From: Brian Wolff Date: Fri, 17 Aug 2012 00:41:13 +0000 (-0300) Subject: (sort of bug 32410) Fix EXIF GPSAltitude calculation when below sea level. X-Git-Tag: 1.31.0-rc.0~22616 X-Git-Url: http://git.cyclocoop.org/%7B%24www_url%7Dadmin/compta/operations/recherche.php?a=commitdiff_plain;h=d1f0c5a7d8e25d378b06fb0850727a357b6b39f6;p=lhc%2Fweb%2Fwiklou.git (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 cb67f24690..fcab7216e1 100644 --- 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 f5dc020c8d..784a6018c0 100644 --- 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 / 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 75fdd96dff..36660b3dcc 100644 --- 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 f99b484d5e..40137340e5 100644 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 index 0000000000..2d1243d559 --- /dev/null +++ b/tests/phpunit/data/xmp/gps.result.php @@ -0,0 +1,12 @@ + + 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 index 0000000000..e52d2c8a6b --- /dev/null +++ b/tests/phpunit/data/xmp/gps.xmp @@ -0,0 +1,17 @@ + + + + + + 103993/33102 + 1 + 5/1 + 88,31.083333N + 21,7.414167W + 2.2.0.0 + + + + + diff --git a/tests/phpunit/includes/media/ExifTest.php b/tests/phpunit/includes/media/ExifTest.php index 80aaac2332..fff8c0bcf3 100644 --- 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 c952b23c94..942cc3a989 100644 --- 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' );