[libcamera-devel,v2,4/4] android: jpeg: exif: change GPS method encoding from ASCII to NoEncoding

11535 diff mbox series

According to the EXIF specification, the GPS method should be UNDEFINED,
and the first 8 bytes will designate the type. However, CTS expects the
first 8 bytes to be part of the data. Remove the 8-byte encoding
designator by changing the encoding to NoEncoding 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>

---
Changes in v2:
- keep UNDEFINED instead of changing it to ASCII, and use NoEncoding
---
 src/android/jpeg/exif.cpp | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

Hi Paul,

Thank you for the patch.

On Tue, Mar 09, 2021 at 05:52:35PM +0900, 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
> first 8 bytes to be part of the data. Remove the 8-byte encoding
> designator by changing the encoding to NoEncoding 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>

Exif is such an amazing specification. I don't see a way to find an
authoritative answer to the question of what we should do here, so being
CTS-compliant is probably as good as it gets.

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

> ---
> Changes in v2:
> - keep UNDEFINED instead of changing it to ASCII, and use NoEncoding
> ---
>  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 628382a9..7e47c1ac 100644
> --- a/src/android/jpeg/exif.cpp
> +++ b/src/android/jpeg/exif.cpp
> @@ -407,7 +407,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_UNDEFINED, method, NoEncoding);
>  }
>  
>  void Exif::setOrientation(int orientation)

Hi Paul,

On Tue, Mar 09, 2021 at 05:52:35PM +0900, 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
> first 8 bytes to be part of the data. Remove the 8-byte encoding
> designator by changing the encoding to NoEncoding 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>
>

I'm not sure I fully get the reasons behind this, but as long as it
fixes one more CTS test, I'm happy

Reviewed-by: Jacopo Mondi <jacopo@jmondi.org>

Thanks
   j

> ---
> Changes in v2:
> - keep UNDEFINED instead of changing it to ASCII, and use NoEncoding
> ---
>  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 628382a9..7e47c1ac 100644
> --- a/src/android/jpeg/exif.cpp
> +++ b/src/android/jpeg/exif.cpp
> @@ -407,7 +407,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_UNDEFINED, method, NoEncoding);
>  }
>
>  void Exif::setOrientation(int orientation)
> --
> 2.27.0
>
> _______________________________________________
> libcamera-devel mailing list
> libcamera-devel@lists.libcamera.org
> https://lists.libcamera.org/listinfo/libcamera-devel