[{"id":14329,"web_url":"https://patchwork.libcamera.org/comment/14329/","msgid":"","date":"2020-12-25T22:06:05","subject":"Re: [libcamera-devel] [PATCH 0/2] Improve BayerFormat class","submitter":{"id":2,"url":"https://patchwork.libcamera.org/api/people/2/","name":"Laurent Pinchart","email":"laurent.pinchart@ideasonboard.com"},"content":"Hi Sebastian,\n\nOn Wed, Dec 23, 2020 at 01:10:53PM +0100, Sebastian Fricke wrote:\n> This patch series adds unit-tests and the `fromV4L2PixelFormat` method\n> to the BayerFormat class.\n> \n> I also wanted to discuss one alternative that I played around with.\n> We could maybe drop one of the two mapping tables and use the following\n> logic to get the map key from a mapped value.\n> \n> ```\n> #include \n> ...\n> BayerFormat BayerFormat::fromV4L2PixelFormat(V4L2PixelFormat v4l2Format) const\n> {\n> auto it = std::find_if(\n> bayerToV4l2.begin(),\n> bayerToV4l2.end(),\n> [v4l2Format](const auto& i) { return i->second == v4l2Format; }\n> );\n> if (it != bayerToV4l2.end())\n> return it->first;\n> \n> return BayerFormat();\n> }\n> ```\n\nThis would certainly work. The question is whether we can to optimize\nthe memory consumption (dropping the second lookup table), or the\nexecution time (a lookup operation for a map has a O(log(n)) complexity,\ncompared to O(n) for the code above).\n\n> Sebastian Fricke (2):\n> libcamera: Add the fromV4L2PixelFormat method\n> test: Add unit tests for the BayerFormat class\n> \n> include/libcamera/internal/bayer_format.h | 1 +\n> src/libcamera/bayer_format.cpp | 14 ++\n> test/bayer_format.cpp | 154 ++++++++++++++++++++++\n> test/meson.build | 1 +\n> 4 files changed, 170 insertions(+)\n> create mode 100644 test/bayer_format.cpp\n>","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 A4317C0F1A\n\tfor ;\n\tFri, 25 Dec 2020 22:06:22 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id 235F2615AC;\n\tFri, 25 Dec 2020 23:06:22 +0100 (CET)","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 616ED60525\n\tfor ;\n\tFri, 25 Dec 2020 23:06:20 +0100 (CET)","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 2A5EF18B7;\n\tFri, 25 Dec 2020 23:06:17 +0100 (CET)"],"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=\"wmn1p7nf\"; dkim-atps=neutral","DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com;\n\ts=mail; t=1608933979;\n\tbh=3qcjEG8YQ9zrKF8pWD0OFNPrrXPI9qfSOOKYZoGTj7A=;\n\th=Date:From:To:Cc:Subject:References:In-Reply-To:From;\n\tb=wmn1p7nfoD5dI4ey9caUaOgCPeDr4rH+EXn2wqWsXNsYlcJg4SgCIhGmhWkmFUuF4\n\tCu53s+uX92O7aDxyWwg5IX6iKxZSJ7ikxt3F7TVYQ6fJFexugnDLGPuaDsqRHFJw4I\n\tdL6WvR4fwGmM0WdqsYWR/e34rbW0CCF6/Y5e81eA=","Date":"Sat, 26 Dec 2020 00:06:05 +0200","From":"Laurent Pinchart ","To":"Sebastian Fricke ","Message-ID":"","References":"<20201223121055.14178-1-sebastian.fricke.linux@gmail.com>","MIME-Version":"1.0","Content-Disposition":"inline","In-Reply-To":"<20201223121055.14178-1-sebastian.fricke.linux@gmail.com>","Subject":"Re: [libcamera-devel] [PATCH 0/2] Improve BayerFormat class","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\" "}},{"id":14367,"web_url":"https://patchwork.libcamera.org/comment/14367/","msgid":"<20201228070528.abg2wm2nrupoze33@basti-TUXEDO-Book-XA1510>","date":"2020-12-28T07:05:28","subject":"Re: [libcamera-devel] [PATCH 0/2] Improve BayerFormat class","submitter":{"id":73,"url":"https://patchwork.libcamera.org/api/people/73/","name":"Sebastian Fricke","email":"sebastian.fricke.linux@gmail.com"},"content":"On 26.12.2020 00:06, Laurent Pinchart wrote:\n>Hi Sebastian,\n\nHey Laurent,\n\n>\n>On Wed, Dec 23, 2020 at 01:10:53PM +0100, Sebastian Fricke wrote:\n>> This patch series adds unit-tests and the `fromV4L2PixelFormat` method\n>> to the BayerFormat class.\n>>\n>> I also wanted to discuss one alternative that I played around with.\n>> We could maybe drop one of the two mapping tables and use the following\n>> logic to get the map key from a mapped value.\n>>\n>> ```\n>> #include \n>> ...\n>> BayerFormat BayerFormat::fromV4L2PixelFormat(V4L2PixelFormat v4l2Format) const\n>> {\n>> auto it = std::find_if(\n>> bayerToV4l2.begin(),\n>> bayerToV4l2.end(),\n>> [v4l2Format](const auto& i) { return i->second == v4l2Format; }\n>> );\n>> if (it != bayerToV4l2.end())\n>> return it->first;\n>>\n>> return BayerFormat();\n>> }\n>> ```\n>\n>This would certainly work. The question is whether we can to optimize\n>the memory consumption (dropping the second lookup table), or the\n>execution time (a lookup operation for a map has a O(log(n)) complexity,\n>compared to O(n) for the code above).\n\nWhich probably boils down to the question: 'How often do we expect to\nuse this function?'.\nCan you recall from the current code bases if there are cases, where the\nconstructor is used frequently in a row? I would suspect that the\nconstructor/static function should be used quite seldomly.\n\n>\n>> Sebastian Fricke (2):\n>> libcamera: Add the fromV4L2PixelFormat method\n>> test: Add unit tests for the BayerFormat class\n>>\n>> include/libcamera/internal/bayer_format.h | 1 +\n>> src/libcamera/bayer_format.cpp | 14 ++\n>> test/bayer_format.cpp | 154 ++++++++++++++++++++++\n>> test/meson.build | 1 +\n>> 4 files changed, 170 insertions(+)\n>> create mode 100644 test/bayer_format.cpp\n>>\n>\n>-- \n>Regards,\n>\n>Laurent Pinchart","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 5F9ECC0F1B\n\tfor ;\n\tMon, 28 Dec 2020 07:05:32 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id E8EF660525;\n\tMon, 28 Dec 2020 08:05:31 +0100 (CET)","from mail-ej1-x62f.google.com (mail-ej1-x62f.google.com\n\t[IPv6:2a00:1450:4864:20::62f])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTPS id A9E6A6031B\n\tfor ;\n\tMon, 28 Dec 2020 08:05:29 +0100 (CET)","by mail-ej1-x62f.google.com with SMTP id jx16so12990880ejb.10\n\tfor ;\n\tSun, 27 Dec 2020 23:05:29 -0800 (PST)","from localhost ([2001:16b8:577a:e001:7981:6f77:43f3:9d36])\n\tby smtp.gmail.com with ESMTPSA id\n\tf18sm35775929edt.60.2020.12.27.23.05.28\n\t(version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256);\n\tSun, 27 Dec 2020 23:05:28 -0800 (PST)"],"Authentication-Results":"lancelot.ideasonboard.com;\n\tdkim=fail reason=\"signature verification failed\" (2048-bit key;\n\tunprotected) header.d=gmail.com header.i=@gmail.com\n\theader.b=\"UcSjxBj7\"; dkim-atps=neutral","DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/relaxed; d=gmail.com; s=20161025;\n\th=date:from:to:cc:subject:message-id:reply-to:references:mime-version\n\t:content-disposition:in-reply-to;\n\tbh=f0CFozTxcT3ca0965kQvNKdQhjhDxflJXuP+nCakaOA=;\n\tb=UcSjxBj7pcbrVFWluoMsaOVjyQSb+0z8ShUsuIFXGLnBzVah8MeLzphjO2cUaoMzfk\n\tDyqr5KHj6XJyogGzzmo8bkj5xnYxls7NTuSouopi00570DgPLLE3EGDRIIcHxTXxuwWm\n\thcgVcl/9X0V18fis0iNMsXs+5+E2BPeqeqt8dgXqkmZhleU5L9bHOMPwcyoC1ff5qOWF\n\t9GByRcmtfFRhlEu+OpilzPJorMYDLwiZZG+Wd+e20HzjbOhIrk2Xk/CHu+ztg7/idId1\n\tVop0gAxoH3WU9HT/4Etd12IoE0Skl7YZxsycCG7qVrpFOI9CePdmQpGNOHgIorQDPqfR\n\t6ACg==","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:reply-to\n\t:references:mime-version:content-disposition:in-reply-to;\n\tbh=f0CFozTxcT3ca0965kQvNKdQhjhDxflJXuP+nCakaOA=;\n\tb=nbjhS70Ky5j7z/n+PzHa2hwxpHWx66qXbTJvrfD9C61PQJnz5bJDH7VRQNuEDDUd6H\n\tWo4yfLKy9GHcyGDIfPFj0Xnv4Uu4ctUntQXwJScwhfKrAfY8JNJeBI55OSh+g6Dt8bb+\n\tpdWwnmA946QuRJ6X8dZAvRag32AobOmfEzhFFox8BEr9/9KMpGLuv85WRl4fT9GxvClH\n\trlP++KIc5kZLqy4BSmJ2T+Q5Kqf88gdS161rmMHFFM+Qy4kQgBTKEIJa6X58JJLWi2/m\n\tJmEgyvtj2K3SI5i6iT5VsRvgGV2mPbNJ1sNQ9PVjo86m7SbVr0AGZx/KQ/FKWLCTGYsT\n\t4c+Q==","X-Gm-Message-State":"AOAM531ZbQTksNNMbN58ZkFffsJCb/CiWihCrH1eLFsLDBb+hRUQj9wG\n\tG8h8ysUh0YJsTnPcOkJsBWsQIGVDEkI=","X-Google-Smtp-Source":"ABdhPJw+jOSxPfDUBG50oVuWjZIEah/Ds+BjHLtra4+ki3IQzfSHdkjfJgXZMZsODUVDD7C5Hi3oKQ==","X-Received":"by 2002:a17:906:2358:: with SMTP id\n\tm24mr39715496eja.198.1609139129335; \n\tSun, 27 Dec 2020 23:05:29 -0800 (PST)","Date":"Mon, 28 Dec 2020 08:05:28 +0100","From":"Sebastian Fricke ","To":"Laurent Pinchart ","Message-ID":"<20201228070528.abg2wm2nrupoze33@basti-TUXEDO-Book-XA1510>","References":"<20201223121055.14178-1-sebastian.fricke.linux@gmail.com>\n\t","MIME-Version":"1.0","Content-Disposition":"inline","In-Reply-To":"","Subject":"Re: [libcamera-devel] [PATCH 0/2] Improve BayerFormat class","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","Reply-To":"Sebastian Fricke ","Cc":"libcamera-devel@lists.libcamera.org","Content-Transfer-Encoding":"7bit","Content-Type":"text/plain; charset=\"us-ascii\"; Format=\"flowed\"","Errors-To":"libcamera-devel-bounces@lists.libcamera.org","Sender":"\"libcamera-devel\" "}},{"id":14380,"web_url":"https://patchwork.libcamera.org/comment/14380/","msgid":"","date":"2020-12-28T10:55:27","subject":"Re: [libcamera-devel] [PATCH 0/2] Improve BayerFormat class","submitter":{"id":2,"url":"https://patchwork.libcamera.org/api/people/2/","name":"Laurent Pinchart","email":"laurent.pinchart@ideasonboard.com"},"content":"Hi Sebastian,\n\nOn Mon, Dec 28, 2020 at 08:05:28AM +0100, Sebastian Fricke wrote:\n> On 26.12.2020 00:06, Laurent Pinchart wrote:\n> >On Wed, Dec 23, 2020 at 01:10:53PM +0100, Sebastian Fricke wrote:\n> >> This patch series adds unit-tests and the `fromV4L2PixelFormat` method\n> >> to the BayerFormat class.\n> >>\n> >> I also wanted to discuss one alternative that I played around with.\n> >> We could maybe drop one of the two mapping tables and use the following\n> >> logic to get the map key from a mapped value.\n> >>\n> >> ```\n> >> #include \n> >> ...\n> >> BayerFormat BayerFormat::fromV4L2PixelFormat(V4L2PixelFormat v4l2Format) const\n> >> {\n> >> auto it = std::find_if(\n> >> bayerToV4l2.begin(),\n> >> bayerToV4l2.end(),\n> >> [v4l2Format](const auto& i) { return i->second == v4l2Format; }\n> >> );\n> >> if (it != bayerToV4l2.end())\n> >> return it->first;\n> >>\n> >> return BayerFormat();\n> >> }\n> >> ```\n> >\n> > This would certainly work. The question is whether we can to optimize\n> > the memory consumption (dropping the second lookup table), or the\n> > execution time (a lookup operation for a map has a O(log(n)) complexity,\n> > compared to O(n) for the code above).\n> \n> Which probably boils down to the question: 'How often do we expect to\n> use this function?'.\n> Can you recall from the current code bases if there are cases, where the\n> constructor is used frequently in a row? I would suspect that the\n> constructor/static function should be used quite seldomly.\n\nI don't expect this to be used in any hot path.\n\n> >> Sebastian Fricke (2):\n> >> libcamera: Add the fromV4L2PixelFormat method\n> >> test: Add unit tests for the BayerFormat class\n> >>\n> >> include/libcamera/internal/bayer_format.h | 1 +\n> >> src/libcamera/bayer_format.cpp | 14 ++\n> >> test/bayer_format.cpp | 154 ++++++++++++++++++++++\n> >> test/meson.build | 1 +\n> >> 4 files changed, 170 insertions(+)\n> >> create mode 100644 test/bayer_format.cpp","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 34B7EC0F1A\n\tfor ;\n\tMon, 28 Dec 2020 10:55:41 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id A3C03615B2;\n\tMon, 28 Dec 2020 11:55:40 +0100 (CET)","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 A25FC60525\n\tfor ;\n\tMon, 28 Dec 2020 11:55:38 +0100 (CET)","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 0EDE525B;\n\tMon, 28 Dec 2020 11:55:38 +0100 (CET)"],"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=\"Gmckdhlc\"; dkim-atps=neutral","DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com;\n\ts=mail; t=1609152938;\n\tbh=O82hPviO6pJMQajU+HhHlZ8BvXnsnKfcK/AFQAlk7II=;\n\th=Date:From:To:Cc:Subject:References:In-Reply-To:From;\n\tb=GmckdhlcXsPE7R8zMjJJ+uObjO33DCtZuDaqsr3Rf/PEGZmcTfJet7CClnnEPqRL0\n\t+tRO54ED2RMrsAZZ/n9Eff3Dn1C76z/2wt/NsgzJPxApryK3jMdyirDDsT0dma9xKO\n\tpgfRugIj3/rl9Iu61YqMPy9VCiW3hPYPMxFh/2hQ=","Date":"Mon, 28 Dec 2020 12:55:27 +0200","From":"Laurent Pinchart ","To":"Sebastian Fricke ","Message-ID":"","References":"<20201223121055.14178-1-sebastian.fricke.linux@gmail.com>\n\t\n\t<20201228070528.abg2wm2nrupoze33@basti-TUXEDO-Book-XA1510>","MIME-Version":"1.0","Content-Disposition":"inline","In-Reply-To":"<20201228070528.abg2wm2nrupoze33@basti-TUXEDO-Book-XA1510>","Subject":"Re: [libcamera-devel] [PATCH 0/2] Improve BayerFormat class","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\" "}}]