[libcamera-devel] [RFC v1 PATCH] android: jpeg: exif: Set timezone information
Umang Jain
email at uajain.com
Wed Sep 23 14:53:23 CEST 2020
Hi Kieran
On 9/23/20 4:26 PM, Kieran Bingham wrote:
> Hi Umang,
>
> On 23/09/2020 10:13, Umang Jain wrote:
>> Hi,
>>
>> On 9/22/20 9:56 PM, Umang Jain wrote:
>>> Get timezone information from the timestamp, although the resolution
>>> for EXIF_TAG_TIME_ZONE_OFFSET is fairly limited (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
>>>
>>> The EXIF specification defines three other tags (OffsetTime,
>>> OffsetTimeOriginal, OffsetTimeDigitized), in the EXIF IFD.
>>> These are not supported by libexif yet.
>>>
>>> Signed-off-by: Umang Jain <email at uajain.com>
>>> ---
>>> src/android/jpeg/exif.cpp | 31 +++++++++++++++++++++++++++++++
>>> src/android/jpeg/exif.h | 1 +
>>> 2 files changed, 32 insertions(+)
>>>
>>> ---
>>>
>>> Hi Laurent/Kieran,
>>>
>>> Can you soft review(or test) this patch to see if anything
>>> is wrong with it?
>>> The reason I am asking, because I cannot make it work :(
>>> exiftool with a captured frame shows empty Timezone Offset tag as:
>>> ```
>>> Time Zone Offset :
>>> ```
>>>
>>> exif tool shows nothing.
> Hrm, maybe adding some debug to the exif tool and rebuilding might help
> identify what data it actually parses or why it can't identify the field.
I didn't find any particular debug option in exiftool that would give
out a error message. Although I found -v3 verbose output which can give
hexdump of each tag. So attaching the various outputs here:
With:
a) setString(EXIF_IFD_0, EXIF_TAG_TIME_ZONE_OFFSET, EXIF_FORMAT_ASCII, "6");
o/p:
| 8) TimeZoneOffset = 6
| - Tag 0x882a (2 bytes, string[2]):
| 0090: 36 00 [6.]
JPEG DQT (65 bytes):
b) setSShort(EXIF_IFD_0, EXIF_TAG_TIME_ZONE_OFFSET, '6' or 6);
o/p:
| 8) TimeZoneOffset =
| - Tag 0x882a (0 bytes, undef):
>
> What does exiv2 report on the generated image?
`exiv2` tool doesn't report the tag in any of the cases.
`exif` tool reports it properly only if setString is used...
>
>>> I made sure a valid value being generated for timezone
>>> offset in setTimeStamp().
>>> I also checked the case where it might be preset using
>>> exif_data_fix and not been able to re-set again.
>>> But no, that doesn't seem the problem in my testing.
>>>
>>>
>>> diff --git a/src/android/jpeg/exif.cpp b/src/android/jpeg/exif.cpp
>>> index c0dbfcc..9c23cfb 100644
>>> --- a/src/android/jpeg/exif.cpp
>>> +++ b/src/android/jpeg/exif.cpp
>>> @@ -7,6 +7,8 @@
>>> #include "exif.h"
>>> +#include <stdlib.h>
>>> +
>>> #include "libcamera/internal/log.h"
>>> using namespace libcamera;
>>> @@ -135,6 +137,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);
>> Getting to bottom of this in the morning, exif_set_sshort seems to
>> convert its passed int16_t value to character. I don't know what's the
>> logic behind this [1] (apart from EXIF's quirky-ness), but this simply
>> doesn't look right then. So..
>
> Oh - converting a short to a char certainly doesn't sound right at all
> for a function called 'set_short'.
>
> Char's are 8-bits. Shorts are 16....
>
> Oh - Now I've followed to [1], I see that's simply dealing with
> byte-ordering issues. So it shouldn't be a problem.
oh, I misinterpreted things then.
>
>
>>> + exif_entry_unref(entry);
>>> +}
>>> +
>>> void Exif::setLong(ExifIfd ifd, ExifTag tag, uint32_t item)
>>> {
>>> ExifEntry *entry = createEntry(ifd, tag);
>>> @@ -196,6 +208,25 @@ 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.
> Where have you found the information that it is only 'per-hour' resolution?
It's what the tools use to do. They report on per-hour basis only. I
have mentioned the behaviour in the commit message.
Also stumbled upon
https://github.com/jim-easterbrook/Photini/commit/686d26 literally few
minutes ago.
<https://github.com/jim-easterbrook/Photini/commit/686d26>
>
>
>>> + */
>>> + int r = strftime(str, sizeof(str), "%z", &tm);
>>> + if (r > 0) {
>>> + int16_t timezone = atoi(str);
>>> + int16_t hour = timezone / 100;
>>> + int16_t min = timezone % 100;;
>> Complier should have errored out on ;; , but it didn't.
>> Rectified locally on noticing.
> ;; isn't going to generate a compiler error because the second ';' is
> just an empty statement that does nothing.
>
> Still worth cleaning up of course ;-)
>
>
>>> +
>>> + if (min <= -30)
>>> + hour--;
>>> + else if (min >= 30)
>>> + hour++;
>>> +
>>> + setSShort(EXIF_IFD_0, EXIF_TAG_TIME_ZONE_OFFSET, hour);
>> Instead of setSShort call, if we use setString, things seems to get
>> moving. Something like:
>> setString(EXIF_IFD_0, EXIF_TAG_TIME_ZONE_OFFSET, EXIF_FORMAT_ASCII, "6");
>>
>> got parsed correctly by exiftool and exif commandline utilities.
> Ok, so lets tie down exactly what the right format should be written to
> this field.
>
> Writing '6' as ascii infers that the actual value that got written to
> memory in that place was 54....
>
> What was presented with the tools? Did it show a '6' or the integer value?
It showed '6', no integer/ascii value of '6' was spotted anytime with
any tool.
>
> If this really is a field for a short, we should write the ascii value
> to the short, not a string... which (currently) adds null padding too!?
tried to setSShort with ascii value of 6, no improvements.
If setSShort is used, the exiftool reports it as 'empty' value :(
https://paste.debian.net/1164408/
>
> Interestingly:
> https://github.com/libexif/libexif/blob/master/libexif/exif-tag.c#L523
> seems to state that EXIF_TAG_TIME_ZONE_OFFSET is not in Exif-2.2...
>
> Indeed, examining closely I see no reference to a tag with id 0x882a
>
> https://www.exiv2.org/tags.html lists Exif.Image.TimeZoneOffset as an
> SShort at 0x882a (34858), with the following text:
>
> """
> This optional tag encodes the time zone of the camera clock (relative to
> Greenwich Mean Time) used to create the DataTimeOriginal tag-value when
> the picture was taken. It may also contain the time zone offset of the
> clock used to create the DateTime tag-value when the image was modified.
> """
>
> But that doesn't fully explain how the value is encoded.
>
>
>
>
> And ... now I've gone further down the rabbit-hole, I've discovered Exif
> 2.31 is available from :
>
> http://cipa.jp/std/documents/download_e.html?DC-008-Translation-2016-E
>
> It also doesn't list a tag at 0x882a, but does explicitly mention a tag
> "OffsetTime" (0x9010) which is an ascii field matching the string
> generated by the strftime(str, sizeof(str), "%z", &tm) call ;-)
>
>
> See page 49 in that document from cipa.jp.
There are 3 more timezone offset related tags that are mentioned but not
supported by libexif. Laurent has pointed this out in previous replies
to this thread.
>
> I expect we should explicitly set our exif version to "0231" if we use it?
hmm, yea, looking at the above pastebin output, my exif version is
'0210' so I would agree setting it 0231 because that's where the tag is
introduced.
My local version of libexif is 0.6.21 which already seems to support the
timezone offset tag. Although, I would expect it to report that the tag
I am trying set needs a higher exif version.
(Explicitly set exif version to 0231, no improvements observed)
>
> Do the exif/exiv2 libraries support Exif 2.31?
hmm, probably : https://github.com/libexif/libexif/commit/54333d8b8c
Not sure, how they keep up with minor versions, OR rather support new
tags directly.
>
> --
> Regards
>
> Kieran
>
>
>
>> [1]:
>> https://github.com/libexif/libexif/blob/master/libexif/exif-utils.c#L107-L121
>>
>>> + }
>>> }
>>> void Exif::setOrientation(int orientation)
>>> diff --git a/src/android/jpeg/exif.h b/src/android/jpeg/exif.h
>>> index f04cefc..9c9cc3b 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);
>> _______________________________________________
>> libcamera-devel mailing list
>> libcamera-devel at lists.libcamera.org
>> https://lists.libcamera.org/listinfo/libcamera-devel
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <https://lists.libcamera.org/pipermail/libcamera-devel/attachments/20200923/4682720f/attachment-0001.htm>
More information about the libcamera-devel
mailing list