[libcamera-devel,3/4] android: jpeg: exif: Fix setGPSLocation longitude

11512 diff mbox series

There was a copy-paste error that caused the latitude to be set twice and
the longitude never. Fix this.

This is part of the fix that allows the following CTS test to pass:
- android.hardware.cts.CameraTest#testJpegExif

Signed-off-by: Paul Elder <paul.elder@ideasonboard.com>
---
 src/android/jpeg/exif.cpp | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

Hi Paul,

On 08/03/2021 10:13, Paul Elder wrote:
> There was a copy-paste error that caused the latitude to be set twice and
> the longitude never. Fix this.
> 

Eeep. Fine as long as you're on a very narrow line around the world
where Y=X I suppose.

But that doesn't cover much - so it's good that we get this fixed ;)

Reviewed-by: Kieran Bingham <kieran.bingham@ideasonboard.com>

> This is part of the fix that allows the following CTS test to pass:
> - android.hardware.cts.CameraTest#testJpegExif
> 
> Signed-off-by: Paul Elder <paul.elder@ideasonboard.com>
> ---
>  src/android/jpeg/exif.cpp | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/src/android/jpeg/exif.cpp b/src/android/jpeg/exif.cpp
> index c8b2f072..5225d17f 100644
> --- a/src/android/jpeg/exif.cpp
> +++ b/src/android/jpeg/exif.cpp
> @@ -392,7 +392,7 @@ void Exif::setGPSLocation(const double *coords)
>  	std::tie<int, int, int>(deg, min, sec) = degreesToDMS(coords[1]);
>  	setString(EXIF_IFD_GPS, static_cast<ExifTag>(EXIF_TAG_GPS_LONGITUDE_REF),
>  		  EXIF_FORMAT_ASCII, deg >= 0 ? "E" : "W");
> -	setGPSDMS(EXIF_IFD_GPS, static_cast<ExifTag>(EXIF_TAG_GPS_LATITUDE),
> +	setGPSDMS(EXIF_IFD_GPS, static_cast<ExifTag>(EXIF_TAG_GPS_LONGITUDE),
>  		  std::abs(deg), min, sec);
>  
>  	setByte(EXIF_IFD_GPS, static_cast<ExifTag>(EXIF_TAG_GPS_ALTITUDE_REF),
>

Hi Paul,

Thank you for the patch.

On Mon, Mar 08, 2021 at 10:51:10AM +0000, Kieran Bingham wrote:
> On 08/03/2021 10:13, Paul Elder wrote:
> > There was a copy-paste error that caused the latitude to be set twice and
> > the longitude never. Fix this.
> 
> Eeep. Fine as long as you're on a very narrow line around the world
> where Y=X I suppose.

"Looking for libcamera developer. Has to be located where latitude
equals longitude". I'm not sure we'd get many candidates :-)

> But that doesn't cover much - so it's good that we get this fixed ;)
> 
> Reviewed-by: Kieran Bingham <kieran.bingham@ideasonboard.com>

Reviewed-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>

> > This is part of the fix that allows the following CTS test to pass:
> > - android.hardware.cts.CameraTest#testJpegExif
> > 
> > Signed-off-by: Paul Elder <paul.elder@ideasonboard.com>
> > ---
> >  src/android/jpeg/exif.cpp | 2 +-
> >  1 file changed, 1 insertion(+), 1 deletion(-)
> > 
> > diff --git a/src/android/jpeg/exif.cpp b/src/android/jpeg/exif.cpp
> > index c8b2f072..5225d17f 100644
> > --- a/src/android/jpeg/exif.cpp
> > +++ b/src/android/jpeg/exif.cpp
> > @@ -392,7 +392,7 @@ void Exif::setGPSLocation(const double *coords)
> >  	std::tie<int, int, int>(deg, min, sec) = degreesToDMS(coords[1]);
> >  	setString(EXIF_IFD_GPS, static_cast<ExifTag>(EXIF_TAG_GPS_LONGITUDE_REF),
> >  		  EXIF_FORMAT_ASCII, deg >= 0 ? "E" : "W");
> > -	setGPSDMS(EXIF_IFD_GPS, static_cast<ExifTag>(EXIF_TAG_GPS_LATITUDE),
> > +	setGPSDMS(EXIF_IFD_GPS, static_cast<ExifTag>(EXIF_TAG_GPS_LONGITUDE),
> >  		  std::abs(deg), min, sec);
> >  
> >  	setByte(EXIF_IFD_GPS, static_cast<ExifTag>(EXIF_TAG_GPS_ALTITUDE_REF),

On 08/03/2021 14:40, Laurent Pinchart wrote:
>> Eeep. Fine as long as you're on a very narrow line around the world
>> where Y=X I suppose.
> 
> "Looking for libcamera developer. Has to be located where latitude
> equals longitude". I'm not sure we'd get many candidates :-)

It's a niche role ;-)

They would also be required to have at least 5 years experience with
libcamera :-)