[libcamera-devel,v2,2/4] android: jpeg: exif: Simplify setGPSDateTimestamp and setGPSDMS

11533 diff mbox series

Now that setRational() supports setting multiple rational values, use
that in setGPSDateTimestamp and setGPSDMS which previously set every
rational manually.

Signed-off-by: Paul Elder <paul.elder@ideasonboard.com>
Reviewed-by: Kieran Bingham <kieran.bingham@ideasonboard.com>
Reviewed-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>

---
Changes in v2:
- use Span
---
 src/android/jpeg/exif.cpp | 25 +++----------------------
 1 file changed, 3 insertions(+), 22 deletions(-)

Hi Paul,

On Tue, Mar 09, 2021 at 05:52:33PM +0900, Paul Elder wrote:
> Now that setRational() supports setting multiple rational values, use
> that in setGPSDateTimestamp and setGPSDMS which previously set every
> rational manually.
> 
> Signed-off-by: Paul Elder <paul.elder@ideasonboard.com>
> Reviewed-by: Kieran Bingham <kieran.bingham@ideasonboard.com>
> Reviewed-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
> 
> ---
> Changes in v2:
> - use Span
> ---
>  src/android/jpeg/exif.cpp | 25 +++----------------------
>  1 file changed, 3 insertions(+), 22 deletions(-)
> 
> diff --git a/src/android/jpeg/exif.cpp b/src/android/jpeg/exif.cpp
> index 929ac542..cf9e632a 100644
> --- a/src/android/jpeg/exif.cpp
> +++ b/src/android/jpeg/exif.cpp
> @@ -347,24 +347,14 @@ void Exif::setGPSDateTimestamp(time_t timestamp)
>  		  EXIF_FORMAT_ASCII, tsStr);
>  
>  	/* Set GPS_TIME_STAMP */
> -	ExifEntry *entry =
> -		createEntry(EXIF_IFD_GPS,
> -			    static_cast<ExifTag>(EXIF_TAG_GPS_TIME_STAMP),
> -			    EXIF_FORMAT_RATIONAL, 3, 3 * sizeof(ExifRational));
> -	if (!entry)
> -		return;
> -
>  	ExifRational ts[] = {
>  		{ static_cast<ExifLong>(tm.tm_hour), 1 },
>  		{ static_cast<ExifLong>(tm.tm_min),  1 },
>  		{ static_cast<ExifLong>(tm.tm_sec),  1 },
>  	};
>  
> -	for (int i = 0; i < 3; i++)
> -		exif_set_rational(entry->data + i * sizeof(ExifRational),
> -				  order_, ts[i]);
> -
> -	exif_entry_unref(entry);
> +	setRational(EXIF_IFD_GPS, static_cast<ExifTag>(EXIF_TAG_GPS_TIME_STAMP),
> +		    Span<const ExifRational>{ ts, 3 });

You can write

	setRational(EXIF_IFD_GPS, static_cast<ExifTag>(EXIF_TAG_GPS_TIME_STAMP),
		    ts);

Same below. Isn't Span magic ? :-)

