(sort of bug 32410) Fix EXIF GPSAltitude calculation when below sea level.

In EXIF, GPSAltitude is stored as a fraction string like "1/2".
For values below sea level we were negating this value, in order
to represent the sign and the magnitude in the same value. However,
I forgot to convert that to an integer before negating it. PHP was nice
enough to do a best effort conversion of the string to an integer.
This resulted in altitudes below sea level being taken as just the
numerator of the altitude, which gives results that can be significantly
off.

Also add unit tests for the GPS related image metadata stuff. Change the
existing GPS test to use a fractional altitude (Since this issue isn't
appearent if the denominator is 1). Add tests for XMP as well, since
XMP had same issue, and has to do same processing as EXIF stuff does.

In some future time, may want to consider just converting all exif rational values
to real numbers during the extraction process for generally better sanity.

Patchset 2: rebase

Change-Id: I49032b52a4c840b28e667a6a2b8ae23c508df247

diff --git a/RELEASE-NOTES-1.20 b/RELEASE-NOTES-1.20
index cb67f24..fcab721 100644 (file)
--- a/RELEASE-NOTES-1.20
+++ b/RELEASE-NOTES-1.20
@@ -220,6 +220,7 @@ upgrade PHP if you have not done so prior to upgrading MediaWiki.
 * (bug 30390) Suggested file name on Special:Upload should not contain
   illegal characters.
 * (bug 27111) Cascading foreign file repos now fetch shared descriptions properly
+* EXIF below sea level GPS altitude data is now shown correctly
 
 === API changes in 1.20 ===
 * (bug 34316) Add ability to retrieve maximum upload size from MediaWiki API.

