[libcamera-devel] [PATCH v2 1/4] android: jpeg: exif: Fix and expand setRational

[Jacopo Mondi]

On Tue, Mar 09, 2021 at 05:52:32PM +0900, Paul Elder wrote:
> setRational was not working properly for EXIF tags in the GPS IFD due to
> libexif not supporting those tags in exif_entry_initialize().  Manually
> specify the size of the EXIF entry to fix this. While at it, add support
> for setting multiple rationals, as that is a common use case for
> rational EXIF tags.
>
> As Rational types are no longer initialized by libexif directly, the
> EXIF_TAG_{X,Y}_RESOLUTION exif tags will not have their default values
> populated.
>
> This allows the GPS altitude to be set properly, and is part of the fix
> to allow the following CTS test to pass:
> - android.hardware.cts.CameraTest#testJpegExif
>
> Signed-off-by: Paul Elder <paul.elder at ideasonboard.com>
> Reviewed-by: Kieran Bingham <kieran.bingham at ideasonboard.com>
> Reviewed-by: Laurent Pinchart <laurent.pinchart at ideasonboard.com>
>
> ---
> Changes in v2:
> - use span instead of size + array
> - expand commit message
> ---
>  src/android/jpeg/exif.cpp | 15 +++++++++++++--
>  src/android/jpeg/exif.h   |  2 ++
>  2 files changed, 15 insertions(+), 2 deletions(-)
>
> diff --git a/src/android/jpeg/exif.cpp b/src/android/jpeg/exif.cpp
> index fdd46070..929ac542 100644
> --- a/src/android/jpeg/exif.cpp
> +++ b/src/android/jpeg/exif.cpp
> @@ -14,6 +14,8 @@
>  #include <tuple>
>  #include <uchar.h>
>
> +#include <libcamera/span.h>
> +

span.h is already included in exif.h so you can drop it here I guess

>  #include "libcamera/internal/log.h"
>  #include "libcamera/internal/utils.h"
>
> @@ -187,11 +189,20 @@ void Exif::setLong(ExifIfd ifd, ExifTag tag, uint32_t item)
>
>  void Exif::setRational(ExifIfd ifd, ExifTag tag, ExifRational item)
>  {
> -	ExifEntry *entry = createEntry(ifd, tag);
> +	setRational(ifd, tag, Span<const ExifRational>{ &item, 1 });

and you can write this as
	setRational(ifd, tag, { &item, 1 });

according to my compiler :)

The rest looks good
Reviewed-by: Jacopo Mondi <jacopo at jmondi.org>

Thanks
  j

> +}
> +
> +void Exif::setRational(ExifIfd ifd, ExifTag tag, Span<const ExifRational> items)
> +{
> +	ExifEntry *entry = createEntry(ifd, tag, EXIF_FORMAT_RATIONAL,
> +				       items.size(),
> +				       items.size() * sizeof(ExifRational));
>  	if (!entry)
>  		return;
>
> -	exif_set_rational(entry->data, order_, item);
> +	for (size_t i = 0; i < items.size(); i++)
> +		exif_set_rational(entry->data + i * sizeof(ExifRational),
> +				  order_, items[i]);
>  	exif_entry_unref(entry);
>  }
>
> diff --git a/src/android/jpeg/exif.h b/src/android/jpeg/exif.h
> index b0d61402..8aa1b123 100644
> --- a/src/android/jpeg/exif.h
> +++ b/src/android/jpeg/exif.h
> @@ -89,6 +89,8 @@ private:
>  		       const std::string &item,
>  		       StringEncoding encoding = NoEncoding);
>  	void setRational(ExifIfd ifd, ExifTag tag, ExifRational item);
> +	void setRational(ExifIfd ifd, ExifTag tag,
> +			 libcamera::Span<const ExifRational> items);
>
>  	std::tuple<int, int, int> degreesToDMS(double decimalDegrees);
>  	void setGPSDMS(ExifIfd ifd, ExifTag tag, int deg, int min, int sec);
> --
> 2.27.0
>
> _______________________________________________
> libcamera-devel mailing list
> libcamera-devel at lists.libcamera.org
> https://lists.libcamera.org/listinfo/libcamera-devel