[{"id":12808,"web_url":"https://patchwork.libcamera.org/comment/12808/","msgid":"<9100e408-7109-e0de-118d-41448083fd83@uajain.com>","date":"2020-09-28T12:36:49","subject":"Re: [libcamera-devel] [PATCH v5 7/7] qcam: dng_writer: Record\n\tcamera model","submitter":{"id":1,"url":"https://patchwork.libcamera.org/api/people/1/","name":"Umang Jain","email":"email@uajain.com"},"content":"Hi Niklas,\n\nThank you for this work.\n\nOn 9/25/20 8:37 PM, Niklas Söderlund wrote:\n> Record the model property of the camera if available.\n>\n> Signed-off-by: Niklas Söderlund <niklas.soderlund@ragnatech.se>\n> Reviewed-by: Kieran Bingham <kieran.bingham@ideasonboard.com>\n> ---\n>   src/qcam/dng_writer.cpp | 13 ++++++++++---\n>   1 file changed, 10 insertions(+), 3 deletions(-)\n>\n> diff --git a/src/qcam/dng_writer.cpp b/src/qcam/dng_writer.cpp\n> index 030d1387118d91e9..f6d004d0407f8641 100644\n> --- a/src/qcam/dng_writer.cpp\n> +++ b/src/qcam/dng_writer.cpp\n> @@ -15,6 +15,7 @@\n>   \n>   #include <libcamera/control_ids.h>\n>   #include <libcamera/formats.h>\n> +#include <libcamera/property_ids.h>\n>   \n>   using namespace libcamera;\n>   \n> @@ -353,6 +354,8 @@ int DNGWriter::write(const char *filename, const Camera *camera,\n>   \t\t     [[maybe_unused]] const FrameBuffer *buffer,\n>   \t\t     const void *data)\n>   {\n> +\tconst ControlList &cameraProperties = camera->properties();\n> +\n>   \tconst auto it = formatInfo.find(config.pixelFormat);\n>   \tif (it == formatInfo.cend()) {\n>   \t\tstd::cerr << \"Unsupported pixel format\" << std::endl;\n> @@ -387,9 +390,13 @@ int DNGWriter::write(const char *filename, const Camera *camera,\n>   \tTIFFSetField(tif, TIFFTAG_DNGBACKWARDVERSION, version);\n>   \tTIFFSetField(tif, TIFFTAG_FILLORDER, FILLORDER_MSB2LSB);\n>   \tTIFFSetField(tif, TIFFTAG_MAKE, \"libcamera\");\n> -\t/* \\todo Report a real model string instead of id. */\n> -\tTIFFSetField(tif, TIFFTAG_MODEL, camera->id().c_str());\n> -\tTIFFSetField(tif, TIFFTAG_UNIQUECAMERAMODEL, camera->id().c_str());\n> +\n> +\tif (cameraProperties.contains(properties::Model)) {\n> +\t\tstd::string model = cameraProperties.get(properties::Model);\n> +\t\tTIFFSetField(tif, TIFFTAG_MODEL, model.c_str());\n> +\t\tTIFFSetField(tif, TIFFTAG_UNIQUECAMERAMODEL, model.c_str());\nLooking at TIFFTAG_UNIQUECAMERAMODEL description in the spec, it states \nthat:\n\n > UniqueCameraModel defines a unique, non-localized name for the camera \nmodel that created the image in the raw file. This name should include \nthe manufacturer's name to avoid conflicts ...\nE.g. mentions - \"Canon EOS 300D\" , \"Nikon DIX\" , \"Sony F828\" etc.\n\nSo, I think we can skip setting the UNIQUECAMERAMODEL for now if we \ncan't get the mfg.'s name (I have similiar issue for setting related \nEXIF tag \"setMake()\" in src/android/cameraDevice.cpp).  OR atleast \nintroduce a \\todo to follow up on the work.\n\nApart for that:\nReviewed-by: Umang Jain <email@uajain.com>\n> +\t}\n> +\n>   \tTIFFSetField(tif, TIFFTAG_SOFTWARE, \"qcam\");\n>   \tTIFFSetField(tif, TIFFTAG_ORIENTATION, ORIENTATION_TOPLEFT);\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 9E346C3B5C\n\tfor <parsemail@patchwork.libcamera.org>;\n\tMon, 28 Sep 2020 12:37:00 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id 2B28160BD7;\n\tMon, 28 Sep 2020 14:37:00 +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 6DE3F60364\n\tfor <libcamera-devel@lists.libcamera.org>;\n\tMon, 28 Sep 2020 14:36:58 +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=\"jFs87htP\"; dkim-atps=neutral","DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/simple; d=uajain.com; s=mail;\n\tt=1601296617; bh=7zHFKjGT3vR+3dJiMxtkOQvGjPQQgVo8cN/RgiLpqWg=;\n\th=Subject:To:References:From:In-Reply-To;\n\tb=jFs87htPPJIbKWmhQrr9NwFDveaoel50D3Cun6NX+hNhVJH3+h/AhRkht25vr+3EL\n\tGlC3+TlX5bnGpwMyuMxnaliudwPTDzm3vMY3VyyGk7UorRjgSpK1JJdjcaBbw6hagD\n\teaWobDh7fmh2b8IGPPhvnPp9kRqG7Df8OKgACf9BG12z7ZhsaHy2viEHX+mnXFXwAG\n\tk2bCJBlbv6VpA/i0MEbpjtHJqClu76GJPlQZNT2a7CuuTaP4bFSYvHkWA4mJ4mTkBj\n\tDLevf2qKO/YOJf7xgRovAtkExFi9+soRKgiacfM3DisHnfVfRQGfDA4bJhHSbV2qGV\n\thgbxmziT70Eqg==","To":"libcamera-devel@lists.libcamera.org","References":"<20200925150743.1822226-1-niklas.soderlund@ragnatech.se>\n\t<20200925150743.1822226-8-niklas.soderlund@ragnatech.se>","From":"Umang Jain <email@uajain.com>","Message-ID":"<9100e408-7109-e0de-118d-41448083fd83@uajain.com>","Date":"Mon, 28 Sep 2020 18:06:49 +0530","Mime-Version":"1.0","In-Reply-To":"<20200925150743.1822226-8-niklas.soderlund@ragnatech.se>","Content-Language":"en-US","Subject":"Re: [libcamera-devel] [PATCH v5 7/7] qcam: dng_writer: Record\n\tcamera model","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":"base64","Content-Type":"text/plain; charset=\"utf-8\"; Format=\"flowed\"","Errors-To":"libcamera-devel-bounces@lists.libcamera.org","Sender":"\"libcamera-devel\" <libcamera-devel-bounces@lists.libcamera.org>"}},{"id":12817,"web_url":"https://patchwork.libcamera.org/comment/12817/","msgid":"<20200928173016.GJ23539@pendragon.ideasonboard.com>","date":"2020-09-28T17:30:16","subject":"Re: [libcamera-devel] [PATCH v5 7/7] qcam: dng_writer: Record\n\tcamera model","submitter":{"id":2,"url":"https://patchwork.libcamera.org/api/people/2/","name":"Laurent Pinchart","email":"laurent.pinchart@ideasonboard.com"},"content":"Hi Niklas and Umang,\n\nOn Mon, Sep 28, 2020 at 06:06:49PM +0530, Umang Jain wrote:\n> On 9/25/20 8:37 PM, Niklas Söderlund wrote:\n> > Record the model property of the camera if available.\n> >\n> > Signed-off-by: Niklas Söderlund <niklas.soderlund@ragnatech.se>\n> > Reviewed-by: Kieran Bingham <kieran.bingham@ideasonboard.com>\n> > ---\n> >   src/qcam/dng_writer.cpp | 13 ++++++++++---\n> >   1 file changed, 10 insertions(+), 3 deletions(-)\n> >\n> > diff --git a/src/qcam/dng_writer.cpp b/src/qcam/dng_writer.cpp\n> > index 030d1387118d91e9..f6d004d0407f8641 100644\n> > --- a/src/qcam/dng_writer.cpp\n> > +++ b/src/qcam/dng_writer.cpp\n> > @@ -15,6 +15,7 @@\n> >   \n> >   #include <libcamera/control_ids.h>\n> >   #include <libcamera/formats.h>\n> > +#include <libcamera/property_ids.h>\n> >   \n> >   using namespace libcamera;\n> >   \n> > @@ -353,6 +354,8 @@ int DNGWriter::write(const char *filename, const Camera *camera,\n> >   \t\t     [[maybe_unused]] const FrameBuffer *buffer,\n> >   \t\t     const void *data)\n> >   {\n> > +\tconst ControlList &cameraProperties = camera->properties();\n> > +\n> >   \tconst auto it = formatInfo.find(config.pixelFormat);\n> >   \tif (it == formatInfo.cend()) {\n> >   \t\tstd::cerr << \"Unsupported pixel format\" << std::endl;\n> > @@ -387,9 +390,13 @@ int DNGWriter::write(const char *filename, const Camera *camera,\n> >   \tTIFFSetField(tif, TIFFTAG_DNGBACKWARDVERSION, version);\n> >   \tTIFFSetField(tif, TIFFTAG_FILLORDER, FILLORDER_MSB2LSB);\n> >   \tTIFFSetField(tif, TIFFTAG_MAKE, \"libcamera\");\n> > -\t/* \\todo Report a real model string instead of id. */\n> > -\tTIFFSetField(tif, TIFFTAG_MODEL, camera->id().c_str());\n> > -\tTIFFSetField(tif, TIFFTAG_UNIQUECAMERAMODEL, camera->id().c_str());\n> > +\n> > +\tif (cameraProperties.contains(properties::Model)) {\n> > +\t\tstd::string model = cameraProperties.get(properties::Model);\n> > +\t\tTIFFSetField(tif, TIFFTAG_MODEL, model.c_str());\n> > +\t\tTIFFSetField(tif, TIFFTAG_UNIQUECAMERAMODEL, model.c_str());\n>\n> Looking at TIFFTAG_UNIQUECAMERAMODEL description in the spec, it states \n> that:\n> \n> > UniqueCameraModel defines a unique, non-localized name for the camera \n> > model that created the image in the raw file. This name should include \n> > the manufacturer's name to avoid conflicts ...\n> > E.g. mentions - \"Canon EOS 300D\" , \"Nikon DIX\" , \"Sony F828\" etc.\n> \n> So, I think we can skip setting the UNIQUECAMERAMODEL for now if we \n> can't get the mfg.'s name (I have similiar issue for setting related \n> EXIF tag \"setMake()\" in src/android/cameraDevice.cpp).  OR atleast \n> introduce a \\todo to follow up on the work.\n\nI agree. Following the DNG spec exactly is difficult here, as qcam\ndoesn't have access to anything ressembling a product name in the first\nplace.\n\nUniqueCameraModel is supposed to combine the vendor name and the product\nname. as the Make tag is set to \"libcamera\", it would make sense to use\nit here too, as the vendor name. We could also use the platform name,\npossibly in the form of a pipeline handler name or something similar\n(that's a candidate for future improvements, not a blocker for this\nseries). In any case, I would add a \"libcamera\" prefix to the\nUniqueCameraModel, and add \\todo comment. With that addressed,\n\nReviewed-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>\n\n> Apart for that:\n> Reviewed-by: Umang Jain <email@uajain.com>\n>\n> > +\t}\n> > +\n> >   \tTIFFSetField(tif, TIFFTAG_SOFTWARE, \"qcam\");\n> >   \tTIFFSetField(tif, TIFFTAG_ORIENTATION, ORIENTATION_TOPLEFT);\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 18053C3B5B\n\tfor <parsemail@patchwork.libcamera.org>;\n\tMon, 28 Sep 2020 17:30:53 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id 9A67660BF7;\n\tMon, 28 Sep 2020 19:30:52 +0200 (CEST)","from perceval.ideasonboard.com (perceval.ideasonboard.com\n\t[213.167.242.64])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTPS id 4477C60366\n\tfor <libcamera-devel@lists.libcamera.org>;\n\tMon, 28 Sep 2020 19:30:51 +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 AC53654E;\n\tMon, 28 Sep 2020 19:30:50 +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=\"fFJ3aWBE\"; dkim-atps=neutral","DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com;\n\ts=mail; t=1601314250;\n\tbh=YI9c2JGqMvbWkJ1Rt9XrE6xkpEnK9XHr660j/dBVFyo=;\n\th=Date:From:To:Cc:Subject:References:In-Reply-To:From;\n\tb=fFJ3aWBEE3jol6QUUXVVCoHg3p2gudFJwP5nLyObJi+ptZy+Hg1ykaBIyeR5mMuZi\n\tbjWeICy6MjwHYlFYVj7EQSv4ndTuYr8DePwSdLWF6V9USFN8poEcpcxz2P+P+SWmsT\n\tELX/pz3Jfr6/1XoyOTBW/CAb0BihEkr/UqzwzgNQ=","Date":"Mon, 28 Sep 2020 20:30:16 +0300","From":"Laurent Pinchart <laurent.pinchart@ideasonboard.com>","To":"Umang Jain <email@uajain.com>, Niklas =?utf-8?q?S=C3=B6derlund?=\n\t<niklas.soderlund@ragnatech.se>","Message-ID":"<20200928173016.GJ23539@pendragon.ideasonboard.com>","References":"<20200925150743.1822226-1-niklas.soderlund@ragnatech.se>\n\t<20200925150743.1822226-8-niklas.soderlund@ragnatech.se>\n\t<9100e408-7109-e0de-118d-41448083fd83@uajain.com>","MIME-Version":"1.0","Content-Disposition":"inline","In-Reply-To":"<9100e408-7109-e0de-118d-41448083fd83@uajain.com>","Subject":"Re: [libcamera-devel] [PATCH v5 7/7] qcam: dng_writer: Record\n\tcamera model","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>"}}]