[{"id":12655,"web_url":"https://patchwork.libcamera.org/comment/12655/","msgid":"<70e975f3-ce7c-891c-af41-3775cb9d0fee@uajain.com>","date":"2020-09-23T09:13:40","subject":"Re: [libcamera-devel] [RFC v1 PATCH] android: jpeg: exif: Set\n\ttimezone information","submitter":{"id":1,"url":"https://patchwork.libcamera.org/api/people/1/","name":"Umang Jain","email":"email@uajain.com"},"content":"Hi,\n\nOn 9/22/20 9:56 PM, Umang Jain wrote:\n> Get timezone information from the timestamp, although the resolution\n> for EXIF_TAG_TIME_ZONE_OFFSET is fairly limited (per-hour only).\n>\n> Experimentation with 'exiftool', commandline utility to read/write\n> exif metadata on images, resulted in rounding off the hours if the\n> minutes came out to >= 30. Hence, the behaviour is inspired from\n> exiftool itself. For e.g.,\n>\n> Timezone      Tag's value\n>   +1015     =>     10\n>   +0945     =>     10\n>   -1145     =>    -12\n>\n> The EXIF specification defines three other tags (OffsetTime,\n> OffsetTimeOriginal, OffsetTimeDigitized), in the EXIF IFD.\n> These are not supported by libexif yet.\n>\n> Signed-off-by: Umang Jain <email@uajain.com>\n> ---\n>   src/android/jpeg/exif.cpp | 31 +++++++++++++++++++++++++++++++\n>   src/android/jpeg/exif.h   |  1 +\n>   2 files changed, 32 insertions(+)\n>\n> ---\n>\n> Hi Laurent/Kieran,\n>\n> Can you soft review(or test) this patch to see if anything\n> is wrong with it?\n> The reason I am asking, because I cannot make it work :(\n> exiftool with a captured frame shows empty Timezone Offset tag as:\n> ```\n> Time Zone Offset                :\n> ```\n>\n> exif tool shows nothing.\n>\n> I made sure a valid value being generated for timezone\n> offset in setTimeStamp().\n> I also checked the case where it might be preset using\n> exif_data_fix and not been able to re-set again.\n> But no, that doesn't seem the problem in my testing.\n>\n>\n> diff --git a/src/android/jpeg/exif.cpp b/src/android/jpeg/exif.cpp\n> index c0dbfcc..9c23cfb 100644\n> --- a/src/android/jpeg/exif.cpp\n> +++ b/src/android/jpeg/exif.cpp\n> @@ -7,6 +7,8 @@\n>   \n>   #include \"exif.h\"\n>   \n> +#include <stdlib.h>\n> +\n>   #include \"libcamera/internal/log.h\"\n>   \n>   using namespace libcamera;\n> @@ -135,6 +137,16 @@ void Exif::setShort(ExifIfd ifd, ExifTag tag, uint16_t item)\n>   \texif_entry_unref(entry);\n>   }\n>   \n> +void Exif::setSShort(ExifIfd ifd, ExifTag tag, int16_t item)\n> +{\n> +\tExifEntry *entry = createEntry(ifd, tag);\n> +\tif (!entry)\n> +\t\treturn;\n> +\n> +\texif_set_sshort(entry->data, EXIF_BYTE_ORDER_INTEL, item);\nGetting to bottom of this in the morning, exif_set_sshort seems to \nconvert its passed int16_t value to character. I don't know what's the \nlogic behind this [1] (apart from EXIF's quirky-ness), but this simply \ndoesn't look right then. So..\n> +\texif_entry_unref(entry);\n> +}\n> +\n>   void Exif::setLong(ExifIfd ifd, ExifTag tag, uint32_t item)\n>   {\n>   \tExifEntry *entry = createEntry(ifd, tag);\n> @@ -196,6 +208,25 @@ void Exif::setTimestamp(time_t timestamp)\n>   \tsetString(EXIF_IFD_0, EXIF_TAG_DATE_TIME, EXIF_FORMAT_ASCII, ts);\n>   \tsetString(EXIF_IFD_EXIF, EXIF_TAG_DATE_TIME_ORIGINAL, EXIF_FORMAT_ASCII, ts);\n>   \tsetString(EXIF_IFD_EXIF, EXIF_TAG_DATE_TIME_DIGITIZED, EXIF_FORMAT_ASCII, ts);\n> +\n> +\t/*\n> +\t * If possible, query and set timezone information via\n> +\t * EXIF_TAG_TIME_ZONE_OFFSET. There is only per-hour resolution for tag\n> +\t * hence, round up hours if minutes >= 30.\n> +\t */\n> +\tint r = strftime(str, sizeof(str), \"%z\", &tm);\n> +\tif (r > 0) {\n> +\t\tint16_t timezone = atoi(str);\n> +\t\tint16_t hour = timezone / 100;\n> +\t\tint16_t min = timezone % 100;;\nComplier should have errored out on ;; , but it didn't.\nRectified locally on noticing.\n> +\n> +\t\tif (min <= -30)\n> +\t\t\thour--;\n> +\t\telse if (min >= 30)\n> +\t\t\thour++;\n> +\n> +\t\tsetSShort(EXIF_IFD_0, EXIF_TAG_TIME_ZONE_OFFSET, hour);\nInstead of setSShort call, if we use setString, things seems to get \nmoving. Something like:\nsetString(EXIF_IFD_0, EXIF_TAG_TIME_ZONE_OFFSET, EXIF_FORMAT_ASCII, \"6\");\n\ngot parsed correctly by exiftool and exif commandline utilities.\n\n[1]: \nhttps://github.com/libexif/libexif/blob/master/libexif/exif-utils.c#L107-L121 \n\n> +\t}\n>   }\n>   \n>   void Exif::setOrientation(int orientation)\n> diff --git a/src/android/jpeg/exif.h b/src/android/jpeg/exif.h\n> index f04cefc..9c9cc3b 100644\n> --- a/src/android/jpeg/exif.h\n> +++ b/src/android/jpeg/exif.h\n> @@ -37,6 +37,7 @@ private:\n>   \t\t\t       unsigned long components, unsigned int size);\n>   \n>   \tvoid setShort(ExifIfd ifd, ExifTag tag, uint16_t item);\n> +\tvoid setSShort(ExifIfd ifd, ExifTag tag, int16_t item);\n>   \tvoid setLong(ExifIfd ifd, ExifTag tag, uint32_t item);\n>   \tvoid setString(ExifIfd ifd, ExifTag tag, ExifFormat format,\n>   \t\t       const std::string &item);","headers":{"Return-Path":"<libcamera-devel-bounces@lists.libcamera.org>","X-Original-To":"parsemail@patchwork.libcamera.org","Delivered-To":"parsemail@patchwork.libcamera.org","Received":["from lancelot.ideasonboard.com (lancelot.ideasonboard.com\n\t[92.243.16.209])\n\tby patchwork.libcamera.org (Postfix) with ESMTPS id 909D1C3B5C\n\tfor <parsemail@patchwork.libcamera.org>;\n\tWed, 23 Sep 2020 09:13:50 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id 1794562FDC;\n\tWed, 23 Sep 2020 11:13:50 +0200 (CEST)","from mail.uajain.com (static.126.159.217.95.clients.your-server.de\n\t[95.217.159.126])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTPS id 1E5E460363\n\tfor <libcamera-devel@lists.libcamera.org>;\n\tWed, 23 Sep 2020 11:13:48 +0200 (CEST)"],"Authentication-Results":"lancelot.ideasonboard.com;\n\tdkim=fail reason=\"signature verification failed\" (2048-bit key;\n\tunprotected) header.d=uajain.com header.i=@uajain.com\n\theader.b=\"ae7oqy7Z\"; dkim-atps=neutral","DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/simple; d=uajain.com; s=mail;\n\tt=1600852427; bh=Nz6EZZFCAA7vCU81/5Lzwdd8xjTzw9gvVHOqFmtBvUs=;\n\th=Subject:To:References:From:In-Reply-To;\n\tb=ae7oqy7ZAPXvRznQpc3Q4+tY/dfhvePe8J+QdtDLr2YXzAtTkvg+O3M29DK/mzdny\n\twfeeN6j+k+XfGPBwbjU+6JPE+dsQDFjJQ0EVl8kwUZ5MeXgxArhBDqFA3MOlG/iG3Y\n\tzfNDCnDlJdLdf43+oMC2hVuZNykXW/wEYmh5JT0qUPaUdsX7GJmmwja1xtfvejoYGx\n\tOLEFU20dVc6o9XbXZTc+69LHs/z7wrDfas1epq4J1SRcj/wIW1RO4hsa5uwnT298NX\n\tWYw0jrfFTk9jPuaFY4iVi9f8++h06DDwBA52JlyJ6r+vcPmqGtqJxZeVyES5XZq+3i\n\t7kNfq5ooixX9Q==","To":"libcamera-devel@lists.libcamera.org","References":"<20200909153206.7700-1-email@uajain.com>\n\t<20200922162624.32321-1-email@uajain.com>","From":"Umang Jain <email@uajain.com>","Message-ID":"<70e975f3-ce7c-891c-af41-3775cb9d0fee@uajain.com>","Date":"Wed, 23 Sep 2020 14:43:40 +0530","Mime-Version":"1.0","In-Reply-To":"<20200922162624.32321-1-email@uajain.com>","Content-Language":"en-US","Subject":"Re: [libcamera-devel] [RFC v1 PATCH] android: jpeg: exif: Set\n\ttimezone information","X-BeenThere":"libcamera-devel@lists.libcamera.org","X-Mailman-Version":"2.1.29","Precedence":"list","List-Id":"<libcamera-devel.lists.libcamera.org>","List-Unsubscribe":"<https://lists.libcamera.org/options/libcamera-devel>,\n\t<mailto:libcamera-devel-request@lists.libcamera.org?subject=unsubscribe>","List-Archive":"<https://lists.libcamera.org/pipermail/libcamera-devel/>","List-Post":"<mailto:libcamera-devel@lists.libcamera.org>","List-Help":"<mailto:libcamera-devel-request@lists.libcamera.org?subject=help>","List-Subscribe":"<https://lists.libcamera.org/listinfo/libcamera-devel>,\n\t<mailto:libcamera-devel-request@lists.libcamera.org?subject=subscribe>","Content-Transfer-Encoding":"7bit","Content-Type":"text/plain; charset=\"us-ascii\"; Format=\"flowed\"","Errors-To":"libcamera-devel-bounces@lists.libcamera.org","Sender":"\"libcamera-devel\" <libcamera-devel-bounces@lists.libcamera.org>"}},{"id":12661,"web_url":"https://patchwork.libcamera.org/comment/12661/","msgid":"<9f991efd-324d-3b37-1a5d-0901b25860e1@ideasonboard.com>","date":"2020-09-23T10:56:32","subject":"Re: [libcamera-devel] [RFC v1 PATCH] android: jpeg: exif: Set\n\ttimezone information","submitter":{"id":4,"url":"https://patchwork.libcamera.org/api/people/4/","name":"Kieran Bingham","email":"kieran.bingham@ideasonboard.com"},"content":"Hi Umang,\n\nOn 23/09/2020 10:13, Umang Jain wrote:\n> Hi,\n> \n> On 9/22/20 9:56 PM, Umang Jain wrote:\n>> Get timezone information from the timestamp, although the resolution\n>> for EXIF_TAG_TIME_ZONE_OFFSET is fairly limited (per-hour only).\n>>\n>> Experimentation with 'exiftool', commandline utility to read/write\n>> exif metadata on images, resulted in rounding off the hours if the\n>> minutes came out to >= 30. Hence, the behaviour is inspired from\n>> exiftool itself. For e.g.,\n>>\n>> Timezone      Tag's value\n>>   +1015     =>     10\n>>   +0945     =>     10\n>>   -1145     =>    -12\n>>\n>> The EXIF specification defines three other tags (OffsetTime,\n>> OffsetTimeOriginal, OffsetTimeDigitized), in the EXIF IFD.\n>> These are not supported by libexif yet.\n>>\n>> Signed-off-by: Umang Jain <email@uajain.com>\n>> ---\n>>   src/android/jpeg/exif.cpp | 31 +++++++++++++++++++++++++++++++\n>>   src/android/jpeg/exif.h   |  1 +\n>>   2 files changed, 32 insertions(+)\n>>\n>> ---\n>>\n>> Hi Laurent/Kieran,\n>>\n>> Can you soft review(or test) this patch to see if anything\n>> is wrong with it?\n>> The reason I am asking, because I cannot make it work :(\n>> exiftool with a captured frame shows empty Timezone Offset tag as:\n>> ```\n>> Time Zone Offset                :\n>> ```\n>>\n>> exif tool shows nothing.\n\nHrm, maybe adding some debug to the exif tool and rebuilding might help\nidentify what data it actually parses or why it can't identify the field.\n\nWhat does exiv2 report on the generated image?\n\n>>\n>> I made sure a valid value being generated for timezone\n>> offset in setTimeStamp().\n>> I also checked the case where it might be preset using\n>> exif_data_fix and not been able to re-set again.\n>> But no, that doesn't seem the problem in my testing.\n>>\n>>\n>> diff --git a/src/android/jpeg/exif.cpp b/src/android/jpeg/exif.cpp\n>> index c0dbfcc..9c23cfb 100644\n>> --- a/src/android/jpeg/exif.cpp\n>> +++ b/src/android/jpeg/exif.cpp\n>> @@ -7,6 +7,8 @@\n>>     #include \"exif.h\"\n>>   +#include <stdlib.h>\n>> +\n>>   #include \"libcamera/internal/log.h\"\n>>     using namespace libcamera;\n>> @@ -135,6 +137,16 @@ void Exif::setShort(ExifIfd ifd, ExifTag tag,\n>> uint16_t item)\n>>       exif_entry_unref(entry);\n>>   }\n>>   +void Exif::setSShort(ExifIfd ifd, ExifTag tag, int16_t item)\n>> +{\n>> +    ExifEntry *entry = createEntry(ifd, tag);\n>> +    if (!entry)\n>> +        return;\n>> +\n>> +    exif_set_sshort(entry->data, EXIF_BYTE_ORDER_INTEL, item);\n> Getting to bottom of this in the morning, exif_set_sshort seems to\n> convert its passed int16_t value to character. I don't know what's the\n> logic behind this [1] (apart from EXIF's quirky-ness), but this simply\n> doesn't look right then. So..\n\n\nOh - converting a short to a char certainly doesn't sound right at all\nfor a function called 'set_short'.\n\nChar's are 8-bits. Shorts are 16....\n\nOh - Now I've followed to [1], I see that's simply dealing with\nbyte-ordering issues. So it shouldn't be a problem.\n\n\n\n>> +    exif_entry_unref(entry);\n>> +}\n>> +\n>>   void Exif::setLong(ExifIfd ifd, ExifTag tag, uint32_t item)\n>>   {\n>>       ExifEntry *entry = createEntry(ifd, tag);\n>> @@ -196,6 +208,25 @@ void Exif::setTimestamp(time_t timestamp)\n>>       setString(EXIF_IFD_0, EXIF_TAG_DATE_TIME, EXIF_FORMAT_ASCII, ts);\n>>       setString(EXIF_IFD_EXIF, EXIF_TAG_DATE_TIME_ORIGINAL,\n>> EXIF_FORMAT_ASCII, ts);\n>>       setString(EXIF_IFD_EXIF, EXIF_TAG_DATE_TIME_DIGITIZED,\n>> EXIF_FORMAT_ASCII, ts);\n>> +\n>> +    /*\n>> +     * If possible, query and set timezone information via\n>> +     * EXIF_TAG_TIME_ZONE_OFFSET. There is only per-hour resolution\n>> for tag\n>> +     * hence, round up hours if minutes >= 30.\n\nWhere have you found the information that it is only 'per-hour' resolution?\n\n\n>> +     */\n>> +    int r = strftime(str, sizeof(str), \"%z\", &tm);\n>> +    if (r > 0) {\n>> +        int16_t timezone = atoi(str);\n>> +        int16_t hour = timezone / 100;\n>> +        int16_t min = timezone % 100;;\n> Complier should have errored out on ;; , but it didn't.\n> Rectified locally on noticing.\n\n;; isn't going to generate a compiler error because the second ';' is\njust an empty statement that does nothing.\n\nStill worth cleaning up of course ;-)\n\n\n>> +\n>> +        if (min <= -30)\n>> +            hour--;\n>> +        else if (min >= 30)\n>> +            hour++;\n>> +\n>> +        setSShort(EXIF_IFD_0, EXIF_TAG_TIME_ZONE_OFFSET, hour);\n> Instead of setSShort call, if we use setString, things seems to get\n> moving. Something like:\n> setString(EXIF_IFD_0, EXIF_TAG_TIME_ZONE_OFFSET, EXIF_FORMAT_ASCII, \"6\");\n> \n> got parsed correctly by exiftool and exif commandline utilities.\n\nOk, so lets tie down exactly what the right format should be written to\nthis field.\n\nWriting '6' as ascii infers that the actual value that got written to\nmemory in that place was 54....\n\nWhat was presented with the tools? Did it show a '6' or the integer value?\n\nIf this really is a field for a short, we should write the ascii value\nto the short, not a string... which (currently) adds null padding too!?\n\nInterestingly:\nhttps://github.com/libexif/libexif/blob/master/libexif/exif-tag.c#L523\nseems to state that EXIF_TAG_TIME_ZONE_OFFSET is not in Exif-2.2...\n\nIndeed, examining closely I see no reference to a tag with id 0x882a\n\nhttps://www.exiv2.org/tags.html lists Exif.Image.TimeZoneOffset as an\nSShort at 0x882a (34858), with the following text:\n\n\"\"\"\nThis optional tag encodes the time zone of the camera clock (relative to\nGreenwich Mean Time) used to create the DataTimeOriginal tag-value when\nthe picture was taken. It may also contain the time zone offset of the\nclock used to create the DateTime tag-value when the image was modified.\n\"\"\"\n\nBut that doesn't fully explain how the value is encoded.\n\n\n\n\nAnd ... now I've gone further down the rabbit-hole, I've discovered Exif\n2.31 is available from :\n\n  http://cipa.jp/std/documents/download_e.html?DC-008-Translation-2016-E\n\nIt also doesn't list a tag at 0x882a, but does explicitly mention a tag\n\"OffsetTime\" (0x9010) which is an ascii field matching the string\ngenerated by the strftime(str, sizeof(str), \"%z\", &tm) call ;-)\n\n\nSee page 49 in that document from cipa.jp.\n\nI expect we should explicitly set our exif version to \"0231\" if we use it?\n\nDo the exif/exiv2 libraries support Exif 2.31?\n\n--\nRegards\n\nKieran\n\n\n\n> [1]:\n> https://github.com/libexif/libexif/blob/master/libexif/exif-utils.c#L107-L121\n> \n>> +    }\n>>   }\n>>     void Exif::setOrientation(int orientation)\n>> diff --git a/src/android/jpeg/exif.h b/src/android/jpeg/exif.h\n>> index f04cefc..9c9cc3b 100644\n>> --- a/src/android/jpeg/exif.h\n>> +++ b/src/android/jpeg/exif.h\n>> @@ -37,6 +37,7 @@ private:\n>>                      unsigned long components, unsigned int size);\n>>         void setShort(ExifIfd ifd, ExifTag tag, uint16_t item);\n>> +    void setSShort(ExifIfd ifd, ExifTag tag, int16_t item);\n>>       void setLong(ExifIfd ifd, ExifTag tag, uint32_t item);\n>>       void setString(ExifIfd ifd, ExifTag tag, ExifFormat format,\n>>                  const std::string &item);\n> \n> _______________________________________________\n> libcamera-devel mailing list\n> libcamera-devel@lists.libcamera.org\n> https://lists.libcamera.org/listinfo/libcamera-devel","headers":{"Return-Path":"<libcamera-devel-bounces@lists.libcamera.org>","X-Original-To":"parsemail@patchwork.libcamera.org","Delivered-To":"parsemail@patchwork.libcamera.org","Received":["from lancelot.ideasonboard.com (lancelot.ideasonboard.com\n\t[92.243.16.209])\n\tby patchwork.libcamera.org (Postfix) with ESMTPS id 2F6ACC3B5C\n\tfor <parsemail@patchwork.libcamera.org>;\n\tWed, 23 Sep 2020 10:56:38 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id AF7F762FD8;\n\tWed, 23 Sep 2020 12:56:37 +0200 (CEST)","from perceval.ideasonboard.com (perceval.ideasonboard.com\n\t[213.167.242.64])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTPS id 100A262FD2\n\tfor <libcamera-devel@lists.libcamera.org>;\n\tWed, 23 Sep 2020 12:56:36 +0200 (CEST)","from [192.168.0.20]\n\t(cpc89244-aztw30-2-0-cust3082.18-1.cable.virginm.net [86.31.172.11])\n\tby perceval.ideasonboard.com (Postfix) with ESMTPSA id 7D4FB555;\n\tWed, 23 Sep 2020 12:56:35 +0200 (CEST)"],"Authentication-Results":"lancelot.ideasonboard.com;\n\tdkim=fail reason=\"signature verification failed\" (1024-bit key;\n\tunprotected) header.d=ideasonboard.com header.i=@ideasonboard.com\n\theader.b=\"fSKMcv9U\"; dkim-atps=neutral","DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com;\n\ts=mail; t=1600858595;\n\tbh=b27qBq/weVZv1IzC3X7XNZj2EpP6bwE/JRffkT7Ucfk=;\n\th=Reply-To:Subject:To:References:From:Date:In-Reply-To:From;\n\tb=fSKMcv9U2Shz8T7RDUarjibtGQ0KJAcgJBzeOUH/YkI01sy/J4wvxChxi44qlWgzF\n\tRtY+qrKrj6hNKaZ8BvcJbiN/wdkXYznqk4cnBGyHJhcwiXEWWi1YL220NMYjZnV6TT\n\tiyhln2UKc08gm7yUOFVD+r3lRSoBSxfjt5mTZQ4k=","To":"Umang Jain <email@uajain.com>, libcamera-devel@lists.libcamera.org","References":"<20200909153206.7700-1-email@uajain.com>\n\t<20200922162624.32321-1-email@uajain.com>\n\t<70e975f3-ce7c-891c-af41-3775cb9d0fee@uajain.com>","From":"Kieran Bingham <kieran.bingham@ideasonboard.com>","Autocrypt":"addr=kieran.bingham@ideasonboard.com; keydata=\n\tmQINBFYE/WYBEACs1PwjMD9rgCu1hlIiUA1AXR4rv2v+BCLUq//vrX5S5bjzxKAryRf0uHat\n\tV/zwz6hiDrZuHUACDB7X8OaQcwhLaVlq6byfoBr25+hbZG7G3+5EUl9cQ7dQEdvNj6V6y/SC\n\trRanWfelwQThCHckbobWiQJfK9n7rYNcPMq9B8e9F020LFH7Kj6YmO95ewJGgLm+idg1Kb3C\n\tpotzWkXc1xmPzcQ1fvQMOfMwdS+4SNw4rY9f07Xb2K99rjMwZVDgESKIzhsDB5GY465sCsiQ\n\tcSAZRxqE49RTBq2+EQsbrQpIc8XiffAB8qexh5/QPzCmR4kJgCGeHIXBtgRj+nIkCJPZvZtf\n\tKr2EAbc6tgg6DkAEHJb+1okosV09+0+TXywYvtEop/WUOWQ+zo+Y/OBd+8Ptgt1pDRyOBzL8\n\tRXa8ZqRf0Mwg75D+dKntZeJHzPRJyrlfQokngAAs4PaFt6UfS+ypMAF37T6CeDArQC41V3ko\n\tlPn1yMsVD0p+6i3DPvA/GPIksDC4owjnzVX9kM8Zc5Cx+XoAN0w5Eqo4t6qEVbuettxx55gq\n\t8K8FieAjgjMSxngo/HST8TpFeqI5nVeq0/lqtBRQKumuIqDg+Bkr4L1V/PSB6XgQcOdhtd36\n\tOe9X9dXB8YSNt7VjOcO7BTmFn/Z8r92mSAfHXpb07YJWJosQOQARAQABtDBLaWVyYW4gQmlu\n\tZ2hhbSA8a2llcmFuLmJpbmdoYW1AaWRlYXNvbmJvYXJkLmNvbT6JAlcEEwEKAEECGwMFCwkI\n\tBwIGFQgJCgsCBBYCAwECHgECF4ACGQEWIQSQLdeYP70o/eNy1HqhHkZyEKRh/QUCXWTtygUJ\n\tCyJXZAAKCRChHkZyEKRh/f8dEACTDsbLN2nioNZMwyLuQRUAFcXNolDX48xcUXsWS2QjxaPm\n\tVsJx8Uy8aYkS85mdPBh0C83OovQR/OVbr8AxhGvYqBs3nQvbWuTl/+4od7DfK2VZOoKBAu5S\n\tQK2FYuUcikDqYcFWJ8DQnubxfE8dvzojHEkXw0sA4igINHDDFX3HJGZtLio+WpEFQtCbfTAG\n\tYZslasz1YZRbwEdSsmO3/kqy5eMnczlm8a21A3fKUo3g8oAZEFM+f4DUNzqIltg31OAB/kZS\n\tenKZQ/SWC8PmLg/ZXBrReYakxXtkP6w3FwMlzOlhGxqhIRNiAJfXJBaRhuUWzPOpEDE9q5YJ\n\tBmqQL2WJm1VSNNVxbXJHpaWMH1sA2R00vmvRrPXGwyIO0IPYeUYQa3gsy6k+En/aMQJd27dp\n\taScf9am9PFICPY5T4ppneeJLif2lyLojo0mcHOV+uyrds9XkLpp14GfTkeKPdPMrLLTsHRfH\n\tfA4I4OBpRrEPiGIZB/0im98MkGY/Mu6qxeZmYLCcgD6qz4idOvfgVOrNh+aA8HzIVR+RMW8H\n\tQGBN9f0E3kfwxuhl3omo6V7lDw8XOdmuWZNC9zPq1UfryVHANYbLGz9KJ4Aw6M+OgBC2JpkD\n\thXMdHUkC+d20dwXrwHTlrJi1YNp6rBc+xald3wsUPOZ5z8moTHUX/uPA/qhGsbkCDQRWBP1m\n\tARAAzijkb+Sau4hAncr1JjOY+KyFEdUNxRy+hqTJdJfaYihxyaj0Ee0P0zEi35CbE6lgU0Uz\n\ttih9fiUbSV3wfsWqg1Ut3/5rTKu7kLFp15kF7eqvV4uezXRD3Qu4yjv/rMmEJbbD4cTvGCYI\n\td6MDC417f7vK3hCbCVIZSp3GXxyC1LU+UQr3fFcOyCwmP9vDUR9JV0BSqHHxRDdpUXE26Dk6\n\tmhf0V1YkspE5St814ETXpEus2urZE5yJIUROlWPIL+hm3NEWfAP06vsQUyLvr/GtbOT79vXl\n\tEn1aulcYyu20dRRxhkQ6iILaURcxIAVJJKPi8dsoMnS8pB0QW12AHWuirPF0g6DiuUfPmrA5\n\tPKe56IGlpkjc8cO51lIxHkWTpCMWigRdPDexKX+Sb+W9QWK/0JjIc4t3KBaiG8O4yRX8ml2R\n\t+rxfAVKM6V769P/hWoRGdgUMgYHFpHGSgEt80OKK5HeUPy2cngDUXzwrqiM5Sz6Od0qw5pCk\n\tNlXqI0W/who0iSVM+8+RmyY0OEkxEcci7rRLsGnM15B5PjLJjh1f2ULYkv8s4SnDwMZ/kE04\n\t/UqCMK/KnX8pwXEMCjz0h6qWNpGwJ0/tYIgQJZh6bqkvBrDogAvuhf60Sogw+mH8b+PBlx1L\n\toeTK396wc+4c3BfiC6pNtUS5GpsPMMjYMk7kVvEAEQEAAYkCPAQYAQoAJgIbDBYhBJAt15g/\n\tvSj943LUeqEeRnIQpGH9BQJdizzIBQkLSKZiAAoJEKEeRnIQpGH9eYgQAJpjaWNgqNOnMTmD\n\tMJggbwjIotypzIXfhHNCeTkG7+qCDlSaBPclcPGYrTwCt0YWPU2TgGgJrVhYT20ierN8LUvj\n\t6qOPTd+Uk7NFzL65qkh80ZKNBFddx1AabQpSVQKbdcLb8OFs85kuSvFdgqZwgxA1vl4TFhNz\n\tPZ79NAmXLackAx3sOVFhk4WQaKRshCB7cSl+RIng5S/ThOBlwNlcKG7j7W2MC06BlTbdEkUp\n\tECzuuRBv8wX4OQl+hbWbB/VKIx5HKlLu1eypen/5lNVzSqMMIYkkZcjV2SWQyUGxSwq0O/sx\n\tS0A8/atCHUXOboUsn54qdxrVDaK+6jIAuo8JiRWctP16KjzUM7MO0/+4zllM8EY57rXrj48j\n\tsbEYX0YQnzaj+jO6kJtoZsIaYR7rMMq9aUAjyiaEZpmP1qF/2sYenDx0Fg2BSlLvLvXM0vU8\n\tpQk3kgDu7kb/7PRYrZvBsr21EIQoIjXbZxDz/o7z95frkP71EaICttZ6k9q5oxxA5WC6sTXc\n\tMW8zs8avFNuA9VpXt0YupJd2ijtZy2mpZNG02fFVXhIn4G807G7+9mhuC4XG5rKlBBUXTvPU\n\tAfYnB4JBDLmLzBFavQfvonSfbitgXwCG3vS+9HEwAjU30Bar1PEOmIbiAoMzuKeRm2LVpmq4\n\tWZw01QYHU/GUV/zHJSFk","Organization":"Ideas on Board","Message-ID":"<9f991efd-324d-3b37-1a5d-0901b25860e1@ideasonboard.com>","Date":"Wed, 23 Sep 2020 11:56:32 +0100","User-Agent":"Mozilla/5.0 (X11; Linux x86_64; rv:68.0) Gecko/20100101\n\tThunderbird/68.10.0","MIME-Version":"1.0","In-Reply-To":"<70e975f3-ce7c-891c-af41-3775cb9d0fee@uajain.com>","Content-Language":"en-GB","Subject":"Re: [libcamera-devel] [RFC v1 PATCH] android: jpeg: exif: Set\n\ttimezone information","X-BeenThere":"libcamera-devel@lists.libcamera.org","X-Mailman-Version":"2.1.29","Precedence":"list","List-Id":"<libcamera-devel.lists.libcamera.org>","List-Unsubscribe":"<https://lists.libcamera.org/options/libcamera-devel>,\n\t<mailto:libcamera-devel-request@lists.libcamera.org?subject=unsubscribe>","List-Archive":"<https://lists.libcamera.org/pipermail/libcamera-devel/>","List-Post":"<mailto:libcamera-devel@lists.libcamera.org>","List-Help":"<mailto:libcamera-devel-request@lists.libcamera.org?subject=help>","List-Subscribe":"<https://lists.libcamera.org/listinfo/libcamera-devel>,\n\t<mailto:libcamera-devel-request@lists.libcamera.org?subject=subscribe>","Reply-To":"kieran.bingham@ideasonboard.com","Content-Type":"text/plain; charset=\"utf-8\"","Content-Transfer-Encoding":"base64","Errors-To":"libcamera-devel-bounces@lists.libcamera.org","Sender":"\"libcamera-devel\" <libcamera-devel-bounces@lists.libcamera.org>"}},{"id":12673,"web_url":"https://patchwork.libcamera.org/comment/12673/","msgid":"<6c058a32-2761-88c2-193f-45095ad61977@uajain.com>","date":"2020-09-23T12:53:23","subject":"Re: [libcamera-devel] [RFC v1 PATCH] android: jpeg: exif: Set\n\ttimezone information","submitter":{"id":1,"url":"https://patchwork.libcamera.org/api/people/1/","name":"Umang Jain","email":"email@uajain.com"},"content":"Hi Kieran\n\nOn 9/23/20 4:26 PM, Kieran Bingham wrote:\n> Hi Umang,\n>\n> On 23/09/2020 10:13, Umang Jain wrote:\n>> Hi,\n>>\n>> On 9/22/20 9:56 PM, Umang Jain wrote:\n>>> Get timezone information from the timestamp, although the resolution\n>>> for EXIF_TAG_TIME_ZONE_OFFSET is fairly limited (per-hour only).\n>>>\n>>> Experimentation with 'exiftool', commandline utility to read/write\n>>> exif metadata on images, resulted in rounding off the hours if the\n>>> minutes came out to >= 30. Hence, the behaviour is inspired from\n>>> exiftool itself. For e.g.,\n>>>\n>>> Timezone      Tag's value\n>>>    +1015     =>     10\n>>>    +0945     =>     10\n>>>    -1145     =>    -12\n>>>\n>>> The EXIF specification defines three other tags (OffsetTime,\n>>> OffsetTimeOriginal, OffsetTimeDigitized), in the EXIF IFD.\n>>> These are not supported by libexif yet.\n>>>\n>>> Signed-off-by: Umang Jain <email@uajain.com>\n>>> ---\n>>>    src/android/jpeg/exif.cpp | 31 +++++++++++++++++++++++++++++++\n>>>    src/android/jpeg/exif.h   |  1 +\n>>>    2 files changed, 32 insertions(+)\n>>>\n>>> ---\n>>>\n>>> Hi Laurent/Kieran,\n>>>\n>>> Can you soft review(or test) this patch to see if anything\n>>> is wrong with it?\n>>> The reason I am asking, because I cannot make it work :(\n>>> exiftool with a captured frame shows empty Timezone Offset tag as:\n>>> ```\n>>> Time Zone Offset                :\n>>> ```\n>>>\n>>> exif tool shows nothing.\n> Hrm, maybe adding some debug to the exif tool and rebuilding might help\n> identify what data it actually parses or why it can't identify the field.\nI didn't find any particular debug option in exiftool that would give \nout a error message. Although I found -v3 verbose output which can give \nhexdump of each tag. So attaching the various outputs here:\n\nWith:\na) setString(EXIF_IFD_0, EXIF_TAG_TIME_ZONE_OFFSET, EXIF_FORMAT_ASCII, \"6\");\no/p:\n   | 8)  TimeZoneOffset = 6\n   |     - Tag 0x882a (2 bytes, string[2]):\n   |         0090: 36 00 [6.]\nJPEG DQT (65 bytes):\n\n\nb) setSShort(EXIF_IFD_0, EXIF_TAG_TIME_ZONE_OFFSET, '6' or 6);\no/p:\n   | 8)  TimeZoneOffset =\n   |     - Tag 0x882a (0 bytes, undef):\n\n>\n> What does exiv2 report on the generated image?\n`exiv2` tool doesn't report the tag in any of the cases.\n`exif` tool reports it properly only if setString is used...\n>\n>>> I made sure a valid value being generated for timezone\n>>> offset in setTimeStamp().\n>>> I also checked the case where it might be preset using\n>>> exif_data_fix and not been able to re-set again.\n>>> But no, that doesn't seem the problem in my testing.\n>>>\n>>>\n>>> diff --git a/src/android/jpeg/exif.cpp b/src/android/jpeg/exif.cpp\n>>> index c0dbfcc..9c23cfb 100644\n>>> --- a/src/android/jpeg/exif.cpp\n>>> +++ b/src/android/jpeg/exif.cpp\n>>> @@ -7,6 +7,8 @@\n>>>      #include \"exif.h\"\n>>>    +#include <stdlib.h>\n>>> +\n>>>    #include \"libcamera/internal/log.h\"\n>>>      using namespace libcamera;\n>>> @@ -135,6 +137,16 @@ void Exif::setShort(ExifIfd ifd, ExifTag tag,\n>>> uint16_t item)\n>>>        exif_entry_unref(entry);\n>>>    }\n>>>    +void Exif::setSShort(ExifIfd ifd, ExifTag tag, int16_t item)\n>>> +{\n>>> +    ExifEntry *entry = createEntry(ifd, tag);\n>>> +    if (!entry)\n>>> +        return;\n>>> +\n>>> +    exif_set_sshort(entry->data, EXIF_BYTE_ORDER_INTEL, item);\n>> Getting to bottom of this in the morning, exif_set_sshort seems to\n>> convert its passed int16_t value to character. I don't know what's the\n>> logic behind this [1] (apart from EXIF's quirky-ness), but this simply\n>> doesn't look right then. So..\n>\n> Oh - converting a short to a char certainly doesn't sound right at all\n> for a function called 'set_short'.\n>\n> Char's are 8-bits. Shorts are 16....\n>\n> Oh - Now I've followed to [1], I see that's simply dealing with\n> byte-ordering issues. So it shouldn't be a problem.\noh, I misinterpreted things then.\n>\n>\n>>> +    exif_entry_unref(entry);\n>>> +}\n>>> +\n>>>    void Exif::setLong(ExifIfd ifd, ExifTag tag, uint32_t item)\n>>>    {\n>>>        ExifEntry *entry = createEntry(ifd, tag);\n>>> @@ -196,6 +208,25 @@ void Exif::setTimestamp(time_t timestamp)\n>>>        setString(EXIF_IFD_0, EXIF_TAG_DATE_TIME, EXIF_FORMAT_ASCII, ts);\n>>>        setString(EXIF_IFD_EXIF, EXIF_TAG_DATE_TIME_ORIGINAL,\n>>> EXIF_FORMAT_ASCII, ts);\n>>>        setString(EXIF_IFD_EXIF, EXIF_TAG_DATE_TIME_DIGITIZED,\n>>> EXIF_FORMAT_ASCII, ts);\n>>> +\n>>> +    /*\n>>> +     * If possible, query and set timezone information via\n>>> +     * EXIF_TAG_TIME_ZONE_OFFSET. There is only per-hour resolution\n>>> for tag\n>>> +     * hence, round up hours if minutes >= 30.\n> Where have you found the information that it is only 'per-hour' resolution?\nIt's what the tools use to do. They report on per-hour basis only. I \nhave mentioned the behaviour in the commit message.\nAlso stumbled upon \nhttps://github.com/jim-easterbrook/Photini/commit/686d26 literally few \nminutes ago.\n<https://github.com/jim-easterbrook/Photini/commit/686d26>\n>\n>\n>>> +     */\n>>> +    int r = strftime(str, sizeof(str), \"%z\", &tm);\n>>> +    if (r > 0) {\n>>> +        int16_t timezone = atoi(str);\n>>> +        int16_t hour = timezone / 100;\n>>> +        int16_t min = timezone % 100;;\n>> Complier should have errored out on ;; , but it didn't.\n>> Rectified locally on noticing.\n> ;; isn't going to generate a compiler error because the second ';' is\n> just an empty statement that does nothing.\n>\n> Still worth cleaning up of course ;-)\n>\n>\n>>> +\n>>> +        if (min <= -30)\n>>> +            hour--;\n>>> +        else if (min >= 30)\n>>> +            hour++;\n>>> +\n>>> +        setSShort(EXIF_IFD_0, EXIF_TAG_TIME_ZONE_OFFSET, hour);\n>> Instead of setSShort call, if we use setString, things seems to get\n>> moving. Something like:\n>> setString(EXIF_IFD_0, EXIF_TAG_TIME_ZONE_OFFSET, EXIF_FORMAT_ASCII, \"6\");\n>>\n>> got parsed correctly by exiftool and exif commandline utilities.\n> Ok, so lets tie down exactly what the right format should be written to\n> this field.\n>\n> Writing '6' as ascii infers that the actual value that got written to\n> memory in that place was 54....\n>\n> What was presented with the tools? Did it show a '6' or the integer value?\nIt showed '6', no integer/ascii value of '6' was spotted anytime with \nany tool.\n>\n> If this really is a field for a short, we should write the ascii value\n> to the short, not a string... which (currently) adds null padding too!?\ntried to setSShort with ascii value of 6, no improvements.\nIf setSShort is used, the exiftool reports it as 'empty' value :(\nhttps://paste.debian.net/1164408/\n>\n> Interestingly:\n> https://github.com/libexif/libexif/blob/master/libexif/exif-tag.c#L523\n> seems to state that EXIF_TAG_TIME_ZONE_OFFSET is not in Exif-2.2...\n>\n> Indeed, examining closely I see no reference to a tag with id 0x882a\n>\n> https://www.exiv2.org/tags.html lists Exif.Image.TimeZoneOffset as an\n> SShort at 0x882a (34858), with the following text:\n>\n> \"\"\"\n> This optional tag encodes the time zone of the camera clock (relative to\n> Greenwich Mean Time) used to create the DataTimeOriginal tag-value when\n> the picture was taken. It may also contain the time zone offset of the\n> clock used to create the DateTime tag-value when the image was modified.\n> \"\"\"\n>\n> But that doesn't fully explain how the value is encoded.\n>\n>\n>\n>\n> And ... now I've gone further down the rabbit-hole, I've discovered Exif\n> 2.31 is available from :\n>\n>    http://cipa.jp/std/documents/download_e.html?DC-008-Translation-2016-E\n>\n> It also doesn't list a tag at 0x882a, but does explicitly mention a tag\n> \"OffsetTime\" (0x9010) which is an ascii field matching the string\n> generated by the strftime(str, sizeof(str), \"%z\", &tm) call ;-)\n>\n>\n> See page 49 in that document from cipa.jp.\nThere are 3 more timezone offset related tags that are mentioned but not \nsupported by libexif.  Laurent has pointed this out in previous replies \nto this thread.\n>\n> I expect we should explicitly set our exif version to \"0231\" if we use it?\nhmm, yea, looking at the above pastebin output, my exif version is \n'0210' so I would agree setting it 0231 because that's where the tag is \nintroduced.\n\nMy local version of libexif is 0.6.21 which already seems to support the \ntimezone offset tag. Although, I would expect it to report that the tag \nI am trying set needs a higher exif version.\n\n(Explicitly set exif version to 0231, no improvements observed)\n>\n> Do the exif/exiv2 libraries support Exif 2.31?\nhmm, probably : https://github.com/libexif/libexif/commit/54333d8b8c\n\nNot sure, how they keep up with minor versions, OR rather support new \ntags directly.\n>\n> --\n> Regards\n>\n> Kieran\n>\n>\n>\n>> [1]:\n>> https://github.com/libexif/libexif/blob/master/libexif/exif-utils.c#L107-L121\n>>\n>>> +    }\n>>>    }\n>>>      void Exif::setOrientation(int orientation)\n>>> diff --git a/src/android/jpeg/exif.h b/src/android/jpeg/exif.h\n>>> index f04cefc..9c9cc3b 100644\n>>> --- a/src/android/jpeg/exif.h\n>>> +++ b/src/android/jpeg/exif.h\n>>> @@ -37,6 +37,7 @@ private:\n>>>                       unsigned long components, unsigned int size);\n>>>          void setShort(ExifIfd ifd, ExifTag tag, uint16_t item);\n>>> +    void setSShort(ExifIfd ifd, ExifTag tag, int16_t item);\n>>>        void setLong(ExifIfd ifd, ExifTag tag, uint32_t item);\n>>>        void setString(ExifIfd ifd, ExifTag tag, ExifFormat format,\n>>>                   const std::string &item);\n>> _______________________________________________\n>> libcamera-devel mailing list\n>> libcamera-devel@lists.libcamera.org\n>> https://lists.libcamera.org/listinfo/libcamera-devel","headers":{"Return-Path":"<libcamera-devel-bounces@lists.libcamera.org>","X-Original-To":"parsemail@patchwork.libcamera.org","Delivered-To":"parsemail@patchwork.libcamera.org","Received":["from lancelot.ideasonboard.com (lancelot.ideasonboard.com\n\t[92.243.16.209])\n\tby patchwork.libcamera.org (Postfix) with ESMTPS id A24BFC3B5B\n\tfor <parsemail@patchwork.libcamera.org>;\n\tWed, 23 Sep 2020 12:53:30 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id 238A462FE3;\n\tWed, 23 Sep 2020 14:53:30 +0200 (CEST)","from mail.uajain.com (static.126.159.217.95.clients.your-server.de\n\t[95.217.159.126])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTPS id E5B9660576\n\tfor <libcamera-devel@lists.libcamera.org>;\n\tWed, 23 Sep 2020 14:53:27 +0200 (CEST)"],"Authentication-Results":"lancelot.ideasonboard.com;\n\tdkim=fail reason=\"signature verification failed\" (2048-bit key;\n\tunprotected) header.d=uajain.com header.i=@uajain.com\n\theader.b=\"nREGcQ4W\"; dkim-atps=neutral","DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/simple; d=uajain.com; s=mail;\n\tt=1600865607; bh=JUvvcVZ1TTGIGOqixKj1ViIWzGfhkNFKj33lpio0ETQ=;\n\th=Subject:To:References:From:In-Reply-To;\n\tb=nREGcQ4WvEyx1TtSSobdxhrlcqZh+eGmUuJh071UfJzttA/n1jD/X0Q7MZjYXLMan\n\tI96XYAxLAl0/b718QO0Y/WL/4R2lTUuUmg44J9YQ8R9zQ3KBVx89TsOYmdruxQxKVy\n\tOfCD+1Pf8YLFsLnURmlGqUePRz9t9NYYITJ29z4AK5sJprQ4eByMKeXwZCAx9yP0Zy\n\tBfM9leO3I0UBAvsnBBJ2F8AnegADlzOJXjXDLE3Z0PGZyO7qPORUC/mRrHZxrwUFXe\n\tMYydTfdJeaRE1jGBlO4Hs3ASg99NKJ2Zn2kJ0vE5bNlfkIma2eEoJN3l6dZVecFmTK\n\tMxZyqjXPY+r+A==","To":"kieran.bingham@ideasonboard.com, libcamera-devel@lists.libcamera.org","References":"<20200909153206.7700-1-email@uajain.com>\n\t<20200922162624.32321-1-email@uajain.com>\n\t<70e975f3-ce7c-891c-af41-3775cb9d0fee@uajain.com>\n\t<9f991efd-324d-3b37-1a5d-0901b25860e1@ideasonboard.com>","From":"Umang Jain <email@uajain.com>","Message-ID":"<6c058a32-2761-88c2-193f-45095ad61977@uajain.com>","Date":"Wed, 23 Sep 2020 18:23:23 +0530","Mime-Version":"1.0","In-Reply-To":"<9f991efd-324d-3b37-1a5d-0901b25860e1@ideasonboard.com>","Content-Language":"en-US","Subject":"Re: [libcamera-devel] [RFC v1 PATCH] android: jpeg: exif: Set\n\ttimezone information","X-BeenThere":"libcamera-devel@lists.libcamera.org","X-Mailman-Version":"2.1.29","Precedence":"list","List-Id":"<libcamera-devel.lists.libcamera.org>","List-Unsubscribe":"<https://lists.libcamera.org/options/libcamera-devel>,\n\t<mailto:libcamera-devel-request@lists.libcamera.org?subject=unsubscribe>","List-Archive":"<https://lists.libcamera.org/pipermail/libcamera-devel/>","List-Post":"<mailto:libcamera-devel@lists.libcamera.org>","List-Help":"<mailto:libcamera-devel-request@lists.libcamera.org?subject=help>","List-Subscribe":"<https://lists.libcamera.org/listinfo/libcamera-devel>,\n\t<mailto:libcamera-devel-request@lists.libcamera.org?subject=subscribe>","Content-Type":"multipart/mixed;\n\tboundary=\"===============8441832717016013044==\"","Errors-To":"libcamera-devel-bounces@lists.libcamera.org","Sender":"\"libcamera-devel\" <libcamera-devel-bounces@lists.libcamera.org>"}},{"id":12676,"web_url":"https://patchwork.libcamera.org/comment/12676/","msgid":"<20200923132934.GC3980@pendragon.ideasonboard.com>","date":"2020-09-23T13:29:34","subject":"Re: [libcamera-devel] [RFC v1 PATCH] android: jpeg: exif: Set\n\ttimezone information","submitter":{"id":2,"url":"https://patchwork.libcamera.org/api/people/2/","name":"Laurent Pinchart","email":"laurent.pinchart@ideasonboard.com"},"content":"Hello,\n\nOn Wed, Sep 23, 2020 at 06:23:23PM +0530, Umang Jain wrote:\n> On 9/23/20 4:26 PM, Kieran Bingham wrote:\n> > On 23/09/2020 10:13, Umang Jain wrote:\n> >> On 9/22/20 9:56 PM, Umang Jain wrote:\n> >>> Get timezone information from the timestamp, although the resolution\n> >>> for EXIF_TAG_TIME_ZONE_OFFSET is fairly limited (per-hour only).\n> >>>\n> >>> Experimentation with 'exiftool', commandline utility to read/write\n> >>> exif metadata on images, resulted in rounding off the hours if the\n> >>> minutes came out to >= 30. Hence, the behaviour is inspired from\n> >>> exiftool itself. For e.g.,\n> >>>\n> >>> Timezone      Tag's value\n> >>>    +1015     =>     10\n> >>>    +0945     =>     10\n> >>>    -1145     =>    -12\n> >>>\n> >>> The EXIF specification defines three other tags (OffsetTime,\n> >>> OffsetTimeOriginal, OffsetTimeDigitized), in the EXIF IFD.\n> >>> These are not supported by libexif yet.\n> >>>\n> >>> Signed-off-by: Umang Jain <email@uajain.com>\n> >>> ---\n> >>>    src/android/jpeg/exif.cpp | 31 +++++++++++++++++++++++++++++++\n> >>>    src/android/jpeg/exif.h   |  1 +\n> >>>    2 files changed, 32 insertions(+)\n> >>>\n> >>> ---\n> >>>\n> >>> Hi Laurent/Kieran,\n> >>>\n> >>> Can you soft review(or test) this patch to see if anything\n> >>> is wrong with it?\n> >>> The reason I am asking, because I cannot make it work :(\n> >>> exiftool with a captured frame shows empty Timezone Offset tag as:\n> >>> ```\n> >>> Time Zone Offset                :\n> >>> ```\n> >>>\n> >>> exif tool shows nothing.\n> >\n> > Hrm, maybe adding some debug to the exif tool and rebuilding might help\n> > identify what data it actually parses or why it can't identify the field.\n>\n> I didn't find any particular debug option in exiftool that would give \n> out a error message. Although I found -v3 verbose output which can give \n> hexdump of each tag. So attaching the various outputs here:\n> \n> With:\n> a) setString(EXIF_IFD_0, EXIF_TAG_TIME_ZONE_OFFSET, EXIF_FORMAT_ASCII, \"6\");\n> o/p:\n>    | 8)  TimeZoneOffset = 6\n>    |     - Tag 0x882a (2 bytes, string[2]):\n>    |         0090: 36 00 [6.]\n> JPEG DQT (65 bytes):\n> \n> \n> b) setSShort(EXIF_IFD_0, EXIF_TAG_TIME_ZONE_OFFSET, '6' or 6);\n> o/p:\n>    | 8)  TimeZoneOffset =\n>    |     - Tag 0x882a (0 bytes, undef):\n> \n> > What does exiv2 report on the generated image?\n>\n> `exiv2` tool doesn't report the tag in any of the cases.\n> `exif` tool reports it properly only if setString is used...\n>\n> >>> I made sure a valid value being generated for timezone\n> >>> offset in setTimeStamp().\n> >>> I also checked the case where it might be preset using\n> >>> exif_data_fix and not been able to re-set again.\n> >>> But no, that doesn't seem the problem in my testing.\n> >>>\n> >>>\n> >>> diff --git a/src/android/jpeg/exif.cpp b/src/android/jpeg/exif.cpp\n> >>> index c0dbfcc..9c23cfb 100644\n> >>> --- a/src/android/jpeg/exif.cpp\n> >>> +++ b/src/android/jpeg/exif.cpp\n> >>> @@ -7,6 +7,8 @@\n> >>>      #include \"exif.h\"\n> >>>    +#include <stdlib.h>\n> >>> +\n> >>>    #include \"libcamera/internal/log.h\"\n> >>>      using namespace libcamera;\n> >>> @@ -135,6 +137,16 @@ void Exif::setShort(ExifIfd ifd, ExifTag tag,\n> >>> uint16_t item)\n> >>>        exif_entry_unref(entry);\n> >>>    }\n> >>>    +void Exif::setSShort(ExifIfd ifd, ExifTag tag, int16_t item)\n> >>> +{\n> >>> +    ExifEntry *entry = createEntry(ifd, tag);\n> >>> +    if (!entry)\n> >>> +        return;\n> >>> +\n> >>> +    exif_set_sshort(entry->data, EXIF_BYTE_ORDER_INTEL, item);\n> >>\n> >> Getting to bottom of this in the morning, exif_set_sshort seems to\n> >> convert its passed int16_t value to character. I don't know what's the\n> >> logic behind this [1] (apart from EXIF's quirky-ness), but this simply\n> >> doesn't look right then. So..\n> >\n> > Oh - converting a short to a char certainly doesn't sound right at all\n> > for a function called 'set_short'.\n> >\n> > Char's are 8-bits. Shorts are 16....\n> >\n> > Oh - Now I've followed to [1], I see that's simply dealing with\n> > byte-ordering issues. So it shouldn't be a problem.\n>\n> oh, I misinterpreted things then.\n>\n> >>> +    exif_entry_unref(entry);\n> >>> +}\n> >>> +\n> >>>    void Exif::setLong(ExifIfd ifd, ExifTag tag, uint32_t item)\n> >>>    {\n> >>>        ExifEntry *entry = createEntry(ifd, tag);\n> >>> @@ -196,6 +208,25 @@ void Exif::setTimestamp(time_t timestamp)\n> >>>        setString(EXIF_IFD_0, EXIF_TAG_DATE_TIME, EXIF_FORMAT_ASCII, ts);\n> >>>        setString(EXIF_IFD_EXIF, EXIF_TAG_DATE_TIME_ORIGINAL, EXIF_FORMAT_ASCII, ts);\n> >>>        setString(EXIF_IFD_EXIF, EXIF_TAG_DATE_TIME_DIGITIZED, EXIF_FORMAT_ASCII, ts);\n> >>> +\n> >>> +    /*\n> >>> +     * If possible, query and set timezone information via\n> >>> +     * EXIF_TAG_TIME_ZONE_OFFSET. There is only per-hour resolution for tag\n> >>> +     * hence, round up hours if minutes >= 30.\n> >\n> > Where have you found the information that it is only 'per-hour' resolution?\n>\n> It's what the tools use to do. They report on per-hour basis only. I \n> have mentioned the behaviour in the commit message.\n> Also stumbled upon \n> https://github.com/jim-easterbrook/Photini/commit/686d26 literally few \n> minutes ago.\n\nTimeZoneOffset is defined in the TIFF/EP specification (ISO/DIS\n12234-2).\n\n5.2.48 TimeZoneOffset\n\nThis optional tag encodes the time zone of the camera clock (relative to\nGreenwich Mean Time) used to create the DataTimeOriginal tag-value when\nthe picture was taken. It may also contain the time zone offset of the\nclock used to create the DateTime tag-value when the image was modified.\n\nTag Name = TimeZoneOffset\nTag = 34858 (882A.H)\nType = SSHORT\nN = 1 or 2\nValue = VALUE\nThe allowed values are -12 to +11.\n\nSSHORT 0 Time Zone Offset (in hours) of DateTimeOriginal tag-value relative to Greenwich Mean Time\nSSHORT 1 If present, Time Zone Offset (in hours) of DateTime tag-value relative to Greenwich Mean Time\n\nUsage: IFD0\n\nIt's fairly ill-defined as time zones extend to UTC+14, but it's clearly\na signed short, with one or two values, expressed as a number of hours.\nWe should really use the time zone tags of the EXIF specification\ninstead, but they're not supported by libexif.\n\n> >>> +     */\n> >>> +    int r = strftime(str, sizeof(str), \"%z\", &tm);\n> >>> +    if (r > 0) {\n> >>> +        int16_t timezone = atoi(str);\n> >>> +        int16_t hour = timezone / 100;\n> >>> +        int16_t min = timezone % 100;;\n> >>\n> >> Complier should have errored out on ;; , but it didn't.\n> >> Rectified locally on noticing.\n> >\n> > ;; isn't going to generate a compiler error because the second ';' is\n> > just an empty statement that does nothing.\n> >\n> > Still worth cleaning up of course ;-)\n> >\n> >>> +\n> >>> +        if (min <= -30)\n> >>> +            hour--;\n> >>> +        else if (min >= 30)\n> >>> +            hour++;\n> >>> +\n> >>> +        setSShort(EXIF_IFD_0, EXIF_TAG_TIME_ZONE_OFFSET, hour);\n> >>\n> >> Instead of setSShort call, if we use setString, things seems to get\n> >> moving. Something like:\n> >> setString(EXIF_IFD_0, EXIF_TAG_TIME_ZONE_OFFSET, EXIF_FORMAT_ASCII, \"6\");\n> >>\n> >> got parsed correctly by exiftool and exif commandline utilities.\n> >\n> > Ok, so lets tie down exactly what the right format should be written to\n> > this field.\n> >\n> > Writing '6' as ascii infers that the actual value that got written to\n> > memory in that place was 54....\n> >\n> > What was presented with the tools? Did it show a '6' or the integer value?\n>\n> It showed '6', no integer/ascii value of '6' was spotted anytime with \n> any tool.\n>\n> > If this really is a field for a short, we should write the ascii value\n> > to the short, not a string... which (currently) adds null padding too!?\n>\n> tried to setSShort with ascii value of 6, no improvements.\n> If setSShort is used, the exiftool reports it as 'empty' value :(\n> https://paste.debian.net/1164408/\n\nCan you provide a sample image that has the time zone offset tag set\nwith setSShort() ?\n\n> > Interestingly:\n> > https://github.com/libexif/libexif/blob/master/libexif/exif-tag.c#L523\n> > seems to state that EXIF_TAG_TIME_ZONE_OFFSET is not in Exif-2.2...\n> >\n> > Indeed, examining closely I see no reference to a tag with id 0x882a\n> >\n> > https://www.exiv2.org/tags.html lists Exif.Image.TimeZoneOffset as an\n> > SShort at 0x882a (34858), with the following text:\n> >\n> > \"\"\"\n> > This optional tag encodes the time zone of the camera clock (relative to\n> > Greenwich Mean Time) used to create the DataTimeOriginal tag-value when\n> > the picture was taken. It may also contain the time zone offset of the\n> > clock used to create the DateTime tag-value when the image was modified.\n> > \"\"\"\n> >\n> > But that doesn't fully explain how the value is encoded.\n> >\n> > And ... now I've gone further down the rabbit-hole, I've discovered Exif\n> > 2.31 is available from :\n> >\n> >    http://cipa.jp/std/documents/download_e.html?DC-008-Translation-2016-E\n> >\n> > It also doesn't list a tag at 0x882a, but does explicitly mention a tag\n> > \"OffsetTime\" (0x9010) which is an ascii field matching the string\n> > generated by the strftime(str, sizeof(str), \"%z\", &tm) call ;-)\n> >\n> >\n> > See page 49 in that document from cipa.jp.\n>\n> There are 3 more timezone offset related tags that are mentioned but not \n> supported by libexif.  Laurent has pointed this out in previous replies \n> to this thread.\n>\n> > I expect we should explicitly set our exif version to \"0231\" if we use it?\n>\n> hmm, yea, looking at the above pastebin output, my exif version is \n> '0210' so I would agree setting it 0231 because that's where the tag is \n> introduced.\n\nThe TimeZoneOffset tag doesn't exist in any EXIF specification, so I\ndon't think the EXIF version will make any difference.\n\n> My local version of libexif is 0.6.21 which already seems to support the \n> timezone offset tag. Although, I would expect it to report that the tag \n> I am trying set needs a higher exif version.\n> \n> (Explicitly set exif version to 0231, no improvements observed)\n>\n> > Do the exif/exiv2 libraries support Exif 2.31?\n>\n> hmm, probably : https://github.com/libexif/libexif/commit/54333d8b8c\n> \n> Not sure, how they keep up with minor versions, OR rather support new \n> tags directly.\n>\n> >> [1]:\n> >> https://github.com/libexif/libexif/blob/master/libexif/exif-utils.c#L107-L121\n> >>\n> >>> +    }\n> >>>    }\n> >>>      void Exif::setOrientation(int orientation)\n> >>> diff --git a/src/android/jpeg/exif.h b/src/android/jpeg/exif.h\n> >>> index f04cefc..9c9cc3b 100644\n> >>> --- a/src/android/jpeg/exif.h\n> >>> +++ b/src/android/jpeg/exif.h\n> >>> @@ -37,6 +37,7 @@ private:\n> >>>                       unsigned long components, unsigned int size);\n> >>>          void setShort(ExifIfd ifd, ExifTag tag, uint16_t item);\n> >>> +    void setSShort(ExifIfd ifd, ExifTag tag, int16_t item);\n> >>>        void setLong(ExifIfd ifd, ExifTag tag, uint32_t item);\n> >>>        void setString(ExifIfd ifd, ExifTag tag, ExifFormat format,\n> >>>                   const std::string &item);","headers":{"Return-Path":"<libcamera-devel-bounces@lists.libcamera.org>","X-Original-To":"parsemail@patchwork.libcamera.org","Delivered-To":"parsemail@patchwork.libcamera.org","Received":["from lancelot.ideasonboard.com (lancelot.ideasonboard.com\n\t[92.243.16.209])\n\tby patchwork.libcamera.org (Postfix) with ESMTPS id 7375EC3B5B\n\tfor <parsemail@patchwork.libcamera.org>;\n\tWed, 23 Sep 2020 13:30:09 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id 41C2762FE6;\n\tWed, 23 Sep 2020 15:30:09 +0200 (CEST)","from perceval.ideasonboard.com (perceval.ideasonboard.com\n\t[IPv6:2001:4b98:dc2:55:216:3eff:fef7:d647])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTPS id BBB8E60576\n\tfor <libcamera-devel@lists.libcamera.org>;\n\tWed, 23 Sep 2020 15:30:07 +0200 (CEST)","from pendragon.ideasonboard.com (62-78-145-57.bb.dnainternet.fi\n\t[62.78.145.57])\n\tby perceval.ideasonboard.com (Postfix) with ESMTPSA id 47541555;\n\tWed, 23 Sep 2020 15:30:07 +0200 (CEST)"],"Authentication-Results":"lancelot.ideasonboard.com;\n\tdkim=fail reason=\"signature verification failed\" (1024-bit key;\n\tunprotected) header.d=ideasonboard.com header.i=@ideasonboard.com\n\theader.b=\"XmlzkusL\"; dkim-atps=neutral","DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com;\n\ts=mail; t=1600867807;\n\tbh=7EWdn8Bq+q+FtVhkxBvn1YYEVViXOGAiRmytNdcAusg=;\n\th=Date:From:To:Cc:Subject:References:In-Reply-To:From;\n\tb=XmlzkusL2d4qvk2H+Zh2hpFnGxBxaa/kCYjxEPMQNLHTrL7wmyEwWgOoNtFcDUdIs\n\tI7qWRGW4pSYNFILh0VHQJzmWPNwCDEnUzWFyuhnTf/NBNEuaul6w6+l9D95jhmVTzu\n\tDa77RLMfrDt8gSQWlelr//1kLdX+iWeioN5YU0mU=","Date":"Wed, 23 Sep 2020 16:29:34 +0300","From":"Laurent Pinchart <laurent.pinchart@ideasonboard.com>","To":"Umang Jain <email@uajain.com>","Message-ID":"<20200923132934.GC3980@pendragon.ideasonboard.com>","References":"<20200909153206.7700-1-email@uajain.com>\n\t<20200922162624.32321-1-email@uajain.com>\n\t<70e975f3-ce7c-891c-af41-3775cb9d0fee@uajain.com>\n\t<9f991efd-324d-3b37-1a5d-0901b25860e1@ideasonboard.com>\n\t<6c058a32-2761-88c2-193f-45095ad61977@uajain.com>","MIME-Version":"1.0","Content-Disposition":"inline","In-Reply-To":"<6c058a32-2761-88c2-193f-45095ad61977@uajain.com>","Subject":"Re: [libcamera-devel] [RFC v1 PATCH] android: jpeg: exif: Set\n\ttimezone information","X-BeenThere":"libcamera-devel@lists.libcamera.org","X-Mailman-Version":"2.1.29","Precedence":"list","List-Id":"<libcamera-devel.lists.libcamera.org>","List-Unsubscribe":"<https://lists.libcamera.org/options/libcamera-devel>,\n\t<mailto:libcamera-devel-request@lists.libcamera.org?subject=unsubscribe>","List-Archive":"<https://lists.libcamera.org/pipermail/libcamera-devel/>","List-Post":"<mailto:libcamera-devel@lists.libcamera.org>","List-Help":"<mailto:libcamera-devel-request@lists.libcamera.org?subject=help>","List-Subscribe":"<https://lists.libcamera.org/listinfo/libcamera-devel>,\n\t<mailto:libcamera-devel-request@lists.libcamera.org?subject=subscribe>","Cc":"libcamera-devel@lists.libcamera.org","Content-Type":"text/plain; charset=\"utf-8\"","Content-Transfer-Encoding":"base64","Errors-To":"libcamera-devel-bounces@lists.libcamera.org","Sender":"\"libcamera-devel\" <libcamera-devel-bounces@lists.libcamera.org>"}},{"id":12677,"web_url":"https://patchwork.libcamera.org/comment/12677/","msgid":"<502e4ff5-e5e5-c6e1-4f0b-58b9c69f50e0@uajain.com>","date":"2020-09-23T13:48:03","subject":"Re: [libcamera-devel] [RFC v1 PATCH] android: jpeg: exif: Set\n\ttimezone information","submitter":{"id":1,"url":"https://patchwork.libcamera.org/api/people/1/","name":"Umang Jain","email":"email@uajain.com"},"content":"Hi Laurent,\n\nOn 9/23/20 6:59 PM, Laurent Pinchart wrote:\n> Hello,\n>\n> On Wed, Sep 23, 2020 at 06:23:23PM +0530, Umang Jain wrote:\n>> On 9/23/20 4:26 PM, Kieran Bingham wrote:\n>>> On 23/09/2020 10:13, Umang Jain wrote:\n>>>> On 9/22/20 9:56 PM, Umang Jain wrote:\n>>>>> Get timezone information from the timestamp, although the resolution\n>>>>> for EXIF_TAG_TIME_ZONE_OFFSET is fairly limited (per-hour only).\n>>>>>\n>>>>> Experimentation with 'exiftool', commandline utility to read/write\n>>>>> exif metadata on images, resulted in rounding off the hours if the\n>>>>> minutes came out to >= 30. Hence, the behaviour is inspired from\n>>>>> exiftool itself. For e.g.,\n>>>>>\n>>>>> Timezone      Tag's value\n>>>>>     +1015     =>     10\n>>>>>     +0945     =>     10\n>>>>>     -1145     =>    -12\n>>>>>\n>>>>> The EXIF specification defines three other tags (OffsetTime,\n>>>>> OffsetTimeOriginal, OffsetTimeDigitized), in the EXIF IFD.\n>>>>> These are not supported by libexif yet.\n>>>>>\n>>>>> Signed-off-by: Umang Jain <email@uajain.com>\n>>>>> ---\n>>>>>     src/android/jpeg/exif.cpp | 31 +++++++++++++++++++++++++++++++\n>>>>>     src/android/jpeg/exif.h   |  1 +\n>>>>>     2 files changed, 32 insertions(+)\n>>>>>\n>>>>> ---\n>>>>>\n>>>>> Hi Laurent/Kieran,\n>>>>>\n>>>>> Can you soft review(or test) this patch to see if anything\n>>>>> is wrong with it?\n>>>>> The reason I am asking, because I cannot make it work :(\n>>>>> exiftool with a captured frame shows empty Timezone Offset tag as:\n>>>>> ```\n>>>>> Time Zone Offset                :\n>>>>> ```\n>>>>>\n>>>>> exif tool shows nothing.\n>>> Hrm, maybe adding some debug to the exif tool and rebuilding might help\n>>> identify what data it actually parses or why it can't identify the field.\n>> I didn't find any particular debug option in exiftool that would give\n>> out a error message. Although I found -v3 verbose output which can give\n>> hexdump of each tag. So attaching the various outputs here:\n>>\n>> With:\n>> a) setString(EXIF_IFD_0, EXIF_TAG_TIME_ZONE_OFFSET, EXIF_FORMAT_ASCII, \"6\");\n>> o/p:\n>>     | 8)  TimeZoneOffset = 6\n>>     |     - Tag 0x882a (2 bytes, string[2]):\n>>     |         0090: 36 00 [6.]\n>> JPEG DQT (65 bytes):\n>>\n>>\n>> b) setSShort(EXIF_IFD_0, EXIF_TAG_TIME_ZONE_OFFSET, '6' or 6);\n>> o/p:\n>>     | 8)  TimeZoneOffset =\n>>     |     - Tag 0x882a (0 bytes, undef):\n>>\n>>> What does exiv2 report on the generated image?\n>> `exiv2` tool doesn't report the tag in any of the cases.\n>> `exif` tool reports it properly only if setString is used...\n>>\n>>>>> I made sure a valid value being generated for timezone\n>>>>> offset in setTimeStamp().\n>>>>> I also checked the case where it might be preset using\n>>>>> exif_data_fix and not been able to re-set again.\n>>>>> But no, that doesn't seem the problem in my testing.\n>>>>>\n>>>>>\n>>>>> diff --git a/src/android/jpeg/exif.cpp b/src/android/jpeg/exif.cpp\n>>>>> index c0dbfcc..9c23cfb 100644\n>>>>> --- a/src/android/jpeg/exif.cpp\n>>>>> +++ b/src/android/jpeg/exif.cpp\n>>>>> @@ -7,6 +7,8 @@\n>>>>>       #include \"exif.h\"\n>>>>>     +#include <stdlib.h>\n>>>>> +\n>>>>>     #include \"libcamera/internal/log.h\"\n>>>>>       using namespace libcamera;\n>>>>> @@ -135,6 +137,16 @@ void Exif::setShort(ExifIfd ifd, ExifTag tag,\n>>>>> uint16_t item)\n>>>>>         exif_entry_unref(entry);\n>>>>>     }\n>>>>>     +void Exif::setSShort(ExifIfd ifd, ExifTag tag, int16_t item)\n>>>>> +{\n>>>>> +    ExifEntry *entry = createEntry(ifd, tag);\n>>>>> +    if (!entry)\n>>>>> +        return;\n>>>>> +\n>>>>> +    exif_set_sshort(entry->data, EXIF_BYTE_ORDER_INTEL, item);\n>>>> Getting to bottom of this in the morning, exif_set_sshort seems to\n>>>> convert its passed int16_t value to character. I don't know what's the\n>>>> logic behind this [1] (apart from EXIF's quirky-ness), but this simply\n>>>> doesn't look right then. So..\n>>> Oh - converting a short to a char certainly doesn't sound right at all\n>>> for a function called 'set_short'.\n>>>\n>>> Char's are 8-bits. Shorts are 16....\n>>>\n>>> Oh - Now I've followed to [1], I see that's simply dealing with\n>>> byte-ordering issues. So it shouldn't be a problem.\n>> oh, I misinterpreted things then.\n>>\n>>>>> +    exif_entry_unref(entry);\n>>>>> +}\n>>>>> +\n>>>>>     void Exif::setLong(ExifIfd ifd, ExifTag tag, uint32_t item)\n>>>>>     {\n>>>>>         ExifEntry *entry = createEntry(ifd, tag);\n>>>>> @@ -196,6 +208,25 @@ void Exif::setTimestamp(time_t timestamp)\n>>>>>         setString(EXIF_IFD_0, EXIF_TAG_DATE_TIME, EXIF_FORMAT_ASCII, ts);\n>>>>>         setString(EXIF_IFD_EXIF, EXIF_TAG_DATE_TIME_ORIGINAL, EXIF_FORMAT_ASCII, ts);\n>>>>>         setString(EXIF_IFD_EXIF, EXIF_TAG_DATE_TIME_DIGITIZED, EXIF_FORMAT_ASCII, ts);\n>>>>> +\n>>>>> +    /*\n>>>>> +     * If possible, query and set timezone information via\n>>>>> +     * EXIF_TAG_TIME_ZONE_OFFSET. There is only per-hour resolution for tag\n>>>>> +     * hence, round up hours if minutes >= 30.\n>>> Where have you found the information that it is only 'per-hour' resolution?\n>> It's what the tools use to do. They report on per-hour basis only. I\n>> have mentioned the behaviour in the commit message.\n>> Also stumbled upon\n>> https://github.com/jim-easterbrook/Photini/commit/686d26 literally few\n>> minutes ago.\n> TimeZoneOffset is defined in the TIFF/EP specification (ISO/DIS\n> 12234-2).\n>\n> 5.2.48 TimeZoneOffset\n>\n> This optional tag encodes the time zone of the camera clock (relative to\n> Greenwich Mean Time) used to create the DataTimeOriginal tag-value when\n> the picture was taken. It may also contain the time zone offset of the\n> clock used to create the DateTime tag-value when the image was modified.\n>\n> Tag Name = TimeZoneOffset\n> Tag = 34858 (882A.H)\n> Type = SSHORT\n> N = 1 or 2\n> Value = VALUE\n> The allowed values are -12 to +11.\n>\n> SSHORT 0 Time Zone Offset (in hours) of DateTimeOriginal tag-value relative to Greenwich Mean Time\n> SSHORT 1 If present, Time Zone Offset (in hours) of DateTime tag-value relative to Greenwich Mean Time\n>\n> Usage: IFD0\n>\n> It's fairly ill-defined as time zones extend to UTC+14, but it's clearly\n> a signed short, with one or two values, expressed as a number of hours.\n> We should really use the time zone tags of the EXIF specification\n> instead, but they're not supported by libexif.\n>\n>>>>> +     */\n>>>>> +    int r = strftime(str, sizeof(str), \"%z\", &tm);\n>>>>> +    if (r > 0) {\n>>>>> +        int16_t timezone = atoi(str);\n>>>>> +        int16_t hour = timezone / 100;\n>>>>> +        int16_t min = timezone % 100;;\n>>>> Complier should have errored out on ;; , but it didn't.\n>>>> Rectified locally on noticing.\n>>> ;; isn't going to generate a compiler error because the second ';' is\n>>> just an empty statement that does nothing.\n>>>\n>>> Still worth cleaning up of course ;-)\n>>>\n>>>>> +\n>>>>> +        if (min <= -30)\n>>>>> +            hour--;\n>>>>> +        else if (min >= 30)\n>>>>> +            hour++;\n>>>>> +\n>>>>> +        setSShort(EXIF_IFD_0, EXIF_TAG_TIME_ZONE_OFFSET, hour);\n>>>> Instead of setSShort call, if we use setString, things seems to get\n>>>> moving. Something like:\n>>>> setString(EXIF_IFD_0, EXIF_TAG_TIME_ZONE_OFFSET, EXIF_FORMAT_ASCII, \"6\");\n>>>>\n>>>> got parsed correctly by exiftool and exif commandline utilities.\n>>> Ok, so lets tie down exactly what the right format should be written to\n>>> this field.\n>>>\n>>> Writing '6' as ascii infers that the actual value that got written to\n>>> memory in that place was 54....\n>>>\n>>> What was presented with the tools? Did it show a '6' or the integer value?\n>> It showed '6', no integer/ascii value of '6' was spotted anytime with\n>> any tool.\n>>\n>>> If this really is a field for a short, we should write the ascii value\n>>> to the short, not a string... which (currently) adds null padding too!?\n>> tried to setSShort with ascii value of 6, no improvements.\n>> If setSShort is used, the exiftool reports it as 'empty' value :(\n>> https://paste.debian.net/1164408/\n> Can you provide a sample image that has the time zone offset tag set\n> with setSShort() ?\nPlease find attached a sample image to this mail.\n>\n>>> Interestingly:\n>>> https://github.com/libexif/libexif/blob/master/libexif/exif-tag.c#L523\n>>> seems to state that EXIF_TAG_TIME_ZONE_OFFSET is not in Exif-2.2...\n>>>\n>>> Indeed, examining closely I see no reference to a tag with id 0x882a\n>>>\n>>> https://www.exiv2.org/tags.html lists Exif.Image.TimeZoneOffset as an\n>>> SShort at 0x882a (34858), with the following text:\n>>>\n>>> \"\"\"\n>>> This optional tag encodes the time zone of the camera clock (relative to\n>>> Greenwich Mean Time) used to create the DataTimeOriginal tag-value when\n>>> the picture was taken. It may also contain the time zone offset of the\n>>> clock used to create the DateTime tag-value when the image was modified.\n>>> \"\"\"\n>>>\n>>> But that doesn't fully explain how the value is encoded.\n>>>\n>>> And ... now I've gone further down the rabbit-hole, I've discovered Exif\n>>> 2.31 is available from :\n>>>\n>>>     http://cipa.jp/std/documents/download_e.html?DC-008-Translation-2016-E\n>>>\n>>> It also doesn't list a tag at 0x882a, but does explicitly mention a tag\n>>> \"OffsetTime\" (0x9010) which is an ascii field matching the string\n>>> generated by the strftime(str, sizeof(str), \"%z\", &tm) call ;-)\n>>>\n>>>\n>>> See page 49 in that document from cipa.jp.\n>> There are 3 more timezone offset related tags that are mentioned but not\n>> supported by libexif.  Laurent has pointed this out in previous replies\n>> to this thread.\n>>\n>>> I expect we should explicitly set our exif version to \"0231\" if we use it?\n>> hmm, yea, looking at the above pastebin output, my exif version is\n>> '0210' so I would agree setting it 0231 because that's where the tag is\n>> introduced.\n> The TimeZoneOffset tag doesn't exist in any EXIF specification, so I\n> don't think the EXIF version will make any difference.\nAh, right. Booooing! :head_bang:\n>\n>> My local version of libexif is 0.6.21 which already seems to support the\n>> timezone offset tag. Although, I would expect it to report that the tag\n>> I am trying set needs a higher exif version.\n>>\n>> (Explicitly set exif version to 0231, no improvements observed)\n>>\n>>> Do the exif/exiv2 libraries support Exif 2.31?\n>> hmm, probably : https://github.com/libexif/libexif/commit/54333d8b8c\n>>\n>> Not sure, how they keep up with minor versions, OR rather support new\n>> tags directly.\n>>\n>>>> [1]:\n>>>> https://github.com/libexif/libexif/blob/master/libexif/exif-utils.c#L107-L121\n>>>>\n>>>>> +    }\n>>>>>     }\n>>>>>       void Exif::setOrientation(int orientation)\n>>>>> diff --git a/src/android/jpeg/exif.h b/src/android/jpeg/exif.h\n>>>>> index f04cefc..9c9cc3b 100644\n>>>>> --- a/src/android/jpeg/exif.h\n>>>>> +++ b/src/android/jpeg/exif.h\n>>>>> @@ -37,6 +37,7 @@ private:\n>>>>>                        unsigned long components, unsigned int size);\n>>>>>           void setShort(ExifIfd ifd, ExifTag tag, uint16_t item);\n>>>>> +    void setSShort(ExifIfd ifd, ExifTag tag, int16_t item);\n>>>>>         void setLong(ExifIfd ifd, ExifTag tag, uint32_t item);\n>>>>>         void setString(ExifIfd ifd, ExifTag tag, ExifFormat format,\n>>>>>                    const std::string &item);","headers":{"Return-Path":"<libcamera-devel-bounces@lists.libcamera.org>","X-Original-To":"parsemail@patchwork.libcamera.org","Delivered-To":"parsemail@patchwork.libcamera.org","Received":["from lancelot.ideasonboard.com (lancelot.ideasonboard.com\n\t[92.243.16.209])\n\tby patchwork.libcamera.org (Postfix) with ESMTPS id A1ADBC3B5C\n\tfor <parsemail@patchwork.libcamera.org>;\n\tWed, 23 Sep 2020 13:48:11 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id 2D0BF62FDE;\n\tWed, 23 Sep 2020 15:48:11 +0200 (CEST)","from mail.uajain.com (static.126.159.217.95.clients.your-server.de\n\t[95.217.159.126])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTPS id 5153060576\n\tfor <libcamera-devel@lists.libcamera.org>;\n\tWed, 23 Sep 2020 15:48:09 +0200 (CEST)"],"Authentication-Results":"lancelot.ideasonboard.com;\n\tdkim=fail reason=\"signature verification failed\" (2048-bit key;\n\tunprotected) header.d=uajain.com header.i=@uajain.com\n\theader.b=\"OBlRBNwJ\"; dkim-atps=neutral","DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/simple; d=uajain.com; s=mail;\n\tt=1600868888; bh=UZyq1BxqP+GevO0i5IJ6KZ+5tzHa1fGlzJ8zIO2sebQ=;\n\th=Subject:To:Cc:References:From:In-Reply-To;\n\tb=OBlRBNwJDALg+H10dfzkNd3oFMoz5ou/OcmYqRXkq8HgyLl4qHt3c0WktEvBWs7n1\n\t+ohDWPt18I1fMUmetlYwBW8hRXEvi1O2jNSz7lYNZgepr4gCxKoK74nKTB4ttUook9\n\tTxg61flQaReODiqZ20Ye/+6uI5aW0clCM0KjAyLYLurJhgfFkFhUqcSFLLtxt3YYTN\n\tFJ6D7Y0EN3HL4hsVKmvp3KgHK8mFO2cIrAoIyt32nzc5jdgsVvcvrVZQeDH8zwFhQa\n\tmSlf/K/Iv1lucOKpz5RbMmc9z/A1TALKwofYBKrOUbTWrhHBr4yiEEhFYY7qy592jl\n\t15M7imb86RlOA==","To":"Laurent Pinchart <laurent.pinchart@ideasonboard.com>","References":"<20200909153206.7700-1-email@uajain.com>\n\t<20200922162624.32321-1-email@uajain.com>\n\t<70e975f3-ce7c-891c-af41-3775cb9d0fee@uajain.com>\n\t<9f991efd-324d-3b37-1a5d-0901b25860e1@ideasonboard.com>\n\t<6c058a32-2761-88c2-193f-45095ad61977@uajain.com>\n\t<20200923132934.GC3980@pendragon.ideasonboard.com>","From":"Umang Jain <email@uajain.com>","Message-ID":"<502e4ff5-e5e5-c6e1-4f0b-58b9c69f50e0@uajain.com>","Date":"Wed, 23 Sep 2020 19:18:03 +0530","Mime-Version":"1.0","In-Reply-To":"<20200923132934.GC3980@pendragon.ideasonboard.com>","Content-Type":"multipart/mixed;\n\tboundary=\"------------17BE6537604A8EBEF8BB369C\"","Content-Language":"en-US","Subject":"Re: [libcamera-devel] [RFC v1 PATCH] android: jpeg: exif: Set\n\ttimezone information","X-BeenThere":"libcamera-devel@lists.libcamera.org","X-Mailman-Version":"2.1.29","Precedence":"list","List-Id":"<libcamera-devel.lists.libcamera.org>","List-Unsubscribe":"<https://lists.libcamera.org/options/libcamera-devel>,\n\t<mailto:libcamera-devel-request@lists.libcamera.org?subject=unsubscribe>","List-Archive":"<https://lists.libcamera.org/pipermail/libcamera-devel/>","List-Post":"<mailto:libcamera-devel@lists.libcamera.org>","List-Help":"<mailto:libcamera-devel-request@lists.libcamera.org?subject=help>","List-Subscribe":"<https://lists.libcamera.org/listinfo/libcamera-devel>,\n\t<mailto:libcamera-devel-request@lists.libcamera.org?subject=subscribe>","Cc":"libcamera-devel@lists.libcamera.org","Errors-To":"libcamera-devel-bounces@lists.libcamera.org","Sender":"\"libcamera-devel\" <libcamera-devel-bounces@lists.libcamera.org>"}},{"id":12679,"web_url":"https://patchwork.libcamera.org/comment/12679/","msgid":"<20200923154112.GD3980@pendragon.ideasonboard.com>","date":"2020-09-23T15:41:12","subject":"Re: [libcamera-devel] [RFC v1 PATCH] android: jpeg: exif: Set\n\ttimezone information","submitter":{"id":2,"url":"https://patchwork.libcamera.org/api/people/2/","name":"Laurent Pinchart","email":"laurent.pinchart@ideasonboard.com"},"content":"Hi Umang,\n\nOn Wed, Sep 23, 2020 at 07:18:03PM +0530, Umang Jain wrote:\n> On 9/23/20 6:59 PM, Laurent Pinchart wrote:\n> > On Wed, Sep 23, 2020 at 06:23:23PM +0530, Umang Jain wrote:\n> >> On 9/23/20 4:26 PM, Kieran Bingham wrote:\n> >>> On 23/09/2020 10:13, Umang Jain wrote:\n> >>>> On 9/22/20 9:56 PM, Umang Jain wrote:\n> >>>>> Get timezone information from the timestamp, although the resolution\n> >>>>> for EXIF_TAG_TIME_ZONE_OFFSET is fairly limited (per-hour only).\n> >>>>>\n> >>>>> Experimentation with 'exiftool', commandline utility to read/write\n> >>>>> exif metadata on images, resulted in rounding off the hours if the\n> >>>>> minutes came out to >= 30. Hence, the behaviour is inspired from\n> >>>>> exiftool itself. For e.g.,\n> >>>>>\n> >>>>> Timezone      Tag's value\n> >>>>>     +1015     =>     10\n> >>>>>     +0945     =>     10\n> >>>>>     -1145     =>    -12\n> >>>>>\n> >>>>> The EXIF specification defines three other tags (OffsetTime,\n> >>>>> OffsetTimeOriginal, OffsetTimeDigitized), in the EXIF IFD.\n> >>>>> These are not supported by libexif yet.\n> >>>>>\n> >>>>> Signed-off-by: Umang Jain <email@uajain.com>\n> >>>>> ---\n> >>>>>     src/android/jpeg/exif.cpp | 31 +++++++++++++++++++++++++++++++\n> >>>>>     src/android/jpeg/exif.h   |  1 +\n> >>>>>     2 files changed, 32 insertions(+)\n> >>>>>\n> >>>>> ---\n> >>>>>\n> >>>>> Hi Laurent/Kieran,\n> >>>>>\n> >>>>> Can you soft review(or test) this patch to see if anything\n> >>>>> is wrong with it?\n> >>>>> The reason I am asking, because I cannot make it work :(\n> >>>>> exiftool with a captured frame shows empty Timezone Offset tag as:\n> >>>>> ```\n> >>>>> Time Zone Offset                :\n> >>>>> ```\n> >>>>>\n> >>>>> exif tool shows nothing.\n> >>>\n> >>> Hrm, maybe adding some debug to the exif tool and rebuilding might help\n> >>> identify what data it actually parses or why it can't identify the field.\n> >>\n> >> I didn't find any particular debug option in exiftool that would give\n> >> out a error message. Although I found -v3 verbose output which can give\n> >> hexdump of each tag. So attaching the various outputs here:\n> >>\n> >> With:\n> >> a) setString(EXIF_IFD_0, EXIF_TAG_TIME_ZONE_OFFSET, EXIF_FORMAT_ASCII, \"6\");\n> >> o/p:\n> >>     | 8)  TimeZoneOffset = 6\n> >>     |     - Tag 0x882a (2 bytes, string[2]):\n> >>     |         0090: 36 00 [6.]\n> >> JPEG DQT (65 bytes):\n> >>\n> >>\n> >> b) setSShort(EXIF_IFD_0, EXIF_TAG_TIME_ZONE_OFFSET, '6' or 6);\n> >> o/p:\n> >>     | 8)  TimeZoneOffset =\n> >>     |     - Tag 0x882a (0 bytes, undef):\n> >>\n> >>> What does exiv2 report on the generated image?\n> >>\n> >> `exiv2` tool doesn't report the tag in any of the cases.\n> >> `exif` tool reports it properly only if setString is used...\n> >>\n> >>>>> I made sure a valid value being generated for timezone\n> >>>>> offset in setTimeStamp().\n> >>>>> I also checked the case where it might be preset using\n> >>>>> exif_data_fix and not been able to re-set again.\n> >>>>> But no, that doesn't seem the problem in my testing.\n> >>>>>\n> >>>>>\n> >>>>> diff --git a/src/android/jpeg/exif.cpp b/src/android/jpeg/exif.cpp\n> >>>>> index c0dbfcc..9c23cfb 100644\n> >>>>> --- a/src/android/jpeg/exif.cpp\n> >>>>> +++ b/src/android/jpeg/exif.cpp\n> >>>>> @@ -7,6 +7,8 @@\n> >>>>>       #include \"exif.h\"\n> >>>>>     +#include <stdlib.h>\n> >>>>> +\n> >>>>>     #include \"libcamera/internal/log.h\"\n> >>>>>       using namespace libcamera;\n> >>>>> @@ -135,6 +137,16 @@ void Exif::setShort(ExifIfd ifd, ExifTag tag,\n> >>>>> uint16_t item)\n> >>>>>         exif_entry_unref(entry);\n> >>>>>     }\n> >>>>>     +void Exif::setSShort(ExifIfd ifd, ExifTag tag, int16_t item)\n> >>>>> +{\n> >>>>> +    ExifEntry *entry = createEntry(ifd, tag);\n> >>>>> +    if (!entry)\n> >>>>> +        return;\n> >>>>> +\n> >>>>> +    exif_set_sshort(entry->data, EXIF_BYTE_ORDER_INTEL, item);\n> >>>>\n> >>>> Getting to bottom of this in the morning, exif_set_sshort seems to\n> >>>> convert its passed int16_t value to character. I don't know what's the\n> >>>> logic behind this [1] (apart from EXIF's quirky-ness), but this simply\n> >>>> doesn't look right then. So..\n> >>>\n> >>> Oh - converting a short to a char certainly doesn't sound right at all\n> >>> for a function called 'set_short'.\n> >>>\n> >>> Char's are 8-bits. Shorts are 16....\n> >>>\n> >>> Oh - Now I've followed to [1], I see that's simply dealing with\n> >>> byte-ordering issues. So it shouldn't be a problem.\n> >>\n> >> oh, I misinterpreted things then.\n> >>\n> >>>>> +    exif_entry_unref(entry);\n> >>>>> +}\n> >>>>> +\n> >>>>>     void Exif::setLong(ExifIfd ifd, ExifTag tag, uint32_t item)\n> >>>>>     {\n> >>>>>         ExifEntry *entry = createEntry(ifd, tag);\n> >>>>> @@ -196,6 +208,25 @@ void Exif::setTimestamp(time_t timestamp)\n> >>>>>         setString(EXIF_IFD_0, EXIF_TAG_DATE_TIME, EXIF_FORMAT_ASCII, ts);\n> >>>>>         setString(EXIF_IFD_EXIF, EXIF_TAG_DATE_TIME_ORIGINAL, EXIF_FORMAT_ASCII, ts);\n> >>>>>         setString(EXIF_IFD_EXIF, EXIF_TAG_DATE_TIME_DIGITIZED, EXIF_FORMAT_ASCII, ts);\n> >>>>> +\n> >>>>> +    /*\n> >>>>> +     * If possible, query and set timezone information via\n> >>>>> +     * EXIF_TAG_TIME_ZONE_OFFSET. There is only per-hour resolution for tag\n> >>>>> +     * hence, round up hours if minutes >= 30.\n> >>>\n> >>> Where have you found the information that it is only 'per-hour' resolution?\n> >>\n> >> It's what the tools use to do. They report on per-hour basis only. I\n> >> have mentioned the behaviour in the commit message.\n> >> Also stumbled upon\n> >> https://github.com/jim-easterbrook/Photini/commit/686d26 literally few\n> >> minutes ago.\n> >\n> > TimeZoneOffset is defined in the TIFF/EP specification (ISO/DIS\n> > 12234-2).\n> >\n> > 5.2.48 TimeZoneOffset\n> >\n> > This optional tag encodes the time zone of the camera clock (relative to\n> > Greenwich Mean Time) used to create the DataTimeOriginal tag-value when\n> > the picture was taken. It may also contain the time zone offset of the\n> > clock used to create the DateTime tag-value when the image was modified.\n> >\n> > Tag Name = TimeZoneOffset\n> > Tag = 34858 (882A.H)\n> > Type = SSHORT\n> > N = 1 or 2\n> > Value = VALUE\n> > The allowed values are -12 to +11.\n> >\n> > SSHORT 0 Time Zone Offset (in hours) of DateTimeOriginal tag-value relative to Greenwich Mean Time\n> > SSHORT 1 If present, Time Zone Offset (in hours) of DateTime tag-value relative to Greenwich Mean Time\n> >\n> > Usage: IFD0\n> >\n> > It's fairly ill-defined as time zones extend to UTC+14, but it's clearly\n> > a signed short, with one or two values, expressed as a number of hours.\n> > We should really use the time zone tags of the EXIF specification\n> > instead, but they're not supported by libexif.\n> >\n> >>>>> +     */\n> >>>>> +    int r = strftime(str, sizeof(str), \"%z\", &tm);\n> >>>>> +    if (r > 0) {\n> >>>>> +        int16_t timezone = atoi(str);\n> >>>>> +        int16_t hour = timezone / 100;\n> >>>>> +        int16_t min = timezone % 100;;\n> >>>>\n> >>>> Complier should have errored out on ;; , but it didn't.\n> >>>> Rectified locally on noticing.\n> >>>\n> >>> ;; isn't going to generate a compiler error because the second ';' is\n> >>> just an empty statement that does nothing.\n> >>>\n> >>> Still worth cleaning up of course ;-)\n> >>>\n> >>>>> +\n> >>>>> +        if (min <= -30)\n> >>>>> +            hour--;\n> >>>>> +        else if (min >= 30)\n> >>>>> +            hour++;\n> >>>>> +\n> >>>>> +        setSShort(EXIF_IFD_0, EXIF_TAG_TIME_ZONE_OFFSET, hour);\n> >>>>\n> >>>> Instead of setSShort call, if we use setString, things seems to get\n> >>>> moving. Something like:\n> >>>> setString(EXIF_IFD_0, EXIF_TAG_TIME_ZONE_OFFSET, EXIF_FORMAT_ASCII, \"6\");\n> >>>>\n> >>>> got parsed correctly by exiftool and exif commandline utilities.\n> >>>\n> >>> Ok, so lets tie down exactly what the right format should be written to\n> >>> this field.\n> >>>\n> >>> Writing '6' as ascii infers that the actual value that got written to\n> >>> memory in that place was 54....\n> >>>\n> >>> What was presented with the tools? Did it show a '6' or the integer value?\n> >>\n> >> It showed '6', no integer/ascii value of '6' was spotted anytime with\n> >> any tool.\n> >>\n> >>> If this really is a field for a short, we should write the ascii value\n> >>> to the short, not a string... which (currently) adds null padding too!?\n> >>\n> >> tried to setSShort with ascii value of 6, no improvements.\n> >> If setSShort is used, the exiftool reports it as 'empty' value :(\n> >> https://paste.debian.net/1164408/\n> >\n> > Can you provide a sample image that has the time zone offset tag set\n> > with setSShort() ?\n>\n> Please find attached a sample image to this mail.\n\nThere's clearly something wrong. The IFD entry is set to\n\n2A 88 -> Tag = 0x882a, TimeZoneOffset\n07 00 -> Type = Undefined\n00 00 00 00 -> Count = 0\n00 00 00 00 -> Value = 0\n\nA short investigation in exif_entry_initialize() showed that it doesn't\nsupport EXIF_TAG_TIME_ZONE_OFFSET.\n\nI see two options, either adding the tag manually instead of calling\nexif_entry_initialize(), or stop bothering with time zones until libexif\ngets proper support for them.\n\n> >>> Interestingly:\n> >>> https://github.com/libexif/libexif/blob/master/libexif/exif-tag.c#L523\n> >>> seems to state that EXIF_TAG_TIME_ZONE_OFFSET is not in Exif-2.2...\n> >>>\n> >>> Indeed, examining closely I see no reference to a tag with id 0x882a\n> >>>\n> >>> https://www.exiv2.org/tags.html lists Exif.Image.TimeZoneOffset as an\n> >>> SShort at 0x882a (34858), with the following text:\n> >>>\n> >>> \"\"\"\n> >>> This optional tag encodes the time zone of the camera clock (relative to\n> >>> Greenwich Mean Time) used to create the DataTimeOriginal tag-value when\n> >>> the picture was taken. It may also contain the time zone offset of the\n> >>> clock used to create the DateTime tag-value when the image was modified.\n> >>> \"\"\"\n> >>>\n> >>> But that doesn't fully explain how the value is encoded.\n> >>>\n> >>> And ... now I've gone further down the rabbit-hole, I've discovered Exif\n> >>> 2.31 is available from :\n> >>>\n> >>>     http://cipa.jp/std/documents/download_e.html?DC-008-Translation-2016-E\n> >>>\n> >>> It also doesn't list a tag at 0x882a, but does explicitly mention a tag\n> >>> \"OffsetTime\" (0x9010) which is an ascii field matching the string\n> >>> generated by the strftime(str, sizeof(str), \"%z\", &tm) call ;-)\n> >>>\n> >>>\n> >>> See page 49 in that document from cipa.jp.\n> >>\n> >> There are 3 more timezone offset related tags that are mentioned but not\n> >> supported by libexif.  Laurent has pointed this out in previous replies\n> >> to this thread.\n> >>\n> >>> I expect we should explicitly set our exif version to \"0231\" if we use it?\n> >>\n> >> hmm, yea, looking at the above pastebin output, my exif version is\n> >> '0210' so I would agree setting it 0231 because that's where the tag is\n> >> introduced.\n> >\n> > The TimeZoneOffset tag doesn't exist in any EXIF specification, so I\n> > don't think the EXIF version will make any difference.\n>\n> Ah, right. Booooing! :head_bang:\n>\n> >> My local version of libexif is 0.6.21 which already seems to support the\n> >> timezone offset tag. Although, I would expect it to report that the tag\n> >> I am trying set needs a higher exif version.\n> >>\n> >> (Explicitly set exif version to 0231, no improvements observed)\n> >>\n> >>> Do the exif/exiv2 libraries support Exif 2.31?\n> >>\n> >> hmm, probably : https://github.com/libexif/libexif/commit/54333d8b8c\n> >>\n> >> Not sure, how they keep up with minor versions, OR rather support new\n> >> tags directly.\n> >>\n> >>>> [1]:\n> >>>> https://github.com/libexif/libexif/blob/master/libexif/exif-utils.c#L107-L121\n> >>>>\n> >>>>> +    }\n> >>>>>     }\n> >>>>>       void Exif::setOrientation(int orientation)\n> >>>>> diff --git a/src/android/jpeg/exif.h b/src/android/jpeg/exif.h\n> >>>>> index f04cefc..9c9cc3b 100644\n> >>>>> --- a/src/android/jpeg/exif.h\n> >>>>> +++ b/src/android/jpeg/exif.h\n> >>>>> @@ -37,6 +37,7 @@ private:\n> >>>>>                        unsigned long components, unsigned int size);\n> >>>>>           void setShort(ExifIfd ifd, ExifTag tag, uint16_t item);\n> >>>>> +    void setSShort(ExifIfd ifd, ExifTag tag, int16_t item);\n> >>>>>         void setLong(ExifIfd ifd, ExifTag tag, uint32_t item);\n> >>>>>         void setString(ExifIfd ifd, ExifTag tag, ExifFormat format,\n> >>>>>                    const std::string &item);","headers":{"Return-Path":"<libcamera-devel-bounces@lists.libcamera.org>","X-Original-To":"parsemail@patchwork.libcamera.org","Delivered-To":"parsemail@patchwork.libcamera.org","Received":["from lancelot.ideasonboard.com (lancelot.ideasonboard.com\n\t[92.243.16.209])\n\tby patchwork.libcamera.org (Postfix) with ESMTPS id A410EC3B5C\n\tfor <parsemail@patchwork.libcamera.org>;\n\tWed, 23 Sep 2020 15:41:50 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id F3A8E62FDE;\n\tWed, 23 Sep 2020 17:41:49 +0200 (CEST)","from perceval.ideasonboard.com (perceval.ideasonboard.com\n\t[IPv6:2001:4b98:dc2:55:216:3eff:fef7:d647])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTPS id BFE8860576\n\tfor <libcamera-devel@lists.libcamera.org>;\n\tWed, 23 Sep 2020 17:41:48 +0200 (CEST)","from pendragon.ideasonboard.com (62-78-145-57.bb.dnainternet.fi\n\t[62.78.145.57])\n\tby perceval.ideasonboard.com (Postfix) with ESMTPSA id F16F6555;\n\tWed, 23 Sep 2020 17:41:45 +0200 (CEST)"],"Authentication-Results":"lancelot.ideasonboard.com;\n\tdkim=fail reason=\"signature verification failed\" (1024-bit key;\n\tunprotected) header.d=ideasonboard.com header.i=@ideasonboard.com\n\theader.b=\"cEluKHvU\"; dkim-atps=neutral","DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com;\n\ts=mail; t=1600875706;\n\tbh=4+enw7R/poYON5z0WwILtGJrG+NWeEKtl5jtEd43AjU=;\n\th=Date:From:To:Cc:Subject:References:In-Reply-To:From;\n\tb=cEluKHvU1VUYDZUPwq5ag1P5i0x6wgRM+Frqsc4xGLT1G+dYqiXOs+fy7+djcrWYD\n\tpWWsoGaBmnAaljhDZ/w3ds4TLRFaDhMAzKyLQ0Zsd2PYZHkHWwa53qd4KKGj3KYyRP\n\ti91XVPVUtG4iTTTl7CKJUSUcwbr4alon2JiZRH04=","Date":"Wed, 23 Sep 2020 18:41:12 +0300","From":"Laurent Pinchart <laurent.pinchart@ideasonboard.com>","To":"Umang Jain <email@uajain.com>","Message-ID":"<20200923154112.GD3980@pendragon.ideasonboard.com>","References":"<20200909153206.7700-1-email@uajain.com>\n\t<20200922162624.32321-1-email@uajain.com>\n\t<70e975f3-ce7c-891c-af41-3775cb9d0fee@uajain.com>\n\t<9f991efd-324d-3b37-1a5d-0901b25860e1@ideasonboard.com>\n\t<6c058a32-2761-88c2-193f-45095ad61977@uajain.com>\n\t<20200923132934.GC3980@pendragon.ideasonboard.com>\n\t<502e4ff5-e5e5-c6e1-4f0b-58b9c69f50e0@uajain.com>","MIME-Version":"1.0","Content-Disposition":"inline","In-Reply-To":"<502e4ff5-e5e5-c6e1-4f0b-58b9c69f50e0@uajain.com>","Subject":"Re: [libcamera-devel] [RFC v1 PATCH] android: jpeg: exif: Set\n\ttimezone information","X-BeenThere":"libcamera-devel@lists.libcamera.org","X-Mailman-Version":"2.1.29","Precedence":"list","List-Id":"<libcamera-devel.lists.libcamera.org>","List-Unsubscribe":"<https://lists.libcamera.org/options/libcamera-devel>,\n\t<mailto:libcamera-devel-request@lists.libcamera.org?subject=unsubscribe>","List-Archive":"<https://lists.libcamera.org/pipermail/libcamera-devel/>","List-Post":"<mailto:libcamera-devel@lists.libcamera.org>","List-Help":"<mailto:libcamera-devel-request@lists.libcamera.org?subject=help>","List-Subscribe":"<https://lists.libcamera.org/listinfo/libcamera-devel>,\n\t<mailto:libcamera-devel-request@lists.libcamera.org?subject=subscribe>","Cc":"libcamera-devel@lists.libcamera.org","Content-Type":"text/plain; charset=\"utf-8\"","Content-Transfer-Encoding":"base64","Errors-To":"libcamera-devel-bounces@lists.libcamera.org","Sender":"\"libcamera-devel\" <libcamera-devel-bounces@lists.libcamera.org>"}}]