<html>
  <head>
    <meta http-equiv="Content-Type" content="text/html; charset=UTF-8">
  </head>
  <body>
    Hi Kieran<br>
    <br>
    <div class="moz-cite-prefix">On 9/23/20 4:26 PM, Kieran Bingham
      wrote:<br>
    </div>
    <blockquote type="cite"
      cite="mid:9f991efd-324d-3b37-1a5d-0901b25860e1@ideasonboard.com">
      <pre class="moz-quote-pre" wrap="">Hi Umang,

On 23/09/2020 10:13, Umang Jain wrote:
</pre>
      <blockquote type="cite">
        <pre class="moz-quote-pre" wrap="">Hi,

On 9/22/20 9:56 PM, Umang Jain wrote:
</pre>
        <blockquote type="cite">
          <pre class="moz-quote-pre" wrap="">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 <a class="moz-txt-link-rfc2396E" href="mailto:email@uajain.com"><email@uajain.com></a>
---
  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.
</pre>
        </blockquote>
      </blockquote>
      <pre class="moz-quote-pre" wrap="">
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.</pre>
    </blockquote>
    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:<br>
    <br>
    With:<br>
    a) setString(EXIF_IFD_0, EXIF_TAG_TIME_ZONE_OFFSET,
    EXIF_FORMAT_ASCII, "6");<br>
    o/p:<br>
      | 8)  TimeZoneOffset = 6<br>
      |     - Tag 0x882a (2 bytes, string[2]):<br>
      |         0090: 36 00                                          
    [6.]<br>
    JPEG DQT (65 bytes):<br>
    <br>
    <br>
    b) setSShort(EXIF_IFD_0, EXIF_TAG_TIME_ZONE_OFFSET, '6' or 6);<br>
    o/p: <br>
      | 8)  TimeZoneOffset = <br>
      |     - Tag 0x882a (0 bytes, undef):<br>
    <br>
    <blockquote type="cite"
      cite="mid:9f991efd-324d-3b37-1a5d-0901b25860e1@ideasonboard.com">
      <pre class="moz-quote-pre" wrap="">

What does exiv2 report on the generated image?</pre>
    </blockquote>
    `exiv2` tool doesn't report the tag in any of the cases.<br>
    `exif` tool reports it properly only if setString is used... <br>
    <blockquote type="cite"
      cite="mid:9f991efd-324d-3b37-1a5d-0901b25860e1@ideasonboard.com">
      <pre class="moz-quote-pre" wrap="">

</pre>
      <blockquote type="cite">
        <blockquote type="cite">
          <pre class="moz-quote-pre" wrap="">
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.</pre>
        </blockquote>
      </blockquote>
    </blockquote>
    <blockquote type="cite"
      cite="mid:9f991efd-324d-3b37-1a5d-0901b25860e1@ideasonboard.com">
      <blockquote type="cite">
        <blockquote type="cite">
          <pre class="moz-quote-pre" wrap="">


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);
</pre>
        </blockquote>
        <pre class="moz-quote-pre" wrap="">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..
</pre>
      </blockquote>
      <pre class="moz-quote-pre" wrap="">

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.
</pre>
    </blockquote>
    oh, I misinterpreted things then.
    <blockquote type="cite"
      cite="mid:9f991efd-324d-3b37-1a5d-0901b25860e1@ideasonboard.com">
      <pre class="moz-quote-pre" wrap="">


</pre>
      <blockquote type="cite">
        <blockquote type="cite">
          <pre class="moz-quote-pre" wrap="">+    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.
</pre>
        </blockquote>
      </blockquote>
      <pre class="moz-quote-pre" wrap="">
Where have you found the information that it is only 'per-hour' resolution?</pre>
    </blockquote>
    It's what the tools use to do. They report on per-hour basis only. 
    I have mentioned the behaviour in the commit message.<br>
    Also stumbled upon <a
      href="https://github.com/jim-easterbrook/Photini/commit/686d26">https://github.com/jim-easterbrook/Photini/commit/686d26</a>
    literally few minutes ago.<br>
    <a href="https://github.com/jim-easterbrook/Photini/commit/686d26"></a>
    <blockquote type="cite"
      cite="mid:9f991efd-324d-3b37-1a5d-0901b25860e1@ideasonboard.com">
      <pre class="moz-quote-pre" wrap="">


</pre>
      <blockquote type="cite">
        <blockquote type="cite">
          <pre class="moz-quote-pre" wrap="">+     */
