[{"id":5172,"web_url":"https://patchwork.libcamera.org/comment/5172/","msgid":"<20200611024752.GB13598@pendragon.ideasonboard.com>","date":"2020-06-11T02:47:52","subject":"Re: [libcamera-devel] [PATCH] qcam: dng_writer: Record creation\n\ttime in the EXIF directory","submitter":{"id":2,"url":"https://patchwork.libcamera.org/api/people/2/","name":"Laurent Pinchart","email":"laurent.pinchart@ideasonboard.com"},"content":"Hi Niklas,\n\nThank you for the patch.\n\nOn Thu, Jun 11, 2020 at 03:46:14AM +0200, Niklas Söderlund wrote:\n> If the EXIF directory is empty due to no metadata being available tools\n> such as tiffinfo complains that the directory is malformed.\n> \n> TIFFFetchDirectory: Sanity check on directory count failed, this is probably not a valid IFD offset.\n> TIFFReadCustomDirectory: Failed to read custom directory at offset 0.\n> \n> Always record the creation time in the EXIF directory instead of adding\n> complexity to skip creating the EXIF directory if there is no metadata\n> to record. This ensures there are at least one entry in the EXIF\n> directory and that makes tiffinfo happy.\n> \n> Signed-off-by: Niklas Söderlund \n> ---\n> src/qcam/dng_writer.cpp | 11 +++++++++++\n> 1 file changed, 11 insertions(+)\n> \n> diff --git a/src/qcam/dng_writer.cpp b/src/qcam/dng_writer.cpp\n> index 330b169af25b7ac7..c1c3833740d96630 100644\n> --- a/src/qcam/dng_writer.cpp\n> +++ b/src/qcam/dng_writer.cpp\n> @@ -426,6 +426,17 @@ int DNGWriter::write(const char *filename, const Camera *camera,\n> \t/* Create a new IFD for the EXIF data and fill it. */\n> \tTIFFCreateEXIFDirectory(tif);\n> \n> +\t/* Store creation time. */\n> +\ttime_t rawtime;\n> +\tstruct tm *timeinfo;\n> +\tchar strTime[20];\n> +\n> +\ttime(&rawtime);\n\nCould we use the timestamp from the buffer instead ?\n\n> +\ttimeinfo = localtime(&rawtime);\n> +\tstrftime(strTime, 20, \"%Y:%m:%d %H:%M:%S\", timeinfo);\n\nHow are time zones supposed to be handled ?\n\n> +\n> +\tTIFFSetField(tif, EXIFTAG_DATETIMEDIGITIZED, strTime);\n\nShould we also set EXIFTAG_DATETIMEORIGINAL to the same value ?\n\n> +\n> \tif (metadata.contains(controls::AnalogueGain)) {\n> \t\tfloat gain = metadata.get(controls::AnalogueGain);\n> \t\tuint16_t iso = std::min(std::max(gain * 100, 0.0f), 65535.0f);","headers":{"Return-Path":"","Received":["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 6BAF6600F7\n\tfor ;\n\tThu, 11 Jun 2020 04:48:13 +0200 (CEST)","from pendragon.ideasonboard.com (81-175-216-236.bb.dnainternet.fi\n\t[81.175.216.236])\n\tby perceval.ideasonboard.com (Postfix) with ESMTPSA id C96D426A;\n\tThu, 11 Jun 2020 04:48:12 +0200 (CEST)"],"Authentication-Results":"lancelot.ideasonboard.com; dkim=pass (1024-bit key; \n\tunprotected) header.d=ideasonboard.com\n\theader.i=@ideasonboard.com\n\theader.b=\"g+CoEUtJ\"; dkim-atps=neutral","DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com;\n\ts=mail; t=1591843693;\n\tbh=73FAOzt5mCZ3vpnpehOc3aKyGcogZzbGJbASuVX3Ux0=;\n\th=Date:From:To:Cc:Subject:References:In-Reply-To:From;\n\tb=g+CoEUtJUJSPN85nWRsm8yxLWTV9sy8zHgl95GrnofVDykCcAIyNvfScHD/nNoTra\n\ttSmGzpv6ehhTCM/cnAZ7wIkJi4DuQWFzFMo/w21oQ7BWLIKw5SHT7+JQB3k8m/WEPU\n\tjv2BfpZKEAzAfju7lZTTsE4m/K1boCb3p/52qtD0=","Date":"Thu, 11 Jun 2020 05:47:52 +0300","From":"Laurent Pinchart ","To":"Niklas =?utf-8?q?S=C3=B6derlund?= ","Cc":"libcamera-devel@lists.libcamera.org","Message-ID":"<20200611024752.GB13598@pendragon.ideasonboard.com>","References":"<20200611014614.1683623-1-niklas.soderlund@ragnatech.se>","MIME-Version":"1.0","Content-Type":"text/plain; charset=utf-8","Content-Disposition":"inline","Content-Transfer-Encoding":"8bit","In-Reply-To":"<20200611014614.1683623-1-niklas.soderlund@ragnatech.se>","Subject":"Re: [libcamera-devel] [PATCH] qcam: dng_writer: Record creation\n\ttime in the EXIF directory","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","X-List-Received-Date":"Thu, 11 Jun 2020 02:48:13 -0000"}},{"id":5184,"web_url":"https://patchwork.libcamera.org/comment/5184/","msgid":"<20200611183725.GM192296@oden.dyn.berto.se>","date":"2020-06-11T18:37:25","subject":"Re: [libcamera-devel] [PATCH] qcam: dng_writer: Record creation\n\ttime in the EXIF directory","submitter":{"id":5,"url":"https://patchwork.libcamera.org/api/people/5/","name":"Niklas Söderlund","email":"niklas.soderlund@ragnatech.se"},"content":"Hi Laurent,\n\nThanks for your feedback.\n\nOn 2020-06-11 05:47:52 +0300, Laurent Pinchart wrote:\n> Hi Niklas,\n> \n> Thank you for the patch.\n> \n> On Thu, Jun 11, 2020 at 03:46:14AM +0200, Niklas Söderlund wrote:\n> > If the EXIF directory is empty due to no metadata being available tools\n> > such as tiffinfo complains that the directory is malformed.\n> > \n> > TIFFFetchDirectory: Sanity check on directory count failed, this is probably not a valid IFD offset.\n> > TIFFReadCustomDirectory: Failed to read custom directory at offset 0.\n> > \n> > Always record the creation time in the EXIF directory instead of adding\n> > complexity to skip creating the EXIF directory if there is no metadata\n> > to record. This ensures there are at least one entry in the EXIF\n> > directory and that makes tiffinfo happy.\n> > \n> > Signed-off-by: Niklas Söderlund \n> > ---\n> > src/qcam/dng_writer.cpp | 11 +++++++++++\n> > 1 file changed, 11 insertions(+)\n> > \n> > diff --git a/src/qcam/dng_writer.cpp b/src/qcam/dng_writer.cpp\n> > index 330b169af25b7ac7..c1c3833740d96630 100644\n> > --- a/src/qcam/dng_writer.cpp\n> > +++ b/src/qcam/dng_writer.cpp\n> > @@ -426,6 +426,17 @@ int DNGWriter::write(const char *filename, const Camera *camera,\n> > \t/* Create a new IFD for the EXIF data and fill it. */\n> > \tTIFFCreateEXIFDirectory(tif);\n> > \n> > +\t/* Store creation time. */\n> > +\ttime_t rawtime;\n> > +\tstruct tm *timeinfo;\n> > +\tchar strTime[20];\n> > +\n> > +\ttime(&rawtime);\n> \n> Could we use the timestamp from the buffer instead ?\n\nWe can't as the timestamp in the buffer is copied from the v4l2 buffer \nand not guaranteed to be an offset from the epoch.\n\n> \n> > +\ttimeinfo = localtime(&rawtime);\n> > +\tstrftime(strTime, 20, \"%Y:%m:%d %H:%M:%S\", timeinfo);\n> \n> How are time zones supposed to be handled ?\n\nThe documentation states noting about timezones so I think the best \noption is the use the local timezone of the device running qcam.\n\n> \n> > +\n> > +\tTIFFSetField(tif, EXIFTAG_DATETIMEDIGITIZED, strTime);\n> \n> Should we also set EXIFTAG_DATETIMEORIGINAL to the same value ?\n\nWe should will send a v2.\n\n> \n> > +\n> > \tif (metadata.contains(controls::AnalogueGain)) {\n> > \t\tfloat gain = metadata.get(controls::AnalogueGain);\n> > \t\tuint16_t iso = std::min(std::max(gain * 100, 0.0f), 65535.0f);\n> \n> -- \n> Regards,\n> \n> Laurent Pinchart","headers":{"Return-Path":"","Received":["from mail-lf1-x142.google.com (mail-lf1-x142.google.com\n\t[IPv6:2a00:1450:4864:20::142])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTPS id 4FF6761027\n\tfor ;\n\tThu, 11 Jun 2020 20:37:28 +0200 (CEST)","by mail-lf1-x142.google.com with SMTP id x22so4074877lfd.4\n\tfor ;\n\tThu, 11 Jun 2020 11:37:28 -0700 (PDT)","from localhost (h-209-203.A463.priv.bahnhof.se. [155.4.209.203])\n\tby smtp.gmail.com with ESMTPSA id\n\t2sm900141lji.100.2020.06.11.11.37.25\n\t(version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256);\n\tThu, 11 Jun 2020 11:37:25 -0700 (PDT)"],"Authentication-Results":"lancelot.ideasonboard.com; dkim=pass (2048-bit key; \n\tunprotected)\n\theader.d=ragnatech-se.20150623.gappssmtp.com\n\theader.i=@ragnatech-se.20150623.gappssmtp.com header.b=\"AJRlQsYB\"; \n\tdkim-atps=neutral","DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/relaxed;\n\td=ragnatech-se.20150623.gappssmtp.com; s=20150623;\n\th=date:from:to:cc:subject:message-id:references:mime-version\n\t:content-disposition:content-transfer-encoding:in-reply-to;\n\tbh=FilEeWKFWIPEW9mWXfqicPxL8GxFvuycmuHVyl8txl8=;\n\tb=AJRlQsYB9kD8h03ys8WHTuZABl2O2i52e39lUtKXjHjNOs7OkWbYHqLatU7U6C1JQq\n\t18StSLD4WMUAkxSC1SMDnnQHovP8eNW0RZ5XuSr8cJrNfSznXIJJHWXbvcspguKr5EOm\n\t9Qp13t8LMS3h+ImbQ3SVGBqqFwYn26uLBQ1PQkdTyVrED5qC0SJfQ/1tADslwf1oN9Gy\n\tVVM9BnzesBUoojC/78jarJNBZHgIAw9RRjYeTHFWjkuqisIh32qD8Ob3mSk08Ch//nBu\n\t2cMNXHvvL68rsPAdUvQyc0McMFSDEUj1cWFeMpIMy2NfYZH9qeBO1OeHzwJCmfFylDe3\n\t/3hw==","X-Google-DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/relaxed;\n\td=1e100.net; s=20161025;\n\th=x-gm-message-state:date:from:to:cc:subject:message-id:references\n\t:mime-version:content-disposition:content-transfer-encoding\n\t:in-reply-to;\n\tbh=FilEeWKFWIPEW9mWXfqicPxL8GxFvuycmuHVyl8txl8=;\n\tb=T0nZfF/tGdn4S3THWjOl3dE28gJqpkFZAp3XiD5KWHRpIHZrooD7/d90R9pPY6Eh81\n\tqElPP4ocJljv1Wzc9nbaxQIT2lFG5BcjmaTBZ89diOHKmzqqjH1n0+5nnRVYd+3DD777\n\tV+fd2QeG/KVSRJGlNSgiw4opF+GAJrQf6q++xMJ+WE3McCjXWc6amUS0uVptEuPfmMbO\n\tSyLtamCLFbF6upPTb5V1y+QVqpDOFtpoZHPWw831hbUrNmA+QMajSPHrXlwum5rDPyyw\n\tZp+WLaYthnJNKS7ul3T4LEZkQ1Pd4PVOww4qWB8b+vs5iXmpR1tw7OuJZW0f5iAyax3K\n\t4PBQ==","X-Gm-Message-State":"AOAM532ydY96c46TweWAhkEdah0hP+dPIlmypWCZjKGAf/R4UzVH9zT9\n\tmTR/YRJwkvtw8z+Rhg9BlYutfQ==","X-Google-Smtp-Source":"ABdhPJzge72fHgqzQ+/FJCXHMhFejvvAle2/aZ9DPhNFTxUHEG+BZHZ4ugKkewa26eLsnaa5ikcc2w==","X-Received":"by 2002:ac2:48a3:: with SMTP id u3mr4980325lfg.115.1591900647304;\n\tThu, 11 Jun 2020 11:37:27 -0700 (PDT)","Date":"Thu, 11 Jun 2020 20:37:25 +0200","From":"Niklas =?iso-8859-1?q?S=F6derlund?= ","To":"Laurent Pinchart ","Cc":"libcamera-devel@lists.libcamera.org","Message-ID":"<20200611183725.GM192296@oden.dyn.berto.se>","References":"<20200611014614.1683623-1-niklas.soderlund@ragnatech.se>\n\t<20200611024752.GB13598@pendragon.ideasonboard.com>","MIME-Version":"1.0","Content-Type":"text/plain; charset=iso-8859-1","Content-Disposition":"inline","Content-Transfer-Encoding":"8bit","In-Reply-To":"<20200611024752.GB13598@pendragon.ideasonboard.com>","Subject":"Re: [libcamera-devel] [PATCH] qcam: dng_writer: Record creation\n\ttime in the EXIF directory","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","X-List-Received-Date":"Thu, 11 Jun 2020 18:37:28 -0000"}},{"id":5185,"web_url":"https://patchwork.libcamera.org/comment/5185/","msgid":"<20200611184657.GB6031@pendragon.ideasonboard.com>","date":"2020-06-11T18:46:57","subject":"Re: [libcamera-devel] [PATCH] qcam: dng_writer: Record creation\n\ttime in the EXIF directory","submitter":{"id":2,"url":"https://patchwork.libcamera.org/api/people/2/","name":"Laurent Pinchart","email":"laurent.pinchart@ideasonboard.com"},"content":"Hi Niklas,\n\nOn Thu, Jun 11, 2020 at 08:37:25PM +0200, Niklas Söderlund wrote:\n> On 2020-06-11 05:47:52 +0300, Laurent Pinchart wrote:\n> > On Thu, Jun 11, 2020 at 03:46:14AM +0200, Niklas Söderlund wrote:\n> > > If the EXIF directory is empty due to no metadata being available tools\n> > > such as tiffinfo complains that the directory is malformed.\n> > > \n> > > TIFFFetchDirectory: Sanity check on directory count failed, this is probably not a valid IFD offset.\n> > > TIFFReadCustomDirectory: Failed to read custom directory at offset 0.\n> > > \n> > > Always record the creation time in the EXIF directory instead of adding\n> > > complexity to skip creating the EXIF directory if there is no metadata\n> > > to record. This ensures there are at least one entry in the EXIF\n> > > directory and that makes tiffinfo happy.\n> > > \n> > > Signed-off-by: Niklas Söderlund \n> > > ---\n> > > src/qcam/dng_writer.cpp | 11 +++++++++++\n> > > 1 file changed, 11 insertions(+)\n> > > \n> > > diff --git a/src/qcam/dng_writer.cpp b/src/qcam/dng_writer.cpp\n> > > index 330b169af25b7ac7..c1c3833740d96630 100644\n> > > --- a/src/qcam/dng_writer.cpp\n> > > +++ b/src/qcam/dng_writer.cpp\n> > > @@ -426,6 +426,17 @@ int DNGWriter::write(const char *filename, const Camera *camera,\n> > > \t/* Create a new IFD for the EXIF data and fill it. */\n> > > \tTIFFCreateEXIFDirectory(tif);\n> > > \n> > > +\t/* Store creation time. */\n> > > +\ttime_t rawtime;\n> > > +\tstruct tm *timeinfo;\n> > > +\tchar strTime[20];\n> > > +\n> > > +\ttime(&rawtime);\n> > \n> > Could we use the timestamp from the buffer instead ?\n> \n> We can't as the timestamp in the buffer is copied from the v4l2 buffer \n> and not guaranteed to be an offset from the epoch.\n\nSo how do we fix that ? :-) It's not a prerequisite for this patch, but\nit seems to me that the problem needs to be addressed. We should also\nadd a \\todo here to mention that the timestamp should come from the\nbuffer.\n\n> > > +\ttimeinfo = localtime(&rawtime);\n> > > +\tstrftime(strTime, 20, \"%Y:%m:%d %H:%M:%S\", timeinfo);\n> > \n> > How are time zones supposed to be handled ?\n> \n> The documentation states noting about timezones so I think the best \n> option is the use the local timezone of the device running qcam.\n\nHow about the OffsetTime, OffsetTimeOriginal and OffsetTimeDigitized\ntags ?\n\n> > > +\n> > > +\tTIFFSetField(tif, EXIFTAG_DATETIMEDIGITIZED, strTime);\n> > \n> > Should we also set EXIFTAG_DATETIMEORIGINAL to the same value ?\n> \n> We should will send a v2.\n> \n> > > +\n> > > \tif (metadata.contains(controls::AnalogueGain)) {\n> > > \t\tfloat gain = metadata.get(controls::AnalogueGain);\n> > > \t\tuint16_t iso = std::min(std::max(gain * 100, 0.0f), 65535.0f);","headers":{"Return-Path":"","Received":["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 6B6BC61027\n\tfor ;\n\tThu, 11 Jun 2020 20:47:19 +0200 (CEST)","from pendragon.ideasonboard.com (81-175-216-236.bb.dnainternet.fi\n\t[81.175.216.236])\n\tby perceval.ideasonboard.com (Postfix) with ESMTPSA id CFA3524F;\n\tThu, 11 Jun 2020 20:47:18 +0200 (CEST)"],"Authentication-Results":"lancelot.ideasonboard.com; dkim=pass (1024-bit key; \n\tunprotected) header.d=ideasonboard.com\n\theader.i=@ideasonboard.com\n\theader.b=\"HVKr5/Mz\"; dkim-atps=neutral","DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com;\n\ts=mail; t=1591901239;\n\tbh=q8ymB+wZsUru/ef8m//jlLC7uIlILdhT9BnMIVk2+SA=;\n\th=Date:From:To:Cc:Subject:References:In-Reply-To:From;\n\tb=HVKr5/MzKEyJk0DWS9de2i6Ud8PQlgDGJtEtZoWGk7fcH6n/1/dM7HrK2aT/hGmdq\n\tW1xGVCcLaadHS0GcX+XeIolia/rKsRjSe2aFQHP+nwtriYFJMEndB7fLmj9pkIQB89\n\ttg3+B3UBQhUanB69y6uvzTHtBD0X/jKge2mXqqVI=","Date":"Thu, 11 Jun 2020 21:46:57 +0300","From":"Laurent Pinchart ","To":"Niklas =?utf-8?q?S=C3=B6derlund?= ","Cc":"libcamera-devel@lists.libcamera.org","Message-ID":"<20200611184657.GB6031@pendragon.ideasonboard.com>","References":"<20200611014614.1683623-1-niklas.soderlund@ragnatech.se>\n\t<20200611024752.GB13598@pendragon.ideasonboard.com>\n\t<20200611183725.GM192296@oden.dyn.berto.se>","MIME-Version":"1.0","Content-Type":"text/plain; charset=utf-8","Content-Disposition":"inline","Content-Transfer-Encoding":"8bit","In-Reply-To":"<20200611183725.GM192296@oden.dyn.berto.se>","Subject":"Re: [libcamera-devel] [PATCH] qcam: dng_writer: Record creation\n\ttime in the EXIF directory","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","X-List-Received-Date":"Thu, 11 Jun 2020 18:47:19 -0000"}},{"id":5186,"web_url":"https://patchwork.libcamera.org/comment/5186/","msgid":"<20200611201605.GN192296@oden.dyn.berto.se>","date":"2020-06-11T20:16:05","subject":"Re: [libcamera-devel] [PATCH] qcam: dng_writer: Record creation\n\ttime in the EXIF directory","submitter":{"id":5,"url":"https://patchwork.libcamera.org/api/people/5/","name":"Niklas Söderlund","email":"niklas.soderlund@ragnatech.se"},"content":"Hi Laurent,\n\nThanks for your feedback.\n\nOn 2020-06-11 21:46:57 +0300, Laurent Pinchart wrote:\n> Hi Niklas,\n> \n> On Thu, Jun 11, 2020 at 08:37:25PM +0200, Niklas Söderlund wrote:\n> > On 2020-06-11 05:47:52 +0300, Laurent Pinchart wrote:\n> > > On Thu, Jun 11, 2020 at 03:46:14AM +0200, Niklas Söderlund wrote:\n> > > > If the EXIF directory is empty due to no metadata being available tools\n> > > > such as tiffinfo complains that the directory is malformed.\n> > > > \n> > > > TIFFFetchDirectory: Sanity check on directory count failed, this is probably not a valid IFD offset.\n> > > > TIFFReadCustomDirectory: Failed to read custom directory at offset 0.\n> > > > \n> > > > Always record the creation time in the EXIF directory instead of adding\n> > > > complexity to skip creating the EXIF directory if there is no metadata\n> > > > to record. This ensures there are at least one entry in the EXIF\n> > > > directory and that makes tiffinfo happy.\n> > > > \n> > > > Signed-off-by: Niklas Söderlund \n> > > > ---\n> > > > src/qcam/dng_writer.cpp | 11 +++++++++++\n> > > > 1 file changed, 11 insertions(+)\n> > > > \n> > > > diff --git a/src/qcam/dng_writer.cpp b/src/qcam/dng_writer.cpp\n> > > > index 330b169af25b7ac7..c1c3833740d96630 100644\n> > > > --- a/src/qcam/dng_writer.cpp\n> > > > +++ b/src/qcam/dng_writer.cpp\n> > > > @@ -426,6 +426,17 @@ int DNGWriter::write(const char *filename, const Camera *camera,\n> > > > \t/* Create a new IFD for the EXIF data and fill it. */\n> > > > \tTIFFCreateEXIFDirectory(tif);\n> > > > \n> > > > +\t/* Store creation time. */\n> > > > +\ttime_t rawtime;\n> > > > +\tstruct tm *timeinfo;\n> > > > +\tchar strTime[20];\n> > > > +\n> > > > +\ttime(&rawtime);\n> > > \n> > > Could we use the timestamp from the buffer instead ?\n> > \n> > We can't as the timestamp in the buffer is copied from the v4l2 buffer \n> > and not guaranteed to be an offset from the epoch.\n> \n> So how do we fix that ? :-) It's not a prerequisite for this patch, but\n> it seems to me that the problem needs to be addressed. We should also\n> add a \\todo here to mention that the timestamp should come from the\n> buffer.\n\nI'm not sure we should attempt to fix it in user-space. I think it's \nmore value for applications to have as accurate timestamps as possible \nto compare between captured buffers then it's to be able to record in a \nfile on disk the exact nano second compared to the epoch it was \ncaptured, unless we manage to get libcamera running on ATLAS at CERN... \n:-)\n\nSo if we are to fix this I think we need to solve it in the kernel \ninterface so that all buffer timestamps are indeed an epoch number.\n\n> \n> > > > +\ttimeinfo = localtime(&rawtime);\n> > > > +\tstrftime(strTime, 20, \"%Y:%m:%d %H:%M:%S\", timeinfo);\n> > > \n> > > How are time zones supposed to be handled ?\n> > \n> > The documentation states noting about timezones so I think the best \n> > option is the use the local timezone of the device running qcam.\n> \n> How about the OffsetTime, OffsetTimeOriginal and OffsetTimeDigitized\n> tags ?\n\nI looked at those, but they are not present in my libtiff (4.1.0), there \nare no #defines for the tags. If I use the numeric value from the EXIF \ndocumentation I endup with warnings creating the DNG file.\n\n TIFFSetField: /home/chrx/test.dng: Unknown tag 36880.\n TIFFSetField: /home/chrx/test.dng: Unknown tag 36881.\n TIFFSetField: /home/chrx/test.dng: Unknown tag 36882.\n\nI'm not sure if they are recorded to the output DNG file since the \nwarnings are printed, but nothing shows up when I run tiffinfo on the \ndng file that still gets created.\n\nSo I can't find a good option to record the timezone information in the \nDNG. So I think we have two options reccord the time in the local \ntimezone or GMT. My preference would be to store it in the local \ntimezone but I have no strong feelings, let me know what you think.\n\n> \n> > > > +\n> > > > +\tTIFFSetField(tif, EXIFTAG_DATETIMEDIGITIZED, strTime);\n> > > \n> > > Should we also set EXIFTAG_DATETIMEORIGINAL to the same value ?\n> > \n> > We should will send a v2.\n> > \n> > > > +\n> > > > \tif (metadata.contains(controls::AnalogueGain)) {\n> > > > \t\tfloat gain = metadata.get(controls::AnalogueGain);\n> > > > \t\tuint16_t iso = std::min(std::max(gain * 100, 0.0f), 65535.0f);\n> \n> -- \n> Regards,\n> \n> Laurent Pinchart","headers":{"Return-Path":"","Received":["from mail-lj1-x243.google.com (mail-lj1-x243.google.com\n\t[IPv6:2a00:1450:4864:20::243])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTPS id 11FA361027\n\tfor ;\n\tThu, 11 Jun 2020 22:16:08 +0200 (CEST)","by mail-lj1-x243.google.com with SMTP id q19so8500031lji.2\n\tfor ;\n\tThu, 11 Jun 2020 13:16:08 -0700 (PDT)","from localhost (h-209-203.A463.priv.bahnhof.se. [155.4.209.203])\n\tby smtp.gmail.com with ESMTPSA id\n\tj17sm968054lja.30.2020.06.11.13.16.05\n\t(version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256);\n\tThu, 11 Jun 2020 13:16:06 -0700 (PDT)"],"Authentication-Results":"lancelot.ideasonboard.com; dkim=pass (2048-bit key; \n\tunprotected)\n\theader.d=ragnatech-se.20150623.gappssmtp.com\n\theader.i=@ragnatech-se.20150623.gappssmtp.com header.b=\"oCphrzcz\"; \n\tdkim-atps=neutral","DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/relaxed;\n\td=ragnatech-se.20150623.gappssmtp.com; s=20150623;\n\th=date:from:to:cc:subject:message-id:references:mime-version\n\t:content-disposition:content-transfer-encoding:in-reply-to;\n\tbh=FdYszeLUsyvYT5jUKA9dURXBUBOHjpYwNFRqsDRFiDU=;\n\tb=oCphrzczITxMYD3vT2ucfDrjMHeKGNjlS28AwBo4WgtsG//a49sWnkclCCX88y+nRn\n\tnl76JArsViylrRMH1mqB9Lr6uKsEwCeQSAIKaAs8Kt/rUL9oK4+ZsD/b+hiYk5LS1HTD\n\tdniIk6NOJ8s5GcA7iddIFIbJ9ZsTzvdNfVAs6eBWT/K5ihKdd1TOv6mIlIp+wC81pk6I\n\tOAvnbGJ1KdlhcQXVIczPHI85EmqKu17i+i0HZdJg+6nzsSZfeLuAUvZvZlM85Y9i1Evx\n\t0sM4N0b3kTywFQ11S5x77SJmb4N6xu2IURhqls9gpNoKAsVggW2pwbLh/sTzMby8eF3U\n\t1iaw==","X-Google-DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/relaxed;\n\td=1e100.net; s=20161025;\n\th=x-gm-message-state:date:from:to:cc:subject:message-id:references\n\t:mime-version:content-disposition:content-transfer-encoding\n\t:in-reply-to;\n\tbh=FdYszeLUsyvYT5jUKA9dURXBUBOHjpYwNFRqsDRFiDU=;\n\tb=L8Hw4wfvTDlr8WgYfZ1Zng8eD2Xotgotc+R1Cd4iRrpYt/YrAJoUmBjvKE039WP7vG\n\tjZIcpC+CG6ywFlTOGSrURGPK39OJ/ztG+8X0Lrq1IBckhfxlDrFPgZVDxUFP+CYddyqv\n\tkgXVJtCzaplw9IXC+fet9Nmk1O8rcfmo5kPsenwsK3Y9S4azKWUw56F9W0wDfB1CpaeB\n\thyCObjIUDnyu7V0B0iwqAdPh2sOVH8oL7aoqjQK4vd8LIa8uVKjA2hJX5DNF9W/vESvP\n\trGPaAIpwcEiwRoTKnDdz8HERZZwciw4piP/LHHHehd9FBgWEUwIX0Qgf78gwVsdbW6Hk\n\t+pmQ==","X-Gm-Message-State":"AOAM531RWWLmqPL7Boj7H238/9UHYEC4vYi1QSGsLh81c7DHLqNXvzeJ\n\tyi6bMlozQ6qQSLXi77AYljKxtg==","X-Google-Smtp-Source":"ABdhPJygEcfIvBpdz24rjn02fAHEwlpcBKslOafTyf4Z1IjE5QaPT5Xo6RFLn85Cke6RKXSWgueuJA==","X-Received":"by 2002:a2e:860f:: with SMTP id\n\ta15mr5401650lji.127.1591906566848; \n\tThu, 11 Jun 2020 13:16:06 -0700 (PDT)","Date":"Thu, 11 Jun 2020 22:16:05 +0200","From":"Niklas =?iso-8859-1?q?S=F6derlund?= ","To":"Laurent Pinchart ","Cc":"libcamera-devel@lists.libcamera.org","Message-ID":"<20200611201605.GN192296@oden.dyn.berto.se>","References":"<20200611014614.1683623-1-niklas.soderlund@ragnatech.se>\n\t<20200611024752.GB13598@pendragon.ideasonboard.com>\n\t<20200611183725.GM192296@oden.dyn.berto.se>\n\t<20200611184657.GB6031@pendragon.ideasonboard.com>","MIME-Version":"1.0","Content-Type":"text/plain; charset=iso-8859-1","Content-Disposition":"inline","Content-Transfer-Encoding":"8bit","In-Reply-To":"<20200611184657.GB6031@pendragon.ideasonboard.com>","Subject":"Re: [libcamera-devel] [PATCH] qcam: dng_writer: Record creation\n\ttime in the EXIF directory","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","X-List-Received-Date":"Thu, 11 Jun 2020 20:16:08 -0000"}},{"id":5187,"web_url":"https://patchwork.libcamera.org/comment/5187/","msgid":"<20200612080021.GB5957@pendragon.ideasonboard.com>","date":"2020-06-12T08:00:21","subject":"Re: [libcamera-devel] [PATCH] qcam: dng_writer: Record creation\n\ttime in the EXIF directory","submitter":{"id":2,"url":"https://patchwork.libcamera.org/api/people/2/","name":"Laurent Pinchart","email":"laurent.pinchart@ideasonboard.com"},"content":"Hi Niklas,\n\nOn Thu, Jun 11, 2020 at 10:16:05PM +0200, Niklas Söderlund wrote:\n> On 2020-06-11 21:46:57 +0300, Laurent Pinchart wrote:\n> > On Thu, Jun 11, 2020 at 08:37:25PM +0200, Niklas Söderlund wrote:\n> > > On 2020-06-11 05:47:52 +0300, Laurent Pinchart wrote:\n> > > > On Thu, Jun 11, 2020 at 03:46:14AM +0200, Niklas Söderlund wrote:\n> > > > > If the EXIF directory is empty due to no metadata being available tools\n> > > > > such as tiffinfo complains that the directory is malformed.\n> > > > > \n> > > > > TIFFFetchDirectory: Sanity check on directory count failed, this is probably not a valid IFD offset.\n> > > > > TIFFReadCustomDirectory: Failed to read custom directory at offset 0.\n> > > > > \n> > > > > Always record the creation time in the EXIF directory instead of adding\n> > > > > complexity to skip creating the EXIF directory if there is no metadata\n> > > > > to record. This ensures there are at least one entry in the EXIF\n> > > > > directory and that makes tiffinfo happy.\n> > > > > \n> > > > > Signed-off-by: Niklas Söderlund \n> > > > > ---\n> > > > > src/qcam/dng_writer.cpp | 11 +++++++++++\n> > > > > 1 file changed, 11 insertions(+)\n> > > > > \n> > > > > diff --git a/src/qcam/dng_writer.cpp b/src/qcam/dng_writer.cpp\n> > > > > index 330b169af25b7ac7..c1c3833740d96630 100644\n> > > > > --- a/src/qcam/dng_writer.cpp\n> > > > > +++ b/src/qcam/dng_writer.cpp\n> > > > > @@ -426,6 +426,17 @@ int DNGWriter::write(const char *filename, const Camera *camera,\n> > > > > \t/* Create a new IFD for the EXIF data and fill it. */\n> > > > > \tTIFFCreateEXIFDirectory(tif);\n> > > > > \n> > > > > +\t/* Store creation time. */\n> > > > > +\ttime_t rawtime;\n> > > > > +\tstruct tm *timeinfo;\n> > > > > +\tchar strTime[20];\n> > > > > +\n> > > > > +\ttime(&rawtime);\n> > > > \n> > > > Could we use the timestamp from the buffer instead ?\n> > > \n> > > We can't as the timestamp in the buffer is copied from the v4l2 buffer \n> > > and not guaranteed to be an offset from the epoch.\n> > \n> > So how do we fix that ? :-) It's not a prerequisite for this patch, but\n> > it seems to me that the problem needs to be addressed. We should also\n> > add a \\todo here to mention that the timestamp should come from the\n> > buffer.\n> \n> I'm not sure we should attempt to fix it in user-space. I think it's \n> more value for applications to have as accurate timestamps as possible \n> to compare between captured buffers then it's to be able to record in a \n> file on disk the exact nano second compared to the epoch it was \n> captured, unless we manage to get libcamera running on ATLAS at CERN... \n> :-)\n> \n> So if we are to fix this I think we need to solve it in the kernel \n> interface so that all buffer timestamps are indeed an epoch number.\n\nI agree with the first part of your statement, the kernel should give\nuserspace high-precision timestamps, not a calendar date. I'm however\npuzzled by why you think we couldn't rely on those timestamps here, and\nconvert them to the time representation we need. This involves figuring\nout the delta between the two, and maybe in the end we will lose so much\nprecision there that using the high-precision timestamp wouldn't be\nworth it in the first place.\n\n> > > > > +\ttimeinfo = localtime(&rawtime);\n> > > > > +\tstrftime(strTime, 20, \"%Y:%m:%d %H:%M:%S\", timeinfo);\n> > > > \n> > > > How are time zones supposed to be handled ?\n> > > \n> > > The documentation states noting about timezones so I think the best \n> > > option is the use the local timezone of the device running qcam.\n> > \n> > How about the OffsetTime, OffsetTimeOriginal and OffsetTimeDigitized\n> > tags ?\n> \n> I looked at those, but they are not present in my libtiff (4.1.0), there \n> are no #defines for the tags. If I use the numeric value from the EXIF \n> documentation I endup with warnings creating the DNG file.\n> \n> TIFFSetField: /home/chrx/test.dng: Unknown tag 36880.\n> TIFFSetField: /home/chrx/test.dng: Unknown tag 36881.\n> TIFFSetField: /home/chrx/test.dng: Unknown tag 36882.\n> \n> I'm not sure if they are recorded to the output DNG file since the \n> warnings are printed, but nothing shows up when I run tiffinfo on the \n> dng file that still gets created.\n\nlibtiff has an API to write custom tags, but it's awkward to use. It\ninvolves creating a custom directory with TIFFCreateCustomDirectory(),\nand passing a table of tag metadata to that function. As we use\nTIFFCreateEXIFDirectory() here, we would have to reimplement a large\npart of that, which isn't practical. The fact that\nTIFFCreateCustomDirectory() takes a TIFFFieldArray pointer, with\n\ntypedef struct _TIFFFieldArray TIFFFieldArray;\n\nand the structure _TIFFFieldArray not being defined in public headers\nmakes me think this isn't a widely used API...\n\nSupport for the timezone offsets has been added in libtiff, I suppose it\nwill make it to the next release. Can you record a \\todo here ?\n\n> So I can't find a good option to record the timezone information in the \n> DNG. So I think we have two options reccord the time in the local \n> timezone or GMT. My preference would be to store it in the local \n> timezone but I have no strong feelings, let me know what you think.\n\nThe local time is best I think, that seems to be in line with what most\ncameras do.\n\n> > > > > +\n> > > > > +\tTIFFSetField(tif, EXIFTAG_DATETIMEDIGITIZED, strTime);\n> > > > \n> > > > Should we also set EXIFTAG_DATETIMEORIGINAL to the same value ?\n> > > \n> > > We should will send a v2.\n> > > \n> > > > > +\n> > > > > \tif (metadata.contains(controls::AnalogueGain)) {\n> > > > > \t\tfloat gain = metadata.get(controls::AnalogueGain);\n> > > > > \t\tuint16_t iso = std::min(std::max(gain * 100, 0.0f), 65535.0f);","headers":{"Return-Path":"","Received":["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 95ADD603C7\n\tfor ;\n\tFri, 12 Jun 2020 10:00:42 +0200 (CEST)","from pendragon.ideasonboard.com (81-175-216-236.bb.dnainternet.fi\n\t[81.175.216.236])\n\tby perceval.ideasonboard.com (Postfix) with ESMTPSA id 0CBCA24F;\n\tFri, 12 Jun 2020 10:00:41 +0200 (CEST)"],"Authentication-Results":"lancelot.ideasonboard.com; dkim=pass (1024-bit key; \n\tunprotected) header.d=ideasonboard.com\n\theader.i=@ideasonboard.com\n\theader.b=\"EkFwdWHi\"; dkim-atps=neutral","DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com;\n\ts=mail; t=1591948842;\n\tbh=spdkMQgKm3VxNuIbg0WnMKjLKJIrTPAjo1lOMebmRIw=;\n\th=Date:From:To:Cc:Subject:References:In-Reply-To:From;\n\tb=EkFwdWHiYSFHkgcok33rBMGo2s9WLjQD5njnT+0WC3+A3ziR8f2PTWJWhGnsMCaZC\n\tLsXVzGgT45p0A5c9+Narzobk87sBgh8DlTVWlN5xYcXmkb73cuM5pqynFiOmTtZYP3\n\ti99c+diqTRzNDbOYlPgT1e73NXPWxmGJ5SrSLH2A=","Date":"Fri, 12 Jun 2020 11:00:21 +0300","From":"Laurent Pinchart ","To":"Niklas =?utf-8?q?S=C3=B6derlund?= ","Cc":"libcamera-devel@lists.libcamera.org","Message-ID":"<20200612080021.GB5957@pendragon.ideasonboard.com>","References":"<20200611014614.1683623-1-niklas.soderlund@ragnatech.se>\n\t<20200611024752.GB13598@pendragon.ideasonboard.com>\n\t<20200611183725.GM192296@oden.dyn.berto.se>\n\t<20200611184657.GB6031@pendragon.ideasonboard.com>\n\t<20200611201605.GN192296@oden.dyn.berto.se>","MIME-Version":"1.0","Content-Type":"text/plain; charset=utf-8","Content-Disposition":"inline","Content-Transfer-Encoding":"8bit","In-Reply-To":"<20200611201605.GN192296@oden.dyn.berto.se>","Subject":"Re: [libcamera-devel] [PATCH] qcam: dng_writer: Record creation\n\ttime in the EXIF directory","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","X-List-Received-Date":"Fri, 12 Jun 2020 08:00:42 -0000"}},{"id":5191,"web_url":"https://patchwork.libcamera.org/comment/5191/","msgid":"<20200612103515.GA915368@oden.dyn.berto.se>","date":"2020-06-12T10:35:15","subject":"Re: [libcamera-devel] [PATCH] qcam: dng_writer: Record creation\n\ttime in the EXIF directory","submitter":{"id":5,"url":"https://patchwork.libcamera.org/api/people/5/","name":"Niklas Söderlund","email":"niklas.soderlund@ragnatech.se"},"content":"Hello Laurent,\n\nOn 2020-06-12 11:00:21 +0300, Laurent Pinchart wrote:\n> Hi Niklas,\n> \n> On Thu, Jun 11, 2020 at 10:16:05PM +0200, Niklas Söderlund wrote:\n> > On 2020-06-11 21:46:57 +0300, Laurent Pinchart wrote:\n> > > On Thu, Jun 11, 2020 at 08:37:25PM +0200, Niklas Söderlund wrote:\n> > > > On 2020-06-11 05:47:52 +0300, Laurent Pinchart wrote:\n> > > > > On Thu, Jun 11, 2020 at 03:46:14AM +0200, Niklas Söderlund wrote:\n> > > > > > If the EXIF directory is empty due to no metadata being available tools\n> > > > > > such as tiffinfo complains that the directory is malformed.\n> > > > > > \n> > > > > > TIFFFetchDirectory: Sanity check on directory count failed, this is probably not a valid IFD offset.\n> > > > > > TIFFReadCustomDirectory: Failed to read custom directory at offset 0.\n> > > > > > \n> > > > > > Always record the creation time in the EXIF directory instead of adding\n> > > > > > complexity to skip creating the EXIF directory if there is no metadata\n> > > > > > to record. This ensures there are at least one entry in the EXIF\n> > > > > > directory and that makes tiffinfo happy.\n> > > > > > \n> > > > > > Signed-off-by: Niklas Söderlund \n> > > > > > ---\n> > > > > > src/qcam/dng_writer.cpp | 11 +++++++++++\n> > > > > > 1 file changed, 11 insertions(+)\n> > > > > > \n> > > > > > diff --git a/src/qcam/dng_writer.cpp b/src/qcam/dng_writer.cpp\n> > > > > > index 330b169af25b7ac7..c1c3833740d96630 100644\n> > > > > > --- a/src/qcam/dng_writer.cpp\n> > > > > > +++ b/src/qcam/dng_writer.cpp\n> > > > > > @@ -426,6 +426,17 @@ int DNGWriter::write(const char *filename, const Camera *camera,\n> > > > > > \t/* Create a new IFD for the EXIF data and fill it. */\n> > > > > > \tTIFFCreateEXIFDirectory(tif);\n> > > > > > \n> > > > > > +\t/* Store creation time. */\n> > > > > > +\ttime_t rawtime;\n> > > > > > +\tstruct tm *timeinfo;\n> > > > > > +\tchar strTime[20];\n> > > > > > +\n> > > > > > +\ttime(&rawtime);\n> > > > > \n> > > > > Could we use the timestamp from the buffer instead ?\n> > > > \n> > > > We can't as the timestamp in the buffer is copied from the v4l2 buffer \n> > > > and not guaranteed to be an offset from the epoch.\n> > > \n> > > So how do we fix that ? :-) It's not a prerequisite for this patch, but\n> > > it seems to me that the problem needs to be addressed. We should also\n> > > add a \\todo here to mention that the timestamp should come from the\n> > > buffer.\n> > \n> > I'm not sure we should attempt to fix it in user-space. I think it's \n> > more value for applications to have as accurate timestamps as possible \n> > to compare between captured buffers then it's to be able to record in a \n> > file on disk the exact nano second compared to the epoch it was \n> > captured, unless we manage to get libcamera running on ATLAS at CERN... \n> > :-)\n> > \n> > So if we are to fix this I think we need to solve it in the kernel \n> > interface so that all buffer timestamps are indeed an epoch number.\n> \n> I agree with the first part of your statement, the kernel should give\n> userspace high-precision timestamps, not a calendar date. I'm however\n> puzzled by why you think we couldn't rely on those timestamps here, and\n> convert them to the time representation we need. This involves figuring\n> out the delta between the two, and maybe in the end we will lose so much\n> precision there that using the high-precision timestamp wouldn't be\n> worth it in the first place.\n\nI'm sorry if I was unclear. What I tried to say was that\n\n- The timestamps from V4L2 may not be in relation to any calendar date \n and can only be treated as a counter where the only meaning one can \n extract from it is comparing the stamp from two buffers from the same \n source to figure out the delta time.\n\n- I think it's a bad idea for libcamera to insert a calendar timestamp \n into the FrameBuffer object once it get holds of the V4L2 buffer. I \n think we should do it the way it's done today, copy the timestamp from \n the V4L2 buffer. I think it's more value for applications to be able \n to calculate a high precision delta time between two buffers then to \n have a calender timestamp with high jitter as we would have if we \n create it in user-space.\n\n- If we want to use a timestamp to produce a calender time here and we \n want the source of the timestamp to come from the FrameBuffer it's my \n opinion we need to fix the kernel API so that the high precision \n timestamp created by the kernel for the V4L2 buffer is in addition to \n a high precision also guaranteed to be relatable to calendar time.\n\n If we can't do that I think producing a calendar time in the DNG \n writer is just as accurate as doing so in the libcamera core and \n storing it in an additional field in FrameBuffer.\n\n> \n> > > > > > +\ttimeinfo = localtime(&rawtime);\n> > > > > > +\tstrftime(strTime, 20, \"%Y:%m:%d %H:%M:%S\", timeinfo);\n> > > > > \n> > > > > How are time zones supposed to be handled ?\n> > > > \n> > > > The documentation states noting about timezones so I think the best \n> > > > option is the use the local timezone of the device running qcam.\n> > > \n> > > How about the OffsetTime, OffsetTimeOriginal and OffsetTimeDigitized\n> > > tags ?\n> > \n> > I looked at those, but they are not present in my libtiff (4.1.0), there \n> > are no #defines for the tags. If I use the numeric value from the EXIF \n> > documentation I endup with warnings creating the DNG file.\n> > \n> > TIFFSetField: /home/chrx/test.dng: Unknown tag 36880.\n> > TIFFSetField: /home/chrx/test.dng: Unknown tag 36881.\n> > TIFFSetField: /home/chrx/test.dng: Unknown tag 36882.\n> > \n> > I'm not sure if they are recorded to the output DNG file since the \n> > warnings are printed, but nothing shows up when I run tiffinfo on the \n> > dng file that still gets created.\n> \n> libtiff has an API to write custom tags, but it's awkward to use. It\n> involves creating a custom directory with TIFFCreateCustomDirectory(),\n> and passing a table of tag metadata to that function. As we use\n> TIFFCreateEXIFDirectory() here, we would have to reimplement a large\n> part of that, which isn't practical. The fact that\n> TIFFCreateCustomDirectory() takes a TIFFFieldArray pointer, with\n> \n> typedef struct _TIFFFieldArray TIFFFieldArray;\n> \n> and the structure _TIFFFieldArray not being defined in public headers\n> makes me think this isn't a widely used API...\n> \n> Support for the timezone offsets has been added in libtiff, I suppose it\n> will make it to the next release. Can you record a \\todo here ?\n\nI will add an \\todo to add OffsetTimeOriginal and OffsetTimeDigitized \ntags here once libtiff catches up.\n\n> \n> > So I can't find a good option to record the timezone information in the \n> > DNG. So I think we have two options reccord the time in the local \n> > timezone or GMT. My preference would be to store it in the local \n> > timezone but I have no strong feelings, let me know what you think.\n> \n> The local time is best I think, that seems to be in line with what most\n> cameras do.\n> \n> > > > > > +\n> > > > > > +\tTIFFSetField(tif, EXIFTAG_DATETIMEDIGITIZED, strTime);\n> > > > > \n> > > > > Should we also set EXIFTAG_DATETIMEORIGINAL to the same value ?\n> > > > \n> > > > We should will send a v2.\n> > > > \n> > > > > > +\n> > > > > > \tif (metadata.contains(controls::AnalogueGain)) {\n> > > > > > \t\tfloat gain = metadata.get(controls::AnalogueGain);\n> > > > > > \t\tuint16_t iso = std::min(std::max(gain * 100, 0.0f), 65535.0f);\n> \n> -- \n> Regards,\n> \n> Laurent Pinchart","headers":{"Return-Path":"","Received":["from mail-lj1-x241.google.com (mail-lj1-x241.google.com\n\t[IPv6:2a00:1450:4864:20::241])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTPS id 3FBDA60C4E\n\tfor ;\n\tFri, 12 Jun 2020 12:35:18 +0200 (CEST)","by mail-lj1-x241.google.com with SMTP id q19so10528803lji.2\n\tfor ;\n\tFri, 12 Jun 2020 03:35:18 -0700 (PDT)","from localhost (h-209-203.A463.priv.bahnhof.se. [155.4.209.203])\n\tby smtp.gmail.com with ESMTPSA id\n\tu26sm332757lfe.9.2020.06.12.03.35.15\n\t(version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256);\n\tFri, 12 Jun 2020 03:35:16 -0700 (PDT)"],"Authentication-Results":"lancelot.ideasonboard.com; dkim=pass (2048-bit key; \n\tunprotected)\n\theader.d=ragnatech-se.20150623.gappssmtp.com\n\theader.i=@ragnatech-se.20150623.gappssmtp.com header.b=\"SSwnhP+f\"; \n\tdkim-atps=neutral","DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/relaxed;\n\td=ragnatech-se.20150623.gappssmtp.com; s=20150623;\n\th=date:from:to:cc:subject:message-id:references:mime-version\n\t:content-disposition:content-transfer-encoding:in-reply-to;\n\tbh=aQM2SVydq8yv7OB8h9T2p5zGrl2G0MTIpZgS2H5CDFU=;\n\tb=SSwnhP+foMivDX1aroffU+IP0CdSQpbbLyLtLaxWfi/3xBA4yMWcwXVfwY26bKsigR\n\tnTHjxUWdL3AW5VxIAy7+gMqM2+JXJcE7gMxroUc69Ol6ZosRis4Ginq+1Soh+njJqfYA\n\t9Rx9jk4ZbGFWPyVRTYtlXeh3qonXAPUw3/VPDLPg+4hYEaTDtQlpFQycqcXElzrm3Nrd\n\tzVaxRD702yHSs5PlX8E4ld3adnwYWeH8JcN/ytvFGKwmE0iWy7ZZREn50iGbVp9wxIP1\n\tvji/sh1SPvtlrB2zGIkmZxh0DOaJzBax2elJ7GWSn92q0P6Ah+4G4XJs9UCCAcWlfukd\n\tSYoA==","X-Google-DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/relaxed;\n\td=1e100.net; s=20161025;\n\th=x-gm-message-state:date:from:to:cc:subject:message-id:references\n\t:mime-version:content-disposition:content-transfer-encoding\n\t:in-reply-to;\n\tbh=aQM2SVydq8yv7OB8h9T2p5zGrl2G0MTIpZgS2H5CDFU=;\n\tb=eZdrlsgH0IH0ZOIAfAUezCfZsEOVOvMiTPu32JEKnS5OJFvq1X9NDl5hwvuARmOMPS\n\tNnvMuhFnQtxKsLxGmRNK7fz7ediuwNEGiewWTKEf7knkG8s2EH4xH98Jvs7xM5ZcofTY\n\t8Qc+5VY9w7Nxw4Orb5LBrm4uKADyD46YK4pFJYc2z7mZi8hgVz9uSCK92t5PzuU7GZB9\n\tc7Id5ML6JyYbNbOCZH4rPqUIRLSdRA87P14ZtrQckzWxEwMSlaTuSJzlvN/LVmySdXX2\n\tMJEOcCbh1UGawOnKnNjhEGQ5oOo/M3LcfQ+Q6VeOUmF+nrqQkuZ0ODMXsAV318YOrcP1\n\twx2w==","X-Gm-Message-State":"AOAM532rWzmJ+uvlUXsSL7H0MZ80Rv5/YYD5L/r4e0VHUMiaHu6swsGF\n\ts/TaB4gjh+0WsGSxk+UjwVf+iEI/hEs=","X-Google-Smtp-Source":"ABdhPJzRsglwVdVk59GrAJ7cFw5WkFBw4h8GiKLPlDJzypc6HU1v/rQWA7g3KhRgXTAy5doQqVP7jA==","X-Received":"by 2002:a2e:9a54:: with SMTP id\n\tk20mr6542476ljj.106.1591958117257; \n\tFri, 12 Jun 2020 03:35:17 -0700 (PDT)","Date":"Fri, 12 Jun 2020 12:35:15 +0200","From":"Niklas =?iso-8859-1?q?S=F6derlund?= ","To":"Laurent Pinchart ","Cc":"libcamera-devel@lists.libcamera.org","Message-ID":"<20200612103515.GA915368@oden.dyn.berto.se>","References":"<20200611014614.1683623-1-niklas.soderlund@ragnatech.se>\n\t<20200611024752.GB13598@pendragon.ideasonboard.com>\n\t<20200611183725.GM192296@oden.dyn.berto.se>\n\t<20200611184657.GB6031@pendragon.ideasonboard.com>\n\t<20200611201605.GN192296@oden.dyn.berto.se>\n\t<20200612080021.GB5957@pendragon.ideasonboard.com>","MIME-Version":"1.0","Content-Type":"text/plain; charset=iso-8859-1","Content-Disposition":"inline","Content-Transfer-Encoding":"8bit","In-Reply-To":"<20200612080021.GB5957@pendragon.ideasonboard.com>","Subject":"Re: [libcamera-devel] [PATCH] qcam: dng_writer: Record creation\n\ttime in the EXIF directory","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","X-List-Received-Date":"Fri, 12 Jun 2020 10:35:18 -0000"}},{"id":5212,"web_url":"https://patchwork.libcamera.org/comment/5212/","msgid":"<20200616022902.GB29596@pendragon.ideasonboard.com>","date":"2020-06-16T02:29:02","subject":"Re: [libcamera-devel] [PATCH] qcam: dng_writer: Record creation\n\ttime in the EXIF directory","submitter":{"id":2,"url":"https://patchwork.libcamera.org/api/people/2/","name":"Laurent Pinchart","email":"laurent.pinchart@ideasonboard.com"},"content":"Hi Niklas,\n\nOn Fri, Jun 12, 2020 at 12:35:15PM +0200, Niklas Söderlund wrote:\n> On 2020-06-12 11:00:21 +0300, Laurent Pinchart wrote:\n> > On Thu, Jun 11, 2020 at 10:16:05PM +0200, Niklas Söderlund wrote:\n> >> On 2020-06-11 21:46:57 +0300, Laurent Pinchart wrote:\n> >>> On Thu, Jun 11, 2020 at 08:37:25PM +0200, Niklas Söderlund wrote:\n> >>>> On 2020-06-11 05:47:52 +0300, Laurent Pinchart wrote:\n> >>>>> On Thu, Jun 11, 2020 at 03:46:14AM +0200, Niklas Söderlund wrote:\n> >>>>>> If the EXIF directory is empty due to no metadata being available tools\n> >>>>>> such as tiffinfo complains that the directory is malformed.\n> >>>>>> \n> >>>>>> TIFFFetchDirectory: Sanity check on directory count failed, this is probably not a valid IFD offset.\n> >>>>>> TIFFReadCustomDirectory: Failed to read custom directory at offset 0.\n> >>>>>> \n> >>>>>> Always record the creation time in the EXIF directory instead of adding\n> >>>>>> complexity to skip creating the EXIF directory if there is no metadata\n> >>>>>> to record. This ensures there are at least one entry in the EXIF\n> >>>>>> directory and that makes tiffinfo happy.\n> >>>>>> \n> >>>>>> Signed-off-by: Niklas Söderlund \n> >>>>>> ---\n> >>>>>> src/qcam/dng_writer.cpp | 11 +++++++++++\n> >>>>>> 1 file changed, 11 insertions(+)\n> >>>>>> \n> >>>>>> diff --git a/src/qcam/dng_writer.cpp b/src/qcam/dng_writer.cpp\n> >>>>>> index 330b169af25b7ac7..c1c3833740d96630 100644\n> >>>>>> --- a/src/qcam/dng_writer.cpp\n> >>>>>> +++ b/src/qcam/dng_writer.cpp\n> >>>>>> @@ -426,6 +426,17 @@ int DNGWriter::write(const char *filename, const Camera *camera,\n> >>>>>> \t/* Create a new IFD for the EXIF data and fill it. */\n> >>>>>> \tTIFFCreateEXIFDirectory(tif);\n> >>>>>> \n> >>>>>> +\t/* Store creation time. */\n> >>>>>> +\ttime_t rawtime;\n> >>>>>> +\tstruct tm *timeinfo;\n> >>>>>> +\tchar strTime[20];\n> >>>>>> +\n> >>>>>> +\ttime(&rawtime);\n> >>>>> \n> >>>>> Could we use the timestamp from the buffer instead ?\n> >>>> \n> >>>> We can't as the timestamp in the buffer is copied from the v4l2 buffer \n> >>>> and not guaranteed to be an offset from the epoch.\n> >>> \n> >>> So how do we fix that ? :-) It's not a prerequisite for this patch, but\n> >>> it seems to me that the problem needs to be addressed. We should also\n> >>> add a \\todo here to mention that the timestamp should come from the\n> >>> buffer.\n> >> \n> >> I'm not sure we should attempt to fix it in user-space. I think it's \n> >> more value for applications to have as accurate timestamps as possible \n> >> to compare between captured buffers then it's to be able to record in a \n> >> file on disk the exact nano second compared to the epoch it was \n> >> captured, unless we manage to get libcamera running on ATLAS at CERN... \n> >> :-)\n> >> \n> >> So if we are to fix this I think we need to solve it in the kernel \n> >> interface so that all buffer timestamps are indeed an epoch number.\n> > \n> > I agree with the first part of your statement, the kernel should give\n> > userspace high-precision timestamps, not a calendar date. I'm however\n> > puzzled by why you think we couldn't rely on those timestamps here, and\n> > convert them to the time representation we need. This involves figuring\n> > out the delta between the two, and maybe in the end we will lose so much\n> > precision there that using the high-precision timestamp wouldn't be\n> > worth it in the first place.\n> \n> I'm sorry if I was unclear. What I tried to say was that\n> \n> - The timestamps from V4L2 may not be in relation to any calendar date \n> and can only be treated as a counter where the only meaning one can \n> extract from it is comparing the stamp from two buffers from the same \n> source to figure out the delta time.\n\nThey're not related to a calendar date, but they're related to one of\nthe system clocks supported by clock_gettime(). The timestamp isn't\nmeant only to compare buffers from the same source, it's very important\nto be able to synchronize it with timestamps from other sources (motion\nsensors come into mind, there's also audio and display that need to be\ntaken into account).\n\n> - I think it's a bad idea for libcamera to insert a calendar timestamp \n> into the FrameBuffer object once it get holds of the V4L2 buffer. I \n\nThat I don't dispute, I was proposing doing so in qcam, not in the\nlibcamera core.\n\n> think we should do it the way it's done today, copy the timestamp from \n> the V4L2 buffer. I think it's more value for applications to be able \n> to calculate a high precision delta time between two buffers then to \n> have a calender timestamp with high jitter as we would have if we \n> create it in user-space.\n\nUserspace doesn't imply a large jitter. The calendar time should be\nderived from the high precision timestamp provided in the buffer.\n\n> - If we want to use a timestamp to produce a calender time here and we \n> want the source of the timestamp to come from the FrameBuffer it's my \n> opinion we need to fix the kernel API so that the high precision \n> timestamp created by the kernel for the V4L2 buffer is in addition to \n> a high precision also guaranteed to be relatable to calendar time.\n\nI don't see why that should be a job for V4L2. We are provided with all\nthe information we need, as far as I can tell.\n\n> If we can't do that I think producing a calendar time in the DNG \n> writer is just as accurate as doing so in the libcamera core and \n> storing it in an additional field in FrameBuffer.\n\nI wasn't proposing doing it in the libcamera core :-) And I agree it may\nnot be worth it anyway, given the precision we store here (1s).\n\n> >>>>>> +\ttimeinfo = localtime(&rawtime);\n> >>>>>> +\tstrftime(strTime, 20, \"%Y:%m:%d %H:%M:%S\", timeinfo);\n> >>>>> \n> >>>>> How are time zones supposed to be handled ?\n> >>>> \n> >>>> The documentation states noting about timezones so I think the best \n> >>>> option is the use the local timezone of the device running qcam.\n> >>> \n> >>> How about the OffsetTime, OffsetTimeOriginal and OffsetTimeDigitized\n> >>> tags ?\n> >> \n> >> I looked at those, but they are not present in my libtiff (4.1.0), there \n> >> are no #defines for the tags. If I use the numeric value from the EXIF \n> >> documentation I endup with warnings creating the DNG file.\n> >> \n> >> TIFFSetField: /home/chrx/test.dng: Unknown tag 36880.\n> >> TIFFSetField: /home/chrx/test.dng: Unknown tag 36881.\n> >> TIFFSetField: /home/chrx/test.dng: Unknown tag 36882.\n> >> \n> >> I'm not sure if they are recorded to the output DNG file since the \n> >> warnings are printed, but nothing shows up when I run tiffinfo on the \n> >> dng file that still gets created.\n> > \n> > libtiff has an API to write custom tags, but it's awkward to use. It\n> > involves creating a custom directory with TIFFCreateCustomDirectory(),\n> > and passing a table of tag metadata to that function. As we use\n> > TIFFCreateEXIFDirectory() here, we would have to reimplement a large\n> > part of that, which isn't practical. The fact that\n> > TIFFCreateCustomDirectory() takes a TIFFFieldArray pointer, with\n> > \n> > typedef struct _TIFFFieldArray TIFFFieldArray;\n> > \n> > and the structure _TIFFFieldArray not being defined in public headers\n> > makes me think this isn't a widely used API...\n> > \n> > Support for the timezone offsets has been added in libtiff, I suppose it\n> > will make it to the next release. Can you record a \\todo here ?\n> \n> I will add an \\todo to add OffsetTimeOriginal and OffsetTimeDigitized \n> tags here once libtiff catches up.\n> \n> >> So I can't find a good option to record the timezone information in the \n> >> DNG. So I think we have two options reccord the time in the local \n> >> timezone or GMT. My preference would be to store it in the local \n> >> timezone but I have no strong feelings, let me know what you think.\n> > \n> > The local time is best I think, that seems to be in line with what most\n> > cameras do.\n> > \n> >>>>>> +\n> >>>>>> +\tTIFFSetField(tif, EXIFTAG_DATETIMEDIGITIZED, strTime);\n> >>>>> \n> >>>>> Should we also set EXIFTAG_DATETIMEORIGINAL to the same value ?\n> >>>> \n> >>>> We should will send a v2.\n> >>>> \n> >>>>>> +\n> >>>>>> \tif (metadata.contains(controls::AnalogueGain)) {\n> >>>>>> \t\tfloat gain = metadata.get(controls::AnalogueGain);\n> >>>>>> \t\tuint16_t iso = std::min(std::max(gain * 100, 0.0f), 65535.0f);","headers":{"Return-Path":"","Received":["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 7CDC261027\n\tfor ;\n\tTue, 16 Jun 2020 04:29:25 +0200 (CEST)","from pendragon.ideasonboard.com (81-175-216-236.bb.dnainternet.fi\n\t[81.175.216.236])\n\tby perceval.ideasonboard.com (Postfix) with ESMTPSA id E3AD7F9;\n\tTue, 16 Jun 2020 04:29:24 +0200 (CEST)"],"Authentication-Results":"lancelot.ideasonboard.com; dkim=pass (1024-bit key; \n\tunprotected) header.d=ideasonboard.com\n\theader.i=@ideasonboard.com\n\theader.b=\"G5ePzONs\"; dkim-atps=neutral","DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com;\n\ts=mail; t=1592274565;\n\tbh=Ve4+hs13WgVpXKC4jxaZ/t6E//U/NLqHcXxjmSjtPEU=;\n\th=Date:From:To:Cc:Subject:References:In-Reply-To:From;\n\tb=G5ePzONsGXGFdkpqvuVzLdkHx8NKYKt4rNYETm/aEO2WJGYa7u+G+4lTSHdgxX9/x\n\tH+pIk4CJG1flmdwOO+/tIzmpAPZiWAiDcCpY0knP5RcP3EVhNhFdnKIQ4WJMpk/4wY\n\tj0hVqwsLDwSWaq0UrdrX1cZNxOOaC+Q7QfOBFe7c=","Date":"Tue, 16 Jun 2020 05:29:02 +0300","From":"Laurent Pinchart ","To":"Niklas =?utf-8?q?S=C3=B6derlund?= ","Cc":"libcamera-devel@lists.libcamera.org","Message-ID":"<20200616022902.GB29596@pendragon.ideasonboard.com>","References":"<20200611014614.1683623-1-niklas.soderlund@ragnatech.se>\n\t<20200611024752.GB13598@pendragon.ideasonboard.com>\n\t<20200611183725.GM192296@oden.dyn.berto.se>\n\t<20200611184657.GB6031@pendragon.ideasonboard.com>\n\t<20200611201605.GN192296@oden.dyn.berto.se>\n\t<20200612080021.GB5957@pendragon.ideasonboard.com>\n\t<20200612103515.GA915368@oden.dyn.berto.se>","MIME-Version":"1.0","Content-Type":"text/plain; charset=utf-8","Content-Disposition":"inline","Content-Transfer-Encoding":"8bit","In-Reply-To":"<20200612103515.GA915368@oden.dyn.berto.se>","Subject":"Re: [libcamera-devel] [PATCH] qcam: dng_writer: Record creation\n\ttime in the EXIF directory","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","X-List-Received-Date":"Tue, 16 Jun 2020 02:29:25 -0000"}}]