D16617: fix extraction of GPS altitude for exif data
This revision was automatically updated to reflect the committed changes. Closed by commit R286:ca8c11351fe7: fix extraction of GPS altitude for exif data (authored by astippich). REPOSITORY R286 KFileMetaData CHANGES SINCE LAST UPDATE https://phabricator.kde.org/D16617?vs=44723=46624 REVISION DETAIL https://phabricator.kde.org/D16617 AFFECTED FILES autotests/exiv2extractortest.cpp autotests/samplefiles/test.jpg src/extractors/exiv2extractor.cpp src/extractors/exiv2extractor.h To: astippich, #frameworks, bruns Cc: kde-frameworks-devel, #baloo, ashaposhnikov, michaelh, astippich, spoorun, ngraham, bruns, abrahams
D16617: fix extraction of GPS altitude for exif data
astippich added a comment. Thanks INLINE COMMENTS > bruns wrote in exiv2extractor.cpp:217 > This is bogus, why am I not allowed to take photos at sea level? Or in > Greenwhich, at the Equator? > > Invalid data should **not** be signaled by 0.0 Yep, I will change that in another diff REPOSITORY R286 KFileMetaData BRANCH exif_gps_altitude REVISION DETAIL https://phabricator.kde.org/D16617 To: astippich, #frameworks, bruns Cc: kde-frameworks-devel, #baloo, ashaposhnikov, michaelh, astippich, spoorun, ngraham, bruns, abrahams
D16617: fix extraction of GPS altitude for exif data
bruns accepted this revision.
bruns added inline comments.
This revision is now accepted and ready to land.
INLINE COMMENTS
> exiv2extractor.cpp:217
>
> if (altitude) {
> result->add(Property::PhotoGpsAltitude, altitude);
This is bogus, why am I not allowed to take photos at sea level? Or in
Greenwhich, at the Equator?
Invalid data should **not** be signaled by 0.0
> astippich wrote in exiv2extractor.cpp:285
> true, but why not use the converted value directly? Otherwise the conversion
> would have to be made manually by dividing, see fetchGpsDouble()
You are correct, the implicit conversions done by Exiv2 are easy to miss.
> exiv2extractor.cpp:294
> +}
> +return alt;
> +}
You could return `std::numeric_limits::quiet_NaN()` here for the error
case.
REPOSITORY
R286 KFileMetaData
BRANCH
exif_gps_altitude
REVISION DETAIL
https://phabricator.kde.org/D16617
To: astippich, #frameworks, bruns
Cc: kde-frameworks-devel, #baloo, ashaposhnikov, michaelh, astippich, spoorun,
ngraham, bruns, abrahams
D16617: fix extraction of GPS altitude for exif data
astippich added a comment. gentle ping REPOSITORY R286 KFileMetaData REVISION DETAIL https://phabricator.kde.org/D16617 To: astippich, #frameworks, bruns Cc: kde-frameworks-devel, #baloo, ashaposhnikov, michaelh, astippich, spoorun, ngraham, bruns, abrahams
D16617: fix extraction of GPS altitude for exif data
astippich added a dependent revision: D16931: Extract more tags from exif metadata. REPOSITORY R286 KFileMetaData REVISION DETAIL https://phabricator.kde.org/D16617 To: astippich, #frameworks, bruns Cc: kde-frameworks-devel, #baloo, ashaposhnikov, michaelh, astippich, spoorun, ngraham, bruns, abrahams
D16617: fix extraction of GPS altitude for exif data
astippich edited the summary of this revision. REPOSITORY R286 KFileMetaData REVISION DETAIL https://phabricator.kde.org/D16617 To: astippich, #frameworks, bruns Cc: kde-frameworks-devel, #baloo, ashaposhnikov, michaelh, astippich, spoorun, ngraham, bruns, abrahams
D16617: fix extraction of GPS altitude for exif data
astippich added inline comments. INLINE COMMENTS > bruns wrote in exiv2extractor.cpp:285 > According to: > https://www.sno.phy.queensu.ca/~phil/exiftool/TagNames/GPS.html > and > http://www.cipa.jp/std/documents/e/DC-008-Translation-2016-E.pdf > > the absolute altitude value is a rational, like deg/min/sec. true, but why not use the converted value directly? Otherwise the conversion would have to be made manually by dividing, see fetchGpsDouble() REPOSITORY R286 KFileMetaData REVISION DETAIL https://phabricator.kde.org/D16617 To: astippich, #frameworks, bruns Cc: kde-frameworks-devel, #baloo, ashaposhnikov, michaelh, astippich, spoorun, ngraham, bruns, abrahams
D16617: fix extraction of GPS altitude for exif data
bruns requested changes to this revision.
bruns added inline comments.
This revision now requires changes to proceed.
INLINE COMMENTS
> exiv2extractor.cpp:285
> +if (it != data.end()) {
> +alt = it->value().toFloat();
> +it = data.findKey(Exiv2::ExifKey("Exif.GPSInfo.GPSAltitudeRef"));
According to:
https://www.sno.phy.queensu.ca/~phil/exiftool/TagNames/GPS.html
and
http://www.cipa.jp/std/documents/e/DC-008-Translation-2016-E.pdf
the absolute altitude value is a rational, like deg/min/sec.
REPOSITORY
R286 KFileMetaData
REVISION DETAIL
https://phabricator.kde.org/D16617
To: astippich, #frameworks, bruns
Cc: kde-frameworks-devel, #baloo, ashaposhnikov, michaelh, astippich, spoorun,
ngraham, bruns, abrahams
D16617: fix extraction of GPS altitude for exif data
astippich edited the summary of this revision. REPOSITORY R286 KFileMetaData REVISION DETAIL https://phabricator.kde.org/D16617 To: astippich, #frameworks, bruns Cc: kde-frameworks-devel, #baloo, ashaposhnikov, michaelh, astippich, spoorun, ngraham, bruns, abrahams
D16617: fix extraction of GPS altitude for exif data
astippich added a dependency: D16560: create a separate test file for embedded images test. REPOSITORY R286 KFileMetaData REVISION DETAIL https://phabricator.kde.org/D16617 To: astippich, #frameworks, bruns Cc: kde-frameworks-devel, #baloo, ashaposhnikov, michaelh, astippich, spoorun, ngraham, bruns, abrahams
D16617: fix extraction of GPS altitude for exif data
astippich created this revision. astippich added reviewers: Frameworks, bruns. Herald added projects: Frameworks, Baloo. Herald added subscribers: Baloo, kde-frameworks-devel. astippich requested review of this revision. REVISION SUMMARY The previous code was wrong, since the altitude is given with only a single double value below or above sea level. REPOSITORY R286 KFileMetaData BRANCH exif_gps_altitude REVISION DETAIL https://phabricator.kde.org/D16617 AFFECTED FILES autotests/exiv2extractortest.cpp autotests/samplefiles/test.jpg src/extractors/exiv2extractor.cpp src/extractors/exiv2extractor.h To: astippich, #frameworks, bruns Cc: kde-frameworks-devel, #baloo, ashaposhnikov, michaelh, astippich, spoorun, ngraham, bruns, abrahams
