[{"id":12729,"web_url":"https://patchwork.libcamera.org/comment/12729/","msgid":"<20200924133750.GI3968@pendragon.ideasonboard.com>","date":"2020-09-24T13:37:50","subject":"Re: [libcamera-devel] [PATCH] android: jpeg: exif: Set timezone\n\tinformation","submitter":{"id":2,"url":"https://patchwork.libcamera.org/api/people/2/","name":"Laurent Pinchart","email":"laurent.pinchart@ideasonboard.com"},"content":"Hi Umang,\n\nThank you for the patch.\n\nOn Thu, Sep 24, 2020 at 02:36:31PM +0530, Umang Jain wrote:\n> The EXIF specification defines three timezone related tags namely,\n> OffsetTime, OffsetTimeOriginal and OffsetTimeDigitized. However,\n> these are not supported by libexif (as of v0.6.21) hence, carry\n> the tags' positional values in our implementation until we get\n> this support from libexif itself.\n> \n> Since these tags were introduced in EXIF specification v2.31, set\n> the exif version number explicitly too.\n> \n> Signed-off-by: Umang Jain \n> ---\n> src/android/jpeg/exif.cpp | 29 +++++++++++++++++++++++++++++\n> 1 file changed, 29 insertions(+)\n> \n> diff --git a/src/android/jpeg/exif.cpp b/src/android/jpeg/exif.cpp\n> index c0dbfcc..df38765 100644\n> --- a/src/android/jpeg/exif.cpp\n> +++ b/src/android/jpeg/exif.cpp\n> @@ -13,6 +13,16 @@ using namespace libcamera;\n> \n> LOG_DEFINE_CATEGORY(EXIF)\n> \n> +/*\n> + * List of EXIF tags that we set directly because they are not supported\n> + * by libexif version 0.6.21.\n> + */\n> +enum _ExifTag {\n\nI'd make this \"enum class\" as you always use the _ExifTag:: prefix\nbelow.\n\n> +\tOFFSET_TIME = 0x9010,\n> +\tOFFSET_TIME_ORIGINAL = 0x9011,\n> +\tOFFSET_TIME_DIGITIZED = 0x9012,\n> +};\n> +\n> /*\n> * The Exif class should be instantiated and specific properties set\n> * through the exposed public API.\n> @@ -51,6 +61,9 @@ Exif::Exif()\n> \t */\n> \texif_data_set_byte_order(data_, EXIF_BYTE_ORDER_INTEL);\n> \n> +\tsetString(EXIF_IFD_EXIF, EXIF_TAG_EXIF_VERSION,\n> +\t\t EXIF_FORMAT_UNDEFINED, \"0231\");\n> +\n\nAccording to the EXIF standard, \"Since the type is UNDEFINED, it shall\nnot be terminated with NULL\". How about modifying setString() to only\ninclude the NULL terminator when the type is ASCII ? And maybe splitting\nthis to a separate patch ? Up to you.\n\nThe rest looks good to me.\n\n> \t/* Create the mandatory EXIF fields with default data. */\n> \texif_data_fix(data_);\n> }\n> @@ -196,6 +209,22 @@ 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/* Query and set timezone information if available. */\n> +\tint r = strftime(str, sizeof(str), \"%z\", &tm);\n> +\tif (r > 0) {\n> +\t\tstd::string tz(str);\n> +\t\ttz.insert(3, 1, ':');\n> +\t\tsetString(EXIF_IFD_EXIF,\n> +\t\t\t static_cast(_ExifTag::OFFSET_TIME),\n> +\t\t\t EXIF_FORMAT_ASCII, tz);\n> +\t\tsetString(EXIF_IFD_EXIF,\n> +\t\t\t static_cast(_ExifTag::OFFSET_TIME_ORIGINAL),\n> +\t\t\t EXIF_FORMAT_ASCII, tz);\n> +\t\tsetString(EXIF_IFD_EXIF,\n> +\t\t\t static_cast(_ExifTag::OFFSET_TIME_DIGITIZED),\n> +\t\t\t EXIF_FORMAT_ASCII, tz);\n> +\t}\n> }\n> \n> void Exif::setOrientation(int orientation)","headers":{"Return-Path":"","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 3F63AC3B5B\n\tfor ;\n\tThu, 24 Sep 2020 13:38:25 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id BDA1462FDA;\n\tThu, 24 Sep 2020 15:38:24 +0200 (CEST)","from perceval.ideasonboard.com (perceval.ideasonboard.com\n\t[213.167.242.64])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTPS id 29ECB60362\n\tfor ;\n\tThu, 24 Sep 2020 15:38:24 +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 A03942FD;\n\tThu, 24 Sep 2020 15:38:23 +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=\"qxtAhTKf\"; dkim-atps=neutral","DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com;\n\ts=mail; t=1600954703;\n\tbh=cL6h0EXxGYydm+0GGIPITJX0dScVI4w9Gn11p5zPwC4=;\n\th=Date:From:To:Cc:Subject:References:In-Reply-To:From;\n\tb=qxtAhTKft5kX/pCh5FtHvhVNkSWsmjt4M4c6a9ViNhPb+wkpRKWUcTbqYP5HCGoto\n\tcgfXn+aBYbDGDarO3KooHA714lSyxY/r7VwyYWIEM1WG8G9h5YgIn5vbr0K5u6Jocj\n\twppqGoB7TBccnUF2WGiG8EqTz4FEOTbCUlGSweNs=","Date":"Thu, 24 Sep 2020 16:37:50 +0300","From":"Laurent Pinchart ","To":"Umang Jain ","Message-ID":"<20200924133750.GI3968@pendragon.ideasonboard.com>","References":"<20200924090631.18473-1-email@uajain.com>","MIME-Version":"1.0","Content-Disposition":"inline","In-Reply-To":"<20200924090631.18473-1-email@uajain.com>","Subject":"Re: [libcamera-devel] [PATCH] android: jpeg: exif: Set timezone\n\tinformation","X-BeenThere":"libcamera-devel@lists.libcamera.org","X-Mailman-Version":"2.1.29","Precedence":"list","List-Id":"","List-Unsubscribe":",\n\t","List-Archive":"","List-Post":"","List-Help":"","List-Subscribe":",\n\t","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\" "}}]