[{"id":12693,"web_url":"https://patchwork.libcamera.org/comment/12693/","msgid":"<9aaa1d34-8989-9ba1-177d-63dc3a5e5cd5@ideasonboard.com>","date":"2020-09-23T20:44:54","subject":"Re: [libcamera-devel] [RFC PATCH v2] 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 20:17, Umang Jain wrote:\n> Hi,\n> \n> Again posting as a RFC but the patch is now functional as expected.\n> Tested with few range of values (+ -) and didn't see a problem.\n> \n> Let me know what you think.\n> ---\n> \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> This commit also introduces a special case handling for this tag.\n> In course of development, it was found that libexif does not\n> support this tag practically but still carries it among its\n> header files. The manual patching here turned out to be a small\n> detail that can conveniently be removed as and when libexif's\n> support for timezone information improves.\n\nOh, I'm surprised here - I'm not sure I fully understand and would have\nto investigate - but I thought the setXXX() functions are all capable of\ncreating a new entry when required, and they 'remove' any existing entry ...\n\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 | 44 +++++++++++++++++++++++++++++++++++++++\n>  src/android/jpeg/exif.h   |  1 +\n>  2 files changed, 45 insertions(+)\n> \n> diff --git a/src/android/jpeg/exif.cpp b/src/android/jpeg/exif.cpp\n> index c0dbfcc..27d8fde 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,29 @@ 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;\n> +\t/*\n> +\t * Special case handling for EXIF_TAG_TIME_ZONE_OFFSET. We need to\n> +\t * manually create and initialize an ExifEntry for this tag, since\n> +\t * the intialization support is missing from libexif's function\n> +\t * exif_entry_initialize().\n> +\t *\n> +\t * \\todo: Remove this special case when the above issue is fixed in\n> +\t * libexif.\n> +\t */\n> +\tif (tag == EXIF_TAG_TIME_ZONE_OFFSET)\n> +\t\tentry = createEntry (ifd, tag, EXIF_FORMAT_SSHORT, 1, 2);\n> +\telse\n> +\t\tentry = createEntry(ifd, tag);\n\nIs the special case required to determine the size?\n\nOh - I see, createEntry(ifd, tag) uses exif_entry_initialize()\npresumably to initialise from a table of values based on the tag ...\nwhich isn't provided for the EXIF_TAG_TIME_ZONE_OFFSET ...\n\nSo calling the extended version is explicitly setting everything up ...\n\nHrm ... a bit nasty indeed.\n\nBecuase this is a workaround, I'd almost be tempted to handle this in\nsetTimestamp, as that's the only call path that needs it...\n\n\n> +\tif (!entry)\n> +\t\treturn;\n> +\n> +\texif_set_sshort(entry->data, EXIF_BYTE_ORDER_INTEL, item);\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 +221,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;\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);\n\nSo that would be remove that line (I do prefer the setSShort() type\nhelpers in the general case) and add something like:\n\n> \t\t/*\n> \t\t * exif_entry_initialize() does not yet support\n> \t\t * EXIF_TAG_TIME_ZONE_OFFSET, so we have to do this\n> \t\t * manually.\n> \t\t */\n> \t\tExifEntry *entry = createEntry(EXIF_IFD_0, EXIF_TAG_TIME_ZONE_OFFSET,\n> \t\t\t\t\t\tEXIF_FORMAT_SSHORT, 1, 2);\n> \t\tif (entry) {\n> \t\t\texif_set_sshort(entry->data, EXIF_BYTE_ORDER_INTEL, hour);\n> \t\t\texif_entry_unref(entry);\n> \t\t}\n\nWhat do you think? Better - or easier just to have it in the new\nsetSShort() call?\n\n--\nKieran\n\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);\n>","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 12033C3B5C\n\tfor <parsemail@patchwork.libcamera.org>;\n\tWed, 23 Sep 2020 20:45:00 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id 8DBE562FE3;\n\tWed, 23 Sep 2020 22:44:59 +0200 (CEST)","from perceval.ideasonboard.com (perceval.ideasonboard.com\n\t[213.167.242.64])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTPS id 24D2560576\n\tfor <libcamera-devel@lists.libcamera.org>;\n\tWed, 23 Sep 2020 22:44:59 +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 45985555;\n\tWed, 23 Sep 2020 22:44:58 +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=\"aPU/Gcld\"; dkim-atps=neutral","DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com;\n\ts=mail; t=1600893898;\n\tbh=uam/YuAEQUijwM0KcVtb33LzQKGoAFyd3f4YeGwYmhg=;\n\th=Reply-To:Subject:To:References:From:Date:In-Reply-To:From;\n\tb=aPU/GcldBYUVIUuVYLg+3f/HzBcIabNFoZKrPdkEO4R2kNU7/W0FO1cwjzgxINK3P\n\timHzmmNhFh+xVoe2fHwwrs+GVwx9UrxKzmK8/l6kHOtN0TM6UnFS54OZLyZybS05d1\n\tQ3ASLDq/X/GcsogcEwYn9lWpGZcZIuQosXeyg0oA=","To":"Umang Jain <email@uajain.com>, libcamera-devel@lists.libcamera.org","References":"<20200909153206.7700-1-email@uajain.com>\n\t<20200923191731.11433-1-email@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":"<9aaa1d34-8989-9ba1-177d-63dc3a5e5cd5@ideasonboard.com>","Date":"Wed, 23 Sep 2020 21:44:54 +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":"<20200923191731.11433-1-email@uajain.com>","Content-Language":"en-GB","Subject":"Re: [libcamera-devel] [RFC PATCH v2] 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=\"us-ascii\"","Content-Transfer-Encoding":"7bit","Errors-To":"libcamera-devel-bounces@lists.libcamera.org","Sender":"\"libcamera-devel\" <libcamera-devel-bounces@lists.libcamera.org>"}},{"id":12697,"web_url":"https://patchwork.libcamera.org/comment/12697/","msgid":"<20200924010755.GH3980@pendragon.ideasonboard.com>","date":"2020-09-24T01:07:55","subject":"Re: [libcamera-devel] [RFC PATCH v2] 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 Kieran,\n\nOn Wed, Sep 23, 2020 at 09:44:54PM +0100, Kieran Bingham wrote:\n> On 23/09/2020 20:17, Umang Jain wrote:\n> > Hi,\n> > \n> > Again posting as a RFC but the patch is now functional as expected.\n> > Tested with few range of values (+ -) and didn't see a problem.\n> > \n> > Let me know what you think.\n> > ---\n> > \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> > This commit also introduces a special case handling for this tag.\n> > In course of development, it was found that libexif does not\n> > support this tag practically but still carries it among its\n> > header files. The manual patching here turned out to be a small\n> > detail that can conveniently be removed as and when libexif's\n> > support for timezone information improves.\n> \n> Oh, I'm surprised here - I'm not sure I fully understand and would have\n> to investigate - but I thought the setXXX() functions are all capable of\n> creating a new entry when required, and they 'remove' any existing entry ...\n> \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 | 44 +++++++++++++++++++++++++++++++++++++++\n> >  src/android/jpeg/exif.h   |  1 +\n> >  2 files changed, 45 insertions(+)\n> > \n> > diff --git a/src/android/jpeg/exif.cpp b/src/android/jpeg/exif.cpp\n> > index c0dbfcc..27d8fde 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,29 @@ 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;\n> > +\t/*\n> > +\t * Special case handling for EXIF_TAG_TIME_ZONE_OFFSET. We need to\n> > +\t * manually create and initialize an ExifEntry for this tag, since\n> > +\t * the intialization support is missing from libexif's function\n> > +\t * exif_entry_initialize().\n> > +\t *\n> > +\t * \\todo: Remove this special case when the above issue is fixed in\n> > +\t * libexif.\n> > +\t */\n> > +\tif (tag == EXIF_TAG_TIME_ZONE_OFFSET)\n> > +\t\tentry = createEntry (ifd, tag, EXIF_FORMAT_SSHORT, 1, 2);\n> > +\telse\n> > +\t\tentry = createEntry(ifd, tag);\n> \n> Is the special case required to determine the size?\n> \n> Oh - I see, createEntry(ifd, tag) uses exif_entry_initialize()\n> presumably to initialise from a table of values based on the tag ...\n> which isn't provided for the EXIF_TAG_TIME_ZONE_OFFSET ...\n> \n> So calling the extended version is explicitly setting everything up ...\n> \n> Hrm ... a bit nasty indeed.\n> \n> Becuase this is a workaround, I'd almost be tempted to handle this in\n> setTimestamp, as that's the only call path that needs it...\n> \n> \n> > +\tif (!entry)\n> > +\t\treturn;\n> > +\n> > +\texif_set_sshort(entry->data, EXIF_BYTE_ORDER_INTEL, item);\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 +221,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;\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);\n> \n> So that would be remove that line (I do prefer the setSShort() type\n> helpers in the general case) and add something like:\n> \n> > \t\t/*\n> > \t\t * exif_entry_initialize() does not yet support\n> > \t\t * EXIF_TAG_TIME_ZONE_OFFSET, so we have to do this\n> > \t\t * manually.\n> > \t\t */\n> > \t\tExifEntry *entry = createEntry(EXIF_IFD_0, EXIF_TAG_TIME_ZONE_OFFSET,\n> > \t\t\t\t\t\tEXIF_FORMAT_SSHORT, 1, 2);\n> > \t\tif (entry) {\n> > \t\t\texif_set_sshort(entry->data, EXIF_BYTE_ORDER_INTEL, hour);\n> > \t\t\texif_entry_unref(entry);\n> > \t\t}\n> \n> What do you think? Better - or easier just to have it in the new\n> setSShort() call?\n\nThat would be better, but if we go down the route of adding tags that\nare not fully supported by the library, we can as well add the EXIF\ntimezone tags, not the TIFF/EP tag. Those use strings, so setString()\nshould work out of the box.\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 4C5B0C3B5B\n\tfor <parsemail@patchwork.libcamera.org>;\n\tThu, 24 Sep 2020 01:08:31 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id B2F1062FDE;\n\tThu, 24 Sep 2020 03:08:30 +0200 (CEST)","from perceval.ideasonboard.com (perceval.ideasonboard.com\n\t[213.167.242.64])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTPS id B2FF96036A\n\tfor <libcamera-devel@lists.libcamera.org>;\n\tThu, 24 Sep 2020 03:08:29 +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 3319B27B;\n\tThu, 24 Sep 2020 03:08:29 +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=\"KZlsmkAf\"; dkim-atps=neutral","DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com;\n\ts=mail; t=1600909709;\n\tbh=sKJxURSRbOVaQNa3wJgjtDUxt/GAUdDUBmkSmzugYWw=;\n\th=Date:From:To:Cc:Subject:References:In-Reply-To:From;\n\tb=KZlsmkAf1iI9Kfdx4WApuAo/VEBR3uK0NJP3fcLtqM0mswve8pDbfHso+SQGpXy5E\n\teqOj2p8v+/TIotzpKDDeqyfO6TKftb2Gvw5M5GwbER3680oAw5SxR5ohZG/QS3OMNG\n\twm72SDsJhB8KxGeL2OSVpsi9RMLgoMF+on11EjUY=","Date":"Thu, 24 Sep 2020 04:07:55 +0300","From":"Laurent Pinchart <laurent.pinchart@ideasonboard.com>","To":"Kieran Bingham <kieran.bingham@ideasonboard.com>","Message-ID":"<20200924010755.GH3980@pendragon.ideasonboard.com>","References":"<20200909153206.7700-1-email@uajain.com>\n\t<20200923191731.11433-1-email@uajain.com>\n\t<9aaa1d34-8989-9ba1-177d-63dc3a5e5cd5@ideasonboard.com>","MIME-Version":"1.0","Content-Disposition":"inline","In-Reply-To":"<9aaa1d34-8989-9ba1-177d-63dc3a5e5cd5@ideasonboard.com>","Subject":"Re: [libcamera-devel] [RFC PATCH v2] 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=\"us-ascii\"","Content-Transfer-Encoding":"7bit","Errors-To":"libcamera-devel-bounces@lists.libcamera.org","Sender":"\"libcamera-devel\" <libcamera-devel-bounces@lists.libcamera.org>"}}]