Bug 131532 - Minolta exception code can break EXIF rotation
Summary: Minolta exception code can break EXIF rotation
Status: RESOLVED FIXED
Alias: None
Product: digikam
Classification: Applications
Component: Metadata-Orientation (show other bugs)
Version: 0.9.0
Platform: Debian testing Linux
: NOR normal
Target Milestone: ---
Assignee: Digikam Developers
URL:
Keywords:
Depends on:
Blocks:
 
Reported: 2006-07-30 00:31 UTC by Sven Coenye
Modified: 2017-08-11 21:49 UTC (History)
1 user (show)

See Also:
Latest Commit:
Version Fixed In: 0.9.0


Attachments
Crash backtrace (2.45 KB, text/plain)
2006-07-31 21:03 UTC, Sven Coenye
Details

Note You need to log in before you can comment on or make changes to this bug.
Description Sven Coenye 2006-07-30 00:31:31 UTC
Version:           0.9.0-beta1 (using KDE KDE 3.5.0)
Installed from:    Debian testing/unstable Packages
Compiler:          gcc 3.3.5 
OS:                Linux

I could not get Digikam to honor the EXIF rotation settings and fixes in related bug reports didn't bring any relief. The problem occurs for the generation of thumbnails and when opening images. Also, I noticed the Orientation tag to be missing from the verbose metadata listing as well. Finally, I noticed "digikam: Cannot parse Exif Orientation tag using Exiv2 (Invalid key `Exif.MinoltaCs7D.Rotation')" in the console output each time I tried to open an image.

I traced this to DMetadata::getImageOrientation(). This method contains exception code to correct for an erroneous EXIF tag in Minolta's 7D and 5D cameras and tries to retrieve the regular Exif.Image.Orientation tag last. The problem is that the entire block is embedded in a try/catch statement and that at least the version of Exiv2 I use throws an exception when the first invalid key (Exif.MinoltaCs7D.Rotation) is tested. This causes getImageOrientation() to always return ORIENTATION_UNSPECIFIED.

Splitting the three tests up so each sits in its own try/catch statement and recompiling immediately fixed all the problems listed above. (I'm assuming debug logging is suspended when generating thumbnails as I don't see the exception message in the logs when Digikam rebuilds an album's thumbnails?)

Exiv2 is the Debian exiv2-0.9.1-1 collection but recompiled against libstdc++5.so. However, I can't see where the exceptions are coming from. libexiv2 seems to return the end of the iterator as expected. However, I'm not too fluent in C++, so that may explain that...
Comment 1 Marcel Wiesweg 2006-07-31 00:03:46 UTC
You could try to outcomment the try/catch statement and post the backtrace when it aborts. Then re-add the try/catch to make it usable again ;-) And just run digikam/digikam without "make install" then your kioslave is not affected.

Btw, debug output from ioslaves can be found only in your ~/.xsession-errors file.
Comment 2 Sven Coenye 2006-07-31 21:03:59 UTC
Created attachment 17187 [details]
Crash backtrace

Generated by disabling the try/catch block around the Minolta exception code
Comment 3 Sven Coenye 2006-07-31 22:29:56 UTC
The backtrace indicates the crash occurs at

Exiv2::ExifKey minoltaKey1("Exif.MinoltaCs7D.Rotation");

Creating the key calls ExifKey::decomposeKey() which validates the key parts. It can't find "MinoltaCs7D" in its IfdInfo lookup table and throws the exception. MinoltaCs5D is not valid either, so that one suffers the same fate.

This change was introduced in Exiv2 0.7. Exiv2 0.5 did not throw an exception in this case. Exiv2 0.6 is a bit in between: it doesn't throw an exception when a name isn't found, but it can throw one if it can't turn the name in a MakerNote.

