Bug 144604

Isaac,

Witch kipi-plugins, Exiv2, and libkexiv2 releases you use on your computer ?

Gilles Caulier

Andreas,

We have 2 issues : the bug is relevant of Exiv2 or libkexiv2.

Personally, i suspect a problem in libkexiv2. What do you think about ?

Gilles

Gilles,

Ubuntu seems to divide them like this:

kipi-plugins:
  0.1.3-1ubuntu1
libexiv2-0.12:
  0.12-0ubuntu2
exiv2:
  0.12-0ubuntu2
libkexiv2:
  0.1.1-0ubuntu1

Gilles,
This doesn't sound like an exiv2 issue and I have no other feedback of related problems with exiv2 alone.
The exiv2 philosophy is to do exactly what the user asks for. It doesn't add or delete tags by itself and it doesn't modify tag types unless told to do so. But as the bug reporter (and the exiv2 man page) said, it will do things which are not consistent with the Exif standard if you ask it to.

Regarding default types, LONG is used for ImageWidth/Length as well as PixelX/YDimension tags, in conformance with the standard.

Andreas

Andreas,

I have take a look into libkexiv2 and Exiv2 source code, and i think than the problem come from Exiv2. Let's me explain why :

- in libkexiv2, nothing special is done. The rotation tool set the tags values as well without force to use a tag format. All is delegate to Exiv2.

- in Exiv2::tags.cpp, the "Exif.Photo.PixelXDimension", "Exif.Photo.PixelYDimension", "Exif.Image.ImageWidth", and "Exif.Image.ImageLength" tags are define like "Unsigned Long" not "Long", and this is the problem. Like Isac said, the Exif spec. (http://www.exif.org/Exif2-2.PDF page 23, 26, 27) must be "Long", not "Unsigned Long".

Fixing it into Exiv2 is a 5 minutes job (:=)))...

Gilles

Am Thursday 26 April 2007 schrieb Gilles Caulier:
[bugs.kde.org quoted mail]

I wonder if there is a similar problem with the GPS tags. Since a few days I'm 
using the google.maps module in Gallery2.2 which reads the coordinates from 
digikam images. They are wrongly interpreted. If I use jpg from other sources 
than digikam, the coordinates are right. 

I haven't hda the time to dig into the problem, it could well be that the 
gallery modules is at fault. But if you have a look to the tags.cpp anyway, 
Andreas, please check the gps tag section as well.

Thanks 
Gerhard

Gilles,

Exiv2 typedef unsignedLong is correct for Exif type LONG. 

There is a misunderstanding here. The Exif standard uses a terminology which is different from C++:

Types as defined in the Exif standard (page 14)
Value  Name    Desc
4      LONG    A 32 bit unsigned integer
9      SLONG   A 32 bit signed integer

Exiv2 uses typedefs unsignedLong and signedLong and names "Long" and "SLong" respectively.

Can you please point me to the libkexiv2 code I should look at?

Have you tried to reproduce the problem in digiKam on your computer? If you can, can you also reproduce the problem with the exiv2 tool alone, just by running the same commands using the modify action?


Gerhard,

I suggest you report this as a separate issue, once you have more information and it still looks like digiKam/exiv2 is not doing the right thing. The question here is whether the GPS tags in those images conform to the Exif standard. For a start, use exiv2 -pt and exiv2 -pv to check.

-ahu.

Andreas,

This is the method from libkexiv2 used to update image dimensions after rotation :

bool KExiv2::setImageDimensions(const QSize& size, bool setProgramName)
{
    if (!setProgramId(setProgramName))
        return false;

    try
    {    
        d->exifMetadata["Exif.Image.ImageWidth"]      = size.width();
        d->exifMetadata["Exif.Image.ImageLength"]     = size.height();
        d->exifMetadata["Exif.Photo.PixelXDimension"] = size.width();
        d->exifMetadata["Exif.Photo.PixelYDimension"] = size.height();
        return true;
    }
    catch( Exiv2::Error &e )
    {
        printExiv2ExceptionError("Cannot set image dimensions using Exiv2 ", e);
    }        
    
    return false;
}

Full code from libkexiv2 is here :

http://websvn.kde.org/trunk/extragear/libs/libkexiv2/kexiv2.cpp?revision=646581&view=markup

The QSize::width() or QSize::height() method return an integer value.

The code is simple and the problem reported by Isaac is reproductible on my computer. After to rotate an image, the tags type is SLONG.

I have not yet tried to play with Exiv2 command line tool. I will do it. Isac can you perform a test with EXiv2 command line tool too to trying to reproduce the problem ?

Gilles

Andreas,

You have right. if the tags are reported like SLONG by EXIV2, it fine for me. If Isaac, cannot load the image in the exifiron program, this is want mean than the problem is in another place. Perhaps it's a bug in exifiron.

Isaac, are you tried to play with ExifTool program on the rotated image ? Is ExifTool report any problems ?

Gilles 

Am Thursday 26 April 2007 schrieb Andreas Huggel:
[bugs.kde.org quoted mail]

