| Message ID | 20200909153206.7700-1-email@uajain.com |
|---|---|
| State | Accepted |
| Headers | show |
| Series |
|
| Related | show |
Hi Umang, Thank you for the patch. On Wed, Sep 09, 2020 at 09:02:06PM +0530, Umang Jain wrote: > Get timestamp information from the timestamp, although the resolution s/timestamp information/timezone information/ > for EXIF_TAG_TIME_ZONE_OFFSET is fairly limtied (per-hour only). s/limtied/limited/ > > Experimentation with 'exiftool', commandline utility to read/write > exif metadata on images, resulted in rounding off the hours if the > minutes came out to >= 30. Hence, the behaviour is inspired from > exiftool itself. For e.g., > > Timezone Tag's value > +1015 => 10 > +0945 => 10 > -1145 => -12 Sounds good to me. > Signed-off-by: Umang Jain <email@uajain.com> > --- > src/android/jpeg/exif.cpp | 30 ++++++++++++++++++++++++++++++ > src/android/jpeg/exif.h | 1 + > 2 files changed, 31 insertions(+) > > diff --git a/src/android/jpeg/exif.cpp b/src/android/jpeg/exif.cpp > index 031f5f4..d8b4537 100644 > --- a/src/android/jpeg/exif.cpp > +++ b/src/android/jpeg/exif.cpp > @@ -135,6 +135,16 @@ void Exif::setShort(ExifIfd ifd, ExifTag tag, uint16_t item) > exif_entry_unref(entry); > } > > +void Exif::setSShort(ExifIfd ifd, ExifTag tag, int16_t item) > +{ > + ExifEntry *entry = createEntry(ifd, tag); > + if (!entry) > + return; > + > + exif_set_sshort(entry->data, EXIF_BYTE_ORDER_INTEL, item); > + exif_entry_unref(entry); > +} > + > void Exif::setLong(ExifIfd ifd, ExifTag tag, uint32_t item) > { > ExifEntry *entry = createEntry(ifd, tag); > @@ -194,6 +204,26 @@ 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); > + > + /* If possible, query and set timezone information via /* * If possible ... > + * EXIF_TAG_TIME_ZONE_OFFSET. There is only per-hour resolution for tag > + * hence, round up hours if minutes >= 30. > + */ > + char tz[6]; How about reusing the str variable ? > + int r = std::strftime(tz, sizeof(tz), "%z", std::localtime(×tamp)); I've posted "[PATCH] android: jpeg: exif: Use reentrant localtime_r()" which adds a local tm variable that you can reuse instead of calling std::localtime() again. > + if (r > 0) { > + int16_t timezone = atoi(tz); You should include stdlib.h for atoi. > + int16_t hour = timezone / 100; > + int16_t min = abs(timezone) - (abs(hour) * 100); > + > + if (tz[0] == '-' && min >= 30) > + hour--; > + else if (min >= 30) > + hour++; An alternative would be int16_t timezone = atoi(tz); int16_t hour = timezone / 100; int16_t min = timezone % 100; if (min <= -30) hour--; else if (min >= 30) hour++; This relies on the fact that the modulo of a negative number will be a negative number. > + > + /* \todo Check if EXIF IFD is correct here or not. */ It's messy. The TIFF/EP specification defines the TimeZoneOffset tag in IFD0, so that's where I think it should go. The EXIF specification defines three other tags (OffsetTime, OffsetTimeOriginal, OffsetTimeDigitized), in the EXIF IFD, but those are not supported by libexif. Should we mention the unsupported tags in the commit message ? Note that the DateTimeOriginal tag is defined by both the TIFF/EP and EXIF specifications, but in different IFDs (IFD 0 for TIFF/EP and EXIF IFD for EXIF). Lovely... > + setSShort(EXIF_IFD_EXIF, EXIF_TAG_TIME_ZONE_OFFSET, hour); The TIFF/EP specification defined TimeZoneOffset as containing up to two signed short values. The first value relates to DateTime, the second value to DateTimeOriginal. As we don't set DateTimeOriginal in IFD0 I would leave the second value out, but if we added DateTimeOriginal in IFD0 in addition to the EXIF IFD, I would add the second value. > + } > } > > void Exif::setOrientation(int orientation) > diff --git a/src/android/jpeg/exif.h b/src/android/jpeg/exif.h > index 622de4c..4817815 100644 > --- a/src/android/jpeg/exif.h > +++ b/src/android/jpeg/exif.h > @@ -37,6 +37,7 @@ private: > unsigned long components, unsigned int size); > > void setShort(ExifIfd ifd, ExifTag tag, uint16_t item); > + void setSShort(ExifIfd ifd, ExifTag tag, int16_t item); > void setLong(ExifIfd ifd, ExifTag tag, uint32_t item); > void setString(ExifIfd ifd, ExifTag tag, ExifFormat format, > const std::string &item);
Hi Umang, On Thu, Sep 10, 2020 at 07:25:47AM +0300, Laurent Pinchart wrote: > Hi Umang, > > Thank you for the patch. > > On Wed, Sep 09, 2020 at 09:02:06PM +0530, Umang Jain wrote: > > Get timestamp information from the timestamp, although the resolution > > s/timestamp information/timezone information/ > > > for EXIF_TAG_TIME_ZONE_OFFSET is fairly limtied (per-hour only). > > s/limtied/limited/ > > > Experimentation with 'exiftool', commandline utility to read/write > > exif metadata on images, resulted in rounding off the hours if the > > minutes came out to >= 30. Hence, the behaviour is inspired from > > exiftool itself. For e.g., > > > > Timezone Tag's value > > +1015 => 10 > > +0945 => 10 > > -1145 => -12 > > Sounds good to me. > > > Signed-off-by: Umang Jain <email@uajain.com> > > --- > > src/android/jpeg/exif.cpp | 30 ++++++++++++++++++++++++++++++ > > src/android/jpeg/exif.h | 1 + > > 2 files changed, 31 insertions(+) > > > > diff --git a/src/android/jpeg/exif.cpp b/src/android/jpeg/exif.cpp > > index 031f5f4..d8b4537 100644 > > --- a/src/android/jpeg/exif.cpp > > +++ b/src/android/jpeg/exif.cpp > > @@ -135,6 +135,16 @@ void Exif::setShort(ExifIfd ifd, ExifTag tag, uint16_t item) > > exif_entry_unref(entry); > > } > > > > +void Exif::setSShort(ExifIfd ifd, ExifTag tag, int16_t item) > > +{ > > + ExifEntry *entry = createEntry(ifd, tag); > > + if (!entry) > > + return; > > + > > + exif_set_sshort(entry->data, EXIF_BYTE_ORDER_INTEL, item); > > + exif_entry_unref(entry); > > +} > > + > > void Exif::setLong(ExifIfd ifd, ExifTag tag, uint32_t item) > > { > > ExifEntry *entry = createEntry(ifd, tag); > > @@ -194,6 +204,26 @@ 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); > > + > > + /* If possible, query and set timezone information via > > /* > * If possible ... > > > + * EXIF_TAG_TIME_ZONE_OFFSET. There is only per-hour resolution for tag > > + * hence, round up hours if minutes >= 30. > > + */ > > + char tz[6]; > > How about reusing the str variable ? > > > + int r = std::strftime(tz, sizeof(tz), "%z", std::localtime(×tamp)); > > I've posted "[PATCH] android: jpeg: exif: Use reentrant localtime_r()" > which adds a local tm variable that you can reuse instead of calling > std::localtime() again. I've now pushed that patch to master. Could you rebase this one ? > > + if (r > 0) { > > + int16_t timezone = atoi(tz); > > You should include stdlib.h for atoi. > > > + int16_t hour = timezone / 100; > > + int16_t min = abs(timezone) - (abs(hour) * 100); > > + > > + if (tz[0] == '-' && min >= 30) > > + hour--; > > + else if (min >= 30) > > + hour++; > > An alternative would be > > int16_t timezone = atoi(tz); > int16_t hour = timezone / 100; > int16_t min = timezone % 100; > > if (min <= -30) > hour--; > else if (min >= 30) > hour++; > > This relies on the fact that the modulo of a negative number will be a > negative number. > > > + > > + /* \todo Check if EXIF IFD is correct here or not. */ > > It's messy. The TIFF/EP specification defines the TimeZoneOffset tag in > IFD0, so that's where I think it should go. I've also checked how libexif and libexiv2 handle reading this tag (through the exif and exiv2 command line utilities respectively). libexif seems to parse it fine regardless of whether it's stored in IFD0 or in int EXIF IFD, while libexiv2 seem to only handle it when stored in IFD0 (the tag isn't printed when stored in the EXIF IFD). IFD0 thus seems to be the right pick. > The EXIF specification > defines three other tags (OffsetTime, OffsetTimeOriginal, > OffsetTimeDigitized), in the EXIF IFD, but those are not supported by > libexif. Should we mention the unsupported tags in the commit message ? > > Note that the DateTimeOriginal tag is defined by both the TIFF/EP and > EXIF specifications, but in different IFDs (IFD 0 for TIFF/EP and EXIF > IFD for EXIF). Lovely... > > > + setSShort(EXIF_IFD_EXIF, EXIF_TAG_TIME_ZONE_OFFSET, hour); > > The TIFF/EP specification defined TimeZoneOffset as containing up to two > signed short values. The first value relates to DateTime, the second > value to DateTimeOriginal. As we don't set DateTimeOriginal in IFD0 I > would leave the second value out, but if we added DateTimeOriginal in > IFD0 in addition to the EXIF IFD, I would add the second value. > > > + } > > } > > > > void Exif::setOrientation(int orientation) > > diff --git a/src/android/jpeg/exif.h b/src/android/jpeg/exif.h > > index 622de4c..4817815 100644 > > --- a/src/android/jpeg/exif.h > > +++ b/src/android/jpeg/exif.h > > @@ -37,6 +37,7 @@ private: > > unsigned long components, unsigned int size); > > > > void setShort(ExifIfd ifd, ExifTag tag, uint16_t item); > > + void setSShort(ExifIfd ifd, ExifTag tag, int16_t item); > > void setLong(ExifIfd ifd, ExifTag tag, uint32_t item); > > void setString(ExifIfd ifd, ExifTag tag, ExifFormat format, > > const std::string &item);
Hi Laurent, On 9/10/20 9:55 AM, Laurent Pinchart wrote: > Hi Umang, > > Thank you for the patch. > > On Wed, Sep 09, 2020 at 09:02:06PM +0530, Umang Jain wrote: >> Get timestamp information from the timestamp, although the resolution > s/timestamp information/timezone information/ > >> for EXIF_TAG_TIME_ZONE_OFFSET is fairly limtied (per-hour only). > s/limtied/limited/ > >> Experimentation with 'exiftool', commandline utility to read/write >> exif metadata on images, resulted in rounding off the hours if the >> minutes came out to >= 30. Hence, the behaviour is inspired from >> exiftool itself. For e.g., >> >> Timezone Tag's value >> +1015 => 10 >> +0945 => 10 >> -1145 => -12 > Sounds good to me. > >> Signed-off-by: Umang Jain <email@uajain.com> >> --- >> src/android/jpeg/exif.cpp | 30 ++++++++++++++++++++++++++++++ >> src/android/jpeg/exif.h | 1 + >> 2 files changed, 31 insertions(+) >> >> diff --git a/src/android/jpeg/exif.cpp b/src/android/jpeg/exif.cpp >> index 031f5f4..d8b4537 100644 >> --- a/src/android/jpeg/exif.cpp >> +++ b/src/android/jpeg/exif.cpp >> @@ -135,6 +135,16 @@ void Exif::setShort(ExifIfd ifd, ExifTag tag, uint16_t item) >> exif_entry_unref(entry); >> } >> >> +void Exif::setSShort(ExifIfd ifd, ExifTag tag, int16_t item) >> +{ >> + ExifEntry *entry = createEntry(ifd, tag); >> + if (!entry) >> + return; >> + >> + exif_set_sshort(entry->data, EXIF_BYTE_ORDER_INTEL, item); >> + exif_entry_unref(entry); >> +} >> + >> void Exif::setLong(ExifIfd ifd, ExifTag tag, uint32_t item) >> { >> ExifEntry *entry = createEntry(ifd, tag); >> @@ -194,6 +204,26 @@ 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); >> + >> + /* If possible, query and set timezone information via > /* > * If possible ... > >> + * EXIF_TAG_TIME_ZONE_OFFSET. There is only per-hour resolution for tag >> + * hence, round up hours if minutes >= 30. >> + */ >> + char tz[6]; > How about reusing the str variable ? > >> + int r = std::strftime(tz, sizeof(tz), "%z", std::localtime(×tamp)); > I've posted "[PATCH] android: jpeg: exif: Use reentrant localtime_r()" > which adds a local tm variable that you can reuse instead of calling > std::localtime() again. > >> + if (r > 0) { >> + int16_t timezone = atoi(tz); > You should include stdlib.h for atoi. > >> + int16_t hour = timezone / 100; >> + int16_t min = abs(timezone) - (abs(hour) * 100); >> + >> + if (tz[0] == '-' && min >= 30) >> + hour--; >> + else if (min >= 30) >> + hour++; > An alternative would be > > int16_t timezone = atoi(tz); > int16_t hour = timezone / 100; > int16_t min = timezone % 100; > > if (min <= -30) > hour--; > else if (min >= 30) > hour++; > > This relies on the fact that the modulo of a negative number will be a > negative number. > >> + >> + /* \todo Check if EXIF IFD is correct here or not. */ > It's messy. The TIFF/EP specification defines the TimeZoneOffset tag in > IFD0, so that's where I think it should go. The EXIF specification > defines three other tags (OffsetTime, OffsetTimeOriginal, > OffsetTimeDigitized), in the EXIF IFD, but those are not supported by > libexif. Should we mention the unsupported tags in the commit message ? Sure. I think it would be worth it next time someone is touching this piece of code. > > Note that the DateTimeOriginal tag is defined by both the TIFF/EP and > EXIF specifications, but in different IFDs (IFD 0 for TIFF/EP and EXIF > IFD for EXIF). Lovely... It's weird how these specs conflict when one (EXIF) is a reduced subset of another (TIFF/EP). > >> + setSShort(EXIF_IFD_EXIF, EXIF_TAG_TIME_ZONE_OFFSET, hour); > The TIFF/EP specification defined TimeZoneOffset as containing up to two > signed short values. The first value relates to DateTime, the second > value to DateTimeOriginal. As we don't set DateTimeOriginal in IFD0 I > would leave the second value out, but if we added DateTimeOriginal in > IFD0 in addition to the EXIF IFD, I would add the second value. I agree with your analysis. Can I ask when/how would we add DateTimeOriginal in IFD0? I guess when we start supporting TIFF-compliant metadata via libtiff? Also, this feels important decision/information. Should it be also considered to be mentioned as part of commit message? > >> + } >> } >> >> void Exif::setOrientation(int orientation) >> diff --git a/src/android/jpeg/exif.h b/src/android/jpeg/exif.h >> index 622de4c..4817815 100644 >> --- a/src/android/jpeg/exif.h >> +++ b/src/android/jpeg/exif.h >> @@ -37,6 +37,7 @@ private: >> unsigned long components, unsigned int size); >> >> void setShort(ExifIfd ifd, ExifTag tag, uint16_t item); >> + void setSShort(ExifIfd ifd, ExifTag tag, int16_t item); >> void setLong(ExifIfd ifd, ExifTag tag, uint32_t item); >> void setString(ExifIfd ifd, ExifTag tag, ExifFormat format, >> const std::string &item);
Hi Umang, On Tue, Sep 22, 2020 at 05:13:39PM +0530, Umang Jain wrote: > On 9/10/20 9:55 AM, Laurent Pinchart wrote: > > On Wed, Sep 09, 2020 at 09:02:06PM +0530, Umang Jain wrote: > >> Get timestamp information from the timestamp, although the resolution > > > > s/timestamp information/timezone information/ > > > >> for EXIF_TAG_TIME_ZONE_OFFSET is fairly limtied (per-hour only). > > > > s/limtied/limited/ > > > >> Experimentation with 'exiftool', commandline utility to read/write > >> exif metadata on images, resulted in rounding off the hours if the > >> minutes came out to >= 30. Hence, the behaviour is inspired from > >> exiftool itself. For e.g., > >> > >> Timezone Tag's value > >> +1015 => 10 > >> +0945 => 10 > >> -1145 => -12 > > > > Sounds good to me. > > > >> Signed-off-by: Umang Jain <email@uajain.com> > >> --- > >> src/android/jpeg/exif.cpp | 30 ++++++++++++++++++++++++++++++ > >> src/android/jpeg/exif.h | 1 + > >> 2 files changed, 31 insertions(+) > >> > >> diff --git a/src/android/jpeg/exif.cpp b/src/android/jpeg/exif.cpp > >> index 031f5f4..d8b4537 100644 > >> --- a/src/android/jpeg/exif.cpp > >> +++ b/src/android/jpeg/exif.cpp > >> @@ -135,6 +135,16 @@ void Exif::setShort(ExifIfd ifd, ExifTag tag, uint16_t item) > >> exif_entry_unref(entry); > >> } > >> > >> +void Exif::setSShort(ExifIfd ifd, ExifTag tag, int16_t item) > >> +{ > >> + ExifEntry *entry = createEntry(ifd, tag); > >> + if (!entry) > >> + return; > >> + > >> + exif_set_sshort(entry->data, EXIF_BYTE_ORDER_INTEL, item); > >> + exif_entry_unref(entry); > >> +} > >> + > >> void Exif::setLong(ExifIfd ifd, ExifTag tag, uint32_t item) > >> { > >> ExifEntry *entry = createEntry(ifd, tag); > >> @@ -194,6 +204,26 @@ 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); > >> + > >> + /* If possible, query and set timezone information via > > > > /* > > * If possible ... > > > >> + * EXIF_TAG_TIME_ZONE_OFFSET. There is only per-hour resolution for tag > >> + * hence, round up hours if minutes >= 30. > >> + */ > >> + char tz[6]; > > > > How about reusing the str variable ? > > > >> + int r = std::strftime(tz, sizeof(tz), "%z", std::localtime(×tamp)); > > > > I've posted "[PATCH] android: jpeg: exif: Use reentrant localtime_r()" > > which adds a local tm variable that you can reuse instead of calling > > std::localtime() again. > > > >> + if (r > 0) { > >> + int16_t timezone = atoi(tz); > > > > You should include stdlib.h for atoi. > > > >> + int16_t hour = timezone / 100; > >> + int16_t min = abs(timezone) - (abs(hour) * 100); > >> + > >> + if (tz[0] == '-' && min >= 30) > >> + hour--; > >> + else if (min >= 30) > >> + hour++; > > > > An alternative would be > > > > int16_t timezone = atoi(tz); > > int16_t hour = timezone / 100; > > int16_t min = timezone % 100; > > > > if (min <= -30) > > hour--; > > else if (min >= 30) > > hour++; > > > > This relies on the fact that the modulo of a negative number will be a > > negative number. > > > >> + > >> + /* \todo Check if EXIF IFD is correct here or not. */ > > > > It's messy. The TIFF/EP specification defines the TimeZoneOffset tag in > > IFD0, so that's where I think it should go. The EXIF specification > > defines three other tags (OffsetTime, OffsetTimeOriginal, > > OffsetTimeDigitized), in the EXIF IFD, but those are not supported by > > libexif. Should we mention the unsupported tags in the commit message ? > > Sure. I think it would be worth it next time someone is touching this > piece of code. > > > Note that the DateTimeOriginal tag is defined by both the TIFF/EP and > > EXIF specifications, but in different IFDs (IFD 0 for TIFF/EP and EXIF > > IFD for EXIF). Lovely... > > It's weird how these specs conflict when one (EXIF) is a reduced subset > of another (TIFF/EP). Standardization is amazing, right ? :-) > >> + setSShort(EXIF_IFD_EXIF, EXIF_TAG_TIME_ZONE_OFFSET, hour); > > > > The TIFF/EP specification defined TimeZoneOffset as containing up to two > > signed short values. The first value relates to DateTime, the second > > value to DateTimeOriginal. As we don't set DateTimeOriginal in IFD0 I > > would leave the second value out, but if we added DateTimeOriginal in > > IFD0 in addition to the EXIF IFD, I would add the second value. > > I agree with your analysis. Can I ask when/how would we add > DateTimeOriginal in IFD0? I guess when we start supporting > TIFF-compliant metadata via libtiff? The EXIF specification is quite unclear here, it lists DateTimeOriginal as a tag in the EXIF IFD in table 7 ("Exif IFD Attribute Information" in section 4.6.5), but also in table 18 ("Tag Support Levels (2) - 0th IFD Exif Private Tags" in section 4.6.8). Setting DateTimeOriginal in IFD0 in a TIFF/EP file is valid, setting it in a JFIF+EXIF file in IFD0 may or may not be valid. The exif command line tool allows setting DateTimeOriginal in both IFD0 and the EXIF IFD. It however prints the latter only, and doesn't find DateTimeOriginal in IFD0 when specifically asked for it. exiv2 will print both. I think we can probably ignore this for now, keep DateTimeOriginal in the EXIF IFD, with a single value in TimeZoneOffset. > Also, this feels important decision/information. Should it be also > considered to be mentioned as part of commit message? Feel free to record how desperate we are with the TIFF/EP and EXIF specs in the commit message :-) > >> + } > >> } > >> > >> void Exif::setOrientation(int orientation) > >> diff --git a/src/android/jpeg/exif.h b/src/android/jpeg/exif.h > >> index 622de4c..4817815 100644 > >> --- a/src/android/jpeg/exif.h > >> +++ b/src/android/jpeg/exif.h > >> @@ -37,6 +37,7 @@ private: > >> unsigned long components, unsigned int size); > >> > >> void setShort(ExifIfd ifd, ExifTag tag, uint16_t item); > >> + void setSShort(ExifIfd ifd, ExifTag tag, int16_t item); > >> void setLong(ExifIfd ifd, ExifTag tag, uint32_t item); > >> void setString(ExifIfd ifd, ExifTag tag, ExifFormat format, > >> const std::string &item);
diff --git a/src/android/jpeg/exif.cpp b/src/android/jpeg/exif.cpp index 031f5f4..d8b4537 100644 --- a/src/android/jpeg/exif.cpp +++ b/src/android/jpeg/exif.cpp @@ -135,6 +135,16 @@ void Exif::setShort(ExifIfd ifd, ExifTag tag, uint16_t item) exif_entry_unref(entry); } +void Exif::setSShort(ExifIfd ifd, ExifTag tag, int16_t item) +{ + ExifEntry *entry = createEntry(ifd, tag); + if (!entry) + return; + + exif_set_sshort(entry->data, EXIF_BYTE_ORDER_INTEL, item); + exif_entry_unref(entry); +} + void Exif::setLong(ExifIfd ifd, ExifTag tag, uint32_t item) { ExifEntry *entry = createEntry(ifd, tag); @@ -194,6 +204,26 @@ 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); + + /* If possible, query and set timezone information via + * EXIF_TAG_TIME_ZONE_OFFSET. There is only per-hour resolution for tag + * hence, round up hours if minutes >= 30. + */ + char tz[6]; + int r = std::strftime(tz, sizeof(tz), "%z", std::localtime(×tamp)); + if (r > 0) { + int16_t timezone = atoi(tz); + int16_t hour = timezone / 100; + int16_t min = abs(timezone) - (abs(hour) * 100); + + if (tz[0] == '-' && min >= 30) + hour--; + else if (min >= 30) + hour++; + + /* \todo Check if EXIF IFD is correct here or not. */ + setSShort(EXIF_IFD_EXIF, EXIF_TAG_TIME_ZONE_OFFSET, hour); + } } void Exif::setOrientation(int orientation) diff --git a/src/android/jpeg/exif.h b/src/android/jpeg/exif.h index 622de4c..4817815 100644 --- a/src/android/jpeg/exif.h +++ b/src/android/jpeg/exif.h @@ -37,6 +37,7 @@ private: unsigned long components, unsigned int size); void setShort(ExifIfd ifd, ExifTag tag, uint16_t item); + void setSShort(ExifIfd ifd, ExifTag tag, int16_t item); void setLong(ExifIfd ifd, ExifTag tag, uint32_t item); void setString(ExifIfd ifd, ExifTag tag, ExifFormat format, const std::string &item);
Get timestamp information from the timestamp, although the resolution for EXIF_TAG_TIME_ZONE_OFFSET is fairly limtied (per-hour only). Experimentation with 'exiftool', commandline utility to read/write exif metadata on images, resulted in rounding off the hours if the minutes came out to >= 30. Hence, the behaviour is inspired from exiftool itself. For e.g., Timezone Tag's value +1015 => 10 +0945 => 10 -1145 => -12 Signed-off-by: Umang Jain <email@uajain.com> --- src/android/jpeg/exif.cpp | 30 ++++++++++++++++++++++++++++++ src/android/jpeg/exif.h | 1 + 2 files changed, 31 insertions(+)