Exiv2 0.10 introduces the Minolta names in the lookup table as Makernote while the Image tag would return IFD0. I'll try to compile and run with this one, but I'm out of time for today :/
Comment 4 Marcel Wiesweg 2006-07-31 23:14:08 UTC
*** Bug 131603 has been marked as a duplicate of this bug. ***
Comment 5 Marcel Wiesweg 2006-07-31 23:22:31 UTC
SVN commit 568344 by mwiesweg:

Older Exiv2 versions throw an error when they encounter Minolta Makernote key names.
Only query these keys if they are supported by Exiv2.
Otherwise, the Exif.Image.Orientation tag will be ignored as well.

CCBUGS: 131532, 131603



 M  +50 -34    dmetadata.cpp  


--- trunk/extragear/graphics/digikam/libs/dmetadata/dmetadata.cpp #568343:568344
@@ -550,8 +550,21 @@
     if (d->exifMetadata.empty())
        return ORIENTATION_UNSPECIFIED;
 
+    // Workaround for older Exiv2 versions which do not support
+    // Minolta Makernotes and throw an error for such keys.
+    bool supportMinolta = true;
     try
-    {   
+    {
+        Exiv2::ExifKey minoltaKey1("Exif.MinoltaCs7D.Rotation");
+        Exiv2::ExifKey minoltaKey2("Exif.MinoltaCs5D.Rotation");
+    }
+    catch( Exiv2::Error &e )
+    {
+        supportMinolta = false;
+    }
+
+    try
+    {
         Exiv2::ExifData exifData(d->exifMetadata);
         Exiv2::ExifData::iterator it;
         long orientation;
@@ -562,49 +575,52 @@
 
         // -- Minolta Cameras ----------------------------------
 
-        Exiv2::ExifKey minoltaKey1("Exif.MinoltaCs7D.Rotation");
-        it = exifData.findKey(minoltaKey1);
-        
-        if (it != exifData.end())
+        if (supportMinolta)
         {
-            orientation = it->toLong();
-            kdDebug() << "Minolta Makernote Orientation: " << orientation << endl;
-            switch(orientation)
+            Exiv2::ExifKey minoltaKey1("Exif.MinoltaCs7D.Rotation");
+            it = exifData.findKey(minoltaKey1);
+
+            if (it != exifData.end())
             {
-                case 76:
-                    imageOrient = ORIENTATION_ROT_90;
-                    break;
-                case 82:
-                    imageOrient = ORIENTATION_ROT_270;
-                    break;
+                orientation = it->toLong();
+                kdDebug() << "Minolta Makernote Orientation: " << orientation << endl;
+                switch(orientation)
+                {
+                    case 76:
+                        imageOrient = ORIENTATION_ROT_90;
+                        break;
+                    case 82:
+                        imageOrient = ORIENTATION_ROT_270;
+                        break;
+                }
+                return imageOrient;
             }
-            return imageOrient;
-        }
 
-        Exiv2::ExifKey minoltaKey2("Exif.MinoltaCs5D.Rotation");
-        it = exifData.findKey(minoltaKey2);
-        
-        if (it != exifData.end())
-        {
-            orientation = it->toLong();
-            kdDebug() << "Minolta Makernote Orientation: " << orientation << endl;
-            switch(orientation)
+            Exiv2::ExifKey minoltaKey2("Exif.MinoltaCs5D.Rotation");
+            it = exifData.findKey(minoltaKey2);
+
+            if (it != exifData.end())
             {
-                case 76:
-                    imageOrient = ORIENTATION_ROT_90;
-                    break;
-                case 82:
-                    imageOrient = ORIENTATION_ROT_270;
-                    break;
+                orientation = it->toLong();
+                kdDebug() << "Minolta Makernote Orientation: " << orientation << endl;
+                switch(orientation)
+                {
+                    case 76:
+                        imageOrient = ORIENTATION_ROT_90;
+                        break;
+                    case 82:
+                        imageOrient = ORIENTATION_ROT_270;
+                        break;
+                }
+                return imageOrient;
             }
-            return imageOrient;
         }
 
         // -- Standard Exif tag --------------------------------
 
         Exiv2::ExifKey keyStd("Exif.Image.Orientation");
         it = exifData.findKey(keyStd);
-        
+
         if (it != exifData.end())
         {
             orientation = it->toLong();
@@ -617,8 +633,8 @@
         kdDebug() << "Cannot parse Exif Orientation tag using Exiv2 (" 
                   << QString::fromLocal8Bit(e.what().c_str())
                   << ")" << endl;
-    }        
-    
+    }
+
     return ORIENTATION_UNSPECIFIED;
 }
 
