[{"id":26534,"web_url":"https://patchwork.libcamera.org/comment/26534/","msgid":"","date":"2023-03-02T12:12:12","subject":"Re: [libcamera-devel] [PATCH 4/5] libcamera: imx8-isi: Split\n\tBayer/YUV config generation","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:29PM +0100, Jacopo Mondi via libcamera-devel wrote:\n> At generateConfiguration() a YUV/RGB pixel format is preferred for the\n> StillCapture/VideoRecording/Viewfinder roles, but currently there are no\n> guarantees in place that the sensor provides a non-Bayer bus format from\n> which YUV/RGB can be generated.\n> \n> This makes the default configuration generated for those roles not to\n> work if the sensor is a RAW-only one.\n> \n> To improve the situation split the configuration generation in two,\n> one for YUV modes and one for Raw Bayer mode.\n> \n> StreamRoles assigned to a YUV mode will try to first generate a YUV\n> configuration and then fallback to RAW if that's what the sensor can\n> provide.\n> \n> As an additional requirement, for YUV streams, the generated mode has to\n> be validated with the sensor to confirm the desired sizes can be\n> generated. In order to test a format on the sensor introduce\n> CameraSensor::tryFormat().\n\nimo CameraSensor::tryFormat() should be split into a separate patch, but\nit's not that big of a deal.\n\n> \n> Reported-by: Laurent Pinchart \n> Signed-off-by: Jacopo Mondi \n> ---\n> include/libcamera/internal/camera_sensor.h | 1 +\n> src/libcamera/camera_sensor.cpp | 14 ++\n> src/libcamera/pipeline/imx8-isi/imx8-isi.cpp | 231 +++++++++++++------\n> 3 files changed, 170 insertions(+), 76 deletions(-)\n> \n> diff --git a/include/libcamera/internal/camera_sensor.h b/include/libcamera/internal/camera_sensor.h\n> index b9f4d7867854..ce3a790f00ee 100644\n> --- a/include/libcamera/internal/camera_sensor.h\n> +++ b/include/libcamera/internal/camera_sensor.h\n> @@ -54,6 +54,7 @@ public:\n> \tV4L2SubdeviceFormat getFormat(const std::vector &mbusCodes,\n> \t\t\t\t const Size &size) const;\n> \tint setFormat(V4L2SubdeviceFormat *format);\n> +\tint tryFormat(V4L2SubdeviceFormat *format) const;\n> \n> \tconst ControlInfoMap &controls() const;\n> \tControlList getControls(const std::vector &ids);\n> diff --git a/src/libcamera/camera_sensor.cpp b/src/libcamera/camera_sensor.cpp\n> index a210aa4fa370..dfe593033def 100644\n> --- a/src/libcamera/camera_sensor.cpp\n> +++ b/src/libcamera/camera_sensor.cpp\n> @@ -757,6 +757,20 @@ int CameraSensor::setFormat(V4L2SubdeviceFormat *format)\n> \treturn 0;\n> }\n> \n> +/**\n> + * \\brief Try the sensor output format\n> + * \\param[in] format The desired sensor output format\n> + *\n> + * The ranges of any controls associated with the sensor are not updated.\n> + *\n> + * \\return 0 on success or a negative error code otherwise\n> + */\n> +int CameraSensor::tryFormat(V4L2SubdeviceFormat *format) const\n> +{\n> +\treturn subdev_->setFormat(pad_, format,\n> +\t\t\t\t V4L2Subdevice::Whence::TryFormat);\n> +}\n> +\n> /**\n> * \\brief Retrieve the supported V4L2 controls and their information\n> *\n> diff --git a/src/libcamera/pipeline/imx8-isi/imx8-isi.cpp b/src/libcamera/pipeline/imx8-isi/imx8-isi.cpp\n> index 5976a63d27dd..0e1e87c7a2aa 100644\n> --- a/src/libcamera/pipeline/imx8-isi/imx8-isi.cpp\n> +++ b/src/libcamera/pipeline/imx8-isi/imx8-isi.cpp\n> @@ -145,6 +145,10 @@ private:\n> \n> \tPipe *pipeFromStream(Camera *camera, const Stream *stream);\n> \n> +\tStreamConfiguration generateYUVConfiguration(Camera *camera,\n> +\t\t\t\t\t\t const Size &size);\n> +\tStreamConfiguration generateRawConfiguration(Camera *camera);\n> +\n> \tvoid bufferReady(FrameBuffer *buffer);\n> \n> \tMediaDevice *isiDev_;\n> @@ -705,6 +709,129 @@ PipelineHandlerISI::PipelineHandlerISI(CameraManager *manager)\n> {\n> }\n> \n> +/*\n> + * Generate a StreamConfiguration for YUV/RGB use case.\n> + *\n> + * Verify it the sensor can produce a YUV/RGB media bus format and collect\n> + * all the processed pixel formats the ISI can generate as supported stream\n> + * configurations.\n> + */\n> +StreamConfiguration PipelineHandlerISI::generateYUVConfiguration(Camera *camera,\n> +\t\t\t\t\t\t\t\t const Size &size)\n> +{\n> +\tISICameraData *data = cameraData(camera);\n> +\tPixelFormat pixelFormat = formats::YUYV;\n> +\tunsigned int mbusCode;\n> +\n> +\tmbusCode = data->getYuvMediaBusFormat(&pixelFormat);\n> +\tif (!mbusCode)\n> +\t\treturn {};\n> +\n> +\t/* Adjust the requested size to the sensor's capabilities. */\n> +\tconst CameraSensor *sensor = data->sensor_.get();\n> +\n> +\tV4L2SubdeviceFormat sensorFmt;\n> +\tsensorFmt.mbus_code = mbusCode;\n> +\tsensorFmt.size = size;\n> +\n> +\tint ret = sensor->tryFormat(&sensorFmt);\n> +\tif (ret) {\n> +\t\tLOG(ISI, Error) << \"Failed to try sensor format.\";\n> +\t\treturn {};\n> +\t}\n> +\n> +\tSize sensorSize = sensorFmt.size;\n> +\n> +\t/*\n> +\t * Populate the StreamConfiguration.\n> +\t *\n> +\t * As the sensor supports at least one YUV/RGB media bus format all the\n> +\t * processed ones in formatsMap_ can be generated from it.\n> +\t */\n> +\tstd::map> streamFormats;\n> +\n> +\tfor (const auto &[pixFmt, pipeFmt] : ISICameraConfiguration::formatsMap_) {\n> +\t\tconst PixelFormatInfo &info = PixelFormatInfo::info(pixFmt);\n> +\t\tif (info.colourEncoding == PixelFormatInfo::ColourEncodingRAW)\n> +\t\t\tcontinue;\n> +\n> +\t\tstreamFormats[pixFmt] = { { kMinISISize, sensorSize } };\n> +\t}\n> +\n> +\tStreamFormats formats(streamFormats);\n> +\n> +\tStreamConfiguration cfg(formats);\n> +\tcfg.pixelFormat = pixelFormat;\n> +\tcfg.size = sensorSize;\n> +\tcfg.bufferCount = 4;\n> +\n> +\treturn cfg;\n> +}\n> +\n> +/*\n> + * Generate a StreamConfiguration for Raw Bayer use case. Verify if the sensor\n> + * can produce the requested RAW bayer format and eventually adjust it to\n> + * the one with the largest bit-depth the sensor can produce.\n> + */\n> +StreamConfiguration PipelineHandlerISI::generateRawConfiguration(Camera *camera)\n> +{\n> +\tconst std::map rawFormats = {\n\nstatic?\n\n\nOther than that, looks good to me.\n\nReviewed-by: Paul Elder \n\n> +\t\t{ MEDIA_BUS_FMT_SBGGR8_1X8, formats::SBGGR8 },\n> +\t\t{ MEDIA_BUS_FMT_SGBRG8_1X8, formats::SGBRG8 },\n> +\t\t{ MEDIA_BUS_FMT_SGRBG8_1X8, formats::SGRBG8 },\n> +\t\t{ MEDIA_BUS_FMT_SRGGB8_1X8, formats::SRGGB8 },\n> +\t\t{ MEDIA_BUS_FMT_SBGGR10_1X10, formats::SBGGR10 },\n> +\t\t{ MEDIA_BUS_FMT_SGBRG10_1X10, formats::SGBRG10 },\n> +\t\t{ MEDIA_BUS_FMT_SGRBG10_1X10, formats::SGRBG10 },\n> +\t\t{ MEDIA_BUS_FMT_SRGGB10_1X10, formats::SRGGB10 },\n> +\t\t{ MEDIA_BUS_FMT_SBGGR12_1X12, formats::SBGGR12 },\n> +\t\t{ MEDIA_BUS_FMT_SGBRG12_1X12, formats::SGBRG12 },\n> +\t\t{ MEDIA_BUS_FMT_SGRBG12_1X12, formats::SGRBG12 },\n> +\t\t{ MEDIA_BUS_FMT_SRGGB12_1X12, formats::SRGGB12 },\n> +\t};\n> +\n> +\tISICameraData *data = cameraData(camera);\n> +\tPixelFormat pixelFormat = formats::SBGGR10;\n> +\tunsigned int mbusCode;\n> +\n> +\t/* pixelFormat will be adjusted, if the sensor can produce RAW. */\n> +\tmbusCode = data->getRawMediaBusFormat(&pixelFormat);\n> +\tif (!mbusCode)\n> +\t\treturn {};\n> +\n> +\t/*\n> +\t * Populate the StreamConfiguration with all the supported Bayer\n> +\t * formats the sensor can produce.\n> +\t */\n> +\tstd::map> streamFormats;\n> +\tconst CameraSensor *sensor = data->sensor_.get();\n> +\n> +\tfor (unsigned int code : sensor->mbusCodes()) {\n> +\t\t/* Find a Bayer media bus code from the sensor. */\n> +\t\tconst BayerFormat &bayerFormat = BayerFormat::fromMbusCode(code);\n> +\t\tif (!bayerFormat.isValid())\n> +\t\t\tcontinue;\n> +\n> +\t\tauto it = rawFormats.find(code);\n> +\t\tif (it == rawFormats.end()) {\n> +\t\t\tLOG(ISI, Warning) << bayerFormat\n> +\t\t\t\t\t << \" not supported in ISI formats map.\";\n> +\t\t\tcontinue;\n> +\t\t}\n> +\n> +\t\tstreamFormats[it->second] = { { kMinISISize, sensor->resolution() } };\n> +\t}\n> +\n> +\tStreamFormats formats(streamFormats);\n> +\n> +\tStreamConfiguration cfg(formats);\n> +\tcfg.size = sensor->resolution();\n> +\tcfg.pixelFormat = pixelFormat;\n> +\tcfg.bufferCount = 4;\n> +\n> +\treturn cfg;\n> +}\n> +\n> std::unique_ptr\n> PipelineHandlerISI::generateConfiguration(Camera *camera,\n> \t\t\t\t\t const StreamRoles &roles)\n> @@ -722,79 +849,45 @@ PipelineHandlerISI::generateConfiguration(Camera *camera,\n> \t\treturn nullptr;\n> \t}\n> \n> -\tbool isRaw = false;\n> \tfor (const auto &role : roles) {\n> \t\t/*\n> -\t\t * Prefer the following formats\n> +\t\t * Prefer the following formats:\n> \t\t * - Still Capture: Full resolution YUYV\n> \t\t * - ViewFinder/VideoRecording: 1080p YUYV\n> -\t\t * - RAW: sensor's native format and resolution\n> +\t\t * - RAW: Full resolution Bayer\n> \t\t */\n> -\t\tstd::map> streamFormats;\n> -\t\tPixelFormat pixelFormat;\n> -\t\tSize size;\n> +\t\tStreamConfiguration cfg;\n> \n> \t\tswitch (role) {\n> \t\tcase StreamRole::StillCapture:\n> -\t\t\t/*\n> -\t\t\t * \\todo Make sure the sensor can produce non-RAW formats\n> -\t\t\t * compatible with the ones supported by the pipeline.\n> -\t\t\t */\n> -\t\t\tsize = data->sensor_->resolution();\n> -\t\t\tpixelFormat = formats::YUYV;\n> +\t\t\tcfg = generateYUVConfiguration(camera,\n> +\t\t\t\t\t\t data->sensor_->resolution());\n> +\t\t\tif (!cfg.pixelFormat.isValid()) {\n> +\t\t\t\t/*\n> +\t\t\t\t * Fallback to use a Bayer format if that's what\n> +\t\t\t\t * the sensor supports.\n> +\t\t\t\t */\n> +\t\t\t\tcfg = generateRawConfiguration(camera);\n> +\t\t\t}\n> +\n> \t\t\tbreak;\n> \n> \t\tcase StreamRole::Viewfinder:\n> \t\tcase StreamRole::VideoRecording:\n> -\t\t\t/*\n> -\t\t\t * \\todo Make sure the sensor can produce non-RAW formats\n> -\t\t\t * compatible with the ones supported by the pipeline.\n> -\t\t\t */\n> -\t\t\tsize = PipelineHandlerISI::kPreviewSize;\n> -\t\t\tpixelFormat = formats::YUYV;\n> -\t\t\tbreak;\n> -\n> -\t\tcase StreamRole::Raw: {\n> -\t\t\t/*\n> -\t\t\t * Make sure the sensor can generate a RAW format and\n> -\t\t\t * prefer the ones with a larger bitdepth.\n> -\t\t\t */\n> -\t\t\tconst ISICameraConfiguration::FormatMap::value_type *rawPipeFormat = nullptr;\n> -\t\t\tunsigned int maxDepth = 0;\n> -\n> -\t\t\tfor (unsigned int code : data->sensor_->mbusCodes()) {\n> -\t\t\t\tconst BayerFormat &bayerFormat = BayerFormat::fromMbusCode(code);\n> -\t\t\t\tif (!bayerFormat.isValid())\n> -\t\t\t\t\tcontinue;\n> -\n> -\t\t\t\t/* Make sure the format is supported by the pipeline handler. */\n> -\t\t\t\tauto it = std::find_if(ISICameraConfiguration::formatsMap_.begin(),\n> -\t\t\t\t\t\t ISICameraConfiguration::formatsMap_.end(),\n> -\t\t\t\t\t\t [code](auto &isiFormat) {\n> -\t\t\t\t\t\t\t auto &pipe = isiFormat.second;\n> -\t\t\t\t\t\t\t return pipe.sensorCode == code;\n> -\t\t\t\t\t\t });\n> -\t\t\t\tif (it == ISICameraConfiguration::formatsMap_.end())\n> -\t\t\t\t\tcontinue;\n> -\n> -\t\t\t\tif (bayerFormat.bitDepth > maxDepth) {\n> -\t\t\t\t\tmaxDepth = bayerFormat.bitDepth;\n> -\t\t\t\t\trawPipeFormat = &(*it);\n> -\t\t\t\t}\n> -\t\t\t}\n> -\n> -\t\t\tif (!rawPipeFormat) {\n> -\t\t\t\tLOG(ISI, Error)\n> -\t\t\t\t\t<< \"Cannot generate a configuration for RAW stream\";\n> -\t\t\t\treturn nullptr;\n> +\t\t\tcfg = generateYUVConfiguration(camera,\n> +\t\t\t\t\t\t PipelineHandlerISI::kPreviewSize);\n> +\t\t\tif (!cfg.pixelFormat.isValid()) {\n> +\t\t\t\t/*\n> +\t\t\t\t * Fallback to use a Bayer format if that's what\n> +\t\t\t\t * the sensor supports.\n> +\t\t\t\t */\n> +\t\t\t\tcfg = generateRawConfiguration(camera);\n> \t\t\t}\n> \n> -\t\t\tsize = data->sensor_->resolution();\n> -\t\t\tpixelFormat = rawPipeFormat->first;\n> -\n> -\t\t\tstreamFormats[pixelFormat] = { { kMinISISize, size } };\n> -\t\t\tisRaw = true;\n> +\t\t\tbreak;\n> \n> +\t\tcase StreamRole::Raw: {\n> +\t\t\tcfg = generateRawConfiguration(camera);\n> \t\t\tbreak;\n> \t\t}\n> \n> @@ -803,26 +896,12 @@ PipelineHandlerISI::generateConfiguration(Camera *camera,\n> \t\t\treturn nullptr;\n> \t\t}\n> \n> -\t\t/*\n> -\t\t * For non-RAW configurations the ISI can perform colorspace\n> -\t\t * conversion. List all the supported output formats here.\n> -\t\t */\n> -\t\tif (!isRaw) {\n> -\t\t\tfor (const auto &[pixFmt, pipeFmt] : ISICameraConfiguration::formatsMap_) {\n> -\t\t\t\tconst PixelFormatInfo &info = PixelFormatInfo::info(pixFmt);\n> -\t\t\t\tif (info.colourEncoding == PixelFormatInfo::ColourEncodingRAW)\n> -\t\t\t\t\tcontinue;\n> -\n> -\t\t\t\tstreamFormats[pixFmt] = { { kMinISISize, size } };\n> -\t\t\t}\n> +\t\tif (!cfg.pixelFormat.isValid()) {\n> +\t\t\tLOG(ISI, Error)\n> +\t\t\t\t<< \"Cannot generate configuration for role: \" << role;\n> +\t\t\treturn nullptr;\n> \t\t}\n> \n> -\t\tStreamFormats formats(streamFormats);\n> -\n> -\t\tStreamConfiguration cfg(formats);\n> -\t\tcfg.pixelFormat = pixelFormat;\n> -\t\tcfg.size = size;\n> -\t\tcfg.bufferCount = 4;\n> \t\tconfig->addConfiguration(cfg);\n> \t}\n> \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 EF9B4BE080\n\tfor ;\n\tThu, 2 Mar 2023 12:12:22 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id 3ED5B6269C;\n\tThu, 2 Mar 2023 13:12: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 DAA966267E\n\tfor ;\n\tThu, 2 Mar 2023 13:12:19 +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 52D9B56A;\n\tThu, 2 Mar 2023 13:12:18 +0100 (CET)"],"DKIM-Signature":["v=1; a=rsa-sha256; c=relaxed/simple; d=libcamera.org;\n\ts=mail; t=1677759142;\n\tbh=bPWoFBBLy0+ZsInbgjBtbfFvmFMsSewLwg3TCMW2oHY=;\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=wDDzFznmLQsUN3ntgdHRMhc4RhcUaCgufgZMG0td61bZBFOtzQRhba0eGa2XoNCKa\n\tp1kb54RbTj8Obvdlyc5rBcxIvQPsaWPsY7ItS8JmYF+DT+zz77EZ/y8EvtNGIqn203\n\tyFPX1fvQ+MNWidh3R8up/AE5VmM2ZUoY9QWaRxx+bJ8ugAfyGBqrT9dEcriQ1cIjc8\n\tstpL2UjfLg14CENHZU/L5x/gws+aZ3budtWlUDgbf/v270nllvUFU7IRUB3eoIB7Qw\n\tr/t6khzCKtJdvNincr78Gdpivk0gE+s8BS4dEH3NoJXDfatDmPRoPaQxArq8QtMHUc\n\tCKNg9EQxMP1OA==","v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com;\n\ts=mail; t=1677759139;\n\tbh=bPWoFBBLy0+ZsInbgjBtbfFvmFMsSewLwg3TCMW2oHY=;\n\th=Date:From:To:Cc:Subject:References:In-Reply-To:From;\n\tb=O7Tp/AK5L1W4l5U2EcewuAB+OquefKKZFphc41iHxQQIWF23gpwTQbI9EYp8v92Ia\n\tnClrFmT/5BvatyhTvcd5lp2EA8K9UZEpspvPjzgbS3RSnLYK9bvGEAoYFpcZoEp1s3\n\tJCqwKd5sRHWM2VhQ9oCiuRQ1CAec7eN+zgfrO5no="],"Authentication-Results":"lancelot.ideasonboard.com; dkim=pass (1024-bit key; \n\tunprotected) header.d=ideasonboard.com\n\theader.i=@ideasonboard.com\n\theader.b=\"O7Tp/AK5\"; dkim-atps=neutral","Date":"Thu, 2 Mar 2023 21:12:12 +0900","To":"Jacopo Mondi ","Message-ID":"","References":"<20230129135830.27490-1-jacopo.mondi@ideasonboard.com>\n\t<20230129135830.27490-5-jacopo.mondi@ideasonboard.com>","MIME-Version":"1.0","Content-Type":"text/plain; charset=us-ascii","Content-Disposition":"inline","In-Reply-To":"<20230129135830.27490-5-jacopo.mondi@ideasonboard.com>","Subject":"Re: [libcamera-devel] [PATCH 4/5] libcamera: imx8-isi: Split\n\tBayer/YUV config generation","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":26589,"web_url":"https://patchwork.libcamera.org/comment/26589/","msgid":"","date":"2023-03-07T14:14:31","subject":"Re: [libcamera-devel] [PATCH 4/5] libcamera: imx8-isi: Split\n\tBayer/YUV config generation","submitter":{"id":156,"url":"https://patchwork.libcamera.org/api/people/156/","name":"Dan Scally","email":"dan.scally@ideasonboard.com"},"content":"Hi Jacopo\n\nOn 29/01/2023 13:58, Jacopo Mondi wrote:\n> At generateConfiguration() a YUV/RGB pixel format is preferred for the\n> StillCapture/VideoRecording/Viewfinder roles, but currently there are no\n> guarantees in place that the sensor provides a non-Bayer bus format from\n> which YUV/RGB can be generated.\n>\n> This makes the default configuration generated for those roles not to\n> work if the sensor is a RAW-only one.\n>\n> To improve the situation split the configuration generation in two,\n> one for YUV modes and one for Raw Bayer mode.\n>\n> StreamRoles assigned to a YUV mode will try to first generate a YUV\n> configuration and then fallback to RAW if that's what the sensor can\n> provide.\n>\n> As an additional requirement, for YUV streams, the generated mode has to\n> be validated with the sensor to confirm the desired sizes can be\n> generated. In order to test a format on the sensor introduce\n> CameraSensor::tryFormat().\n>\n> Reported-by: Laurent Pinchart \n> Signed-off-by: Jacopo Mondi \n> Reviewed-by: Paul Elder \n> ---\n> include/libcamera/internal/camera_sensor.h | 1 +\n> src/libcamera/camera_sensor.cpp | 14 ++\n> src/libcamera/pipeline/imx8-isi/imx8-isi.cpp | 231 +++++++++++++------\n> 3 files changed, 170 insertions(+), 76 deletions(-)\n>\n> diff --git a/include/libcamera/internal/camera_sensor.h b/include/libcamera/internal/camera_sensor.h\n> index b9f4d7867854..ce3a790f00ee 100644\n> --- a/include/libcamera/internal/camera_sensor.h\n> +++ b/include/libcamera/internal/camera_sensor.h\n> @@ -54,6 +54,7 @@ public:\n> \tV4L2SubdeviceFormat getFormat(const std::vector &mbusCodes,\n> \t\t\t\t const Size &size) const;\n> \tint setFormat(V4L2SubdeviceFormat *format);\n> +\tint tryFormat(V4L2SubdeviceFormat *format) const;\n> \n> \tconst ControlInfoMap &controls() const;\n> \tControlList getControls(const std::vector &ids);\n> diff --git a/src/libcamera/camera_sensor.cpp b/src/libcamera/camera_sensor.cpp\n> index a210aa4fa370..dfe593033def 100644\n> --- a/src/libcamera/camera_sensor.cpp\n> +++ b/src/libcamera/camera_sensor.cpp\n> @@ -757,6 +757,20 @@ int CameraSensor::setFormat(V4L2SubdeviceFormat *format)\n> \treturn 0;\n> }\n> \n> +/**\n> + * \\brief Try the sensor output format\n> + * \\param[in] format The desired sensor output format\n> + *\n> + * The ranges of any controls associated with the sensor are not updated.\n> + *\n> + * \\return 0 on success or a negative error code otherwise\n> + */\n> +int CameraSensor::tryFormat(V4L2SubdeviceFormat *format) const\n> +{\n> +\treturn subdev_->setFormat(pad_, format,\n> +\t\t\t\t V4L2Subdevice::Whence::TryFormat);\n> +}\n> +\n> /**\n> * \\brief Retrieve the supported V4L2 controls and their information\n> *\n> diff --git a/src/libcamera/pipeline/imx8-isi/imx8-isi.cpp b/src/libcamera/pipeline/imx8-isi/imx8-isi.cpp\n> index 5976a63d27dd..0e1e87c7a2aa 100644\n> --- a/src/libcamera/pipeline/imx8-isi/imx8-isi.cpp\n> +++ b/src/libcamera/pipeline/imx8-isi/imx8-isi.cpp\n> @@ -145,6 +145,10 @@ private:\n> \n> \tPipe *pipeFromStream(Camera *camera, const Stream *stream);\n> \n> +\tStreamConfiguration generateYUVConfiguration(Camera *camera,\n> +\t\t\t\t\t\t const Size &size);\n> +\tStreamConfiguration generateRawConfiguration(Camera *camera);\n> +\n> \tvoid bufferReady(FrameBuffer *buffer);\n> \n> \tMediaDevice *isiDev_;\n> @@ -705,6 +709,129 @@ PipelineHandlerISI::PipelineHandlerISI(CameraManager *manager)\n> {\n> }\n> \n> +/*\n> + * Generate a StreamConfiguration for YUV/RGB use case.\n> + *\n> + * Verify it the sensor can produce a YUV/RGB media bus format and collect\n> + * all the processed pixel formats the ISI can generate as supported stream\n> + * configurations.\n> + */\n> +StreamConfiguration PipelineHandlerISI::generateYUVConfiguration(Camera *camera,\n> +\t\t\t\t\t\t\t\t const Size &size)\n> +{\n> +\tISICameraData *data = cameraData(camera);\n> +\tPixelFormat pixelFormat = formats::YUYV;\n> +\tunsigned int mbusCode;\n> +\n> +\tmbusCode = data->getYuvMediaBusFormat(&pixelFormat);\n> +\tif (!mbusCode)\n> +\t\treturn {};\n> +\n> +\t/* Adjust the requested size to the sensor's capabilities. */\n> +\tconst CameraSensor *sensor = data->sensor_.get();\n> +\n> +\tV4L2SubdeviceFormat sensorFmt;\n> +\tsensorFmt.mbus_code = mbusCode;\n> +\tsensorFmt.size = size;\n> +\n> +\tint ret = sensor->tryFormat(&sensorFmt);\n> +\tif (ret) {\n> +\t\tLOG(ISI, Error) << \"Failed to try sensor format.\";\n> +\t\treturn {};\n> +\t}\n> +\n> +\tSize sensorSize = sensorFmt.size;\n> +\n> +\t/*\n> +\t * Populate the StreamConfiguration.\n> +\t *\n> +\t * As the sensor supports at least one YUV/RGB media bus format all the\n> +\t * processed ones in formatsMap_ can be generated from it.\n> +\t */\n> +\tstd::map> streamFormats;\n> +\n> +\tfor (const auto &[pixFmt, pipeFmt] : ISICameraConfiguration::formatsMap_) {\n> +\t\tconst PixelFormatInfo &info = PixelFormatInfo::info(pixFmt);\n> +\t\tif (info.colourEncoding == PixelFormatInfo::ColourEncodingRAW)\n> +\t\t\tcontinue;\n> +\n> +\t\tstreamFormats[pixFmt] = { { kMinISISize, sensorSize } };\n> +\t}\n> +\n> +\tStreamFormats formats(streamFormats);\n> +\n> +\tStreamConfiguration cfg(formats);\n> +\tcfg.pixelFormat = pixelFormat;\n> +\tcfg.size = sensorSize;\n> +\tcfg.bufferCount = 4;\n> +\n> +\treturn cfg;\n> +}\n> +\n> +/*\n> + * Generate a StreamConfiguration for Raw Bayer use case. Verify if the sensor\n> + * can produce the requested RAW bayer format and eventually adjust it to\n> + * the one with the largest bit-depth the sensor can produce.\n> + */\n> +StreamConfiguration PipelineHandlerISI::generateRawConfiguration(Camera *camera)\n> +{\n> +\tconst std::map rawFormats = {\n> +\t\t{ MEDIA_BUS_FMT_SBGGR8_1X8, formats::SBGGR8 },\n> +\t\t{ MEDIA_BUS_FMT_SGBRG8_1X8, formats::SGBRG8 },\n> +\t\t{ MEDIA_BUS_FMT_SGRBG8_1X8, formats::SGRBG8 },\n> +\t\t{ MEDIA_BUS_FMT_SRGGB8_1X8, formats::SRGGB8 },\n> +\t\t{ MEDIA_BUS_FMT_SBGGR10_1X10, formats::SBGGR10 },\n> +\t\t{ MEDIA_BUS_FMT_SGBRG10_1X10, formats::SGBRG10 },\n> +\t\t{ MEDIA_BUS_FMT_SGRBG10_1X10, formats::SGRBG10 },\n> +\t\t{ MEDIA_BUS_FMT_SRGGB10_1X10, formats::SRGGB10 },\n> +\t\t{ MEDIA_BUS_FMT_SBGGR12_1X12, formats::SBGGR12 },\n> +\t\t{ MEDIA_BUS_FMT_SGBRG12_1X12, formats::SGBRG12 },\n> +\t\t{ MEDIA_BUS_FMT_SGRBG12_1X12, formats::SGRBG12 },\n> +\t\t{ MEDIA_BUS_FMT_SRGGB12_1X12, formats::SRGGB12 },\n> +\t};\n> +\n> +\tISICameraData *data = cameraData(camera);\n> +\tPixelFormat pixelFormat = formats::SBGGR10;\n> +\tunsigned int mbusCode;\n> +\n> +\t/* pixelFormat will be adjusted, if the sensor can produce RAW. */\n> +\tmbusCode = data->getRawMediaBusFormat(&pixelFormat);\n> +\tif (!mbusCode)\n> +\t\treturn {};\n> +\n> +\t/*\n> +\t * Populate the StreamConfiguration with all the supported Bayer\n> +\t * formats the sensor can produce.\n> +\t */\n> +\tstd::map> streamFormats;\n> +\tconst CameraSensor *sensor = data->sensor_.get();\n> +\n> +\tfor (unsigned int code : sensor->mbusCodes()) {\n> +\t\t/* Find a Bayer media bus code from the sensor. */\n> +\t\tconst BayerFormat &bayerFormat = BayerFormat::fromMbusCode(code);\n> +\t\tif (!bayerFormat.isValid())\n> +\t\t\tcontinue;\n> +\n> +\t\tauto it = rawFormats.find(code);\n> +\t\tif (it == rawFormats.end()) {\n> +\t\t\tLOG(ISI, Warning) << bayerFormat\n> +\t\t\t\t\t << \" not supported in ISI formats map.\";\n> +\t\t\tcontinue;\n> +\t\t}\n> +\n> +\t\tstreamFormats[it->second] = { { kMinISISize, sensor->resolution() } };\n> +\t}\n> +\n> +\tStreamFormats formats(streamFormats);\n> +\n> +\tStreamConfiguration cfg(formats);\n> +\tcfg.size = sensor->resolution();\n> +\tcfg.pixelFormat = pixelFormat;\n> +\tcfg.bufferCount = 4;\n> +\n> +\treturn cfg;\n> +}\n> +\n> std::unique_ptr\n> PipelineHandlerISI::generateConfiguration(Camera *camera,\n> \t\t\t\t\t const StreamRoles &roles)\n> @@ -722,79 +849,45 @@ PipelineHandlerISI::generateConfiguration(Camera *camera,\n> \t\treturn nullptr;\n> \t}\n> \n> -\tbool isRaw = false;\n> \tfor (const auto &role : roles) {\n> \t\t/*\n> -\t\t * Prefer the following formats\n> +\t\t * Prefer the following formats:\n> \t\t * - Still Capture: Full resolution YUYV\n> \t\t * - ViewFinder/VideoRecording: 1080p YUYV\n> -\t\t * - RAW: sensor's native format and resolution\n> +\t\t * - RAW: Full resolution Bayer\n> \t\t */\n> -\t\tstd::map> streamFormats;\n> -\t\tPixelFormat pixelFormat;\n> -\t\tSize size;\n> +\t\tStreamConfiguration cfg;\n> \n> \t\tswitch (role) {\n> \t\tcase StreamRole::StillCapture:\n> -\t\t\t/*\n> -\t\t\t * \\todo Make sure the sensor can produce non-RAW formats\n> -\t\t\t * compatible with the ones supported by the pipeline.\n> -\t\t\t */\n> -\t\t\tsize = data->sensor_->resolution();\n> -\t\t\tpixelFormat = formats::YUYV;\n> +\t\t\tcfg = generateYUVConfiguration(camera,\n> +\t\t\t\t\t\t data->sensor_->resolution());\n> +\t\t\tif (!cfg.pixelFormat.isValid()) {\n> +\t\t\t\t/*\n> +\t\t\t\t * Fallback to use a Bayer format if that's what\n> +\t\t\t\t * the sensor supports.\n> +\t\t\t\t */\n> +\t\t\t\tcfg = generateRawConfiguration(camera);\n> +\t\t\t}\n> +\n> \t\t\tbreak;\n> \n> \t\tcase StreamRole::Viewfinder:\n> \t\tcase StreamRole::VideoRecording:\n> -\t\t\t/*\n> -\t\t\t * \\todo Make sure the sensor can produce non-RAW formats\n> -\t\t\t * compatible with the ones supported by the pipeline.\n> -\t\t\t */\n> -\t\t\tsize = PipelineHandlerISI::kPreviewSize;\n> -\t\t\tpixelFormat = formats::YUYV;\n> -\t\t\tbreak;\n> -\n> -\t\tcase StreamRole::Raw: {\n> -\t\t\t/*\n> -\t\t\t * Make sure the sensor can generate a RAW format and\n> -\t\t\t * prefer the ones with a larger bitdepth.\n> -\t\t\t */\n> -\t\t\tconst ISICameraConfiguration::FormatMap::value_type *rawPipeFormat = nullptr;\n> -\t\t\tunsigned int maxDepth = 0;\n> -\n> -\t\t\tfor (unsigned int code : data->sensor_->mbusCodes()) {\n> -\t\t\t\tconst BayerFormat &bayerFormat = BayerFormat::fromMbusCode(code);\n> -\t\t\t\tif (!bayerFormat.isValid())\n> -\t\t\t\t\tcontinue;\n> -\n> -\t\t\t\t/* Make sure the format is supported by the pipeline handler. */\n> -\t\t\t\tauto it = std::find_if(ISICameraConfiguration::formatsMap_.begin(),\n> -\t\t\t\t\t\t ISICameraConfiguration::formatsMap_.end(),\n> -\t\t\t\t\t\t [code](auto &isiFormat) {\n> -\t\t\t\t\t\t\t auto &pipe = isiFormat.second;\n> -\t\t\t\t\t\t\t return pipe.sensorCode == code;\n> -\t\t\t\t\t\t });\n> -\t\t\t\tif (it == ISICameraConfiguration::formatsMap_.end())\n> -\t\t\t\t\tcontinue;\n> -\n> -\t\t\t\tif (bayerFormat.bitDepth > maxDepth) {\n> -\t\t\t\t\tmaxDepth = bayerFormat.bitDepth;\n> -\t\t\t\t\trawPipeFormat = &(*it);\n> -\t\t\t\t}\n> -\t\t\t}\n> -\n> -\t\t\tif (!rawPipeFormat) {\n> -\t\t\t\tLOG(ISI, Error)\n> -\t\t\t\t\t<< \"Cannot generate a configuration for RAW stream\";\n> -\t\t\t\treturn nullptr;\n> +\t\t\tcfg = generateYUVConfiguration(camera,\n> +\t\t\t\t\t\t PipelineHandlerISI::kPreviewSize);\n> +\t\t\tif (!cfg.pixelFormat.isValid()) {\n> +\t\t\t\t/*\n> +\t\t\t\t * Fallback to use a Bayer format if that's what\n> +\t\t\t\t * the sensor supports.\n> +\t\t\t\t */\n> +\t\t\t\tcfg = generateRawConfiguration(camera);\n> \t\t\t}\n\n\nNot a big deal at all so feel free to ignore me, but I think I'd have kept the resolution the same \nand just changed the pixel format - it threw me briefly in testing that it wasn't configuring 1080p \nas I expected in ViewFinder role.\n\n> \n> -\t\t\tsize = data->sensor_->resolution();\n> -\t\t\tpixelFormat = rawPipeFormat->first;\n> -\n> -\t\t\tstreamFormats[pixelFormat] = { { kMinISISize, size } };\n> -\t\t\tisRaw = true;\n> +\t\t\tbreak;\n> \n> +\t\tcase StreamRole::Raw: {\n> +\t\t\tcfg = generateRawConfiguration(camera);\n> \t\t\tbreak;\n> \t\t}\n> \n> @@ -803,26 +896,12 @@ PipelineHandlerISI::generateConfiguration(Camera *camera,\n> \t\t\treturn nullptr;\n> \t\t}\n> \n> -\t\t/*\n> -\t\t * For non-RAW configurations the ISI can perform colorspace\n> -\t\t * conversion. List all the supported output formats here.\n> -\t\t */\n> -\t\tif (!isRaw) {\n> -\t\t\tfor (const auto &[pixFmt, pipeFmt] : ISICameraConfiguration::formatsMap_) {\n> -\t\t\t\tconst PixelFormatInfo &info = PixelFormatInfo::info(pixFmt);\n> -\t\t\t\tif (info.colourEncoding == PixelFormatInfo::ColourEncodingRAW)\n> -\t\t\t\t\tcontinue;\n> -\n> -\t\t\t\tstreamFormats[pixFmt] = { { kMinISISize, size } };\n> -\t\t\t}\n> +\t\tif (!cfg.pixelFormat.isValid()) {\n> +\t\t\tLOG(ISI, Error)\n> +\t\t\t\t<< \"Cannot generate configuration for role: \" << role;\n> +\t\t\treturn nullptr;\n> \t\t}\n> \n> -\t\tStreamFormats formats(streamFormats);\n> -\n> -\t\tStreamConfiguration cfg(formats);\n> -\t\tcfg.pixelFormat = pixelFormat;\n> -\t\tcfg.size = size;\n> -\t\tcfg.bufferCount = 4;\n> \t\tconfig->addConfiguration(cfg);\n> \t}\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 CDEC8BDE17\n\tfor ;\n\tTue, 7 Mar 2023 14:14:36 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id EB83D626B4;\n\tTue, 7 Mar 2023 15:14:35 +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 ADC4362693\n\tfor ;\n\tTue, 7 Mar 2023 15:14:34 +0100 (CET)","from [192.168.0.43]\n\t(cpc141996-chfd3-2-0-cust928.12-3.cable.virginm.net [86.13.91.161])\n\tby perceval.ideasonboard.com (Postfix) with ESMTPSA id 1FE3C4AD;\n\tTue, 7 Mar 2023 15:14:34 +0100 (CET)"],"DKIM-Signature":["v=1; a=rsa-sha256; c=relaxed/simple; d=libcamera.org;\n\ts=mail; t=1678198476;\n\tbh=gYulHSXhr6uT6z0e6fkhTFBGP5MNGJ7o7Cwfsd81/8s=;\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:\n\tFrom;\n\tb=iILs44DP9OLIHAu1apaczabdufQNxG03f5p7qLCCH6+M8eWADnfUnWx8tuL70PpBj\n\tVSHBGe7SGfUboTxx6fhr22jsE8/+SdvPegidEkYhRoVsbgNaUTFzad8ossiyw7Vj9N\n\tqwMDJhyMEzvWMX448FAgHysR5DWzkxpHnR0ZWezJ44f6j0MDSVZGGrkTJCo2MH7/2v\n\tYEqpxjyghLgSwaTJWEg93ghtmvzQoXFawereGrqEFmNGMF50IaHoaro2yXbyrg6ZC+\n\tS1MgfmJkMITzvt8CegsCHLTd/ZmNnZ9pRWniGibEhBAVM9+mg3SAaQvt/Gk9CGfRRP\n\tyePZhOfz2dtkg==","v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com;\n\ts=mail; t=1678198474;\n\tbh=gYulHSXhr6uT6z0e6fkhTFBGP5MNGJ7o7Cwfsd81/8s=;\n\th=Date:Subject:To:References:From:In-Reply-To:From;\n\tb=elmH3kYgCdSJsa3vKQXKLaaeiDtJgmas90QFDyYWEO3fpNWX115hxBhzo9Ml4zot6\n\tZuXfjSxYlqHdLNufw9WszKfcCOivLlEbDj8iG73CEiftZb6QGmOHT2PVY+1pI04T+s\n\tPr50vtMLtrloJHcOCuF7uuBKjZ9U6DGk+umiW5rI="],"Authentication-Results":"lancelot.ideasonboard.com; dkim=pass (1024-bit key; \n\tunprotected) header.d=ideasonboard.com\n\theader.i=@ideasonboard.com\n\theader.b=\"elmH3kYg\"; dkim-atps=neutral","Message-ID":"","Date":"Tue, 7 Mar 2023 14:14:31 +0000","MIME-Version":"1.0","User-Agent":"Mozilla/5.0 (X11; Linux x86_64; rv:102.0) Gecko/20100101\n\tThunderbird/102.7.1","Content-Language":"en-US","To":"Jacopo Mondi ,\n\tlibcamera-devel@lists.libcamera.org","References":"<20230129135830.27490-1-jacopo.mondi@ideasonboard.com>\n\t<20230129135830.27490-5-jacopo.mondi@ideasonboard.com>","In-Reply-To":"<20230129135830.27490-5-jacopo.mondi@ideasonboard.com>","Content-Type":"text/plain; charset=UTF-8; format=flowed","Content-Transfer-Encoding":"7bit","Subject":"Re: [libcamera-devel] [PATCH 4/5] libcamera: imx8-isi: Split\n\tBayer/YUV config generation","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":"Dan Scally via libcamera-devel ","Reply-To":"Dan Scally ","Errors-To":"libcamera-devel-bounces@lists.libcamera.org","Sender":"\"libcamera-devel\" "}},{"id":26642,"web_url":"https://patchwork.libcamera.org/comment/26642/","msgid":"<20230312165044.GB2545@pendragon.ideasonboard.com>","date":"2023-03-12T16:50:44","subject":"Re: [libcamera-devel] [PATCH 4/5] libcamera: imx8-isi: Split\n\tBayer/YUV config generation","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:29PM +0100, Jacopo Mondi wrote:\n> At generateConfiguration() a YUV/RGB pixel format is preferred for the\n> StillCapture/VideoRecording/Viewfinder roles, but currently there are no\n> guarantees in place that the sensor provides a non-Bayer bus format from\n> which YUV/RGB can be generated.\n> \n> This makes the default configuration generated for those roles not to\n> work if the sensor is a RAW-only one.\n> \n> To improve the situation split the configuration generation in two,\n> one for YUV modes and one for Raw Bayer mode.\n> \n> StreamRoles assigned to a YUV mode will try to first generate a YUV\n> configuration and then fallback to RAW if that's what the sensor can\n> provide.\n> \n> As an additional requirement, for YUV streams, the generated mode has to\n> be validated with the sensor to confirm the desired sizes can be\n> generated. In order to test a format on the sensor introduce\n> CameraSensor::tryFormat().\n\nI agree with Paul, please split the new function to a separate patch.\n\n> Reported-by: Laurent Pinchart \n> Signed-off-by: Jacopo Mondi \n> ---\n> include/libcamera/internal/camera_sensor.h | 1 +\n> src/libcamera/camera_sensor.cpp | 14 ++\n> src/libcamera/pipeline/imx8-isi/imx8-isi.cpp | 231 +++++++++++++------\n> 3 files changed, 170 insertions(+), 76 deletions(-)\n> \n> diff --git a/include/libcamera/internal/camera_sensor.h b/include/libcamera/internal/camera_sensor.h\n> index b9f4d7867854..ce3a790f00ee 100644\n> --- a/include/libcamera/internal/camera_sensor.h\n> +++ b/include/libcamera/internal/camera_sensor.h\n> @@ -54,6 +54,7 @@ public:\n> \tV4L2SubdeviceFormat getFormat(const std::vector &mbusCodes,\n> \t\t\t\t const Size &size) const;\n> \tint setFormat(V4L2SubdeviceFormat *format);\n> +\tint tryFormat(V4L2SubdeviceFormat *format) const;\n> \n> \tconst ControlInfoMap &controls() const;\n> \tControlList getControls(const std::vector &ids);\n> diff --git a/src/libcamera/camera_sensor.cpp b/src/libcamera/camera_sensor.cpp\n> index a210aa4fa370..dfe593033def 100644\n> --- a/src/libcamera/camera_sensor.cpp\n> +++ b/src/libcamera/camera_sensor.cpp\n> @@ -757,6 +757,20 @@ int CameraSensor::setFormat(V4L2SubdeviceFormat *format)\n> \treturn 0;\n> }\n> \n> +/**\n> + * \\brief Try the sensor output format\n> + * \\param[in] format The desired sensor output format\n> + *\n> + * The ranges of any controls associated with the sensor are not updated.\n> + *\n> + * \\return 0 on success or a negative error code otherwise\n> + */\n> +int CameraSensor::tryFormat(V4L2SubdeviceFormat *format) const\n> +{\n> +\treturn subdev_->setFormat(pad_, format,\n> +\t\t\t\t V4L2Subdevice::Whence::TryFormat);\n> +}\n> +\n> /**\n> * \\brief Retrieve the supported V4L2 controls and their information\n> *\n> diff --git a/src/libcamera/pipeline/imx8-isi/imx8-isi.cpp b/src/libcamera/pipeline/imx8-isi/imx8-isi.cpp\n> index 5976a63d27dd..0e1e87c7a2aa 100644\n> --- a/src/libcamera/pipeline/imx8-isi/imx8-isi.cpp\n> +++ b/src/libcamera/pipeline/imx8-isi/imx8-isi.cpp\n> @@ -145,6 +145,10 @@ private:\n> \n> \tPipe *pipeFromStream(Camera *camera, const Stream *stream);\n> \n> +\tStreamConfiguration generateYUVConfiguration(Camera *camera,\n> +\t\t\t\t\t\t const Size &size);\n> +\tStreamConfiguration generateRawConfiguration(Camera *camera);\n> +\n> \tvoid bufferReady(FrameBuffer *buffer);\n> \n> \tMediaDevice *isiDev_;\n> @@ -705,6 +709,129 @@ PipelineHandlerISI::PipelineHandlerISI(CameraManager *manager)\n> {\n> }\n> \n> +/*\n> + * Generate a StreamConfiguration for YUV/RGB use case.\n> + *\n> + * Verify it the sensor can produce a YUV/RGB media bus format and collect\n> + * all the processed pixel formats the ISI can generate as supported stream\n> + * configurations.\n> + */\n> +StreamConfiguration PipelineHandlerISI::generateYUVConfiguration(Camera *camera,\n> +\t\t\t\t\t\t\t\t const Size &size)\n> +{\n> +\tISICameraData *data = cameraData(camera);\n> +\tPixelFormat pixelFormat = formats::YUYV;\n> +\tunsigned int mbusCode;\n> +\n> +\tmbusCode = data->getYuvMediaBusFormat(&pixelFormat);\n> +\tif (!mbusCode)\n> +\t\treturn {};\n> +\n> +\t/* Adjust the requested size to the sensor's capabilities. */\n> +\tconst CameraSensor *sensor = data->sensor_.get();\n> +\n> +\tV4L2SubdeviceFormat sensorFmt;\n> +\tsensorFmt.mbus_code = mbusCode;\n> +\tsensorFmt.size = size;\n> +\n> +\tint ret = sensor->tryFormat(&sensorFmt);\n> +\tif (ret) {\n> +\t\tLOG(ISI, Error) << \"Failed to try sensor format.\";\n> +\t\treturn {};\n> +\t}\n> +\n> +\tSize sensorSize = sensorFmt.size;\n> +\n> +\t/*\n> +\t * Populate the StreamConfiguration.\n> +\t *\n> +\t * As the sensor supports at least one YUV/RGB media bus format all the\n> +\t * processed ones in formatsMap_ can be generated from it.\n> +\t */\n> +\tstd::map> streamFormats;\n> +\n> +\tfor (const auto &[pixFmt, pipeFmt] : ISICameraConfiguration::formatsMap_) {\n> +\t\tconst PixelFormatInfo &info = PixelFormatInfo::info(pixFmt);\n> +\t\tif (info.colourEncoding == PixelFormatInfo::ColourEncodingRAW)\n> +\t\t\tcontinue;\n> +\n> +\t\tstreamFormats[pixFmt] = { { kMinISISize, sensorSize } };\n> +\t}\n> +\n> +\tStreamFormats formats(streamFormats);\n> +\n> +\tStreamConfiguration cfg(formats);\n> +\tcfg.pixelFormat = pixelFormat;\n> +\tcfg.size = sensorSize;\n> +\tcfg.bufferCount = 4;\n> +\n> +\treturn cfg;\n> +}\n> +\n> +/*\n> + * Generate a StreamConfiguration for Raw Bayer use case. Verify if the sensor\n> + * can produce the requested RAW bayer format and eventually adjust it to\n> + * the one with the largest bit-depth the sensor can produce.\n> + */\n> +StreamConfiguration PipelineHandlerISI::generateRawConfiguration(Camera *camera)\n> +{\n> +\tconst std::map rawFormats = {\n\nstatic const\n\n> +\t\t{ MEDIA_BUS_FMT_SBGGR8_1X8, formats::SBGGR8 },\n> +\t\t{ MEDIA_BUS_FMT_SGBRG8_1X8, formats::SGBRG8 },\n> +\t\t{ MEDIA_BUS_FMT_SGRBG8_1X8, formats::SGRBG8 },\n> +\t\t{ MEDIA_BUS_FMT_SRGGB8_1X8, formats::SRGGB8 },\n> +\t\t{ MEDIA_BUS_FMT_SBGGR10_1X10, formats::SBGGR10 },\n> +\t\t{ MEDIA_BUS_FMT_SGBRG10_1X10, formats::SGBRG10 },\n> +\t\t{ MEDIA_BUS_FMT_SGRBG10_1X10, formats::SGRBG10 },\n> +\t\t{ MEDIA_BUS_FMT_SRGGB10_1X10, formats::SRGGB10 },\n> +\t\t{ MEDIA_BUS_FMT_SBGGR12_1X12, formats::SBGGR12 },\n> +\t\t{ MEDIA_BUS_FMT_SGBRG12_1X12, formats::SGBRG12 },\n> +\t\t{ MEDIA_BUS_FMT_SGRBG12_1X12, formats::SGRBG12 },\n> +\t\t{ MEDIA_BUS_FMT_SRGGB12_1X12, formats::SRGGB12 },\n> +\t};\n> +\n> +\tISICameraData *data = cameraData(camera);\n> +\tPixelFormat pixelFormat = formats::SBGGR10;\n> +\tunsigned int mbusCode;\n> +\n> +\t/* pixelFormat will be adjusted, if the sensor can produce RAW. */\n> +\tmbusCode = data->getRawMediaBusFormat(&pixelFormat);\n> +\tif (!mbusCode)\n> +\t\treturn {};\n> +\n> +\t/*\n> +\t * Populate the StreamConfiguration with all the supported Bayer\n> +\t * formats the sensor can produce.\n> +\t */\n> +\tstd::map> streamFormats;\n> +\tconst CameraSensor *sensor = data->sensor_.get();\n> +\n> +\tfor (unsigned int code : sensor->mbusCodes()) {\n> +\t\t/* Find a Bayer media bus code from the sensor. */\n> +\t\tconst BayerFormat &bayerFormat = BayerFormat::fromMbusCode(code);\n> +\t\tif (!bayerFormat.isValid())\n> +\t\t\tcontinue;\n> +\n> +\t\tauto it = rawFormats.find(code);\n> +\t\tif (it == rawFormats.end()) {\n> +\t\t\tLOG(ISI, Warning) << bayerFormat\n> +\t\t\t\t\t << \" not supported in ISI formats map.\";\n> +\t\t\tcontinue;\n> +\t\t}\n> +\n> +\t\tstreamFormats[it->second] = { { kMinISISize, sensor->resolution() } };\n\nThe ISI can't scale raw formats, so I don't think kMinISISize is right\nhere.\n\n> +\t}\n> +\n> +\tStreamFormats formats(streamFormats);\n> +\n> +\tStreamConfiguration cfg(formats);\n> +\tcfg.size = sensor->resolution();\n> +\tcfg.pixelFormat = pixelFormat;\n> +\tcfg.bufferCount = 4;\n> +\n> +\treturn cfg;\n> +}\n> +\n> std::unique_ptr\n> PipelineHandlerISI::generateConfiguration(Camera *camera,\n> \t\t\t\t\t const StreamRoles &roles)\n> @@ -722,79 +849,45 @@ PipelineHandlerISI::generateConfiguration(Camera *camera,\n> \t\treturn nullptr;\n> \t}\n> \n> -\tbool isRaw = false;\n> \tfor (const auto &role : roles) {\n> \t\t/*\n> -\t\t * Prefer the following formats\n> +\t\t * Prefer the following formats:\n> \t\t * - Still Capture: Full resolution YUYV\n> \t\t * - ViewFinder/VideoRecording: 1080p YUYV\n> -\t\t * - RAW: sensor's native format and resolution\n> +\t\t * - RAW: Full resolution Bayer\n> \t\t */\n> -\t\tstd::map> streamFormats;\n> -\t\tPixelFormat pixelFormat;\n> -\t\tSize size;\n> +\t\tStreamConfiguration cfg;\n> \n> \t\tswitch (role) {\n> \t\tcase StreamRole::StillCapture:\n> -\t\t\t/*\n> -\t\t\t * \\todo Make sure the sensor can produce non-RAW formats\n> -\t\t\t * compatible with the ones supported by the pipeline.\n> -\t\t\t */\n> -\t\t\tsize = data->sensor_->resolution();\n> -\t\t\tpixelFormat = formats::YUYV;\n> +\t\t\tcfg = generateYUVConfiguration(camera,\n> +\t\t\t\t\t\t data->sensor_->resolution());\n> +\t\t\tif (!cfg.pixelFormat.isValid()) {\n> +\t\t\t\t/*\n> +\t\t\t\t * Fallback to use a Bayer format if that's what\n> +\t\t\t\t * the sensor supports.\n> +\t\t\t\t */\n> +\t\t\t\tcfg = generateRawConfiguration(camera);\n> +\t\t\t}\n> +\n> \t\t\tbreak;\n> \n> \t\tcase StreamRole::Viewfinder:\n> \t\tcase StreamRole::VideoRecording:\n> -\t\t\t/*\n> -\t\t\t * \\todo Make sure the sensor can produce non-RAW formats\n> -\t\t\t * compatible with the ones supported by the pipeline.\n> -\t\t\t */\n> -\t\t\tsize = PipelineHandlerISI::kPreviewSize;\n> -\t\t\tpixelFormat = formats::YUYV;\n> -\t\t\tbreak;\n> -\n> -\t\tcase StreamRole::Raw: {\n> -\t\t\t/*\n> -\t\t\t * Make sure the sensor can generate a RAW format and\n> -\t\t\t * prefer the ones with a larger bitdepth.\n> -\t\t\t */\n> -\t\t\tconst ISICameraConfiguration::FormatMap::value_type *rawPipeFormat = nullptr;\n> -\t\t\tunsigned int maxDepth = 0;\n> -\n> -\t\t\tfor (unsigned int code : data->sensor_->mbusCodes()) {\n> -\t\t\t\tconst BayerFormat &bayerFormat = BayerFormat::fromMbusCode(code);\n> -\t\t\t\tif (!bayerFormat.isValid())\n> -\t\t\t\t\tcontinue;\n> -\n> -\t\t\t\t/* Make sure the format is supported by the pipeline handler. */\n> -\t\t\t\tauto it = std::find_if(ISICameraConfiguration::formatsMap_.begin(),\n> -\t\t\t\t\t\t ISICameraConfiguration::formatsMap_.end(),\n> -\t\t\t\t\t\t [code](auto &isiFormat) {\n> -\t\t\t\t\t\t\t auto &pipe = isiFormat.second;\n> -\t\t\t\t\t\t\t return pipe.sensorCode == code;\n> -\t\t\t\t\t\t });\n> -\t\t\t\tif (it == ISICameraConfiguration::formatsMap_.end())\n> -\t\t\t\t\tcontinue;\n> -\n> -\t\t\t\tif (bayerFormat.bitDepth > maxDepth) {\n> -\t\t\t\t\tmaxDepth = bayerFormat.bitDepth;\n> -\t\t\t\t\trawPipeFormat = &(*it);\n> -\t\t\t\t}\n> -\t\t\t}\n> -\n> -\t\t\tif (!rawPipeFormat) {\n> -\t\t\t\tLOG(ISI, Error)\n> -\t\t\t\t\t<< \"Cannot generate a configuration for RAW stream\";\n> -\t\t\t\treturn nullptr;\n> +\t\t\tcfg = generateYUVConfiguration(camera,\n> +\t\t\t\t\t\t PipelineHandlerISI::kPreviewSize);\n> +\t\t\tif (!cfg.pixelFormat.isValid()) {\n> +\t\t\t\t/*\n> +\t\t\t\t * Fallback to use a Bayer format if that's what\n> +\t\t\t\t * the sensor supports.\n> +\t\t\t\t */\n> +\t\t\t\tcfg = generateRawConfiguration(camera);\n> \t\t\t}\n> \n> -\t\t\tsize = data->sensor_->resolution();\n> -\t\t\tpixelFormat = rawPipeFormat->first;\n> -\n> -\t\t\tstreamFormats[pixelFormat] = { { kMinISISize, size } };\n> -\t\t\tisRaw = true;\n> +\t\t\tbreak;\n> \n> +\t\tcase StreamRole::Raw: {\n> +\t\t\tcfg = generateRawConfiguration(camera);\n> \t\t\tbreak;\n> \t\t}\n> \n> @@ -803,26 +896,12 @@ PipelineHandlerISI::generateConfiguration(Camera *camera,\n> \t\t\treturn nullptr;\n> \t\t}\n> \n> -\t\t/*\n> -\t\t * For non-RAW configurations the ISI can perform colorspace\n> -\t\t * conversion. List all the supported output formats here.\n> -\t\t */\n> -\t\tif (!isRaw) {\n> -\t\t\tfor (const auto &[pixFmt, pipeFmt] : ISICameraConfiguration::formatsMap_) {\n> -\t\t\t\tconst PixelFormatInfo &info = PixelFormatInfo::info(pixFmt);\n> -\t\t\t\tif (info.colourEncoding == PixelFormatInfo::ColourEncodingRAW)\n> -\t\t\t\t\tcontinue;\n> -\n> -\t\t\t\tstreamFormats[pixFmt] = { { kMinISISize, size } };\n> -\t\t\t}\n> +\t\tif (!cfg.pixelFormat.isValid()) {\n> +\t\t\tLOG(ISI, Error)\n> +\t\t\t\t<< \"Cannot generate configuration for role: \" << role;\n> +\t\t\treturn nullptr;\n> \t\t}\n> \n> -\t\tStreamFormats formats(streamFormats);\n> -\n> -\t\tStreamConfiguration cfg(formats);\n> -\t\tcfg.pixelFormat = pixelFormat;\n> -\t\tcfg.size = size;\n> -\t\tcfg.bufferCount = 4;\n> \t\tconfig->addConfiguration(cfg);\n> \t}\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 6A615BD80A\n\tfor ;\n\tSun, 12 Mar 2023 16:50:46 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id CE66D62709;\n\tSun, 12 Mar 2023 17:50:45 +0100 (CET)","from perceval.ideasonboard.com (perceval.ideasonboard.com\n\t[213.167.242.64])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTPS id 474E262705\n\tfor ;\n\tSun, 12 Mar 2023 17:50:44 +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 36B0BD5F;\n\tSun, 12 Mar 2023 17:50:43 +0100 (CET)"],"DKIM-Signature":["v=1; a=rsa-sha256; c=relaxed/simple; d=libcamera.org;\n\ts=mail; t=1678639845;\n\tbh=CDGdntH6FpSydkao7A5NRa9nzNaNf8UF1ySf8UFGrr8=;\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=aNUxs6dr0C3kDoIMXAnNSJ5qOK9BpZLFqkA1tjUoobE2tuYdCZZEUHOgKjjbIdsII\n\t91OB9CtMiQAiMzcFa8twejE5MXWlTabI1BAeSNbwFjKXHObbosIU6o0iWkxjxx+tfH\n\t/BdTG7bXq1za2pT/3aZMLFlFLlikZGtll2hRmh2suDJajqQQJxnGI6MCQp5ZF+syib\n\t+5utdrRNSeUw0o0xsp2nVQv+GewTOQfie/RbctMU0ZxGX18vhSurYve6xr1WBAKHqn\n\tuUznSnm2M9TLNq0oy0Ig+k/LAbE3Ks8iGL81x+6Fb+aGOgN5+KKYCBb2uVOKvQMIvf\n\tLywbo3b3mVpXg==","v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com;\n\ts=mail; t=1678639843;\n\tbh=CDGdntH6FpSydkao7A5NRa9nzNaNf8UF1ySf8UFGrr8=;\n\th=Date:From:To:Cc:Subject:References:In-Reply-To:From;\n\tb=qBAyy1JUDlSqPjAwXyfR7Ku5I0jDTISbsArfVrckkj9/BD4ui9vPqu6T3fsjBUl2E\n\tybew8XOVtymMIax2NM5rWLiguJQvS4q6Kk8L9bqBeKoXs18nw+KsM7jW6BDCEWTOoG\n\tdclLjRjvZkFb1pATh5fDvOOG3VvzWKYELVm+aJks="],"Authentication-Results":"lancelot.ideasonboard.com; dkim=pass (1024-bit key; \n\tunprotected) header.d=ideasonboard.com\n\theader.i=@ideasonboard.com\n\theader.b=\"qBAyy1JU\"; dkim-atps=neutral","Date":"Sun, 12 Mar 2023 18:50:44 +0200","To":"Jacopo Mondi ","Message-ID":"<20230312165044.GB2545@pendragon.ideasonboard.com>","References":"<20230129135830.27490-1-jacopo.mondi@ideasonboard.com>\n\t<20230129135830.27490-5-jacopo.mondi@ideasonboard.com>","MIME-Version":"1.0","Content-Type":"text/plain; charset=utf-8","Content-Disposition":"inline","In-Reply-To":"<20230129135830.27490-5-jacopo.mondi@ideasonboard.com>","Subject":"Re: [libcamera-devel] [PATCH 4/5] libcamera: imx8-isi: Split\n\tBayer/YUV config generation","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":26649,"web_url":"https://patchwork.libcamera.org/comment/26649/","msgid":"<20230313085117.e45xmhenhbknn6ha@uno.localdomain>","date":"2023-03-13T08:51:17","subject":"Re: [libcamera-devel] [PATCH 4/5] libcamera: imx8-isi: Split\n\tBayer/YUV config generation","submitter":{"id":143,"url":"https://patchwork.libcamera.org/api/people/143/","name":"Jacopo Mondi","email":"jacopo.mondi@ideasonboard.com"},"content":"Hi Dan\n\nOn Tue, Mar 07, 2023 at 02:14:31PM +0000, Dan Scally via libcamera-devel wrote:\n> Hi Jacopo\n>\n> On 29/01/2023 13:58, Jacopo Mondi wrote:\n> > At generateConfiguration() a YUV/RGB pixel format is preferred for the\n> > StillCapture/VideoRecording/Viewfinder roles, but currently there are no\n> > guarantees in place that the sensor provides a non-Bayer bus format from\n> > which YUV/RGB can be generated.\n> >\n> > This makes the default configuration generated for those roles not to\n> > work if the sensor is a RAW-only one.\n> >\n> > To improve the situation split the configuration generation in two,\n> > one for YUV modes and one for Raw Bayer mode.\n> >\n> > StreamRoles assigned to a YUV mode will try to first generate a YUV\n> > configuration and then fallback to RAW if that's what the sensor can\n> > provide.\n> >\n> > As an additional requirement, for YUV streams, the generated mode has to\n> > be validated with the sensor to confirm the desired sizes can be\n> > generated. In order to test a format on the sensor introduce\n> > CameraSensor::tryFormat().\n> >\n> > Reported-by: Laurent Pinchart \n> > Signed-off-by: Jacopo Mondi \n> > Reviewed-by: Paul Elder \n> > ---\n> > include/libcamera/internal/camera_sensor.h | 1 +\n> > src/libcamera/camera_sensor.cpp | 14 ++\n> > src/libcamera/pipeline/imx8-isi/imx8-isi.cpp | 231 +++++++++++++------\n> > 3 files changed, 170 insertions(+), 76 deletions(-)\n> >\n> > diff --git a/include/libcamera/internal/camera_sensor.h b/include/libcamera/internal/camera_sensor.h\n> > index b9f4d7867854..ce3a790f00ee 100644\n> > --- a/include/libcamera/internal/camera_sensor.h\n> > +++ b/include/libcamera/internal/camera_sensor.h\n> > @@ -54,6 +54,7 @@ public:\n> > \tV4L2SubdeviceFormat getFormat(const std::vector &mbusCodes,\n> > \t\t\t\t const Size &size) const;\n> > \tint setFormat(V4L2SubdeviceFormat *format);\n> > +\tint tryFormat(V4L2SubdeviceFormat *format) const;\n> > \tconst ControlInfoMap &controls() const;\n> > \tControlList getControls(const std::vector &ids);\n> > diff --git a/src/libcamera/camera_sensor.cpp b/src/libcamera/camera_sensor.cpp\n> > index a210aa4fa370..dfe593033def 100644\n> > --- a/src/libcamera/camera_sensor.cpp\n> > +++ b/src/libcamera/camera_sensor.cpp\n> > @@ -757,6 +757,20 @@ int CameraSensor::setFormat(V4L2SubdeviceFormat *format)\n> > \treturn 0;\n> > }\n> > +/**\n> > + * \\brief Try the sensor output format\n> > + * \\param[in] format The desired sensor output format\n> > + *\n> > + * The ranges of any controls associated with the sensor are not updated.\n> > + *\n> > + * \\return 0 on success or a negative error code otherwise\n> > + */\n> > +int CameraSensor::tryFormat(V4L2SubdeviceFormat *format) const\n> > +{\n> > +\treturn subdev_->setFormat(pad_, format,\n> > +\t\t\t\t V4L2Subdevice::Whence::TryFormat);\n> > +}\n> > +\n> > /**\n> > * \\brief Retrieve the supported V4L2 controls and their information\n> > *\n> > diff --git a/src/libcamera/pipeline/imx8-isi/imx8-isi.cpp b/src/libcamera/pipeline/imx8-isi/imx8-isi.cpp\n> > index 5976a63d27dd..0e1e87c7a2aa 100644\n> > --- a/src/libcamera/pipeline/imx8-isi/imx8-isi.cpp\n> > +++ b/src/libcamera/pipeline/imx8-isi/imx8-isi.cpp\n> > @@ -145,6 +145,10 @@ private:\n> > \tPipe *pipeFromStream(Camera *camera, const Stream *stream);\n> > +\tStreamConfiguration generateYUVConfiguration(Camera *camera,\n> > +\t\t\t\t\t\t const Size &size);\n> > +\tStreamConfiguration generateRawConfiguration(Camera *camera);\n> > +\n> > \tvoid bufferReady(FrameBuffer *buffer);\n> > \tMediaDevice *isiDev_;\n> > @@ -705,6 +709,129 @@ PipelineHandlerISI::PipelineHandlerISI(CameraManager *manager)\n> > {\n> > }\n> > +/*\n> > + * Generate a StreamConfiguration for YUV/RGB use case.\n> > + *\n> > + * Verify it the sensor can produce a YUV/RGB media bus format and collect\n> > + * all the processed pixel formats the ISI can generate as supported stream\n> > + * configurations.\n> > + */\n> > +StreamConfiguration PipelineHandlerISI::generateYUVConfiguration(Camera *camera,\n> > +\t\t\t\t\t\t\t\t const Size &size)\n> > +{\n> > +\tISICameraData *data = cameraData(camera);\n> > +\tPixelFormat pixelFormat = formats::YUYV;\n> > +\tunsigned int mbusCode;\n> > +\n> > +\tmbusCode = data->getYuvMediaBusFormat(&pixelFormat);\n> > +\tif (!mbusCode)\n> > +\t\treturn {};\n> > +\n> > +\t/* Adjust the requested size to the sensor's capabilities. */\n> > +\tconst CameraSensor *sensor = data->sensor_.get();\n> > +\n> > +\tV4L2SubdeviceFormat sensorFmt;\n> > +\tsensorFmt.mbus_code = mbusCode;\n> > +\tsensorFmt.size = size;\n> > +\n> > +\tint ret = sensor->tryFormat(&sensorFmt);\n> > +\tif (ret) {\n> > +\t\tLOG(ISI, Error) << \"Failed to try sensor format.\";\n> > +\t\treturn {};\n> > +\t}\n> > +\n> > +\tSize sensorSize = sensorFmt.size;\n> > +\n> > +\t/*\n> > +\t * Populate the StreamConfiguration.\n> > +\t *\n> > +\t * As the sensor supports at least one YUV/RGB media bus format all the\n> > +\t * processed ones in formatsMap_ can be generated from it.\n> > +\t */\n> > +\tstd::map> streamFormats;\n> > +\n> > +\tfor (const auto &[pixFmt, pipeFmt] : ISICameraConfiguration::formatsMap_) {\n> > +\t\tconst PixelFormatInfo &info = PixelFormatInfo::info(pixFmt);\n> > +\t\tif (info.colourEncoding == PixelFormatInfo::ColourEncodingRAW)\n> > +\t\t\tcontinue;\n> > +\n> > +\t\tstreamFormats[pixFmt] = { { kMinISISize, sensorSize } };\n> > +\t}\n> > +\n> > +\tStreamFormats formats(streamFormats);\n> > +\n> > +\tStreamConfiguration cfg(formats);\n> > +\tcfg.pixelFormat = pixelFormat;\n> > +\tcfg.size = sensorSize;\n> > +\tcfg.bufferCount = 4;\n> > +\n> > +\treturn cfg;\n> > +}\n> > +\n> > +/*\n> > + * Generate a StreamConfiguration for Raw Bayer use case. Verify if the sensor\n> > + * can produce the requested RAW bayer format and eventually adjust it to\n> > + * the one with the largest bit-depth the sensor can produce.\n> > + */\n> > +StreamConfiguration PipelineHandlerISI::generateRawConfiguration(Camera *camera)\n> > +{\n> > +\tconst std::map rawFormats = {\n> > +\t\t{ MEDIA_BUS_FMT_SBGGR8_1X8, formats::SBGGR8 },\n> > +\t\t{ MEDIA_BUS_FMT_SGBRG8_1X8, formats::SGBRG8 },\n> > +\t\t{ MEDIA_BUS_FMT_SGRBG8_1X8, formats::SGRBG8 },\n> > +\t\t{ MEDIA_BUS_FMT_SRGGB8_1X8, formats::SRGGB8 },\n> > +\t\t{ MEDIA_BUS_FMT_SBGGR10_1X10, formats::SBGGR10 },\n> > +\t\t{ MEDIA_BUS_FMT_SGBRG10_1X10, formats::SGBRG10 },\n> > +\t\t{ MEDIA_BUS_FMT_SGRBG10_1X10, formats::SGRBG10 },\n> > +\t\t{ MEDIA_BUS_FMT_SRGGB10_1X10, formats::SRGGB10 },\n> > +\t\t{ MEDIA_BUS_FMT_SBGGR12_1X12, formats::SBGGR12 },\n> > +\t\t{ MEDIA_BUS_FMT_SGBRG12_1X12, formats::SGBRG12 },\n> > +\t\t{ MEDIA_BUS_FMT_SGRBG12_1X12, formats::SGRBG12 },\n> > +\t\t{ MEDIA_BUS_FMT_SRGGB12_1X12, formats::SRGGB12 },\n> > +\t};\n> > +\n> > +\tISICameraData *data = cameraData(camera);\n> > +\tPixelFormat pixelFormat = formats::SBGGR10;\n> > +\tunsigned int mbusCode;\n> > +\n> > +\t/* pixelFormat will be adjusted, if the sensor can produce RAW. */\n> > +\tmbusCode = data->getRawMediaBusFormat(&pixelFormat);\n> > +\tif (!mbusCode)\n> > +\t\treturn {};\n> > +\n> > +\t/*\n> > +\t * Populate the StreamConfiguration with all the supported Bayer\n> > +\t * formats the sensor can produce.\n> > +\t */\n> > +\tstd::map> streamFormats;\n> > +\tconst CameraSensor *sensor = data->sensor_.get();\n> > +\n> > +\tfor (unsigned int code : sensor->mbusCodes()) {\n> > +\t\t/* Find a Bayer media bus code from the sensor. */\n> > +\t\tconst BayerFormat &bayerFormat = BayerFormat::fromMbusCode(code);\n> > +\t\tif (!bayerFormat.isValid())\n> > +\t\t\tcontinue;\n> > +\n> > +\t\tauto it = rawFormats.find(code);\n> > +\t\tif (it == rawFormats.end()) {\n> > +\t\t\tLOG(ISI, Warning) << bayerFormat\n> > +\t\t\t\t\t << \" not supported in ISI formats map.\";\n> > +\t\t\tcontinue;\n> > +\t\t}\n> > +\n> > +\t\tstreamFormats[it->second] = { { kMinISISize, sensor->resolution() } };\n> > +\t}\n> > +\n> > +\tStreamFormats formats(streamFormats);\n> > +\n> > +\tStreamConfiguration cfg(formats);\n> > +\tcfg.size = sensor->resolution();\n> > +\tcfg.pixelFormat = pixelFormat;\n> > +\tcfg.bufferCount = 4;\n> > +\n> > +\treturn cfg;\n> > +}\n> > +\n> > std::unique_ptr\n> > PipelineHandlerISI::generateConfiguration(Camera *camera,\n> > \t\t\t\t\t const StreamRoles &roles)\n> > @@ -722,79 +849,45 @@ PipelineHandlerISI::generateConfiguration(Camera *camera,\n> > \t\treturn nullptr;\n> > \t}\n> > -\tbool isRaw = false;\n> > \tfor (const auto &role : roles) {\n> > \t\t/*\n> > -\t\t * Prefer the following formats\n> > +\t\t * Prefer the following formats:\n> > \t\t * - Still Capture: Full resolution YUYV\n> > \t\t * - ViewFinder/VideoRecording: 1080p YUYV\n> > -\t\t * - RAW: sensor's native format and resolution\n> > +\t\t * - RAW: Full resolution Bayer\n> > \t\t */\n> > -\t\tstd::map> streamFormats;\n> > -\t\tPixelFormat pixelFormat;\n> > -\t\tSize size;\n> > +\t\tStreamConfiguration cfg;\n> > \t\tswitch (role) {\n> > \t\tcase StreamRole::StillCapture:\n> > -\t\t\t/*\n> > -\t\t\t * \\todo Make sure the sensor can produce non-RAW formats\n> > -\t\t\t * compatible with the ones supported by the pipeline.\n> > -\t\t\t */\n> > -\t\t\tsize = data->sensor_->resolution();\n> > -\t\t\tpixelFormat = formats::YUYV;\n> > +\t\t\tcfg = generateYUVConfiguration(camera,\n> > +\t\t\t\t\t\t data->sensor_->resolution());\n> > +\t\t\tif (!cfg.pixelFormat.isValid()) {\n> > +\t\t\t\t/*\n> > +\t\t\t\t * Fallback to use a Bayer format if that's what\n> > +\t\t\t\t * the sensor supports.\n> > +\t\t\t\t */\n> > +\t\t\t\tcfg = generateRawConfiguration(camera);\n> > +\t\t\t}\n> > +\n> > \t\t\tbreak;\n> > \t\tcase StreamRole::Viewfinder:\n> > \t\tcase StreamRole::VideoRecording:\n> > -\t\t\t/*\n> > -\t\t\t * \\todo Make sure the sensor can produce non-RAW formats\n> > -\t\t\t * compatible with the ones supported by the pipeline.\n> > -\t\t\t */\n> > -\t\t\tsize = PipelineHandlerISI::kPreviewSize;\n> > -\t\t\tpixelFormat = formats::YUYV;\n> > -\t\t\tbreak;\n> > -\n> > -\t\tcase StreamRole::Raw: {\n> > -\t\t\t/*\n> > -\t\t\t * Make sure the sensor can generate a RAW format and\n> > -\t\t\t * prefer the ones with a larger bitdepth.\n> > -\t\t\t */\n> > -\t\t\tconst ISICameraConfiguration::FormatMap::value_type *rawPipeFormat = nullptr;\n> > -\t\t\tunsigned int maxDepth = 0;\n> > -\n> > -\t\t\tfor (unsigned int code : data->sensor_->mbusCodes()) {\n> > -\t\t\t\tconst BayerFormat &bayerFormat = BayerFormat::fromMbusCode(code);\n> > -\t\t\t\tif (!bayerFormat.isValid())\n> > -\t\t\t\t\tcontinue;\n> > -\n> > -\t\t\t\t/* Make sure the format is supported by the pipeline handler. */\n> > -\t\t\t\tauto it = std::find_if(ISICameraConfiguration::formatsMap_.begin(),\n> > -\t\t\t\t\t\t ISICameraConfiguration::formatsMap_.end(),\n> > -\t\t\t\t\t\t [code](auto &isiFormat) {\n> > -\t\t\t\t\t\t\t auto &pipe = isiFormat.second;\n> > -\t\t\t\t\t\t\t return pipe.sensorCode == code;\n> > -\t\t\t\t\t\t });\n> > -\t\t\t\tif (it == ISICameraConfiguration::formatsMap_.end())\n> > -\t\t\t\t\tcontinue;\n> > -\n> > -\t\t\t\tif (bayerFormat.bitDepth > maxDepth) {\n> > -\t\t\t\t\tmaxDepth = bayerFormat.bitDepth;\n> > -\t\t\t\t\trawPipeFormat = &(*it);\n> > -\t\t\t\t}\n> > -\t\t\t}\n> > -\n> > -\t\t\tif (!rawPipeFormat) {\n> > -\t\t\t\tLOG(ISI, Error)\n> > -\t\t\t\t\t<< \"Cannot generate a configuration for RAW stream\";\n> > -\t\t\t\treturn nullptr;\n> > +\t\t\tcfg = generateYUVConfiguration(camera,\n> > +\t\t\t\t\t\t PipelineHandlerISI::kPreviewSize);\n> > +\t\t\tif (!cfg.pixelFormat.isValid()) {\n> > +\t\t\t\t/*\n> > +\t\t\t\t * Fallback to use a Bayer format if that's what\n> > +\t\t\t\t * the sensor supports.\n> > +\t\t\t\t */\n> > +\t\t\t\tcfg = generateRawConfiguration(camera);\n> > \t\t\t}\n>\n>\n> Not a big deal at all so feel free to ignore me, but I think I'd have kept\n> the resolution the same and just changed the pixel format - it threw me\n> briefly in testing that it wasn't configuring 1080p as I expected in\n> ViewFinder role.\n>\n\nThing is that when switching to RAW formats we can only output what\nthe sensor produces and we always generate a raw configuration with\nits full resolution. We could try to find a size closer to 1080p\nproduced by the sensor, but as there are no guarantees one exists, I\nthink it is more predictable to use full resolution unconditionally.\n\n> > -\t\t\tsize = data->sensor_->resolution();\n> > -\t\t\tpixelFormat = rawPipeFormat->first;\n> > -\n> > -\t\t\tstreamFormats[pixelFormat] = { { kMinISISize, size } };\n> > -\t\t\tisRaw = true;\n> > +\t\t\tbreak;\n> > +\t\tcase StreamRole::Raw: {\n> > +\t\t\tcfg = generateRawConfiguration(camera);\n> > \t\t\tbreak;\n> > \t\t}\n> > @@ -803,26 +896,12 @@ PipelineHandlerISI::generateConfiguration(Camera *camera,\n> > \t\t\treturn nullptr;\n> > \t\t}\n> > -\t\t/*\n> > -\t\t * For non-RAW configurations the ISI can perform colorspace\n> > -\t\t * conversion. List all the supported output formats here.\n> > -\t\t */\n> > -\t\tif (!isRaw) {\n> > -\t\t\tfor (const auto &[pixFmt, pipeFmt] : ISICameraConfiguration::formatsMap_) {\n> > -\t\t\t\tconst PixelFormatInfo &info = PixelFormatInfo::info(pixFmt);\n> > -\t\t\t\tif (info.colourEncoding == PixelFormatInfo::ColourEncodingRAW)\n> > -\t\t\t\t\tcontinue;\n> > -\n> > -\t\t\t\tstreamFormats[pixFmt] = { { kMinISISize, size } };\n> > -\t\t\t}\n> > +\t\tif (!cfg.pixelFormat.isValid()) {\n> > +\t\t\tLOG(ISI, Error)\n> > +\t\t\t\t<< \"Cannot generate configuration for role: \" << role;\n> > +\t\t\treturn nullptr;\n> > \t\t}\n> > -\t\tStreamFormats formats(streamFormats);\n> > -\n> > -\t\tStreamConfiguration cfg(formats);\n> > -\t\tcfg.pixelFormat = pixelFormat;\n> > -\t\tcfg.size = size;\n> > -\t\tcfg.bufferCount = 4;\n> > \t\tconfig->addConfiguration(cfg);\n> > \t}","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 D9927BE080\n\tfor ;\n\tMon, 13 Mar 2023 08:51:23 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id 2C1E262709;\n\tMon, 13 Mar 2023 09:51:23 +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 AE3E3626B2\n\tfor ;\n\tMon, 13 Mar 2023 09:51:21 +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 AB7AC563;\n\tMon, 13 Mar 2023 09:51:20 +0100 (CET)"],"DKIM-Signature":["v=1; a=rsa-sha256; c=relaxed/simple; d=libcamera.org;\n\ts=mail; t=1678697483;\n\tbh=55ARuFFRalwJah4fwxdgVfslDlSwHg1l3OnHmnuGMLQ=;\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=urNeV+4RR/5Qv8hCWXI5iMVZL5bhEFgSRqy1cKx0c9YUt5pLNcaUYT6OEke7SE6QD\n\teBBvfScQe2b3aIwIKpsZE1iPU/pFRmx7vj5Sj3tIliPaOkAfQ1rmoJLPWEYQVA6zUQ\n\tLH/lI4PQW4EA4fLpWizw9i4OACqABJkzikYfdi+hwE/aAWEy+RP4XwFjLZxKNcBID+\n\tzh1pma3YWsU+A3Wh1OdYol6zYoPn58SFUSHgRN30okc706mcyABaEJ7ThvcroNFoP+\n\t7tj6THig6rlkCj8TIT6Il3mjCyOM7HywZD00OsYLIPb093wAm1+8JzO2iGZ5yB5S6Y\n\tsgPOXPf3oRKEQ==","v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com;\n\ts=mail; t=1678697481;\n\tbh=55ARuFFRalwJah4fwxdgVfslDlSwHg1l3OnHmnuGMLQ=;\n\th=Date:From:To:Cc:Subject:References:In-Reply-To:From;\n\tb=b1Ilpfdcb6EIpY+1sagNM1cswb+bQiuPNF/H2VyJWPUBV/Sa9WtW5aZFMouZNMci6\n\tEBn8MbGnJJncDkfV9K/sib8av4A61M0KWjq41RxG2IaPzp72JuuhG9KRm2wJZLgRqa\n\tVxYiSIDR+smi0hzIYX3imYT2dzsSzOB5IVKfmkOM="],"Authentication-Results":"lancelot.ideasonboard.com; dkim=pass (1024-bit key; \n\tunprotected) header.d=ideasonboard.com\n\theader.i=@ideasonboard.com\n\theader.b=\"b1Ilpfdc\"; dkim-atps=neutral","Date":"Mon, 13 Mar 2023 09:51:17 +0100","To":"Dan Scally ","Message-ID":"<20230313085117.e45xmhenhbknn6ha@uno.localdomain>","References":"<20230129135830.27490-1-jacopo.mondi@ideasonboard.com>\n\t<20230129135830.27490-5-jacopo.mondi@ideasonboard.com>\n\t","MIME-Version":"1.0","Content-Type":"text/plain; charset=utf-8","Content-Disposition":"inline","In-Reply-To":"","Subject":"Re: [libcamera-devel] [PATCH 4/5] libcamera: imx8-isi: Split\n\tBayer/YUV config generation","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\" "}}]