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<double>::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

Reply via email to