Bug 121646

I believe that the bug was introduced here:

http://websvn.kde.org/branches/stable/extragear/graphics/digikam/utilities/imageeditor/imlibinterface.cpp?rev=493015&view=rev

And that the fix would be something along the lines of:

- unsigned short jpegID   = 0xD8FF;
+ unsigned char [] jpegID = { 0xFF, 0xD8 };

(and corresponding fixes for TIFF ID)

Sorry I don't currently have time to write and test a proper patch!

Anthony

I probably should have added that the change suggested above is to the ImlibInterface::fileFormat(const QString& filePath) method in branches/stable/extragear/graphics/digikam/utilities/imageeditor/imlibinterface.cpp

CCBUG won't work properlly today...

I have fixed this problem in stable and trunk branch using KFileMetaInfo parser to detect mime type. Please checkout and valid this fix. Thanks in advance

Gilles Caulier

Thx Gilles!

Anthony, Fabrice:  I've put digikam sources that include the
patch at http://www.mpe.mpg.de/~ach/tmp/src-only/

to build on ppc.  Download the 3 files

dpkg-source -x digikam_0.8.1-3.dsc
cd digikam-0.8.1
debuild -us -uc
sudo debi

then start digikam. 
Let us know if this fixes the problem.

Thx,
Achim

Damned. KFileMetaInfo doesn't work properlly to parse all jpeg files...

I have re-commited any changes to svn using only uchar bytes array for JPEG and TIFF. Please test it again...

Gilles

SVN commit 507591 by cgilles:

digikam from stable : using uchar bytes array always working to detect JPEG/TIFF file format.

CCBUGS: 121646

 M  +21 -21    imlibinterface.cpp  


--- branches/stable/extragear/graphics/digikam/utilities/imageeditor/imlibinterface.cpp #507590:507591
@@ -70,7 +70,8 @@
 
 #define MaxRGB 255L
 
