[libcamera-devel,4/4] android: jpeg: exif: change GPS method from UNDEFINED format to ASCII
diff mbox series

Message ID 20210308101356.59333-5-paul.elder@ideasonboard.com
State Superseded
Headers show
Series
  • android: jpeg: exif: Fix GPS altitude
Related show

Commit Message

Paul Elder March 8, 2021, 10:13 a.m. UTC
According to the EXIF specification, the GPS method should be UNDEFINED,
and the first 8 bytes will designate the type. However, CTS expects the
GPS method to be ASCII. Change the format to ASCII to appease CTS.

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(-)

Comments

Kieran Bingham March 8, 2021, 10:26 a.m. UTC | #1
Hi Paul,

On 08/03/2021 10:13, Paul Elder wrote:
> According to the EXIF specification, the GPS method should be UNDEFINED,
> and the first 8 bytes will designate the type. However, CTS expects the
> GPS method to be ASCII. Change the format to ASCII to appease CTS.

If the Spec says it should be UNDEFINED, doesn't that mean that the CTS
is wrong? (and should be fixed there?)

> 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 5225d17f..83e6faba 100644
> --- a/src/android/jpeg/exif.cpp
> +++ b/src/android/jpeg/exif.cpp
> @@ -404,7 +404,7 @@ void Exif::setGPSLocation(const double *coords)
>  void Exif::setGPSMethod(const std::string &method)
>  {
>  	setString(EXIF_IFD_GPS, static_cast<ExifTag>(EXIF_TAG_GPS_PROCESSING_METHOD),
> -		  EXIF_FORMAT_UNDEFINED, method, Unicode);
> +		  EXIF_FORMAT_ASCII, method);
>  }
>  
>  void Exif::setOrientation(int orientation)
>
Laurent Pinchart March 8, 2021, 2:48 p.m. UTC | #2
Hi Paul,

Thank you for the patch.

On Mon, Mar 08, 2021 at 10:26:56AM +0000, Kieran Bingham wrote:
> On 08/03/2021 10:13, Paul Elder wrote:
> > According to the EXIF specification, the GPS method should be UNDEFINED,
> > and the first 8 bytes will designate the type. However, CTS expects the
> > GPS method to be ASCII. Change the format to ASCII to appease CTS.
> 
> If the Spec says it should be UNDEFINED, doesn't that mean that the CTS
> is wrong? (and should be fixed there?)

Does CTS check that the tag type is set to ASCII rather than UNDEFINED,
or does it fail because it it doesn't expect the 8-bytes encoding prefix
? In the latter case, we could switch to the NoEncoding encoding and
keep the UNDEFINED type.

The Exif spec is very confusing here, it documents the tag as UNDEFINED,
but then shows a few examples where it mentions ASCII. It's not clear if
ASCII refers to the tag type or the encoding here.

> > 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 5225d17f..83e6faba 100644
> > --- a/src/android/jpeg/exif.cpp
> > +++ b/src/android/jpeg/exif.cpp
> > @@ -404,7 +404,7 @@ void Exif::setGPSLocation(const double *coords)
> >  void Exif::setGPSMethod(const std::string &method)
> >  {
> >  	setString(EXIF_IFD_GPS, static_cast<ExifTag>(EXIF_TAG_GPS_PROCESSING_METHOD),
> > -		  EXIF_FORMAT_UNDEFINED, method, Unicode);
> > +		  EXIF_FORMAT_ASCII, method);
> >  }
> >  
> >  void Exif::setOrientation(int orientation)

Patch
diff mbox series

diff --git a/src/android/jpeg/exif.cpp b/src/android/jpeg/exif.cpp
index 5225d17f..83e6faba 100644
--- a/src/android/jpeg/exif.cpp
+++ b/src/android/jpeg/exif.cpp
@@ -404,7 +404,7 @@  void Exif::setGPSLocation(const double *coords)
 void Exif::setGPSMethod(const std::string &method)
 {
 	setString(EXIF_IFD_GPS, static_cast<ExifTag>(EXIF_TAG_GPS_PROCESSING_METHOD),
-		  EXIF_FORMAT_UNDEFINED, method, Unicode);
+		  EXIF_FORMAT_ASCII, method);
 }
 
 void Exif::setOrientation(int orientation)