>  }
>  
>  std::tuple<int, int, int> Exif::degreesToDMS(double decimalDegrees)
> @@ -378,22 +368,13 @@ std::tuple<int, int, int> Exif::degreesToDMS(double decimalDegrees)
>  
>  void Exif::setGPSDMS(ExifIfd ifd, ExifTag tag, int deg, int min, int sec)
>  {
> -	ExifEntry *entry = createEntry(ifd, tag, EXIF_FORMAT_RATIONAL, 3,
> -				       3 * sizeof(ExifRational));
> -	if (!entry)
> -		return;
> -
>  	ExifRational coords[] = {
>  		{ static_cast<ExifLong>(deg), 1 },
>  		{ static_cast<ExifLong>(min), 1 },
>  		{ static_cast<ExifLong>(sec), 1 },
>  	};
>  
> -	for (int i = 0; i < 3; i++)
> -		exif_set_rational(entry->data + i * sizeof(ExifRational),
> -				  order_, coords[i]);
> -
> -	exif_entry_unref(entry);
> +	setRational(ifd, tag, Span<const ExifRational>{ coords, 3 });
>  }
>  
>  /*

Hi Paul,

On Tue, Mar 09, 2021 at 05:52:33PM +0900, Paul Elder wrote:
> Now that setRational() supports setting multiple rational values, use
> that in setGPSDateTimestamp and setGPSDMS which previously set every
> rational manually.
>
> Signed-off-by: Paul Elder <paul.elder@ideasonboard.com>
> Reviewed-by: Kieran Bingham <kieran.bingham@ideasonboard.com>
> Reviewed-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
>

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

Thanks
  j

> ---
> Changes in v2:
> - use Span
> ---
>  src/android/jpeg/exif.cpp | 25 +++----------------------
>  1 file changed, 3 insertions(+), 22 deletions(-)
>
> diff --git a/src/android/jpeg/exif.cpp b/src/android/jpeg/exif.cpp
> index 929ac542..cf9e632a 100644
> --- a/src/android/jpeg/exif.cpp
> +++ b/src/android/jpeg/exif.cpp
> @@ -347,24 +347,14 @@ void Exif::setGPSDateTimestamp(time_t timestamp)
>  		  EXIF_FORMAT_ASCII, tsStr);
>
>  	/* Set GPS_TIME_STAMP */
> -	ExifEntry *entry =
> -		createEntry(EXIF_IFD_GPS,
> -			    static_cast<ExifTag>(EXIF_TAG_GPS_TIME_STAMP),
> -			    EXIF_FORMAT_RATIONAL, 3, 3 * sizeof(ExifRational));
> -	if (!entry)
> -		return;
> -
>  	ExifRational ts[] = {
>  		{ static_cast<ExifLong>(tm.tm_hour), 1 },
>  		{ static_cast<ExifLong>(tm.tm_min),  1 },
>  		{ static_cast<ExifLong>(tm.tm_sec),  1 },
>  	};
>
> -	for (int i = 0; i < 3; i++)
> -		exif_set_rational(entry->data + i * sizeof(ExifRational),
> -				  order_, ts[i]);
> -
> -	exif_entry_unref(entry);
> +	setRational(EXIF_IFD_GPS, static_cast<ExifTag>(EXIF_TAG_GPS_TIME_STAMP),
> +		    Span<const ExifRational>{ ts, 3 });
>  }
>
>  std::tuple<int, int, int> Exif::degreesToDMS(double decimalDegrees)
> @@ -378,22 +368,13 @@ std::tuple<int, int, int> Exif::degreesToDMS(double decimalDegrees)
>
>  void Exif::setGPSDMS(ExifIfd ifd, ExifTag tag, int deg, int min, int sec)
>  {
> -	ExifEntry *entry = createEntry(ifd, tag, EXIF_FORMAT_RATIONAL, 3,
> -				       3 * sizeof(ExifRational));
> -	if (!entry)
> -		return;
> -
>  	ExifRational coords[] = {
>  		{ static_cast<ExifLong>(deg), 1 },
>  		{ static_cast<ExifLong>(min), 1 },
>  		{ static_cast<ExifLong>(sec), 1 },
>  	};
>
> -	for (int i = 0; i < 3; i++)
> -		exif_set_rational(entry->data + i * sizeof(ExifRational),
> -				  order_, coords[i]);
> -
> -	exif_entry_unref(entry);
> +	setRational(ifd, tag, Span<const ExifRational>{ coords, 3 });
>  }
>
>  /*
> --
> 2.27.0
>
> _______________________________________________
> libcamera-devel mailing list
> libcamera-devel@lists.libcamera.org
> https://lists.libcamera.org/listinfo/libcamera-devel