Comment 6 Marcel Wiesweg 2006-07-31 23:40:33 UTC
SVN commit 568351 by mwiesweg:

Rotate RAW thumbnails as well, not only JPEGs contains rotation information.

CCBUGS: 131532, 131603


 M  +38 -47    digikamthumbnail.cpp  


--- trunk/extragear/graphics/digikam/kioslave/digikamthumbnail.cpp #568350:568351
@@ -422,65 +422,56 @@
 
 void kio_digikamthumbnailProtocol::exifRotate(const QString& filePath, QImage& thumb)
 {
-    // Check if the file is an JPEG image
-    KFileMetaInfo metaInfo(filePath, "image/jpeg", KFileMetaInfo::Fastest);
+    // Rotate thumbnail based on metadata orientation information
 
-    if (metaInfo.isValid())
-    {
-        if (metaInfo.mimeType() == "image/jpeg" &&
-            metaInfo.containsGroup("Jpeg EXIF Data"))
-        {
-            // Rotate thumbnail from JPEG files based on EXIF rotate tag
+    DMetadata metadata(filePath);
+    DMetadata::ImageOrientation orientation = metadata.getImageOrientation();
 
-            QWMatrix matrix;
-            DMetadata metadata(filePath);
-            DMetadata::ImageOrientation orientation = metadata.getImageOrientation();
+    if (orientation == DMetadata::ORIENTATION_NORMAL ||
+        orientation == DMetadata::ORIENTATION_UNSPECIFIED)
+        return;
 
-            bool doXform = (orientation != DMetadata::ORIENTATION_NORMAL &&
-                            orientation != DMetadata::ORIENTATION_UNSPECIFIED);
+    QWMatrix matrix;
 
-            switch (orientation) 
-            {
-                case DMetadata::ORIENTATION_NORMAL:
-                case DMetadata::ORIENTATION_UNSPECIFIED:
-                    break;
+    switch (orientation)
+    {
+        case DMetadata::ORIENTATION_NORMAL:
+        case DMetadata::ORIENTATION_UNSPECIFIED:
+            break;
 
-                case DMetadata::ORIENTATION_HFLIP:
-                    matrix.scale(-1, 1);
-                    break;
+        case DMetadata::ORIENTATION_HFLIP:
+            matrix.scale(-1, 1);
+            break;
 
-                case DMetadata::ORIENTATION_ROT_180:
-                    matrix.rotate(180);
-                    break;
+        case DMetadata::ORIENTATION_ROT_180:
+            matrix.rotate(180);
+            break;
 
-                case DMetadata::ORIENTATION_VFLIP:
-                    matrix.scale(1, -1);
-                    break;
+        case DMetadata::ORIENTATION_VFLIP:
+            matrix.scale(1, -1);
+            break;
 
-                case DMetadata::ORIENTATION_ROT_90_HFLIP:
-                    matrix.scale(-1, 1);
-                    matrix.rotate(90);
-                    break;
+        case DMetadata::ORIENTATION_ROT_90_HFLIP:
+            matrix.scale(-1, 1);
+            matrix.rotate(90);
+            break;
 
-                case DMetadata::ORIENTATION_ROT_90:
-                    matrix.rotate(90);
-                    break;
+        case DMetadata::ORIENTATION_ROT_90:
+            matrix.rotate(90);
+            break;
 
-                case DMetadata::ORIENTATION_ROT_90_VFLIP:
-                    matrix.scale(1, -1);
-                    matrix.rotate(90);
-                    break;
+        case DMetadata::ORIENTATION_ROT_90_VFLIP:
+            matrix.scale(1, -1);
+            matrix.rotate(90);
+            break;
 
-                case DMetadata::ORIENTATION_ROT_270:
-                    matrix.rotate(270);
-                    break;
-            }
-
-            //transform accordingly
-            if ( doXform )
-                thumb = thumb.xForm( matrix );
-        }
+        case DMetadata::ORIENTATION_ROT_270:
+            matrix.rotate(270);
+            break;
     }
+
+    // transform accordingly
+    thumb = thumb.xForm( matrix );
 }
 
 QImage kio_digikamthumbnailProtocol::loadPNG(const QString& path)
Comment 7 Marcel Wiesweg 2006-08-01 23:24:04 UTC
SVN commit 568671 by mwiesweg:

The same fix as for the thumbnails ioslave, now for the preview ioslave

BUG: 131603, 131532


 M  +38 -47    digikampreview.cpp  


--- trunk/extragear/graphics/digikam/kioslave/digikampreview.cpp #568670:568671
@@ -156,65 +156,56 @@
 
 void kio_digikampreviewProtocol::exifRotate(const QString& filePath, QImage& thumb)
 {
-    // Check if the file is an JPEG image
-    KFileMetaInfo metaInfo(filePath, "image/jpeg", KFileMetaInfo::Fastest);
+    // Rotate thumbnail based on metadata orientation information
 
-    if (metaInfo.isValid())
-    {
-        if (metaInfo.mimeType() == "image/jpeg" &&
-            metaInfo.containsGroup("Jpeg EXIF Data"))
-        {
-            // Rotate thumbnail from JPEG files based on EXIF rotate tag
+    DMetadata metadata(filePath);
+    DMetadata::ImageOrientation orientation = metadata.getImageOrientation();
 
-            QWMatrix matrix;
-            DMetadata metadata(filePath);
-            DMetadata::ImageOrientation orientation = metadata.getImageOrientation();
+    if (orientation == DMetadata::ORIENTATION_NORMAL ||
+        orientation == DMetadata::ORIENTATION_UNSPECIFIED)
+        return;
 
-            bool doXform = (orientation != DMetadata::ORIENTATION_NORMAL &&
-                            orientation != DMetadata::ORIENTATION_UNSPECIFIED);
+    QWMatrix matrix;
 
-            switch (orientation) 
-            {
-                case DMetadata::ORIENTATION_NORMAL:
-                case DMetadata::ORIENTATION_UNSPECIFIED:
-                    break;
+    switch (orientation)
+    {
+        case DMetadata::ORIENTATION_NORMAL:
+        case DMetadata::ORIENTATION_UNSPECIFIED:
+            break;
 
-                case DMetadata::ORIENTATION_HFLIP:
-                    matrix.scale(-1, 1);
-                    break;
+        case DMetadata::ORIENTATION_HFLIP:
+            matrix.scale(-1, 1);
+            break;
 
-                case DMetadata::ORIENTATION_ROT_180:
-                    matrix.rotate(180);
-                    break;
+        case DMetadata::ORIENTATION_ROT_180:
+            matrix.rotate(180);
+            break;
 
-                case DMetadata::ORIENTATION_VFLIP:
-                    matrix.scale(1, -1);
-                    break;
+        case DMetadata::ORIENTATION_VFLIP:
+            matrix.scale(1, -1);
+            break;
 
-                case DMetadata::ORIENTATION_ROT_90_HFLIP:
-                    matrix.scale(-1, 1);
-                    matrix.rotate(90);
-                    break;
+        case DMetadata::ORIENTATION_ROT_90_HFLIP:
+            matrix.scale(-1, 1);
+            matrix.rotate(90);
+            break;
 
-                case DMetadata::ORIENTATION_ROT_90:
-                    matrix.rotate(90);
-                    break;
+        case DMetadata::ORIENTATION_ROT_90:
+            matrix.rotate(90);
+            break;
 
-                case DMetadata::ORIENTATION_ROT_90_VFLIP:
-                    matrix.scale(1, -1);
-                    matrix.rotate(90);
-                    break;
+        case DMetadata::ORIENTATION_ROT_90_VFLIP:
+            matrix.scale(1, -1);
+            matrix.rotate(90);
+            break;
 
-                case DMetadata::ORIENTATION_ROT_270:
-                    matrix.rotate(270);
-                    break;
-            }
-
-            //transform accordingly
-            if ( doXform )
-                thumb = thumb.xForm( matrix );
-        }
+        case DMetadata::ORIENTATION_ROT_270:
+            matrix.rotate(270);
+            break;
     }
+
+    // transform accordingly
+    thumb = thumb.xForm( matrix );
 }
 
 // -- Exif/IPTC preview extraction using Exiv2 --------------------------------------------------------
Comment 8 caulier.gilles 2006-08-17 14:38:56 UTC
SVN commit 573883 by cgilles:

digikam from trunk: Bug fix: with Minolta camera (Dynax5D and 7D), we need to set the Minolta Makernote Orientation tag to "Normal" 
when we set the Exif orientation tag, else preview and thumb will not be orientated properlly after to have created a new image 
from editor using a Minolta picture.
CCBUGS: 131603, 131532



 M  +44 -0     dmetadata.cpp  


--- trunk/extragear/graphics/digikam/libs/dmetadata/dmetadata.cpp #573882:573883
@@ -643,7 +643,20 @@
     if (d->exifMetadata.empty())
        return false;
 
+    // Workaround for older Exiv2 versions which do not support
+    // Minolta Makernotes and throw an error for such keys.
+    bool supportMinolta = true;
     try
+    {
+        Exiv2::ExifKey minoltaKey1("Exif.MinoltaCs7D.Rotation");
+        Exiv2::ExifKey minoltaKey2("Exif.MinoltaCs5D.Rotation");
+    }
+    catch( Exiv2::Error &e )
+    {
+        supportMinolta = false;
+    }
+
+    try
     {    
         if (orientation < ORIENTATION_UNSPECIFIED || orientation > ORIENTATION_ROT_270)
         {
@@ -653,6 +666,37 @@
         
         d->exifMetadata["Exif.Image.Orientation"] = (uint16_t)orientation;
         kdDebug() << "Exif orientation tag set to: " << orientation << endl;
+
+        // -- Minolta Cameras ----------------------------------
+
+        if (supportMinolta)
+        {
+            uint16_t MinoltaOrientation = 72;    // Horizontal (Normal)
+            switch((uint16_t)orientation)
+            {    
+                case ORIENTATION_ROT_90:
+                        MinoltaOrientation = 76; // Rotate 90 CW
+                    break;
+        
+                case ORIENTATION_ROT_270:
+                        MinoltaOrientation = 82; // Rotate 180 CW
+                    break;
+            }
+    
+            Exiv2::ExifData exifData(d->exifMetadata);
+            Exiv2::ExifData::iterator it;
+
+            Exiv2::ExifKey minoltaKey1("Exif.MinoltaCs7D.Rotation");
+            it = exifData.findKey(minoltaKey1);
+            if (it != exifData.end())
+                d->exifMetadata["Exif.MinoltaCs7D.Rotation"] = MinoltaOrientation;
+        
+            Exiv2::ExifKey minoltaKey2("Exif.MinoltaCs5D.Rotation");
+            it = exifData.findKey(minoltaKey2);
+            if (it != exifData.end())
+                d->exifMetadata["Exif.MinoltaCs5D.Rotation"] = MinoltaOrientation;
+        }
+
         return true;
     }
     catch( Exiv2::Error &e )