[{"id":26535,"web_url":"https://patchwork.libcamera.org/comment/26535/","msgid":"","date":"2023-03-02T12:24:17","subject":"Re: [libcamera-devel] [PATCH 5/5] libcamera: imx8-isi: Remove\n\tmbusCode from formatsMap_","submitter":{"id":17,"url":"https://patchwork.libcamera.org/api/people/17/","name":"Paul Elder","email":"paul.elder@ideasonboard.com"},"content":"On Sun, Jan 29, 2023 at 02:58:30PM +0100, Jacopo Mondi via libcamera-devel wrote:\n> Now that the media bus code selection procedure does not depend\n> on the ISICameraConfiguration::formatsMap_ remove the association\n> between PixelFormat supported by the ISI and the media bus code produced\n> by the sensor.\n> \n> Signed-off-by: Jacopo Mondi \n\nLooks good to me.\n\nReviewed-by: Paul Elder \n\n> ---\n> src/libcamera/pipeline/imx8-isi/imx8-isi.cpp | 159 ++++---------------\n> 1 file changed, 29 insertions(+), 130 deletions(-)\n> \n> diff --git a/src/libcamera/pipeline/imx8-isi/imx8-isi.cpp b/src/libcamera/pipeline/imx8-isi/imx8-isi.cpp\n> index 0e1e87c7a2aa..f754f5d77f90 100644\n> --- a/src/libcamera/pipeline/imx8-isi/imx8-isi.cpp\n> +++ b/src/libcamera/pipeline/imx8-isi/imx8-isi.cpp\n> @@ -76,18 +76,6 @@ public:\n> class ISICameraConfiguration : public CameraConfiguration\n> {\n> public:\n> -\t/*\n> -\t * formatsMap_ records the association between an output pixel format\n> -\t * and the combination of V4L2 pixel format and media bus codes that have\n> -\t * to be applied to the pipeline.\n> -\t */\n> -\tstruct PipeFormat {\n> -\t\tunsigned int isiCode;\n> -\t\tunsigned int sensorCode;\n> -\t};\n> -\n> -\tusing FormatMap = std::map;\n> -\n> \tISICameraConfiguration(ISICameraData *data)\n> \t\t: data_(data)\n> \t{\n> @@ -95,7 +83,7 @@ public:\n> \n> \tStatus validate() override;\n> \n> -\tstatic const FormatMap formatsMap_;\n> +\tstatic const std::map formatsMap_;\n> \n> \tV4L2SubdeviceFormat sensorFormat_;\n> \n> @@ -333,121 +321,33 @@ unsigned int ISICameraData::getMediaBusFormat(PixelFormat *pixelFormat) const\n> * Camera Configuration\n> */\n> \n> -/**\n> - * \\todo Do not associate the sensor format to non-RAW pixelformats, as\n> - * the ISI can do colorspace conversion.\n> +/*\n> + * ISICameraConfiguration::formatsMap_ records the association between an output\n> + * pixel format and the ISI source pixel format to be applied to the pipeline.\n> */\n> -const ISICameraConfiguration::FormatMap ISICameraConfiguration::formatsMap_ = {\n> -\t{\n> -\t\tformats::YUYV,\n> -\t\t{ MEDIA_BUS_FMT_YUV8_1X24,\n> -\t\t MEDIA_BUS_FMT_UYVY8_1X16 },\n> -\t},\n> -\t{\n> -\t\tformats::AVUY8888,\n> -\t\t{ MEDIA_BUS_FMT_YUV8_1X24,\n> -\t\t MEDIA_BUS_FMT_UYVY8_1X16 },\n> -\t},\n> -\t{\n> -\t\tformats::NV12,\n> -\t\t{ MEDIA_BUS_FMT_YUV8_1X24,\n> -\t\t MEDIA_BUS_FMT_UYVY8_1X16 },\n> -\t},\n> -\t{\n> -\t\tformats::NV16,\n> -\t\t{ MEDIA_BUS_FMT_YUV8_1X24,\n> -\t\t MEDIA_BUS_FMT_UYVY8_1X16 },\n> -\t},\n> -\t{\n> -\t\tformats::YUV444,\n> -\t\t{ MEDIA_BUS_FMT_YUV8_1X24,\n> -\t\t MEDIA_BUS_FMT_UYVY8_1X16 },\n> -\t},\n> -\t{\n> -\t\tformats::RGB565,\n> -\t\t{ MEDIA_BUS_FMT_RGB888_1X24,\n> -\t\t MEDIA_BUS_FMT_RGB565_1X16 },\n> -\t},\n> -\t{\n> -\t\tformats::BGR888,\n> -\t\t{ MEDIA_BUS_FMT_RGB888_1X24,\n> -\t\t MEDIA_BUS_FMT_RGB565_1X16 },\n> -\t},\n> -\t{\n> -\t\tformats::RGB888,\n> -\t\t{ MEDIA_BUS_FMT_RGB888_1X24,\n> -\t\t MEDIA_BUS_FMT_RGB565_1X16 },\n> -\t},\n> -\t{\n> -\t\tformats::XRGB8888,\n> -\t\t{ MEDIA_BUS_FMT_RGB888_1X24,\n> -\t\t MEDIA_BUS_FMT_RGB565_1X16 },\n> -\t},\n> -\t{\n> -\t\tformats::ABGR8888,\n> -\t\t{ MEDIA_BUS_FMT_RGB888_1X24,\n> -\t\t MEDIA_BUS_FMT_RGB565_1X16 },\n> -\t},\n> -\t{\n> -\t\tformats::SBGGR8,\n> -\t\t{ MEDIA_BUS_FMT_SBGGR8_1X8,\n> -\t\t MEDIA_BUS_FMT_SBGGR8_1X8 },\n> -\t},\n> -\t{\n> -\t\tformats::SGBRG8,\n> -\t\t{ MEDIA_BUS_FMT_SGBRG8_1X8,\n> -\t\t MEDIA_BUS_FMT_SGBRG8_1X8 },\n> -\t},\n> -\t{\n> -\t\tformats::SGRBG8,\n> -\t\t{ MEDIA_BUS_FMT_SGRBG8_1X8,\n> -\t\t MEDIA_BUS_FMT_SGRBG8_1X8 },\n> -\t},\n> -\t{\n> -\t\tformats::SRGGB8,\n> -\t\t{ MEDIA_BUS_FMT_SRGGB8_1X8,\n> -\t\t MEDIA_BUS_FMT_SRGGB8_1X8 },\n> -\t},\n> -\t{\n> -\t\tformats::SBGGR10,\n> -\t\t{ MEDIA_BUS_FMT_SBGGR10_1X10,\n> -\t\t MEDIA_BUS_FMT_SBGGR10_1X10 },\n> -\t},\n> -\t{\n> -\t\tformats::SGBRG10,\n> -\t\t{ MEDIA_BUS_FMT_SGBRG10_1X10,\n> -\t\t MEDIA_BUS_FMT_SGBRG10_1X10 },\n> -\t},\n> -\t{\n> -\t\tformats::SGRBG10,\n> -\t\t{ MEDIA_BUS_FMT_SGRBG10_1X10,\n> -\t\t MEDIA_BUS_FMT_SGRBG10_1X10 },\n> -\t},\n> -\t{\n> -\t\tformats::SRGGB10,\n> -\t\t{ MEDIA_BUS_FMT_SRGGB10_1X10,\n> -\t\t MEDIA_BUS_FMT_SRGGB10_1X10 },\n> -\t},\n> -\t{\n> -\t\tformats::SBGGR12,\n> -\t\t{ MEDIA_BUS_FMT_SBGGR12_1X12,\n> -\t\t MEDIA_BUS_FMT_SBGGR12_1X12 },\n> -\t},\n> -\t{\n> -\t\tformats::SGBRG12,\n> -\t\t{ MEDIA_BUS_FMT_SGBRG12_1X12,\n> -\t\t MEDIA_BUS_FMT_SGBRG12_1X12 },\n> -\t},\n> -\t{\n> -\t\tformats::SGRBG12,\n> -\t\t{ MEDIA_BUS_FMT_SGRBG12_1X12,\n> -\t\t MEDIA_BUS_FMT_SGRBG12_1X12 },\n> -\t},\n> -\t{\n> -\t\tformats::SRGGB12,\n> -\t\t{ MEDIA_BUS_FMT_SRGGB12_1X12,\n> -\t\t MEDIA_BUS_FMT_SRGGB12_1X12 },\n> -\t},\n> +const std::map ISICameraConfiguration::formatsMap_ = {\n> +\t{ formats::YUYV, MEDIA_BUS_FMT_UYVY8_1X16 },\n> +\t{ formats::AVUY8888, MEDIA_BUS_FMT_UYVY8_1X16 },\n> +\t{ formats::NV12, MEDIA_BUS_FMT_UYVY8_1X16 },\n> +\t{ formats::NV16, MEDIA_BUS_FMT_UYVY8_1X16 },\n> +\t{ formats::YUV444, MEDIA_BUS_FMT_UYVY8_1X16 },\n> +\t{ formats::RGB565, MEDIA_BUS_FMT_RGB565_1X16 },\n> +\t{ formats::BGR888, MEDIA_BUS_FMT_RGB565_1X16 },\n> +\t{ formats::RGB888, MEDIA_BUS_FMT_RGB565_1X16 },\n> +\t{ formats::XRGB8888, MEDIA_BUS_FMT_RGB565_1X16 },\n> +\t{ formats::ABGR8888, MEDIA_BUS_FMT_RGB565_1X16 },\n> +\t{ formats::SBGGR8, MEDIA_BUS_FMT_SBGGR8_1X8 },\n> +\t{ formats::SGBRG8, MEDIA_BUS_FMT_SGBRG8_1X8 },\n> +\t{ formats::SGRBG8, MEDIA_BUS_FMT_SGRBG8_1X8 },\n> +\t{ formats::SRGGB8, MEDIA_BUS_FMT_SRGGB8_1X8 },\n> +\t{ formats::SBGGR10, MEDIA_BUS_FMT_SBGGR10_1X10 },\n> +\t{ formats::SGBRG10, MEDIA_BUS_FMT_SGBRG10_1X10 },\n> +\t{ formats::SGRBG10, MEDIA_BUS_FMT_SGRBG10_1X10 },\n> +\t{ formats::SRGGB10, MEDIA_BUS_FMT_SRGGB10_1X10 },\n> +\t{ formats::SBGGR12, MEDIA_BUS_FMT_SBGGR12_1X12 },\n> +\t{ formats::SGBRG12, MEDIA_BUS_FMT_SGBRG12_1X12 },\n> +\t{ formats::SGRBG12, MEDIA_BUS_FMT_SGRBG12_1X12 },\n> +\t{ formats::SRGGB12, MEDIA_BUS_FMT_SRGGB12_1X12 },\n> };\n> \n> /*\n> @@ -991,11 +891,10 @@ int PipelineHandlerISI::configure(Camera *camera, CameraConfiguration *c)\n> \t\t * size is taken from the sink's COMPOSE (or source's CROP,\n> \t\t * if any) rectangles.\n> \t\t */\n> -\t\tconst ISICameraConfiguration::PipeFormat &pipeFormat =\n> -\t\t\tISICameraConfiguration::formatsMap_.at(config.pixelFormat);\n> +\t\tunsigned int isiCode = ISICameraConfiguration::formatsMap_.at(config.pixelFormat);\n> \n> \t\tV4L2SubdeviceFormat isiFormat{};\n> -\t\tisiFormat.mbus_code = pipeFormat.isiCode;\n> +\t\tisiFormat.mbus_code = isiCode;\n> \t\tisiFormat.size = config.size;\n> \n> \t\tret = pipe->isi->setFormat(1, &isiFormat);\n> -- \n> 2.39.0\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 5009DBF415\n\tfor ;\n\tThu, 2 Mar 2023 12:24:27 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id A07D16269C;\n\tThu, 2 Mar 2023 13:24:26 +0100 (CET)","from perceval.ideasonboard.com (perceval.ideasonboard.com\n\t[213.167.242.64])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTPS id 554BA6267E\n\tfor ;\n\tThu, 2 Mar 2023 13:24:25 +0100 (CET)","from pyrite.rasen.tech (h175-177-042-159.catv02.itscom.jp\n\t[175.177.42.159])\n\tby perceval.ideasonboard.com (Postfix) with ESMTPSA id B86C956A;\n\tThu, 2 Mar 2023 13:24:23 +0100 (CET)"],"DKIM-Signature":["v=1; a=rsa-sha256; c=relaxed/simple; d=libcamera.org;\n\ts=mail; t=1677759866;\n\tbh=9ESuAsQ84+XrOKjaFg1fFraTXXzOzSS/AEcHN4wINls=;\n\th=Date:To:References:In-Reply-To:Subject:List-Id:List-Unsubscribe:\n\tList-Archive:List-Post:List-Help:List-Subscribe:From:Reply-To:Cc:\n\tFrom;\n\tb=elFUFUajg9idXdURno+P0TeIY6lCtOTapPMFvMG01vEWnIW00dcf5D8jbvkFcg357\n\tajDFBpUlRelvGlYrK/CKY+UTPuXujq4HDBwwA28TKxQ30hSO32edsSZlUqpiRqX/VQ\n\tMGjhz4KzzArCZUSrihyTunabcjOmEtYSeNPGj3UGUeiE7D3UDAbf4l4l5bqlWDCbq3\n\tH8JWbo/1XOwss/4I+lDe3uACzLE5pFNePYB/RxksLZrJAXGNp/Q+B+M8xxNFe7n1yI\n\tGZgfr2Ma1jP5GdjKI564v8rG6gXW7Hcuob6n+lSjNjVFWfVhO2P5xNAkwkEGGafQdZ\n\t10yo417oGYkSA==","v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com;\n\ts=mail; t=1677759864;\n\tbh=9ESuAsQ84+XrOKjaFg1fFraTXXzOzSS/AEcHN4wINls=;\n\th=Date:From:To:Cc:Subject:References:In-Reply-To:From;\n\tb=XWbSbH1k82XdSYIWrbqazCxcRI5oexQAZmqdlc9KUFJvyQarhR6zcSLALizSf67ba\n\tx1M8+AYJC4PHqWA/WnY71TwNBO6vVW8VKXDq0xMD6/NgihCedAxsnoWOMFUFH054qC\n\tP/NxWSM4HZLN4E8ROvVPJ4+syxj1jt0hU0NXedxM="],"Authentication-Results":"lancelot.ideasonboard.com; dkim=pass (1024-bit key; \n\tunprotected) header.d=ideasonboard.com\n\theader.i=@ideasonboard.com\n\theader.b=\"XWbSbH1k\"; dkim-atps=neutral","Date":"Thu, 2 Mar 2023 21:24:17 +0900","To":"Jacopo Mondi ","Message-ID":"","References":"<20230129135830.27490-1-jacopo.mondi@ideasonboard.com>\n\t<20230129135830.27490-6-jacopo.mondi@ideasonboard.com>","MIME-Version":"1.0","Content-Type":"text/plain; charset=us-ascii","Content-Disposition":"inline","In-Reply-To":"<20230129135830.27490-6-jacopo.mondi@ideasonboard.com>","Subject":"Re: [libcamera-devel] [PATCH 5/5] libcamera: imx8-isi: Remove\n\tmbusCode from formatsMap_","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","From":"Paul Elder via libcamera-devel ","Reply-To":"Paul Elder ","Cc":"libcamera-devel@lists.libcamera.org","Errors-To":"libcamera-devel-bounces@lists.libcamera.org","Sender":"\"libcamera-devel\" "}},{"id":26643,"web_url":"https://patchwork.libcamera.org/comment/26643/","msgid":"<20230312165156.GC2545@pendragon.ideasonboard.com>","date":"2023-03-12T16:51:56","subject":"Re: [libcamera-devel] [PATCH 5/5] libcamera: imx8-isi: Remove\n\tmbusCode from formatsMap_","submitter":{"id":2,"url":"https://patchwork.libcamera.org/api/people/2/","name":"Laurent Pinchart","email":"laurent.pinchart@ideasonboard.com"},"content":"Hi Jacopo,\n\nThank you for the patch.\n\nOn Sun, Jan 29, 2023 at 02:58:30PM +0100, Jacopo Mondi via libcamera-devel wrote:\n> Now that the media bus code selection procedure does not depend\n> on the ISICameraConfiguration::formatsMap_ remove the association\n> between PixelFormat supported by the ISI and the media bus code produced\n> by the sensor.\n> \n> Signed-off-by: Jacopo Mondi \n> ---\n> src/libcamera/pipeline/imx8-isi/imx8-isi.cpp | 159 ++++---------------\n> 1 file changed, 29 insertions(+), 130 deletions(-)\n> \n> diff --git a/src/libcamera/pipeline/imx8-isi/imx8-isi.cpp b/src/libcamera/pipeline/imx8-isi/imx8-isi.cpp\n> index 0e1e87c7a2aa..f754f5d77f90 100644\n> --- a/src/libcamera/pipeline/imx8-isi/imx8-isi.cpp\n> +++ b/src/libcamera/pipeline/imx8-isi/imx8-isi.cpp\n> @@ -76,18 +76,6 @@ public:\n> class ISICameraConfiguration : public CameraConfiguration\n> {\n> public:\n> -\t/*\n> -\t * formatsMap_ records the association between an output pixel format\n> -\t * and the combination of V4L2 pixel format and media bus codes that have\n> -\t * to be applied to the pipeline.\n> -\t */\n> -\tstruct PipeFormat {\n> -\t\tunsigned int isiCode;\n> -\t\tunsigned int sensorCode;\n> -\t};\n> -\n> -\tusing FormatMap = std::map;\n> -\n> \tISICameraConfiguration(ISICameraData *data)\n> \t\t: data_(data)\n> \t{\n> @@ -95,7 +83,7 @@ public:\n> \n> \tStatus validate() override;\n> \n> -\tstatic const FormatMap formatsMap_;\n> +\tstatic const std::map formatsMap_;\n> \n> \tV4L2SubdeviceFormat sensorFormat_;\n> \n> @@ -333,121 +321,33 @@ unsigned int ISICameraData::getMediaBusFormat(PixelFormat *pixelFormat) const\n> * Camera Configuration\n> */\n> \n> -/**\n> - * \\todo Do not associate the sensor format to non-RAW pixelformats, as\n> - * the ISI can do colorspace conversion.\n> +/*\n> + * ISICameraConfiguration::formatsMap_ records the association between an output\n> + * pixel format and the ISI source pixel format to be applied to the pipeline.\n> */\n> -const ISICameraConfiguration::FormatMap ISICameraConfiguration::formatsMap_ = {\n> -\t{\n> -\t\tformats::YUYV,\n> -\t\t{ MEDIA_BUS_FMT_YUV8_1X24,\n> -\t\t MEDIA_BUS_FMT_UYVY8_1X16 },\n> -\t},\n> -\t{\n> -\t\tformats::AVUY8888,\n> -\t\t{ MEDIA_BUS_FMT_YUV8_1X24,\n> -\t\t MEDIA_BUS_FMT_UYVY8_1X16 },\n> -\t},\n> -\t{\n> -\t\tformats::NV12,\n> -\t\t{ MEDIA_BUS_FMT_YUV8_1X24,\n> -\t\t MEDIA_BUS_FMT_UYVY8_1X16 },\n> -\t},\n> -\t{\n> -\t\tformats::NV16,\n> -\t\t{ MEDIA_BUS_FMT_YUV8_1X24,\n> -\t\t MEDIA_BUS_FMT_UYVY8_1X16 },\n> -\t},\n> -\t{\n> -\t\tformats::YUV444,\n> -\t\t{ MEDIA_BUS_FMT_YUV8_1X24,\n> -\t\t MEDIA_BUS_FMT_UYVY8_1X16 },\n> -\t},\n> -\t{\n> -\t\tformats::RGB565,\n> -\t\t{ MEDIA_BUS_FMT_RGB888_1X24,\n> -\t\t MEDIA_BUS_FMT_RGB565_1X16 },\n> -\t},\n> -\t{\n> -\t\tformats::BGR888,\n> -\t\t{ MEDIA_BUS_FMT_RGB888_1X24,\n> -\t\t MEDIA_BUS_FMT_RGB565_1X16 },\n> -\t},\n> -\t{\n> -\t\tformats::RGB888,\n> -\t\t{ MEDIA_BUS_FMT_RGB888_1X24,\n> -\t\t MEDIA_BUS_FMT_RGB565_1X16 },\n> -\t},\n> -\t{\n> -\t\tformats::XRGB8888,\n> -\t\t{ MEDIA_BUS_FMT_RGB888_1X24,\n> -\t\t MEDIA_BUS_FMT_RGB565_1X16 },\n> -\t},\n> -\t{\n> -\t\tformats::ABGR8888,\n> -\t\t{ MEDIA_BUS_FMT_RGB888_1X24,\n> -\t\t MEDIA_BUS_FMT_RGB565_1X16 },\n> -\t},\n> -\t{\n> -\t\tformats::SBGGR8,\n> -\t\t{ MEDIA_BUS_FMT_SBGGR8_1X8,\n> -\t\t MEDIA_BUS_FMT_SBGGR8_1X8 },\n> -\t},\n> -\t{\n> -\t\tformats::SGBRG8,\n> -\t\t{ MEDIA_BUS_FMT_SGBRG8_1X8,\n> -\t\t MEDIA_BUS_FMT_SGBRG8_1X8 },\n> -\t},\n> -\t{\n> -\t\tformats::SGRBG8,\n> -\t\t{ MEDIA_BUS_FMT_SGRBG8_1X8,\n> -\t\t MEDIA_BUS_FMT_SGRBG8_1X8 },\n> -\t},\n> -\t{\n> -\t\tformats::SRGGB8,\n> -\t\t{ MEDIA_BUS_FMT_SRGGB8_1X8,\n> -\t\t MEDIA_BUS_FMT_SRGGB8_1X8 },\n> -\t},\n> -\t{\n> -\t\tformats::SBGGR10,\n> -\t\t{ MEDIA_BUS_FMT_SBGGR10_1X10,\n> -\t\t MEDIA_BUS_FMT_SBGGR10_1X10 },\n> -\t},\n> -\t{\n> -\t\tformats::SGBRG10,\n> -\t\t{ MEDIA_BUS_FMT_SGBRG10_1X10,\n> -\t\t MEDIA_BUS_FMT_SGBRG10_1X10 },\n> -\t},\n> -\t{\n> -\t\tformats::SGRBG10,\n> -\t\t{ MEDIA_BUS_FMT_SGRBG10_1X10,\n> -\t\t MEDIA_BUS_FMT_SGRBG10_1X10 },\n> -\t},\n> -\t{\n> -\t\tformats::SRGGB10,\n> -\t\t{ MEDIA_BUS_FMT_SRGGB10_1X10,\n> -\t\t MEDIA_BUS_FMT_SRGGB10_1X10 },\n> -\t},\n> -\t{\n> -\t\tformats::SBGGR12,\n> -\t\t{ MEDIA_BUS_FMT_SBGGR12_1X12,\n> -\t\t MEDIA_BUS_FMT_SBGGR12_1X12 },\n> -\t},\n> -\t{\n> -\t\tformats::SGBRG12,\n> -\t\t{ MEDIA_BUS_FMT_SGBRG12_1X12,\n> -\t\t MEDIA_BUS_FMT_SGBRG12_1X12 },\n> -\t},\n> -\t{\n> -\t\tformats::SGRBG12,\n> -\t\t{ MEDIA_BUS_FMT_SGRBG12_1X12,\n> -\t\t MEDIA_BUS_FMT_SGRBG12_1X12 },\n> -\t},\n> -\t{\n> -\t\tformats::SRGGB12,\n> -\t\t{ MEDIA_BUS_FMT_SRGGB12_1X12,\n> -\t\t MEDIA_BUS_FMT_SRGGB12_1X12 },\n> -\t},\n> +const std::map ISICameraConfiguration::formatsMap_ = {\n> +\t{ formats::YUYV, MEDIA_BUS_FMT_UYVY8_1X16 },\n> +\t{ formats::AVUY8888, MEDIA_BUS_FMT_UYVY8_1X16 },\n> +\t{ formats::NV12, MEDIA_BUS_FMT_UYVY8_1X16 },\n> +\t{ formats::NV16, MEDIA_BUS_FMT_UYVY8_1X16 },\n> +\t{ formats::YUV444, MEDIA_BUS_FMT_UYVY8_1X16 },\n> +\t{ formats::RGB565, MEDIA_BUS_FMT_RGB565_1X16 },\n> +\t{ formats::BGR888, MEDIA_BUS_FMT_RGB565_1X16 },\n> +\t{ formats::RGB888, MEDIA_BUS_FMT_RGB565_1X16 },\n> +\t{ formats::XRGB8888, MEDIA_BUS_FMT_RGB565_1X16 },\n> +\t{ formats::ABGR8888, MEDIA_BUS_FMT_RGB565_1X16 },\n> +\t{ formats::SBGGR8, MEDIA_BUS_FMT_SBGGR8_1X8 },\n> +\t{ formats::SGBRG8, MEDIA_BUS_FMT_SGBRG8_1X8 },\n> +\t{ formats::SGRBG8, MEDIA_BUS_FMT_SGRBG8_1X8 },\n> +\t{ formats::SRGGB8, MEDIA_BUS_FMT_SRGGB8_1X8 },\n> +\t{ formats::SBGGR10, MEDIA_BUS_FMT_SBGGR10_1X10 },\n> +\t{ formats::SGBRG10, MEDIA_BUS_FMT_SGBRG10_1X10 },\n> +\t{ formats::SGRBG10, MEDIA_BUS_FMT_SGRBG10_1X10 },\n> +\t{ formats::SRGGB10, MEDIA_BUS_FMT_SRGGB10_1X10 },\n> +\t{ formats::SBGGR12, MEDIA_BUS_FMT_SBGGR12_1X12 },\n> +\t{ formats::SGBRG12, MEDIA_BUS_FMT_SGBRG12_1X12 },\n> +\t{ formats::SGRBG12, MEDIA_BUS_FMT_SGRBG12_1X12 },\n> +\t{ formats::SRGGB12, MEDIA_BUS_FMT_SRGGB12_1X12 },\n> };\n> \n> /*\n> @@ -991,11 +891,10 @@ int PipelineHandlerISI::configure(Camera *camera, CameraConfiguration *c)\n> \t\t * size is taken from the sink's COMPOSE (or source's CROP,\n> \t\t * if any) rectangles.\n> \t\t */\n> -\t\tconst ISICameraConfiguration::PipeFormat &pipeFormat =\n> -\t\t\tISICameraConfiguration::formatsMap_.at(config.pixelFormat);\n> +\t\tunsigned int isiCode = ISICameraConfiguration::formatsMap_.at(config.pixelFormat);\n> \n> \t\tV4L2SubdeviceFormat isiFormat{};\n> -\t\tisiFormat.mbus_code = pipeFormat.isiCode;\n> +\t\tisiFormat.mbus_code = isiCode;\n\nUnless I'm mistaken, you've kept the sensorCore, not the isiCode, in the\nformatsMap_.\n\n> \t\tisiFormat.size = config.size;\n> \n> \t\tret = pipe->isi->setFormat(1, &isiFormat);","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 BAA7FBE080\n\tfor ;\n\tSun, 12 Mar 2023 16:51:57 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id 771C062707;\n\tSun, 12 Mar 2023 17:51:57 +0100 (CET)","from perceval.ideasonboard.com (perceval.ideasonboard.com\n\t[213.167.242.64])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTPS id C36C162706\n\tfor ;\n\tSun, 12 Mar 2023 17:51:55 +0100 (CET)","from pendragon.ideasonboard.com (85-76-21-162-nat.elisa-mobile.fi\n\t[85.76.21.162])\n\tby perceval.ideasonboard.com (Postfix) with ESMTPSA id E8757D5F;\n\tSun, 12 Mar 2023 17:51:54 +0100 (CET)"],"DKIM-Signature":["v=1; a=rsa-sha256; c=relaxed/simple; d=libcamera.org;\n\ts=mail; t=1678639917;\n\tbh=rZt6vkoDxSA5GKMk4bJmVf+wsFuwXtVDgVo6bLmC0DM=;\n\th=Date:To:References:In-Reply-To:Subject:List-Id:List-Unsubscribe:\n\tList-Archive:List-Post:List-Help:List-Subscribe:From:Reply-To:Cc:\n\tFrom;\n\tb=1ZqShwm6Gsf3wbkh9ZZb/spjb1zA1tAJ/SV5TpwnvTKb05fNFc/j9mXIdD2ZwmZT7\n\tox2KoK00WVEbJborWzbNsRdHx66ZHS11RN+3Cw+j8te4uQ1d3HEXOJB4jqlGuPZBPC\n\t1R8mJ7kGfPQLN+lTpttu1DIk0qqkG7mkgI+SIjysOtOC5DOuG3qFQ4C1pU/ZuNTAsa\n\tV8wXpNVwNoBqpbemS+Y3s/Adx0eByiyQSwhR8ywWTykqzeET3SqiapoRbMn5v2yzNZ\n\tJCOkRHfIjvGCDaac/HNJp8Sg7zJrpUQZd4cDDKkEk2lUEIAycSUls7kxx7uwpbVRLj\n\tGkAqUc1LEaUJA==","v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com;\n\ts=mail; t=1678639915;\n\tbh=rZt6vkoDxSA5GKMk4bJmVf+wsFuwXtVDgVo6bLmC0DM=;\n\th=Date:From:To:Cc:Subject:References:In-Reply-To:From;\n\tb=Ksh7ZVFBneVlKVVD8nYt9nmYq0N12cYcGhHhSvn7ZhjZUzX4gVwpb9KlRE59JtC3/\n\t18eMUvABu66fAFFKc1Jvq1D89skBOUaGa7bCQcdb+izKny2pKkNy/MROpN1NJGut8e\n\tNvIi1qzIYUNQ5c9t1VH8GJCmn978WGZ6P96Wb4L4="],"Authentication-Results":"lancelot.ideasonboard.com; dkim=pass (1024-bit key; \n\tunprotected) header.d=ideasonboard.com\n\theader.i=@ideasonboard.com\n\theader.b=\"Ksh7ZVFB\"; dkim-atps=neutral","Date":"Sun, 12 Mar 2023 18:51:56 +0200","To":"Jacopo Mondi ","Message-ID":"<20230312165156.GC2545@pendragon.ideasonboard.com>","References":"<20230129135830.27490-1-jacopo.mondi@ideasonboard.com>\n\t<20230129135830.27490-6-jacopo.mondi@ideasonboard.com>","MIME-Version":"1.0","Content-Type":"text/plain; charset=utf-8","Content-Disposition":"inline","In-Reply-To":"<20230129135830.27490-6-jacopo.mondi@ideasonboard.com>","Subject":"Re: [libcamera-devel] [PATCH 5/5] libcamera: imx8-isi: Remove\n\tmbusCode from formatsMap_","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","From":"Laurent Pinchart via libcamera-devel\n\t","Reply-To":"Laurent Pinchart ","Cc":"libcamera-devel@lists.libcamera.org","Errors-To":"libcamera-devel-bounces@lists.libcamera.org","Sender":"\"libcamera-devel\" "}},{"id":26650,"web_url":"https://patchwork.libcamera.org/comment/26650/","msgid":"<20230313085909.aopzzd3ezcihnumn@uno.localdomain>","date":"2023-03-13T08:59:09","subject":"Re: [libcamera-devel] [PATCH 5/5] libcamera: imx8-isi: Remove\n\tmbusCode from formatsMap_","submitter":{"id":143,"url":"https://patchwork.libcamera.org/api/people/143/","name":"Jacopo Mondi","email":"jacopo.mondi@ideasonboard.com"},"content":"Hi Laurent\n\nOn Sun, Mar 12, 2023 at 06:51:56PM +0200, Laurent Pinchart wrote:\n> Hi Jacopo,\n>\n> Thank you for the patch.\n>\n> On Sun, Jan 29, 2023 at 02:58:30PM +0100, Jacopo Mondi via libcamera-devel wrote:\n> > Now that the media bus code selection procedure does not depend\n> > on the ISICameraConfiguration::formatsMap_ remove the association\n> > between PixelFormat supported by the ISI and the media bus code produced\n> > by the sensor.\n> >\n> > Signed-off-by: Jacopo Mondi \n> > ---\n> > src/libcamera/pipeline/imx8-isi/imx8-isi.cpp | 159 ++++---------------\n> > 1 file changed, 29 insertions(+), 130 deletions(-)\n> >\n> > diff --git a/src/libcamera/pipeline/imx8-isi/imx8-isi.cpp b/src/libcamera/pipeline/imx8-isi/imx8-isi.cpp\n> > index 0e1e87c7a2aa..f754f5d77f90 100644\n> > --- a/src/libcamera/pipeline/imx8-isi/imx8-isi.cpp\n> > +++ b/src/libcamera/pipeline/imx8-isi/imx8-isi.cpp\n> > @@ -76,18 +76,6 @@ public:\n> > class ISICameraConfiguration : public CameraConfiguration\n> > {\n> > public:\n> > -\t/*\n> > -\t * formatsMap_ records the association between an output pixel format\n> > -\t * and the combination of V4L2 pixel format and media bus codes that have\n> > -\t * to be applied to the pipeline.\n> > -\t */\n> > -\tstruct PipeFormat {\n> > -\t\tunsigned int isiCode;\n> > -\t\tunsigned int sensorCode;\n> > -\t};\n> > -\n> > -\tusing FormatMap = std::map;\n> > -\n> > \tISICameraConfiguration(ISICameraData *data)\n> > \t\t: data_(data)\n> > \t{\n> > @@ -95,7 +83,7 @@ public:\n> >\n> > \tStatus validate() override;\n> >\n> > -\tstatic const FormatMap formatsMap_;\n> > +\tstatic const std::map formatsMap_;\n> >\n> > \tV4L2SubdeviceFormat sensorFormat_;\n> >\n> > @@ -333,121 +321,33 @@ unsigned int ISICameraData::getMediaBusFormat(PixelFormat *pixelFormat) const\n> > * Camera Configuration\n> > */\n> >\n> > -/**\n> > - * \\todo Do not associate the sensor format to non-RAW pixelformats, as\n> > - * the ISI can do colorspace conversion.\n> > +/*\n> > + * ISICameraConfiguration::formatsMap_ records the association between an output\n> > + * pixel format and the ISI source pixel format to be applied to the pipeline.\n> > */\n> > -const ISICameraConfiguration::FormatMap ISICameraConfiguration::formatsMap_ = {\n> > -\t{\n> > -\t\tformats::YUYV,\n> > -\t\t{ MEDIA_BUS_FMT_YUV8_1X24,\n> > -\t\t MEDIA_BUS_FMT_UYVY8_1X16 },\n> > -\t},\n> > -\t{\n> > -\t\tformats::AVUY8888,\n> > -\t\t{ MEDIA_BUS_FMT_YUV8_1X24,\n> > -\t\t MEDIA_BUS_FMT_UYVY8_1X16 },\n> > -\t},\n> > -\t{\n> > -\t\tformats::NV12,\n> > -\t\t{ MEDIA_BUS_FMT_YUV8_1X24,\n> > -\t\t MEDIA_BUS_FMT_UYVY8_1X16 },\n> > -\t},\n> > -\t{\n> > -\t\tformats::NV16,\n> > -\t\t{ MEDIA_BUS_FMT_YUV8_1X24,\n> > -\t\t MEDIA_BUS_FMT_UYVY8_1X16 },\n> > -\t},\n> > -\t{\n> > -\t\tformats::YUV444,\n> > -\t\t{ MEDIA_BUS_FMT_YUV8_1X24,\n> > -\t\t MEDIA_BUS_FMT_UYVY8_1X16 },\n> > -\t},\n> > -\t{\n> > -\t\tformats::RGB565,\n> > -\t\t{ MEDIA_BUS_FMT_RGB888_1X24,\n> > -\t\t MEDIA_BUS_FMT_RGB565_1X16 },\n> > -\t},\n> > -\t{\n> > -\t\tformats::BGR888,\n> > -\t\t{ MEDIA_BUS_FMT_RGB888_1X24,\n> > -\t\t MEDIA_BUS_FMT_RGB565_1X16 },\n> > -\t},\n> > -\t{\n> > -\t\tformats::RGB888,\n> > -\t\t{ MEDIA_BUS_FMT_RGB888_1X24,\n> > -\t\t MEDIA_BUS_FMT_RGB565_1X16 },\n> > -\t},\n> > -\t{\n> > -\t\tformats::XRGB8888,\n> > -\t\t{ MEDIA_BUS_FMT_RGB888_1X24,\n> > -\t\t MEDIA_BUS_FMT_RGB565_1X16 },\n> > -\t},\n> > -\t{\n> > -\t\tformats::ABGR8888,\n> > -\t\t{ MEDIA_BUS_FMT_RGB888_1X24,\n> > -\t\t MEDIA_BUS_FMT_RGB565_1X16 },\n> > -\t},\n> > -\t{\n> > -\t\tformats::SBGGR8,\n> > -\t\t{ MEDIA_BUS_FMT_SBGGR8_1X8,\n> > -\t\t MEDIA_BUS_FMT_SBGGR8_1X8 },\n> > -\t},\n> > -\t{\n> > -\t\tformats::SGBRG8,\n> > -\t\t{ MEDIA_BUS_FMT_SGBRG8_1X8,\n> > -\t\t MEDIA_BUS_FMT_SGBRG8_1X8 },\n> > -\t},\n> > -\t{\n> > -\t\tformats::SGRBG8,\n> > -\t\t{ MEDIA_BUS_FMT_SGRBG8_1X8,\n> > -\t\t MEDIA_BUS_FMT_SGRBG8_1X8 },\n> > -\t},\n> > -\t{\n> > -\t\tformats::SRGGB8,\n> > -\t\t{ MEDIA_BUS_FMT_SRGGB8_1X8,\n> > -\t\t MEDIA_BUS_FMT_SRGGB8_1X8 },\n> > -\t},\n> > -\t{\n> > -\t\tformats::SBGGR10,\n> > -\t\t{ MEDIA_BUS_FMT_SBGGR10_1X10,\n> > -\t\t MEDIA_BUS_FMT_SBGGR10_1X10 },\n> > -\t},\n> > -\t{\n> > -\t\tformats::SGBRG10,\n> > -\t\t{ MEDIA_BUS_FMT_SGBRG10_1X10,\n> > -\t\t MEDIA_BUS_FMT_SGBRG10_1X10 },\n> > -\t},\n> > -\t{\n> > -\t\tformats::SGRBG10,\n> > -\t\t{ MEDIA_BUS_FMT_SGRBG10_1X10,\n> > -\t\t MEDIA_BUS_FMT_SGRBG10_1X10 },\n> > -\t},\n> > -\t{\n> > -\t\tformats::SRGGB10,\n> > -\t\t{ MEDIA_BUS_FMT_SRGGB10_1X10,\n> > -\t\t MEDIA_BUS_FMT_SRGGB10_1X10 },\n> > -\t},\n> > -\t{\n> > -\t\tformats::SBGGR12,\n> > -\t\t{ MEDIA_BUS_FMT_SBGGR12_1X12,\n> > -\t\t MEDIA_BUS_FMT_SBGGR12_1X12 },\n> > -\t},\n> > -\t{\n> > -\t\tformats::SGBRG12,\n> > -\t\t{ MEDIA_BUS_FMT_SGBRG12_1X12,\n> > -\t\t MEDIA_BUS_FMT_SGBRG12_1X12 },\n> > -\t},\n> > -\t{\n> > -\t\tformats::SGRBG12,\n> > -\t\t{ MEDIA_BUS_FMT_SGRBG12_1X12,\n> > -\t\t MEDIA_BUS_FMT_SGRBG12_1X12 },\n> > -\t},\n> > -\t{\n> > -\t\tformats::SRGGB12,\n> > -\t\t{ MEDIA_BUS_FMT_SRGGB12_1X12,\n> > -\t\t MEDIA_BUS_FMT_SRGGB12_1X12 },\n> > -\t},\n> > +const std::map ISICameraConfiguration::formatsMap_ = {\n> > +\t{ formats::YUYV, MEDIA_BUS_FMT_UYVY8_1X16 },\n> > +\t{ formats::AVUY8888, MEDIA_BUS_FMT_UYVY8_1X16 },\n> > +\t{ formats::NV12, MEDIA_BUS_FMT_UYVY8_1X16 },\n> > +\t{ formats::NV16, MEDIA_BUS_FMT_UYVY8_1X16 },\n> > +\t{ formats::YUV444, MEDIA_BUS_FMT_UYVY8_1X16 },\n> > +\t{ formats::RGB565, MEDIA_BUS_FMT_RGB565_1X16 },\n> > +\t{ formats::BGR888, MEDIA_BUS_FMT_RGB565_1X16 },\n> > +\t{ formats::RGB888, MEDIA_BUS_FMT_RGB565_1X16 },\n> > +\t{ formats::XRGB8888, MEDIA_BUS_FMT_RGB565_1X16 },\n> > +\t{ formats::ABGR8888, MEDIA_BUS_FMT_RGB565_1X16 },\n> > +\t{ formats::SBGGR8, MEDIA_BUS_FMT_SBGGR8_1X8 },\n> > +\t{ formats::SGBRG8, MEDIA_BUS_FMT_SGBRG8_1X8 },\n> > +\t{ formats::SGRBG8, MEDIA_BUS_FMT_SGRBG8_1X8 },\n> > +\t{ formats::SRGGB8, MEDIA_BUS_FMT_SRGGB8_1X8 },\n> > +\t{ formats::SBGGR10, MEDIA_BUS_FMT_SBGGR10_1X10 },\n> > +\t{ formats::SGBRG10, MEDIA_BUS_FMT_SGBRG10_1X10 },\n> > +\t{ formats::SGRBG10, MEDIA_BUS_FMT_SGRBG10_1X10 },\n> > +\t{ formats::SRGGB10, MEDIA_BUS_FMT_SRGGB10_1X10 },\n> > +\t{ formats::SBGGR12, MEDIA_BUS_FMT_SBGGR12_1X12 },\n> > +\t{ formats::SGBRG12, MEDIA_BUS_FMT_SGBRG12_1X12 },\n> > +\t{ formats::SGRBG12, MEDIA_BUS_FMT_SGRBG12_1X12 },\n> > +\t{ formats::SRGGB12, MEDIA_BUS_FMT_SRGGB12_1X12 },\n> > };\n> >\n> > /*\n> > @@ -991,11 +891,10 @@ int PipelineHandlerISI::configure(Camera *camera, CameraConfiguration *c)\n> > \t\t * size is taken from the sink's COMPOSE (or source's CROP,\n> > \t\t * if any) rectangles.\n> > \t\t */\n> > -\t\tconst ISICameraConfiguration::PipeFormat &pipeFormat =\n> > -\t\t\tISICameraConfiguration::formatsMap_.at(config.pixelFormat);\n> > +\t\tunsigned int isiCode = ISICameraConfiguration::formatsMap_.at(config.pixelFormat);\n> >\n> > \t\tV4L2SubdeviceFormat isiFormat{};\n> > -\t\tisiFormat.mbus_code = pipeFormat.isiCode;\n> > +\t\tisiFormat.mbus_code = isiCode;\n>\n> Unless I'm mistaken, you've kept the sensorCore, not the isiCode, in the\n> formatsMap_.\n>\n\nMmmm, not really, the formatsMap_ collects the association between an\noutput pixelformat and the ISI source pad format.\n\nThe sensor format is dynamically computed by\nISICameraData::getMediaBusFormat() and propagated from the sensor to the\nISI sink pad during configure().\n\nDoes this answer your question ?\n\n> > \t\tisiFormat.size = config.size;\n> >\n> > \t\tret = pipe->isi->setFormat(1, &isiFormat);\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 634BCBD80A\n\tfor ;\n\tMon, 13 Mar 2023 08:59:15 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id 929336270A;\n\tMon, 13 Mar 2023 09:59:14 +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 21148626B2\n\tfor ;\n\tMon, 13 Mar 2023 09:59:13 +0100 (CET)","from ideasonboard.com (host-79-33-55-183.retail.telecomitalia.it\n\t[79.33.55.183])\n\tby perceval.ideasonboard.com (Postfix) with ESMTPSA id 59087563;\n\tMon, 13 Mar 2023 09:59:12 +0100 (CET)"],"DKIM-Signature":["v=1; a=rsa-sha256; c=relaxed/simple; d=libcamera.org;\n\ts=mail; t=1678697954;\n\tbh=gssNV+g/yLAS4xbUSJYS8wSIBQqtlSLs0uWgdCQ1BwU=;\n\th=Date:To:References:In-Reply-To:Subject:List-Id:List-Unsubscribe:\n\tList-Archive:List-Post:List-Help:List-Subscribe:From:Reply-To:Cc:\n\tFrom;\n\tb=GSdm5fiPNzFZD0tXhWvv+iQTIEEsIGAj2s0dqeLMPrZjiFZM2t6vQzznLu2+S93v1\n\tSHvWiCQQ8zipM3uCfRx7hB4JAIVcSX//kmJOqNzQR1+ho8A++9CM4JMNYUs7S0XAJS\n\t5e0gVZ4w8ZENLPVCimmFqqs02ff7vKbfuzW9CMLzWNSCyWGKry2JZ+RNfxok6w/pcn\n\tatDuMKaF36zMY043qYECbxxzHUa5UNHH+PJJ3DnDzIB6NsU+p5Yc+rhBwFIPAYkqOy\n\tI8UJSoE/Il5aYQBvI0S4MVeNeVZWxANUxhmc6xHaQwrHDu7mJcByULUymrtbMMUbes\n\t3HEwjLOKk/9VA==","v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com;\n\ts=mail; t=1678697952;\n\tbh=gssNV+g/yLAS4xbUSJYS8wSIBQqtlSLs0uWgdCQ1BwU=;\n\th=Date:From:To:Cc:Subject:References:In-Reply-To:From;\n\tb=Rn25Uyaaw+hXSEK5Bt0xcwcEcZb4WGTPYc/3Dax3+jJ9KVo6ri2oH5pf2cIchudJM\n\taOsWtpbAhEsAIGyMdN8pTt7kON1c3jgRtuHM0Fx9u5CLq1PhWm2qOrBAZXN6YWEZSj\n\taQIRctCty3OTgxCGMYZx3z3MUf0YIfFFboF59hw0="],"Authentication-Results":"lancelot.ideasonboard.com; dkim=pass (1024-bit key; \n\tunprotected) header.d=ideasonboard.com\n\theader.i=@ideasonboard.com\n\theader.b=\"Rn25Uyaa\"; dkim-atps=neutral","Date":"Mon, 13 Mar 2023 09:59:09 +0100","To":"Laurent Pinchart ","Message-ID":"<20230313085909.aopzzd3ezcihnumn@uno.localdomain>","References":"<20230129135830.27490-1-jacopo.mondi@ideasonboard.com>\n\t<20230129135830.27490-6-jacopo.mondi@ideasonboard.com>\n\t<20230312165156.GC2545@pendragon.ideasonboard.com>","MIME-Version":"1.0","Content-Type":"text/plain; charset=utf-8","Content-Disposition":"inline","In-Reply-To":"<20230312165156.GC2545@pendragon.ideasonboard.com>","Subject":"Re: [libcamera-devel] [PATCH 5/5] libcamera: imx8-isi: Remove\n\tmbusCode from formatsMap_","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","From":"Jacopo Mondi via libcamera-devel ","Reply-To":"Jacopo Mondi ","Cc":"Jacopo Mondi ,\n\tlibcamera-devel@lists.libcamera.org","Errors-To":"libcamera-devel-bounces@lists.libcamera.org","Sender":"\"libcamera-devel\" "}},{"id":26653,"web_url":"https://patchwork.libcamera.org/comment/26653/","msgid":"<20230313102837.GG2545@pendragon.ideasonboard.com>","date":"2023-03-13T10:28:37","subject":"Re: [libcamera-devel] [PATCH 5/5] libcamera: imx8-isi: Remove\n\tmbusCode from formatsMap_","submitter":{"id":2,"url":"https://patchwork.libcamera.org/api/people/2/","name":"Laurent Pinchart","email":"laurent.pinchart@ideasonboard.com"},"content":"Hi Jacopo,\n\nOn Mon, Mar 13, 2023 at 09:59:09AM +0100, Jacopo Mondi wrote:\n> On Sun, Mar 12, 2023 at 06:51:56PM +0200, Laurent Pinchart wrote:\n> > On Sun, Jan 29, 2023 at 02:58:30PM +0100, Jacopo Mondi via libcamera-devel wrote:\n> > > Now that the media bus code selection procedure does not depend\n> > > on the ISICameraConfiguration::formatsMap_ remove the association\n> > > between PixelFormat supported by the ISI and the media bus code produced\n> > > by the sensor.\n> > >\n> > > Signed-off-by: Jacopo Mondi \n> > > ---\n> > > src/libcamera/pipeline/imx8-isi/imx8-isi.cpp | 159 ++++---------------\n> > > 1 file changed, 29 insertions(+), 130 deletions(-)\n> > >\n> > > diff --git a/src/libcamera/pipeline/imx8-isi/imx8-isi.cpp b/src/libcamera/pipeline/imx8-isi/imx8-isi.cpp\n> > > index 0e1e87c7a2aa..f754f5d77f90 100644\n> > > --- a/src/libcamera/pipeline/imx8-isi/imx8-isi.cpp\n> > > +++ b/src/libcamera/pipeline/imx8-isi/imx8-isi.cpp\n> > > @@ -76,18 +76,6 @@ public:\n> > > class ISICameraConfiguration : public CameraConfiguration\n> > > {\n> > > public:\n> > > -\t/*\n> > > -\t * formatsMap_ records the association between an output pixel format\n> > > -\t * and the combination of V4L2 pixel format and media bus codes that have\n> > > -\t * to be applied to the pipeline.\n> > > -\t */\n> > > -\tstruct PipeFormat {\n> > > -\t\tunsigned int isiCode;\n> > > -\t\tunsigned int sensorCode;\n> > > -\t};\n> > > -\n> > > -\tusing FormatMap = std::map;\n> > > -\n> > > \tISICameraConfiguration(ISICameraData *data)\n> > > \t\t: data_(data)\n> > > \t{\n> > > @@ -95,7 +83,7 @@ public:\n> > >\n> > > \tStatus validate() override;\n> > >\n> > > -\tstatic const FormatMap formatsMap_;\n> > > +\tstatic const std::map formatsMap_;\n> > >\n> > > \tV4L2SubdeviceFormat sensorFormat_;\n> > >\n> > > @@ -333,121 +321,33 @@ unsigned int ISICameraData::getMediaBusFormat(PixelFormat *pixelFormat) const\n> > > * Camera Configuration\n> > > */\n> > >\n> > > -/**\n> > > - * \\todo Do not associate the sensor format to non-RAW pixelformats, as\n> > > - * the ISI can do colorspace conversion.\n> > > +/*\n> > > + * ISICameraConfiguration::formatsMap_ records the association between an output\n> > > + * pixel format and the ISI source pixel format to be applied to the pipeline.\n> > > */\n> > > -const ISICameraConfiguration::FormatMap ISICameraConfiguration::formatsMap_ = {\n> > > -\t{\n> > > -\t\tformats::YUYV,\n> > > -\t\t{ MEDIA_BUS_FMT_YUV8_1X24,\n> > > -\t\t MEDIA_BUS_FMT_UYVY8_1X16 },\n> > > -\t},\n> > > -\t{\n> > > -\t\tformats::AVUY8888,\n> > > -\t\t{ MEDIA_BUS_FMT_YUV8_1X24,\n> > > -\t\t MEDIA_BUS_FMT_UYVY8_1X16 },\n> > > -\t},\n> > > -\t{\n> > > -\t\tformats::NV12,\n> > > -\t\t{ MEDIA_BUS_FMT_YUV8_1X24,\n> > > -\t\t MEDIA_BUS_FMT_UYVY8_1X16 },\n> > > -\t},\n> > > -\t{\n> > > -\t\tformats::NV16,\n> > > -\t\t{ MEDIA_BUS_FMT_YUV8_1X24,\n> > > -\t\t MEDIA_BUS_FMT_UYVY8_1X16 },\n> > > -\t},\n> > > -\t{\n> > > -\t\tformats::YUV444,\n> > > -\t\t{ MEDIA_BUS_FMT_YUV8_1X24,\n> > > -\t\t MEDIA_BUS_FMT_UYVY8_1X16 },\n> > > -\t},\n> > > -\t{\n> > > -\t\tformats::RGB565,\n> > > -\t\t{ MEDIA_BUS_FMT_RGB888_1X24,\n> > > -\t\t MEDIA_BUS_FMT_RGB565_1X16 },\n> > > -\t},\n> > > -\t{\n> > > -\t\tformats::BGR888,\n> > > -\t\t{ MEDIA_BUS_FMT_RGB888_1X24,\n> > > -\t\t MEDIA_BUS_FMT_RGB565_1X16 },\n> > > -\t},\n> > > -\t{\n> > > -\t\tformats::RGB888,\n> > > -\t\t{ MEDIA_BUS_FMT_RGB888_1X24,\n> > > -\t\t MEDIA_BUS_FMT_RGB565_1X16 },\n> > > -\t},\n> > > -\t{\n> > > -\t\tformats::XRGB8888,\n> > > -\t\t{ MEDIA_BUS_FMT_RGB888_1X24,\n> > > -\t\t MEDIA_BUS_FMT_RGB565_1X16 },\n> > > -\t},\n> > > -\t{\n> > > -\t\tformats::ABGR8888,\n> > > -\t\t{ MEDIA_BUS_FMT_RGB888_1X24,\n> > > -\t\t MEDIA_BUS_FMT_RGB565_1X16 },\n> > > -\t},\n> > > -\t{\n> > > -\t\tformats::SBGGR8,\n> > > -\t\t{ MEDIA_BUS_FMT_SBGGR8_1X8,\n> > > -\t\t MEDIA_BUS_FMT_SBGGR8_1X8 },\n> > > -\t},\n> > > -\t{\n> > > -\t\tformats::SGBRG8,\n> > > -\t\t{ MEDIA_BUS_FMT_SGBRG8_1X8,\n> > > -\t\t MEDIA_BUS_FMT_SGBRG8_1X8 },\n> > > -\t},\n> > > -\t{\n> > > -\t\tformats::SGRBG8,\n> > > -\t\t{ MEDIA_BUS_FMT_SGRBG8_1X8,\n> > > -\t\t MEDIA_BUS_FMT_SGRBG8_1X8 },\n> > > -\t},\n> > > -\t{\n> > > -\t\tformats::SRGGB8,\n> > > -\t\t{ MEDIA_BUS_FMT_SRGGB8_1X8,\n> > > -\t\t MEDIA_BUS_FMT_SRGGB8_1X8 },\n> > > -\t},\n> > > -\t{\n> > > -\t\tformats::SBGGR10,\n> > > -\t\t{ MEDIA_BUS_FMT_SBGGR10_1X10,\n> > > -\t\t MEDIA_BUS_FMT_SBGGR10_1X10 },\n> > > -\t},\n> > > -\t{\n> > > -\t\tformats::SGBRG10,\n> > > -\t\t{ MEDIA_BUS_FMT_SGBRG10_1X10,\n> > > -\t\t MEDIA_BUS_FMT_SGBRG10_1X10 },\n> > > -\t},\n> > > -\t{\n> > > -\t\tformats::SGRBG10,\n> > > -\t\t{ MEDIA_BUS_FMT_SGRBG10_1X10,\n> > > -\t\t MEDIA_BUS_FMT_SGRBG10_1X10 },\n> > > -\t},\n> > > -\t{\n> > > -\t\tformats::SRGGB10,\n> > > -\t\t{ MEDIA_BUS_FMT_SRGGB10_1X10,\n> > > -\t\t MEDIA_BUS_FMT_SRGGB10_1X10 },\n> > > -\t},\n> > > -\t{\n> > > -\t\tformats::SBGGR12,\n> > > -\t\t{ MEDIA_BUS_FMT_SBGGR12_1X12,\n> > > -\t\t MEDIA_BUS_FMT_SBGGR12_1X12 },\n> > > -\t},\n> > > -\t{\n> > > -\t\tformats::SGBRG12,\n> > > -\t\t{ MEDIA_BUS_FMT_SGBRG12_1X12,\n> > > -\t\t MEDIA_BUS_FMT_SGBRG12_1X12 },\n> > > -\t},\n> > > -\t{\n> > > -\t\tformats::SGRBG12,\n> > > -\t\t{ MEDIA_BUS_FMT_SGRBG12_1X12,\n> > > -\t\t MEDIA_BUS_FMT_SGRBG12_1X12 },\n> > > -\t},\n> > > -\t{\n> > > -\t\tformats::SRGGB12,\n> > > -\t\t{ MEDIA_BUS_FMT_SRGGB12_1X12,\n> > > -\t\t MEDIA_BUS_FMT_SRGGB12_1X12 },\n> > > -\t},\n> > > +const std::map ISICameraConfiguration::formatsMap_ = {\n> > > +\t{ formats::YUYV, MEDIA_BUS_FMT_UYVY8_1X16 },\n> > > +\t{ formats::AVUY8888, MEDIA_BUS_FMT_UYVY8_1X16 },\n> > > +\t{ formats::NV12, MEDIA_BUS_FMT_UYVY8_1X16 },\n> > > +\t{ formats::NV16, MEDIA_BUS_FMT_UYVY8_1X16 },\n> > > +\t{ formats::YUV444, MEDIA_BUS_FMT_UYVY8_1X16 },\n> > > +\t{ formats::RGB565, MEDIA_BUS_FMT_RGB565_1X16 },\n> > > +\t{ formats::BGR888, MEDIA_BUS_FMT_RGB565_1X16 },\n> > > +\t{ formats::RGB888, MEDIA_BUS_FMT_RGB565_1X16 },\n> > > +\t{ formats::XRGB8888, MEDIA_BUS_FMT_RGB565_1X16 },\n> > > +\t{ formats::ABGR8888, MEDIA_BUS_FMT_RGB565_1X16 },\n> > > +\t{ formats::SBGGR8, MEDIA_BUS_FMT_SBGGR8_1X8 },\n> > > +\t{ formats::SGBRG8, MEDIA_BUS_FMT_SGBRG8_1X8 },\n> > > +\t{ formats::SGRBG8, MEDIA_BUS_FMT_SGRBG8_1X8 },\n> > > +\t{ formats::SRGGB8, MEDIA_BUS_FMT_SRGGB8_1X8 },\n> > > +\t{ formats::SBGGR10, MEDIA_BUS_FMT_SBGGR10_1X10 },\n> > > +\t{ formats::SGBRG10, MEDIA_BUS_FMT_SGBRG10_1X10 },\n> > > +\t{ formats::SGRBG10, MEDIA_BUS_FMT_SGRBG10_1X10 },\n> > > +\t{ formats::SRGGB10, MEDIA_BUS_FMT_SRGGB10_1X10 },\n> > > +\t{ formats::SBGGR12, MEDIA_BUS_FMT_SBGGR12_1X12 },\n> > > +\t{ formats::SGBRG12, MEDIA_BUS_FMT_SGBRG12_1X12 },\n> > > +\t{ formats::SGRBG12, MEDIA_BUS_FMT_SGRBG12_1X12 },\n> > > +\t{ formats::SRGGB12, MEDIA_BUS_FMT_SRGGB12_1X12 },\n> > > };\n> > >\n> > > /*\n> > > @@ -991,11 +891,10 @@ int PipelineHandlerISI::configure(Camera *camera, CameraConfiguration *c)\n> > > \t\t * size is taken from the sink's COMPOSE (or source's CROP,\n> > > \t\t * if any) rectangles.\n> > > \t\t */\n> > > -\t\tconst ISICameraConfiguration::PipeFormat &pipeFormat =\n> > > -\t\t\tISICameraConfiguration::formatsMap_.at(config.pixelFormat);\n> > > +\t\tunsigned int isiCode = ISICameraConfiguration::formatsMap_.at(config.pixelFormat);\n> > >\n> > > \t\tV4L2SubdeviceFormat isiFormat{};\n> > > -\t\tisiFormat.mbus_code = pipeFormat.isiCode;\n> > > +\t\tisiFormat.mbus_code = isiCode;\n> >\n> > Unless I'm mistaken, you've kept the sensorCore, not the isiCode, in the\n> > formatsMap_.\n> \n> Mmmm, not really, the formatsMap_ collects the association between an\n> output pixelformat and the ISI source pad format.\n> \n> The sensor format is dynamically computed by\n> ISICameraData::getMediaBusFormat() and propagated from the sensor to the\n> ISI sink pad during configure().\n> \n> Does this answer your question ?\n\nLooking at the first entry of the map, you have\n\n-\t\tformats::YUYV,\n-\t\t{ MEDIA_BUS_FMT_YUV8_1X24,\n-\t\t MEDIA_BUS_FMT_UYVY8_1X16 },\n+\t{ formats::YUYV, MEDIA_BUS_FMT_UYVY8_1X16 },\n\nand the map used to store a\n\nstruct PipeFormat {\n\tunsigned int isiCode;\n\tunsigned int sensorCode;\n};\n\nYou have thus kept the sensorCode value, not the isiCode value, while\nyou're modifying the code that uses to map as follows:\n\n+\t\tunsigned int isiCode = ISICameraConfiguration::formatsMap_.at(config.pixelFormat);\n\n \t\tV4L2SubdeviceFormat isiFormat{};\n-\t\tisiFormat.mbus_code = pipeFormat.isiCode;\n+\t\tisiFormat.mbus_code = isiCode;\n\nI would thus have expected the map to now store\n\n\t{ formats::YUYV, MEDIA_BUS_FMT_YUV8_1X24 },\n\n> > > \t\tisiFormat.size = config.size;\n> > >\n> > > \t\tret = pipe->isi->setFormat(1, &isiFormat);","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 A3F5CBD80A\n\tfor ;\n\tMon, 13 Mar 2023 10:28:39 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id AAF2362709;\n\tMon, 13 Mar 2023 11:28:38 +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 2ED48626B2\n\tfor ;\n\tMon, 13 Mar 2023 11:28:36 +0100 (CET)","from pendragon.ideasonboard.com (85-76-109-120-nat.elisa-mobile.fi\n\t[85.76.109.120])\n\tby perceval.ideasonboard.com (Postfix) with ESMTPSA id 4438A6FA;\n\tMon, 13 Mar 2023 11:28:35 +0100 (CET)"],"DKIM-Signature":["v=1; a=rsa-sha256; c=relaxed/simple; d=libcamera.org;\n\ts=mail; t=1678703318;\n\tbh=1hzvwNUzUHKuuehSMxS6Q8R6jNVM6+YKkYUhjnSgP9g=;\n\th=Date:To:References:In-Reply-To:Subject:List-Id:List-Unsubscribe:\n\tList-Archive:List-Post:List-Help:List-Subscribe:From:Reply-To:Cc:\n\tFrom;\n\tb=AnaH+rP9MGPNFam/8VHQWMCqZPqM02xv1Tm0wrgBwC4icy3iU3/MkKMvO41M2y0l4\n\tpBBXaPFTxhBvnQyP2EjZFRq/VMjOjt0PyzOffWXqbdZgxdJddCf79t2dbpno7kkamK\n\tKTDyVZbnRtwZ/bpjPatQzXkppmPCMzd27DME+MS3/ALhERyBCEcEgHfOO7JOyaSq6T\n\tH/2vWfSnXi5wQnaA6RT3XwOkQhM1AUrldc7Qlsbxs2RMXmuxGE2WrBD1VQgUSPV+fg\n\tbym4uycmiJin+lcfVWUxnUOGB19zJOTZsOWkTZJhJQ+gXvNUmcO0ciADQBF9rH0Qz/\n\tLbbVlB9WBAtmw==","v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com;\n\ts=mail; t=1678703315;\n\tbh=1hzvwNUzUHKuuehSMxS6Q8R6jNVM6+YKkYUhjnSgP9g=;\n\th=Date:From:To:Cc:Subject:References:In-Reply-To:From;\n\tb=iKRCAs2oxtTnEDlmqQqPMbKn1Sl8u06mJFT07c8X2qPs8HkvCiJxQ2qNoY8nDOrhu\n\tO4I82vh04vUPnf/lB1XqJ0X545tdUa2Jy7CyinQl7r05/WfVZqjQGkx3TiMYCfxUZD\n\toIQowrz2PIDSe+ZXETjVnUyS4x1n/pi1Zrff/r+c="],"Authentication-Results":"lancelot.ideasonboard.com; dkim=pass (1024-bit key; \n\tunprotected) header.d=ideasonboard.com\n\theader.i=@ideasonboard.com\n\theader.b=\"iKRCAs2o\"; dkim-atps=neutral","Date":"Mon, 13 Mar 2023 12:28:37 +0200","To":"Jacopo Mondi ","Message-ID":"<20230313102837.GG2545@pendragon.ideasonboard.com>","References":"<20230129135830.27490-1-jacopo.mondi@ideasonboard.com>\n\t<20230129135830.27490-6-jacopo.mondi@ideasonboard.com>\n\t<20230312165156.GC2545@pendragon.ideasonboard.com>\n\t<20230313085909.aopzzd3ezcihnumn@uno.localdomain>","MIME-Version":"1.0","Content-Type":"text/plain; charset=utf-8","Content-Disposition":"inline","In-Reply-To":"<20230313085909.aopzzd3ezcihnumn@uno.localdomain>","Subject":"Re: [libcamera-devel] [PATCH 5/5] libcamera: imx8-isi: Remove\n\tmbusCode from formatsMap_","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","From":"Laurent Pinchart via libcamera-devel\n\t","Reply-To":"Laurent Pinchart ","Cc":"libcamera-devel@lists.libcamera.org","Errors-To":"libcamera-devel-bounces@lists.libcamera.org","Sender":"\"libcamera-devel\" "}}]