Bug 215743 - Endianness bug loading 16 bits raw images
Summary: Endianness bug loading 16 bits raw images
Status: RESOLVED FIXED
Alias: None
Product: digikam
Classification: Applications
Component: Plugin-DImg-RAW (show other bugs)
Version: 1.0.0
Platform: Compiled Sources Linux
: NOR normal
Target Milestone: ---
Assignee: Digikam Developers
URL:
Keywords:
Depends on:
Blocks:
 
Reported: 2009-11-22 21:46 UTC by pochini
Modified: 2017-07-31 15:42 UTC (History)
2 users (show)

See Also:
Latest Commit:
Version Fixed In: 1.0.0


Attachments

Note You need to log in before you can comment on or make changes to this bug.
Description pochini 2009-11-22 21:46:20 UTC
Version:           1.0.0-beta6 (using KDE 4.3.3)
Compiler:          gcc (GCC) 4.1.2 (Gentoo 4.1.2 p1.0.2)  powerpc
OS:                Linux
Installed from:    Compiled From Sources

On big-endian systems digikam does not load 16 bits raw files correctly. The following simple patch fixes the bug (it breaks little-endian archs), but I'm not sure if the bug is here or if libkdcraw needs a fix instead.


--- digikam-1.0.0-beta6/libs/dimg/loaders/rawloader.cpp__orig   2009-11-22 19:49:05.000000000 +0100
+++ digikam-1.0.0-beta6/libs/dimg/loaders/rawloader.cpp 2009-11-22 21:37:29.000000000 +0100
@@ -182,9 +182,9 @@ bool RAWLoader::loadedFromDcraw(QByteArr
                 dst[1] = (unsigned short)((src[2]*256 + src[3]) * fac);      // Green
                 dst[2] = (unsigned short)((src[0]*256 + src[1]) * fac);      // Red
 #else
-                dst[0] = (unsigned short)((src[5]*256 + src[4]) * fac);      // Blue
-                dst[1] = (unsigned short)((src[3]*256 + src[2]) * fac);      // Green
-                dst[2] = (unsigned short)((src[1]*256 + src[0]) * fac);      // Red
+                dst[0] = (unsigned short)((src[4]*256 + src[5]) * fac);      // Blue
+                dst[1] = (unsigned short)((src[2]*256 + src[3]) * fac);      // Green
+                dst[2] = (unsigned short)((src[0]*256 + src[1]) * fac);      // Blue
 #endif
                 dst[3] = 0xFFFF;
Comment 1 caulier.gilles 2009-11-22 22:39:24 UTC
Yes, it's libkdcraw as well...

Gilles Caulier
Comment 2 caulier.gilles 2009-11-22 22:41:39 UTC
probably in this loop:

http://lxr.kde.org/source/KDE/kdegraphics/libs/libkdcraw/libkdcraw/kdcraw.cpp#323
Comment 3 pochini 2009-11-23 23:53:23 UTC
Thanks for the link.
Uhm... no, digikam does not use KDcraw::extractRAWData(). It uses KDcraw::loadFromDcraw() instead, which then calls dcraw_make_mem_image() (http://lxr.kde.org/source/KDE/kdegraphics/libs/libkdcraw/libkdcraw/kdcraw.cpp#679). LibRaw docs do not mention the format of the data, but in LibRaw-0.8.4/samples/mem_image.cpp there is a comment that says: "data in img->data is not converted to network byte order. So, we should swap values on some architectures for dcraw compatibility".

I think we should fix digikam. If we change libkdcraw we may break some other apps which already convert the data.
Comment 4 pochini 2009-11-24 22:19:34 UTC
I propose another patch that fixes the bug and does not break anything, except, perhaps, big-endian machines with KDCRAW_VERSION<0x000400 (I didn't test). Little-endian archs are not affected.


--- digikam-1.0.0-beta6/libs/dimg/loaders/rawloader.cpp__orig   2009-11-24 21:51:58.000000000 +0100
+++ digikam-1.0.0-beta6/libs/dimg/loaders/rawloader.cpp 2009-11-24 22:03:02.000000000 +0100
@@ -177,7 +177,7 @@ bool RAWLoader::loadedFromDcraw(QByteArr
 
             for (int w = 0; w < width; ++w)
             {
-#if KDCRAW_VERSION < 0x000400
+#if (KDCRAW_VERSION < 0x000400) ^ (Q_BYTE_ORDER == Q_BIG_ENDIAN)
                 dst[0] = (unsigned short)((src[4]*256 + src[5]) * fac);      // Blue
                 dst[1] = (unsigned short)((src[2]*256 + src[3]) * fac);      // Green
                 dst[2] = (unsigned short)((src[0]*256 + src[1]) * fac);      // Red
Comment 5 Marcel Wiesweg 2009-12-20 16:11:06 UTC
SVN commit 1064293 by mwiesweg:

Fix 16bit raw loading on big endian.
Patch from pochini@shiny.it slightly modified.

BUG: 215743

 M  +2 -1      NEWS  
 M  +18 -9     libs/dimg/loaders/rawloader.cpp  


WebSVN link: http://websvn.kde.org/?view=rev&revision=1064293