-class ImlibInterfacePrivate {
+class ImlibInterfacePrivate
+{
 
 public:
 
@@ -174,20 +175,8 @@
     if ( filePath == QString::null )
         return NONE_IMAGE;
         
-    KFileMetaInfo metaInfo(filePath, QString::null, KFileMetaInfo::Fastest);
-
-    if (metaInfo.isValid())
-    {
-        kdDebug() << k_funcinfo << " : Mime type: " << metaInfo.mimeType() << endl;
-        
-        if (metaInfo.mimeType() == "image/jpeg")
-            return JPEG_IMAGE;
-        
-        if (metaInfo.mimeType() == "image/png")
-            return PNG_IMAGE;
-    }
-
     FILE* f = fopen(QFile::encodeName(filePath), "rb");
+    
     if (!f)
     {
         kdWarning() << "Failed to open file" << endl;
@@ -206,12 +195,22 @@
 
     fclose(f);
     
-    DcrawParse     rawFileParser;
-    unsigned short tiffBigID = 0x4d4d;
-    unsigned short tiffLilID = 0x4949;
-    
-    if (rawFileParser.getCameraModel( QFile::encodeName(filePath), NULL, NULL) == 0)
+    DcrawParse rawFileParser;
+    uchar jpegID[2]    = { 0xFF, 0xD8 };   
+    uchar tiffBigID[2] = { 0x4D, 0x4D };
+    uchar tiffLilID[2] = { 0x49, 0x49 };
+    uchar pngID[8]     = {'\211', 'P', 'N', 'G', '\r', '\n', '\032', '\n'};
+
+    if (memcmp(&header, &jpegID, 2) == 0)            // JPEG file ?
     {
+        return JPEG_IMAGE;
+    }
+    else if (memcmp(&header, &pngID, 8) == 0)        // PNG file ?
+    {
+        return PNG_IMAGE;
+    }
+    else if (rawFileParser.getCameraModel( QFile::encodeName(filePath), NULL, NULL) == 0)
+    {
         // RAW File test using dcraw.  
         // Need to test it before TIFF because any RAW file 
         // formats using TIFF header.
@@ -222,9 +221,10 @@
     {
         return TIFF_IMAGE;
     }
+    
+    // In others cases, QImage will be used to try to open file.
+    return QIMAGE_IMAGE;
 
-    // In others cases, QImage will be used to open file.
-    return QIMAGE_IMAGE;      
 }
 
 bool ImlibInterface::load(const QString& filename, bool *isReadOnly)

SVN commit 507587 by cgilles:

digikam from trunk : using uchar bytes array always working to detect JPEG/TIFF file format.

CCBUGS: 121646

 M  +25 -26    dimg.cpp   [POSSIBLY UNSAFE: scanf]


--- trunk/extragear/graphics/digikam/libs/dimg/dimg.cpp #507586:507587
@@ -280,50 +280,48 @@
 {
     if ( filePath == QString::null )
         return NONE;
-
-    KFileMetaInfo metaInfo(filePath, QString::null, KFileMetaInfo::Fastest);
-
-    if (metaInfo.isValid())
-    {
-        kdDebug() << k_funcinfo << " : Mime type: " << metaInfo.mimeType() << endl;
-        
-        if (metaInfo.mimeType() == "image/jpeg")
-            return JPEG;
-        
-        if (metaInfo.mimeType() == "image/png")
-            return PNG;
-    }
-
+    
     FILE* f = fopen(QFile::encodeName(filePath), "rb");
+    
     if (!f)
     {
         kdDebug() << k_funcinfo << "Failed to open file" << endl;
         return NONE;
     }
-
+    
     const int headerLen = 8;
     unsigned char header[headerLen];
-
+    
     if (fread(&header, 8, 1, f) != 1)
     {
         kdDebug() << k_funcinfo << "Failed to read header" << endl;
         fclose(f);
         return NONE;
     }
-
+    
     fclose(f);
-
-    DcrawParse     rawFileParser;
-    unsigned short tiffBigID = 0x4d4d;
-    unsigned short tiffLilID = 0x4949;
-
-    if (memcmp(&header[0], "P", 1)  == 0 &&
-        memcmp(&header[2], "\n", 1) == 0)       // PPM 16 bits file ?
+    
+    DcrawParse rawFileParser;
+    uchar jpegID[2]    = { 0xFF, 0xD8 };   
+    uchar tiffBigID[2] = { 0x4D, 0x4D };
+    uchar tiffLilID[2] = { 0x49, 0x49 };
+    uchar pngID[8]     = {'\211', 'P', 'N', 'G', '\r', '\n', '\032', '\n'};
+    
+    if (memcmp(&header, &jpegID, 2) == 0)            // JPEG file ?
     {
+        return JPEG;
+    }
+    else if (memcmp(&header, &pngID, 8) == 0)        // PNG file ?
+    {
+        return PNG;
+    }
+    else if (memcmp(&header[0], "P", 1)  == 0 &&
+             memcmp(&header[2], "\n", 1) == 0)       // PPM 16 bits file ?
+    {
         int width, height, rgbmax;
         char nl;
-
         FILE *file = fopen(QFile::encodeName(filePath), "rb");
+        
         if (fscanf (file, "P6 %d %d %d%c", &width, &height, &rgbmax, &nl) == 4) 
         {
             if (rgbmax > 255)
@@ -332,6 +330,7 @@
                 return PPM;
             }
         }
+        
         pclose (file);
     }
     else if (rawFileParser.getCameraModel( QFile::encodeName(filePath), NULL, NULL) == 0)
@@ -346,7 +345,7 @@
     {
         return TIFF;
     }
-
+    
     // In others cases, QImage will be used to try to open file.
     return QIMAGE;
 }

Maybe this is worth a new bug report?  Would be good if KFileMetaInfo
catches all JPEG cases so all KDE image application get it right.

Achim
P.S. I'll build with new patch later ...

do you have an example jpeg where the mime type isn't properly detected? duplicating those detections doesn't sound like a good fix to me..

There is no duplicate method in current implementation. Look in svn...

Gilles

Achim, no need to open a new bug report about KFileMetaInfo. I have removed this solution in current implementation...

Gilles

Hi Gilles,
maybe we missunderstand each other.

AFAIU #5, KFileMetaInfo is _not_ able to detect _all_ kinds of jpeg files correctly on _all/some_ architectures.

If this is true, then we should open a bug/wish against KFileMetaInfo
so this get's fixed (once for all KDE apps).  Then we can

if kde352 > 3.5.2
 your first implementation
else
 your second solution
endif

I assume Dirk understood #5 the same way as I did, therefore he asked
for example jpeg file(s)/URL(s), that are not properly detected by
KFileMetaInfo.

Achim

Trying to check this file :

http://digikam3rdparty.free.fr/Images_2_Test_DImg/JPEG/002.JPG

The file comming directly from my old Olympus C3000Z without correction/change.

The code used in digikam is listed below (from digikam/lib/dimg/dimg.cpp avaialble in svn trunk) :

DImg::FORMAT DImg::fileFormat(const QString& filePath)
{
    if ( filePath == QString::null )
        return NONE;

    // ----------------------------------------
    // FIXME : It's just for testing : remove this code later...
    KFileMetaInfo metaInfo(filePath, QString::null, KFileMetaInfo::Fastest);
    if (metaInfo.isValid())
        kdDebug() << k_funcinfo << " : Mime type: " << metaInfo.mimeType() << endl;
    else
        kdDebug() << k_funcinfo << " KFileMetaInfo is not valid for " << filePath << endl;
    // ----------------------------------------
...

DImg::fileFormat() method is used to parse the image file format before to load image in editor.
On my main computer, i have this message from a console :

digikam: [Digikam::DImg::FORMAT Digikam::DImg::fileFormat(const QString&)]  KFileMetaInfo is not valid for /home/gilles/Images/Albums/Adrien/Photo non traitées/002.JPG
digikam: /home/gilles/Images/Albums/Adrien/Photo non traitées/002.JPG : JPEG file identified
digikam: Reading JPEG metadata: APP1 (size=14403)
digikam: Reading JPEG metadata: COM (size=10)

But on another computer (same KDE version, same configuration, same OS-Mandriva 2006 official) :

digikam: [Digikam::DImg::FORMAT Digikam::DImg::fileFormat(const QString&)]  : Mime type: image/jpeg
digikam: /home/gilles/Images/Test DImg framework/JPEG/002.JPG : JPEG file identified
digikam: Reading JPEG metadata: APP1 (size=14403)
digikam: Reading JPEG metadata: COM (size=10)

crazy no ? 

Gilles

> do you have an example jpeg where the mime type isn't properly detected?

The sample images from 3 different cameras used for the bugreport :
http://centsix.org/debian/tmp/image-10d.jpg
http://centsix.org/debian/tmp/image-300d.jpg
http://centsix.org/debian/tmp/image-sam.jpg

--
Fabrice

#4 : just tested. it's working with this patch.

And another problem using KFileMetaInfo tested with valgrind :

==2615==
==2615== 32 bytes in 1 blocks are definitely lost in loss record 190 of 335
==2615==    at 0x1B8FFC32: operator new(unsigned) (vg_replace_malloc.c:164)
==2615==    by 0x1C0F2292: KFileMimeTypeInfo::GroupInfo::addVariableInfo(QVariant::Type, unsigned) (in /usr/lib/libkio.so.4.2.0)
==2615==    by 0x1C0F234A: KFilePlugin::addVariableInfo(KFileMimeTypeInfo::GroupInfo*, QVariant::Type, unsigned) const (in /usr/lib/libkio.so.4.2.0)
==2615==    by 0x1E2B4D73: KPngPlugin::KPngPlugin(QObject*, char const*, QStringList const&) (in /usr/lib/kde3/kfile_png.so)
==2615==    by 0x1E2B6751: KGenericFactory<KPngPlugin, QObject>::createObject(QObject*, char const*, char const*, QStringList const&) (in /usr/lib/kde3/kfile_png.so)
==2615==    by 0x1C5C3080: KLibFactory::create(QObject*, char const*, char const*, QStringList const&) (in /usr/lib/libkdecore.so.4.2.0)
==2615==    by 0x1C122F6D: KFileMetaInfoProvider::loadPlugin(QString const&, QString const&) (in /usr/lib/libkio.so.4.2.0)
==2615==    by 0x1C12311B: KFileMetaInfoProvider::loadAndRegisterPlugin(QString const&, QString const&) (in /usr/lib/libkio.so.4.2.0)
==2615==    by 0x1C123711: KFileMetaInfoProvider::mimeTypeInfo(QString const&, QString const&) (in /usr/lib/libkio.so.4.2.0)
==2615==    by 0x1C12429F: KFileMetaInfo::init(KURL const&, QString const&, unsigned) (in /usr/lib/libkio.so.4.2.0)
==2615==    by 0x1C1262CD: KFileMetaInfo::KFileMetaInfo(QString const&, QString const&, unsigned) (in /usr/lib/libkio.so.4.2.0)
==2615==    by 0x1BB8648D: Digikam::DImg::fileFormat(QString const&) (dimg.cpp:391)

Gilles Caulier

memleak fixed

ok thanks Dirk.

Fabrice, about jpeg detection and PPC, my last fix work fine now ? Can i close this file ?

Gilles

PPC tests :
The solution proposed in #4 at http://www.mpe.mpg.de/~ach/tmp/src-only/ works (but is not the last one).
The svn trunk branch of Digikam (0.8.2-svn 2006-02-18 14:05) including the last fix is working too.
So this bug can be closed (?)

ok i close this file.

Gilles

commit #5  FWIW:

Gilles you told me that the first patch version using KFileMetaInfo did
not work on one of your systems.  Was this an AMD64 system?

At least a debian user reported that the first version breaks on his
AMD64 (only 64 bit libs).

http://bugs.debian.org/354173

I use now your 2nd version of the PPC fix from #5.

Achim
P.S. Nevertheless I can't imaging that KFileMetaInfo is broken for jpegs on AMD64.  But I can't spot what's wrong with the first version.

> Gilles you told me that the first patch version using KFileMetaInfo did 
> not work on one of your systems.  Was this an AMD64 system? 

No, i never use AMD32/64...
 
>  At least a debian user reported that the first version breaks on his 
> AMD64 (only 64 bit libs). 
 
> http://bugs.debian.org/354173 
 
> I use now your 2nd version of the PPC fix from #5. 
 
> Achim 
> P.S. Nevertheless I can't imaging that KFileMetaInfo is broken for jpegs on AMD64.  But I can't >spot what's wrong with the first version. 

Open konqueror and look if jpeg image haven't associed to RAW file mime type... Here it is and this is why KFileMetaInfo won't work properlly on my computer to detect JPEG image ...

Gilles