| Message ID | 20200924090631.18473-1-email@uajain.com |
|---|---|
| State | Accepted |
| Headers | show |
| Series |
|
| Related | show |
Hi Umang, Thank you for the patch. On Thu, Sep 24, 2020 at 02:36:31PM +0530, Umang Jain wrote: > The EXIF specification defines three timezone related tags namely, > OffsetTime, OffsetTimeOriginal and OffsetTimeDigitized. However, > these are not supported by libexif (as of v0.6.21) hence, carry > the tags' positional values in our implementation until we get > this support from libexif itself. > > Since these tags were introduced in EXIF specification v2.31, set > the exif version number explicitly too. > > Signed-off-by: Umang Jain <email@uajain.com> > --- > src/android/jpeg/exif.cpp | 29 +++++++++++++++++++++++++++++ > 1 file changed, 29 insertions(+) > > diff --git a/src/android/jpeg/exif.cpp b/src/android/jpeg/exif.cpp > index c0dbfcc..df38765 100644 > --- a/src/android/jpeg/exif.cpp > +++ b/src/android/jpeg/exif.cpp > @@ -13,6 +13,16 @@ using namespace libcamera; > > LOG_DEFINE_CATEGORY(EXIF) > > +/* > + * List of EXIF tags that we set directly because they are not supported > + * by libexif version 0.6.21. > + */ > +enum _ExifTag { I'd make this "enum class" as you always use the _ExifTag:: prefix below. > + OFFSET_TIME = 0x9010, > + OFFSET_TIME_ORIGINAL = 0x9011, > + OFFSET_TIME_DIGITIZED = 0x9012, > +}; > + > /* > * The Exif class should be instantiated and specific properties set > * through the exposed public API. > @@ -51,6 +61,9 @@ Exif::Exif() > */ > exif_data_set_byte_order(data_, EXIF_BYTE_ORDER_INTEL); > > + setString(EXIF_IFD_EXIF, EXIF_TAG_EXIF_VERSION, > + EXIF_FORMAT_UNDEFINED, "0231"); > + According to the EXIF standard, "Since the type is UNDEFINED, it shall not be terminated with NULL". How about modifying setString() to only include the NULL terminator when the type is ASCII ? And maybe splitting this to a separate patch ? Up to you. The rest looks good to me. > /* Create the mandatory EXIF fields with default data. */ > exif_data_fix(data_); > } > @@ -196,6 +209,22 @@ void Exif::setTimestamp(time_t timestamp) > setString(EXIF_IFD_0, EXIF_TAG_DATE_TIME, EXIF_FORMAT_ASCII, ts); > setString(EXIF_IFD_EXIF, EXIF_TAG_DATE_TIME_ORIGINAL, EXIF_FORMAT_ASCII, ts); > setString(EXIF_IFD_EXIF, EXIF_TAG_DATE_TIME_DIGITIZED, EXIF_FORMAT_ASCII, ts); > + > + /* Query and set timezone information if available. */ > + int r = strftime(str, sizeof(str), "%z", &tm); > + if (r > 0) { > + std::string tz(str); > + tz.insert(3, 1, ':'); > + setString(EXIF_IFD_EXIF, > + static_cast<ExifTag>(_ExifTag::OFFSET_TIME), > + EXIF_FORMAT_ASCII, tz); > + setString(EXIF_IFD_EXIF, > + static_cast<ExifTag>(_ExifTag::OFFSET_TIME_ORIGINAL), > + EXIF_FORMAT_ASCII, tz); > + setString(EXIF_IFD_EXIF, > + static_cast<ExifTag>(_ExifTag::OFFSET_TIME_DIGITIZED), > + EXIF_FORMAT_ASCII, tz); > + } > } > > void Exif::setOrientation(int orientation)
diff --git a/src/android/jpeg/exif.cpp b/src/android/jpeg/exif.cpp index c0dbfcc..df38765 100644 --- a/src/android/jpeg/exif.cpp +++ b/src/android/jpeg/exif.cpp @@ -13,6 +13,16 @@ using namespace libcamera; LOG_DEFINE_CATEGORY(EXIF) +/* + * List of EXIF tags that we set directly because they are not supported + * by libexif version 0.6.21. + */ +enum _ExifTag { + OFFSET_TIME = 0x9010, + OFFSET_TIME_ORIGINAL = 0x9011, + OFFSET_TIME_DIGITIZED = 0x9012, +}; + /* * The Exif class should be instantiated and specific properties set * through the exposed public API. @@ -51,6 +61,9 @@ Exif::Exif() */ exif_data_set_byte_order(data_, EXIF_BYTE_ORDER_INTEL); + setString(EXIF_IFD_EXIF, EXIF_TAG_EXIF_VERSION, + EXIF_FORMAT_UNDEFINED, "0231"); + /* Create the mandatory EXIF fields with default data. */ exif_data_fix(data_); } @@ -196,6 +209,22 @@ void Exif::setTimestamp(time_t timestamp) setString(EXIF_IFD_0, EXIF_TAG_DATE_TIME, EXIF_FORMAT_ASCII, ts); setString(EXIF_IFD_EXIF, EXIF_TAG_DATE_TIME_ORIGINAL, EXIF_FORMAT_ASCII, ts); setString(EXIF_IFD_EXIF, EXIF_TAG_DATE_TIME_DIGITIZED, EXIF_FORMAT_ASCII, ts); + + /* Query and set timezone information if available. */ + int r = strftime(str, sizeof(str), "%z", &tm); + if (r > 0) { + std::string tz(str); + tz.insert(3, 1, ':'); + setString(EXIF_IFD_EXIF, + static_cast<ExifTag>(_ExifTag::OFFSET_TIME), + EXIF_FORMAT_ASCII, tz); + setString(EXIF_IFD_EXIF, + static_cast<ExifTag>(_ExifTag::OFFSET_TIME_ORIGINAL), + EXIF_FORMAT_ASCII, tz); + setString(EXIF_IFD_EXIF, + static_cast<ExifTag>(_ExifTag::OFFSET_TIME_DIGITIZED), + EXIF_FORMAT_ASCII, tz); + } } void Exif::setOrientation(int orientation)
The EXIF specification defines three timezone related tags namely, OffsetTime, OffsetTimeOriginal and OffsetTimeDigitized. However, these are not supported by libexif (as of v0.6.21) hence, carry the tags' positional values in our implementation until we get this support from libexif itself. Since these tags were introduced in EXIF specification v2.31, set the exif version number explicitly too. Signed-off-by: Umang Jain <email@uajain.com> --- src/android/jpeg/exif.cpp | 29 +++++++++++++++++++++++++++++ 1 file changed, 29 insertions(+)