+    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;;
</pre>
        </blockquote>
        <pre class="moz-quote-pre" wrap="">Complier should have errored out on ;; , but it didn't.
Rectified locally on noticing.
</pre>
      </blockquote>
      <pre class="moz-quote-pre" wrap="">
;; 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 ;-)


</pre>
      <blockquote type="cite">
        <blockquote type="cite">
          <pre class="moz-quote-pre" wrap="">+
+        if (min <= -30)
+            hour--;
+        else if (min >= 30)
+            hour++;
+
+        setSShort(EXIF_IFD_0, EXIF_TAG_TIME_ZONE_OFFSET, hour);
</pre>
        </blockquote>
        <pre class="moz-quote-pre" wrap="">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.
</pre>
      </blockquote>
      <pre class="moz-quote-pre" wrap="">
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?</pre>
    </blockquote>
    It showed '6', no integer/ascii value of '6' was spotted anytime
    with any tool.<br>
    <blockquote type="cite"
      cite="mid:9f991efd-324d-3b37-1a5d-0901b25860e1@ideasonboard.com">
      <pre class="moz-quote-pre" wrap="">

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!?</pre>
    </blockquote>
    tried to setSShort with ascii value of 6, no improvements.<br>
    If setSShort is used, the exiftool reports it as 'empty' value :( <br>
    <a href="https://paste.debian.net/1164408/">https://paste.debian.net/1164408/</a>
    <blockquote type="cite"
      cite="mid:9f991efd-324d-3b37-1a5d-0901b25860e1@ideasonboard.com">
      <pre class="moz-quote-pre" wrap="">

Interestingly:
<a class="moz-txt-link-freetext" href="https://github.com/libexif/libexif/blob/master/libexif/exif-tag.c#L523">https://github.com/libexif/libexif/blob/master/libexif/exif-tag.c#L523</a>
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

<a class="moz-txt-link-freetext" href="https://www.exiv2.org/tags.html">https://www.exiv2.org/tags.html</a> 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 :

  <a class="moz-txt-link-freetext" href="http://cipa.jp/std/documents/download_e.html?DC-008-Translation-2016-E">http://cipa.jp/std/documents/download_e.html?DC-008-Translation-2016-E</a>

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.</pre>
    </blockquote>
    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.<br>
    <blockquote type="cite"
      cite="mid:9f991efd-324d-3b37-1a5d-0901b25860e1@ideasonboard.com">
      <pre class="moz-quote-pre" wrap="">

I expect we should explicitly set our exif version to "0231" if we use it?</pre>
    </blockquote>
    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.<br>
    <br>
    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. <br>
    <br>
    (Explicitly set exif version to 0231, no improvements observed)<br>
    <blockquote type="cite"
      cite="mid:9f991efd-324d-3b37-1a5d-0901b25860e1@ideasonboard.com">
      <pre class="moz-quote-pre" wrap="">

Do the exif/exiv2 libraries support Exif 2.31?</pre>
    </blockquote>
    hmm, probably : <a
      href="https://github.com/libexif/libexif/commit/54333d8b8c">https://github.com/libexif/libexif/commit/54333d8b8c</a><br>
    <br>
    Not sure, how they keep up with minor versions, OR rather support
    new tags directly. <br>
    <blockquote type="cite"
      cite="mid:9f991efd-324d-3b37-1a5d-0901b25860e1@ideasonboard.com">
      <pre class="moz-quote-pre" wrap="">

--
Regards

Kieran



</pre>
      <blockquote type="cite">
        <pre class="moz-quote-pre" wrap="">[1]:
<a class="moz-txt-link-freetext" href="https://github.com/libexif/libexif/blob/master/libexif/exif-utils.c#L107-L121">https://github.com/libexif/libexif/blob/master/libexif/exif-utils.c#L107-L121</a>

</pre>
        <blockquote type="cite">
          <pre class="moz-quote-pre" wrap="">+    }
  }
    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);
</pre>
        </blockquote>
        <pre class="moz-quote-pre" wrap="">
_______________________________________________
libcamera-devel mailing list
<a class="moz-txt-link-abbreviated" href="mailto:libcamera-devel@lists.libcamera.org">libcamera-devel@lists.libcamera.org</a>
<a class="moz-txt-link-freetext" href="https://lists.libcamera.org/listinfo/libcamera-devel">https://lists.libcamera.org/listinfo/libcamera-devel</a>
</pre>
      </blockquote>
      <pre class="moz-quote-pre" wrap="">
</pre>
    </blockquote>
    <br>
  </body>
</html>