[libcamera-devel] [PATCH] src: android: exif: Set the class byte ordering

[Umang Jain]

Hi,

Can we get additional R-b tags to merge these trivial changes?
Thanks.

On 9/24/20 3:20 PM, Kieran Bingham wrote:
> The exif object sets the byte ordering on construction, and then
> during later calls re-states the byte ordering when setting values.
>
> It could be argued that this ordering should already be known to the exif
> library and is redudant, but even so we must provide it.
>
> Ensure we are consistent in always using the same byte ordering by setting
> a private class member to re-use a single value.
>
> Signed-off-by: Kieran Bingham <kieran.bingham at ideasonboard.com>
> ---
>
> This patch itself feels like it might be a bit redundant, but I saw the
> repetition, and whilst we're not likely to change the byte order, I
> thought it might make sense to ensure it's always set from the same
> state variable.
>
> If this is helpful we can add it, but if this is overkill I don't mind
> dropping it. It's just an idea.
>
>   src/android/jpeg/exif.cpp | 11 ++++++-----
>   src/android/jpeg/exif.h   |  1 +
>   2 files changed, 7 insertions(+), 5 deletions(-)
>
> diff --git a/src/android/jpeg/exif.cpp b/src/android/jpeg/exif.cpp
> index 1ced55343ee9..f29a7c71d1ec 100644
> --- a/src/android/jpeg/exif.cpp
> +++ b/src/android/jpeg/exif.cpp
> @@ -25,7 +25,8 @@ LOG_DEFINE_CATEGORY(EXIF)
>    * data can be obtained using the data() method.
>    */
>   Exif::Exif()
> -	: valid_(false), data_(nullptr), exifData_(0), size_(0)
> +	: valid_(false), data_(nullptr), order_(EXIF_BYTE_ORDER_INTEL),
> +	  exifData_(0), size_(0)
>   {
>   	/* Create an ExifMem allocator to construct entries. */
>   	mem_ = exif_mem_new_default();
> @@ -49,7 +50,7 @@ Exif::Exif()
>   	 * Big-Endian: EXIF_BYTE_ORDER_MOTOROLA
>   	 * Little Endian: EXIF_BYTE_ORDER_INTEL
>   	 */
> -	exif_data_set_byte_order(data_, EXIF_BYTE_ORDER_INTEL);
> +	exif_data_set_byte_order(data_, order_);
>   
>   	/* Create the mandatory EXIF fields with default data. */
>   	exif_data_fix(data_);
> @@ -131,7 +132,7 @@ void Exif::setShort(ExifIfd ifd, ExifTag tag, uint16_t item)
>   	if (!entry)
>   		return;
>   
> -	exif_set_short(entry->data, EXIF_BYTE_ORDER_INTEL, item);
> +	exif_set_short(entry->data, order_, item);
>   	exif_entry_unref(entry);
>   }
>   
> @@ -141,7 +142,7 @@ void Exif::setLong(ExifIfd ifd, ExifTag tag, uint32_t item)
>   	if (!entry)
>   		return;
>   
> -	exif_set_long(entry->data, EXIF_BYTE_ORDER_INTEL, item);
> +	exif_set_long(entry->data, order_, item);
>   	exif_entry_unref(entry);
>   }
>   
> @@ -151,7 +152,7 @@ void Exif::setRational(ExifIfd ifd, ExifTag tag, ExifRational item)
>   	if (!entry)
>   		return;
>   
> -	exif_set_rational(entry->data, EXIF_BYTE_ORDER_INTEL, item);
> +	exif_set_rational(entry->data, order_, item);
>   	exif_entry_unref(entry);
>   }
>   
> diff --git a/src/android/jpeg/exif.h b/src/android/jpeg/exif.h
> index 622de4cfd593..6e8ce04a2e67 100644
> --- a/src/android/jpeg/exif.h
> +++ b/src/android/jpeg/exif.h
> @@ -46,6 +46,7 @@ private:
>   
>   	ExifData *data_;
>   	ExifMem *mem_;
> +	ExifByteOrder order_;
>   
>   	unsigned char *exifData_;
>   	unsigned int size_;