[libcamera-devel] [PATCH] src: android: exif: Set the class byte ordering
Umang Jain
email at uajain.com
Thu Sep 24 12:53:30 CEST 2020
Hi Kieran,
Thank you for the patch.
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.
It is indeed helpful and the idea has crossed my mind in the past but
you got to it first. :)
So, fine by me,
Reviewed-by: Umang Jain <email at uajain.com>
>
> 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_;
More information about the libcamera-devel
mailing list