Digikam JPG exiv2 -pt
Exif.GPSInfo.GPSVersionID                    Byte        4  2 0 0 0
Exif.GPSInfo.GPSLatitudeRef                  Ascii       2  North
Exif.GPSInfo.GPSLatitude                     SRational   3  63deg 59.35280'
Exif.GPSInfo.GPSLongitudeRef                 Ascii       2  West
Exif.GPSInfo.GPSLongitude                    SRational   3  22deg 35.68542'
Exif.GPSInfo.GPSAltitudeRef                  Byte        1  Above sea level
Exif.GPSInfo.GPSAltitude                     SRational   1  37.6 m
Exif.GPSInfo.GPSMapDatum                     Ascii       7  WGS-84

exiv2 -pv
0x0000 GPSInfo      GPSVersionID                Byte        4  2 2 0 0
0x0001 GPSInfo      GPSLatitudeRef              Ascii       2  N
0x0002 GPSInfo      GPSLatitude                 Rational    3  29/1 29/1 
2314/100
0x0003 GPSInfo      GPSLongitudeRef             Ascii       2  W
0x0004 GPSInfo      GPSLongitude                Rational    3  89/1 42/1 
723/100
0x0005 GPSInfo      GPSAltitudeRef              Byte        1  0
0x0006 GPSInfo      GPSAltitude                 Rational    1  4294967273/1
0x0007 GPSInfo      GPSTimeStamp                Rational    3  14/1 4/1 31/1

Other source JPG exiv2 -pt (this one works with Gallery 2.2)
Exif.GPSInfo.GPSVersionID                    Byte        4  2 2 0 0
Exif.GPSInfo.GPSLatitudeRef                  Ascii       2  North
Exif.GPSInfo.GPSLatitude                     Rational    3  29deg 29' 23.140"
Exif.GPSInfo.GPSLongitudeRef                 Ascii       2  West
Exif.GPSInfo.GPSLongitude                    Rational    3  89deg 42' 7.230"
Exif.GPSInfo.GPSAltitudeRef                  Byte        1  Above sea level
Exif.GPSInfo.GPSAltitude                     Rational    1  4294967296 m
Exif.GPSInfo.GPSTimeStamp                    Rational    3  14:04:31

exiv2 -pv
0x0000 GPSInfo      GPSVersionID                Byte        4  2 0 0 0
0x0001 GPSInfo      GPSLatitudeRef              Ascii       2  N
0x0002 GPSInfo      GPSLatitude                 SRational   3  63/1 
59352804/1000000 0/1
0x0003 GPSInfo      GPSLongitudeRef             Ascii       2  W
0x0004 GPSInfo      GPSLongitude                SRational   3  22/1 
35685424/1000000 0/1
0x0005 GPSInfo      GPSAltitudeRef              Byte        1  0
0x0006 GPSInfo      GPSAltitude                 SRational   1  376285/10000
0x0012 GPSInfo      GPSMapDatum                 Ascii       7  WGS-84

This is my first diagnostics.
Gerhard

> -ahu.
> _______________________________________________
> Kde-imaging mailing list
> Kde-imaging@kde.org
> https://mail.kde.org/mailman/listinfo/kde-imaging

It seems we're losing focus here - but I think I've found the bug anyway.  The problem is definitely KExiv2.  Here's why:

The KExiv code Gilles pasted uses QSize::width() and QSize::height(), which return int (i.e. signed int); libexiv2 *really* doesn't care about types, and just uses operator=() overloading to decide what type to give the tag based on the RHS of the assignment.  So when you pass an int, you invoke this libexiv2 function (this is from libexiv2, src/exif.cpp, line 148 ish):
    Exifdatum& Exifdatum::operator=(const int32_t& value)
    {
        return Exiv2::setValue(*this, value);
    }


I think the best way to fix the problem is (strangely) to cast the number to a string first.  From the libexiv2 source it looks like this will force the library to choose the correct type by itself.  Depending on your QT version, something like this should work:
    d->exifMetadata["Exif.Image.ImageWidth"]      = QString::number(size.width()).toStdString(); // QT 4
    d->exifMetadata["Exif.Image.ImageWidth"]      = std::string(QString::number(size.width())); // QT 3


If that doesn't work, use a cast:
    d->exifMetadata["Exif.Image.ImageWidth"]      = (uint32_t)size.width();

I'd attach a patch, but I'd have to do loads of work to set it up, and it's 3am right now, and I've found the bug, and...and... :)

You may well find that Gerhard's GPS types issue is caused by the same kind of operator=() cleverness...

Happy hacking.

The code in #8 :

a) generates the two Exif.Image.ImageWidth/Length tags, which the original bug report said were newly introduced.

b) uses the type of QSize::size() to eventually choose the type of the Exif tag. Convert or cast it to uint32_t to get an Exif LONG type, e.g.,

d->exifMetadata["Exif.Photo.PixelXDimension"] =
    static_cast<uint32_t>(size.width());

That will create a LONG type regardless of the original type (which could also be SHORT)

Converting the value to an std::string and assigning that will use even more magic: If the tag already exists and has a value, it will attempt to read the string assuming it is of the type of the current value. If the tag doesn't exist yet or doesn't have a value yet, it will create a new value with the Exiv2 default type for the tag.

