[libcamera-devel] [RFC v1 PATCH] android: jpeg: exif: Set timezone information
Umang Jain
email at uajain.com
Wed Sep 23 15:48:03 CEST 2020
Hi Laurent,
On 9/23/20 6:59 PM, Laurent Pinchart wrote:
> Hello,
>
> On Wed, Sep 23, 2020 at 06:23:23PM +0530, Umang Jain wrote:
>> On 9/23/20 4:26 PM, Kieran Bingham wrote:
>>> On 23/09/2020 10:13, Umang Jain wrote:
>>>> 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.
> TimeZoneOffset is defined in the TIFF/EP specification (ISO/DIS
> 12234-2).
>
> 5.2.48 TimeZoneOffset
>
> 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.
>
> Tag Name = TimeZoneOffset
> Tag = 34858 (882A.H)
> Type = SSHORT
> N = 1 or 2
> Value = VALUE
> The allowed values are -12 to +11.
>
> SSHORT 0 Time Zone Offset (in hours) of DateTimeOriginal tag-value relative to Greenwich Mean Time
> SSHORT 1 If present, Time Zone Offset (in hours) of DateTime tag-value relative to Greenwich Mean Time
>
> Usage: IFD0
>
> It's fairly ill-defined as time zones extend to UTC+14, but it's clearly
> a signed short, with one or two values, expressed as a number of hours.
> We should really use the time zone tags of the EXIF specification
> instead, but they're not supported by libexif.
>
>>>>> + */
>>>>> + 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/
> Can you provide a sample image that has the time zone offset tag set
> with setSShort() ?
Please find attached a sample image to this mail.
>
>>> 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.
> The TimeZoneOffset tag doesn't exist in any EXIF specification, so I
> don't think the EXIF version will make any difference.
Ah, right. Booooing! :head_bang:
>
>> 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.
>>
>>>> [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);
-------------- next part --------------
A non-text attachment was scrubbed...
Name: frame-stream0-000000.jpg
Type: image/jpeg
Size: 40054 bytes
Desc: not available
URL: <https://lists.libcamera.org/pipermail/libcamera-devel/attachments/20200923/a2f3d2c9/attachment-0001.jpg>
More information about the libcamera-devel
mailing list