diff --git a/includes/media/Exif.php b/includes/media/Exif.php
index f5dc020..784a601 100644 (file)
--- a/includes/media/Exif.php
+++ b/includes/media/Exif.php
@@ -370,6 +370,12 @@ class Exif {
                $this->exifGPStoNumber( 'GPSDestLongitude' );
 
                if ( isset( $this->mFilteredExifData['GPSAltitude'] ) && isset( $this->mFilteredExifData['GPSAltitudeRef'] ) ) {
+
+                       // We know altitude data is a <num>/<denom> from the validation functions ran earlier.
+                       // But multiplying such a string by -1 doesn't work well, so convert.
+                       list( $num, $denom ) = explode( '/', $this->mFilteredExifData['GPSAltitude'] );
+                       $this->mFilteredExifData['GPSAltitude'] = $num / $denom;
+
                        if ( $this->mFilteredExifData['GPSAltitudeRef'] === "\1" ) {
                                $this->mFilteredExifData['GPSAltitude'] *= - 1;
                        }

diff --git a/includes/media/XMP.php b/includes/media/XMP.php
index 75fdd96..36660b3 100644 (file)
--- a/includes/media/XMP.php
+++ b/includes/media/XMP.php
@@ -213,10 +213,16 @@ class XMPReader {
                unset( $data['xmp-special'] );
 
                // Convert GPSAltitude to negative if below sea level.
-               if ( isset( $data['xmp-exif']['GPSAltitudeRef'] ) ) {
-                       if ( $data['xmp-exif']['GPSAltitudeRef'] == '1'
-                               && isset( $data['xmp-exif']['GPSAltitude'] )
-                       ) {
+               if ( isset( $data['xmp-exif']['GPSAltitudeRef'] )
+                       && isset( $data['xmp-exif']['GPSAltitude'] )
+               ) {
+
+                       // Must convert to a real before multiplying by -1
+                       // XMPValidate guarantees there will always be a '/' in this value.
+                       list( $nom, $denom ) = explode( '/', $data['xmp-exif']['GPSAltitude'] );
+                       $data['xmp-exif']['GPSAltitude'] = $nom / $denom;
+
+                       if ( $data['xmp-exif']['GPSAltitudeRef'] == '1' ) {
                                $data['xmp-exif']['GPSAltitude'] *= -1;
                        }
                        unset( $data['xmp-exif']['GPSAltitudeRef'] );

diff --git a/tests/phpunit/data/media/exif-gps.jpg b/tests/phpunit/data/media/exif-gps.jpg
index f99b484..4013734 100644 (file)
Binary files a/tests/phpunit/data/media/exif-gps.jpg and b/tests/phpunit/data/media/exif-gps.jpg differ

diff --git a/tests/phpunit/data/xmp/gps.result.php b/tests/phpunit/data/xmp/gps.result.php
new file mode 100644 (file)
index 0000000..2d1243d
--- /dev/null
+++ b/tests/phpunit/data/xmp/gps.result.php
@@ -0,0 +1,12 @@
+<?php
+
+$result = array( 'xmp-exif' =>
+       array(
+               'GPSAltitude' => -3.14159265301,
+               'GPSDOP' => '5/1',
+               'GPSLatitude' => 88.51805555,
+               'GPSLongitude' => -21.12356945,
+               'GPSVersionID' => '2.2.0.0'
+        )
+);
+

diff --git a/tests/phpunit/data/xmp/gps.xmp b/tests/phpunit/data/xmp/gps.xmp
new file mode 100644 (file)
index 0000000..e52d2c8
--- /dev/null
+++ b/tests/phpunit/data/xmp/gps.xmp
@@ -0,0 +1,17 @@
+<?xpacket begin='' id='W5M0MpCehiHzreSzNTczkc9d'?>
+<x:xmpmeta xmlns:x='adobe:ns:meta/' x:xmptk='Image::ExifTool 7.30'>
+<rdf:RDF xmlns:rdf='http://www.w3.org/1999/02/22-rdf-syntax-ns#'>
+
+ <rdf:Description rdf:about=''
+  xmlns:exif='http://ns.adobe.com/exif/1.0/'>
+  <exif:GPSAltitude>103993/33102</exif:GPSAltitude>
+  <exif:GPSAltitudeRef>1</exif:GPSAltitudeRef>
+  <exif:GPSDOP>5/1</exif:GPSDOP>
+  <exif:GPSLatitude>88,31.083333N</exif:GPSLatitude>
+  <exif:GPSLongitude>21,7.414167W</exif:GPSLongitude>
+  <exif:GPSVersionID>2.2.0.0</exif:GPSVersionID>
+ </rdf:Description>
+
+</rdf:RDF>
+</x:xmpmeta>
+<?xpacket end='w'?>

diff --git a/tests/phpunit/includes/media/ExifTest.php b/tests/phpunit/includes/media/ExifTest.php
index 80aaac2..fff8c0b 100644 (file)
--- a/tests/phpunit/includes/media/ExifTest.php
+++ b/tests/phpunit/includes/media/ExifTest.php
@@ -25,7 +25,7 @@ class ExifTest extends MediaWikiTestCase {
                $expected = array(
                        'GPSLatitude' => 88.5180555556,
                        'GPSLongitude' => -21.12357,
-                       'GPSAltitude' => -200,
+                       'GPSAltitude' => -3.141592653,
                        'GPSDOP' => '5/1',
                        'GPSVersionID' => '2.2.0.0',
                );

diff --git a/tests/phpunit/includes/media/XMPTest.php b/tests/phpunit/includes/media/XMPTest.php
index c952b23..942cc3a 100644 (file)
--- a/tests/phpunit/includes/media/XMPTest.php
+++ b/tests/phpunit/includes/media/XMPTest.php
@@ -22,7 +22,7 @@ class XMPTest extends MediaWikiTestCase {
                }
                $reader = new XMPReader;
                $reader->parse( $xmp );
-               $this->assertEquals( $expected, $reader->getResults(), $info );
+               $this->assertEquals( $expected, $reader->getResults(), $info, 0.0000000001 );
        }
 
        public function dataXMPParse() {
@@ -52,6 +52,7 @@ class XMPTest extends MediaWikiTestCase {
                        array( 'utf32BE', 'UTF-32BE encoding' ),
                        array( 'utf32LE', 'UTF-32LE encoding' ),
                        array( 'xmpExt', 'Extended XMP missing second part' ),
+                       array( 'gps', 'Handling of exif GPS parameters in XMP' ),
                 );
                foreach( $xmpFiles as $file ) {
                        $xmp = file_get_contents( $xmpPath . $file[0] . '.xmp' );