On the other hand, if you prefer less magic, you can create an Exiv2::Value of the type you want and assign that.

-ahu.

Gerhard,

At first glance, the only thing that is obviously not right here is the altitude in one of the two samples (I'd be surprised if GPS worked at that altitude :). Try to use the exiv2 tool to set it to something more reasonable and see if this helps.

-ahu.

Andreas,

Ok, i will review libkexiv2 source code to provide a patch about Image Dimensions and GPS stuff (this last one sound like the same cast problem).

Gilles

Oh yes now I see it too: SRATIONAL is wrong, these should be RATIONAL types.
-ahu.

Re comment #12: There seems to be an echo in here.  Andreas - with respect - why did you reword my comment #11 and post it again?

Re comment #14: Best mark this confirmed, then.

Glad we've got this one nailed, anyway.

To Andreas, #15: Please look the KExiv2::setGPSPosition() method, and look if something is wrong for you... Note : i use M$ Vista during this week (holidays) and i'm out of home using a simple RTC internet connexion. It's not trivial to hack under Linux (:=)))

To Isaac, #16: your problem about image dimensions tags type is fixed on my computer following Andreas tips. Gerhard will patch svn at soon.

Gilles

Isaac,

Yes, you found the bug and your analysis was correct. Thanks! 

I just elaborated on the implications of each solution, especially that via an std::string, which you found 'strange', and highlighted that there is also one without 'magic', to help Gilles decide which way he prefers to fix this.

-ahu.

Gilles,

Try this patch (against a recent pre-release of libkexiv2). It compiles but is untested (I don' have the complete digikam devel environment here at the moment):

--- kexiv2.cpp-0.1.2    2007-04-28 10:16:07.000000000 +0800
+++ kexiv2.cpp  2007-04-28 10:25:14.000000000 +0800
@@ -1570,11 +1570,9 @@
         d->exifMetadata.add(Exiv2::ExifKey("Exif.GPSInfo.GPSAltitudeRef"), value.get());

         // And the actual altitude.
-        value = Exiv2::Value::create(Exiv2::signedRational);
         convertToRational(altitude, &nom, &denom, 4);
         snprintf(scratchBuf, 100, "%ld/%ld", nom, denom);
-        value->read(scratchBuf);
-        d->exifMetadata.add(Exiv2::ExifKey("Exif.GPSInfo.GPSAltitude"), value.get());
+        d->exifMetadata["Exif.GPSInfo.GPSAltitude"] = scratchBuf;

         // LATTITUDE
         // Latitude reference: "N" or "S".
@@ -1606,12 +1604,10 @@
         // Further note: original code did not translate between
         //   dd.dddddd to dd mm.mm - that's why we now multiply
         //   by 6000 - x60 to get minutes, x1000000 to get to mmmm/1000000.
-        value = Exiv2::Value::create(Exiv2::signedRational);
         deg   = (int)floor(fabs(latitude)); // Slice off after decimal.
         min   = (int)floor((fabs(latitude) - floor(fabs(latitude))) * 60000000);
         snprintf(scratchBuf, 100, "%ld/1 %ld/1000000 0/1", deg, min);
-        value->read(scratchBuf);
-        d->exifMetadata.add(Exiv2::ExifKey("Exif.GPSInfo.GPSLatitude"), value.get());
+        d->exifMetadata["Exif.GPSInfo.GPSLatitude"] = scratchBuf;

         // LONGITUDE
         // Longitude reference: "E" or "W".
@@ -1643,12 +1639,10 @@
         // Further note: original code did not translate between
         //   dd.dddddd to dd mm.mm - that's why we now multiply
         //   by 6000 - x60 to get minutes, x1000000 to get to mmmm/1000000.
-        value = Exiv2::Value::create(Exiv2::signedRational);
         deg   = (int)floor(fabs(longitude)); // Slice off after decimal.
         min   = (int)floor((fabs(longitude) - floor(fabs(longitude))) * 60000000);
         snprintf(scratchBuf, 100, "%ld/1 %ld/1000000 0/1", deg, min);
-        value->read(scratchBuf);
-        d->exifMetadata.add(Exiv2::ExifKey("Exif.GPSInfo.GPSLongitude"), value.get());
+        d->exifMetadata["Exif.GPSInfo.GPSLongitude"] = scratchBuf;

         return true;
     }

Gerhard, 

Can you test it on your computer and with you Gallery envirronement ? If it's fine for you, you can apply it on svn of course.

Gilles

Ok I tested with patch and Gallery2.2. It works! It was the RATIONAL problem and not the ss/1. My intuition was right :-)

I'll commit it now.
Cheers

Isaac,

Gerhard have patched libkexiv2 about the wrong cast with image dimension Exif tags.

Can you checkout current implementation of libkexiv2, recompile and install all extragears/libs and recompile and install digiKam to test again ?

Thanks in advance

Gilles Caulier 

Angelo, Arnd, 

This be certainly closed...

Gilles

Yes, closing this bug as FIXED.
If there is any remaining problem, don't hesitate to re-open.

Best, Arnd