Bug 140297

Summary: GPS tool truncates input coordinates, introducing inacuracy
Product: digikam Reporter: Pedro Venda <pjvenda>
Component: Plugin-Generic-GeolocationEditAssignee: Digikam Developers <digikam-bugs-null>
Status: RESOLVED FIXED    
Severity: normal CC: caulier.gilles, lure
Priority: NOR    
Version: unspecified   
Target Milestone: ---   
Platform: Gentoo Packages   
OS: Linux   
Latest Commit: Version Fixed In: 0.1.0
Attachments: patch to kexiv2.cpp to avoid the GPS accuracy loss

Description Pedro Venda 2007-01-19 16:48:16 UTC
Version:           0.1.3-rc1-svn (using KDE KDE 3.5.5)
Installed from:    Gentoo Packages
Compiler:          gcc (GCC) 4.1.1 (Gentoo 4.1.1-r3) CFLAGS="-march=pentium-m -mtune=pentium-m -mmmx -msse -msse2 -mfpmath=sse,387 -fomit-frame-pointer -funroll-loops -O2 -pipe"
OS:                Linux

With digikam, using Image menu -> Geolocalisation -> Edit Coordinates dialog to input coordinates into a picture file's metadata, the corresponding kipi-plugin/digikam (not sure which is bugged) apparently truncates the fractional part of the numbers;

Example:
1. Desired coordinates: 40º 19' 40.55" N, 7º 35' 11.32" W (40.32793056 lat, -7.58647778 lon);
2. input lat: 40.32793056, lon: -7.58647778;
3. click ok
4. metadata info is lat: 40.3278333333, lon: -7.58633333333

latitude is off by 2,411e-4% (9.723e-05 or 10,82 m)
longitude is off by 1,90e-3% (-1.444e-4 or less than 16 m)

How to Reproduce:
1. Input desired coordinates in Degree decimal format;
2. Input latitude and longitude;
3. Click "OK";
4. View metadata info and compare the numbers. They differ;

Expected behaviour:
Accuracy should not be wasted.
Comment 1 Pedro Venda 2007-01-19 17:04:44 UTC
Further information about introduced GPS inaccuracy...

Entered data in DMS format:
40º 19' 40.55" N, 7º 35' 11.32" W
Entered data in Degree Decimal format:
40.32793056, -7.58647778

Retrieved data in Degree Decimal format:
40.3278333333, -7.58633333333
Retrieved data in DMS format:
40 19' 40.20" N,  7 35' 10.80"
Comment 2 Fabien 2007-01-19 17:13:23 UTC
Check also this thread :
http://mail.kde.org/pipermail/digikam-devel/2006-November/008726.html
Comment 3 Birkir A. Barkarson 2007-01-19 19:16:39 UTC
Hmm... actually I would expect accuracy should always be preserved.  
Strictly speaking the accuracy should depend on the zoom level when the co-ordinates are set.  If you set the position at the max zoom out level you are not setting the position to an accuracy of five decimal digits, maybe one or two.  (since even a click to a single pixel might not correspond to location up to a metre, most users might click within a 10 or 50 pixel radius of actual desired location).  Best would be if such considerations are taken into account.
Of course it might be a different subject from the one posted here.
Comment 4 Pedro Venda 2007-01-19 19:32:57 UTC
Birkir: I understand your point, and it makes sense; but I believe that it shouldn't influence the manual input of coordinates.

If the user inputs coordinates manually, then it's up to him/her to deal with their accuracy.

What's happening is a little different. Internally, the exiv2 or the kipi plugin or even digikam must be clipping some bits of the entered data, thus throwing away part of the information the user enters.
Comment 5 Arnd Baecker 2007-03-06 09:57:30 UTC
To summarize the thread Fabien points to in #2:

- exiv2 can provide enough digits (i.e. it is not the culprit ;-)
- digikam itself can deal with more accurate GPS coordinates
- but: the "Edit geographical coordinates" tool does not write 
  the necessary number of digits on save.

Gilles then wrote:
"""This is relevant of the of following Exiv2Iface methods :

- setGPSInfo()
- convertToRational()

... where digits are lost during double to rational conversion.

http://websvn.kde.org/trunk/extragear/libs/kipi-plugins/common/exiv2iface/exiv2iface.cpp?rev=606701&view=auto

Andreas, There is method in Exiv2 to do this conversion without lost digits ?
If i can use it instead Exiv2Iface::convertToRational()..."""

Andreas' reply on this:
"""There is no such method in Exiv2. If you are happy with 7 decimal places, you could use a simple conversion algorithm similar to that in crwimage.cpp, from line 1015; that should just fit into 32 bit unsigned ints. (Search for "primitive conversion" in the file.)

http://dev.robotbattle.com/~cvsuser/cgi-bin/ns_viewcvs.cgi/exiv2/trunk/src/crwimage.cpp?rev=861&view=markup
"""

My impression is that 7 decimal places would be sufficient,
also in view of Andreas' accuracy analysis given in the thread in #2.
The code of the primitive conversion is just a few lines, so
it might not do much harm to directly include it?
Comment 6 Arnd Baecker 2007-03-07 22:30:49 UTC
Created attachment 19906 [details]
patch to kexiv2.cpp to avoid the GPS accuracy loss

I think there is a really simple fix: 
because, as Andreas wrote in 
http://mail.kde.org/pipermail/digikam-devel/2006-November/008713.html
"""
In the Exif field, longitude and latitude values must be encoded as 
three rationals (two 32bit integers each): the number of degrees, 
minutes and seconds. Fractions are obviously possible for all three and 
one specific value can be encoded in multiple ways.
"""
one may just use a larger denominator
in libkexiv2/kexiv2.cpp, KExiv2::setGPSInfo.
With 1000000 as denominator, using 
(51.123456789 , 13.987654321 ) as input
in the "Edit geographical coordinates" gives
(51.1234567833, 13.9876543167).
This should be sufficiently many valid digits.

Attached is a corresponding patch.
Comment 7 caulier.gilles 2007-03-08 08:58:43 UTC
Arn,

Thanks you very much for this patch. It's very appreciate.

It's look fine but not yet tested. Luka, can you check this patch and apply it on svn ? Thanks in advance...

Gilles
Comment 8 caulier.gilles 2007-03-14 13:11:38 UTC
SVN commit 642431 by cgilles:

libkexiv2 from trunk : patch from Arnd Baecker about to avoid the GPS accuracy loss 
BUG: 140297


 M  +6 -6      kexiv2.cpp  


--- trunk/extragear/libs/libkexiv2/kexiv2.cpp #642430:642431
@@ -1602,11 +1602,11 @@
         // as the sign is encoded in LatRef.
         // 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, x100 to get to mmmm/100.
+        //   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))) * 6000);
-        snprintf(scratchBuf, 100, "%ld/1 %ld/100 0/1", deg, min);
+        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());
         
@@ -1639,11 +1639,11 @@
         // as the sign is encoded in LongRef.
         // 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, x100 to get to mmmm/100.
+        //   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))) * 6000);
-        snprintf(scratchBuf, 100, "%ld/1 %ld/100 0/1", deg, min);
+        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());