{"id":18221,"url":"https://patchwork.libcamera.org/api/patches/18221/?format=json","web_url":"https://patchwork.libcamera.org/patch/18221/","project":{"id":1,"url":"https://patchwork.libcamera.org/api/projects/1/?format=json","name":"libcamera","link_name":"libcamera","list_id":"libcamera_core","list_email":"libcamera-devel@lists.libcamera.org","web_url":"","scm_url":"","webscm_url":""},"msgid":"<20230129135830.27490-5-jacopo.mondi@ideasonboard.com>","date":"2023-01-29T13:58:29","name":"[libcamera-devel,4/5] libcamera: imx8-isi: Split Bayer/YUV config generation","commit_ref":null,"pull_url":null,"state":"accepted","archived":false,"hash":"07e9fd591ffaeb1e72ec32c19a37d93a1a368bd2","submitter":{"id":143,"url":"https://patchwork.libcamera.org/api/people/143/?format=json","name":"Jacopo Mondi","email":"jacopo.mondi@ideasonboard.com"},"delegate":null,"mbox":"https://patchwork.libcamera.org/patch/18221/mbox/","series":[{"id":3730,"url":"https://patchwork.libcamera.org/api/series/3730/?format=json","web_url":"https://patchwork.libcamera.org/project/libcamera/list/?series=3730","date":"2023-01-29T13:58:25","name":"libcamera: imx8-isi: Remove pixelformat-2-media-bus map","version":1,"mbox":"https://patchwork.libcamera.org/series/3730/mbox/"}],"comments":"https://patchwork.libcamera.org/api/patches/18221/comments/","check":"pending","checks":"https://patchwork.libcamera.org/api/patches/18221/checks/","tags":{},"headers":{"Return-Path":"<libcamera-devel-bounces@lists.libcamera.org>","X-Original-To":"parsemail@patchwork.libcamera.org","Delivered-To":"parsemail@patchwork.libcamera.org","Received":["from lancelot.ideasonboard.com (lancelot.ideasonboard.com\n\t[92.243.16.209])\n\tby patchwork.libcamera.org (Postfix) with ESMTPS id 9681EC3296\n\tfor <parsemail@patchwork.libcamera.org>;\n\tSun, 29 Jan 2023 13:58:53 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id 24882625F0;\n\tSun, 29 Jan 2023 14:58:52 +0100 (CET)","from perceval.ideasonboard.com (perceval.ideasonboard.com\n\t[213.167.242.64])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTPS id 511CE625EE\n\tfor <libcamera-devel@lists.libcamera.org>;\n\tSun, 29 Jan 2023 14:58:49 +0100 (CET)","from uno.localdomain (mob-5-90-54-203.net.vodafone.it\n\t[5.90.54.203])\n\tby perceval.ideasonboard.com (Postfix) with ESMTPSA id 9D1BF327;\n\tSun, 29 Jan 2023 14:58:48 +0100 (CET)"],"DKIM-Signature":["v=1; a=rsa-sha256; c=relaxed/simple; d=libcamera.org;\n\ts=mail; t=1675000732;\n\tbh=nbOeooQSfmwVd0SzRk3TxK91ailqoyrhEauCXlGAvdk=;\n\th=To:Date:In-Reply-To:References:Subject:List-Id:List-Unsubscribe:\n\tList-Archive:List-Post:List-Help:List-Subscribe:From:Reply-To:Cc:\n\tFrom;\n\tb=OJKncxC9cLSuQwtr335lVCmMsxjBcZ8wvCnYZCnx/VlX9V0whWCXm8a8bmMTQOIhY\n\tXBs4Gce6zQEWotg+vmZu5EF7vKKji/bvFh81DPce1x3Ayva5m9oYGSzuREE2WRa2IT\n\t6AA276gR83kDCJTFERGlOu1FXe4B+jaey9Qyul4D3OYLgxSRJh7Hm1G6MITewdkXj9\n\tNq6TxsWYu+U5uZ7Xs+6Q3JO1zG9NUwQcEreVQ7LPM9OfKfNYp384vDVYqWSrL90WQX\n\t6KciHp2wlwWUcE2HwTg/i5bfvR4o/le+JK2m8OqT7Izoo1s2Ak6I8vQ2ainxXuXHxI\n\ttIucRTZ0p2MJA==","v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com;\n\ts=mail; t=1675000729;\n\tbh=nbOeooQSfmwVd0SzRk3TxK91ailqoyrhEauCXlGAvdk=;\n\th=From:To:Cc:Subject:Date:In-Reply-To:References:From;\n\tb=NrNyDNKAHW6PbyaIRzFMYBdCxsqsyFriS2fxbsEVDFL9fpbpmZM1vrKE7gyISvtL+\n\t0r3jQTx7DI/2KbnXED+M4nZfQxH3oYR5hlV6Xuo6crOj8d7Cu4i8RkDwWOE8DFxpps\n\tkISiY5slU1VF3E73Ehvb5dWDchTcEr0qhQB2OTvU="],"Authentication-Results":"lancelot.ideasonboard.com; dkim=pass (1024-bit key; \n\tunprotected) header.d=ideasonboard.com\n\theader.i=@ideasonboard.com\n\theader.b=\"NrNyDNKA\"; dkim-atps=neutral","To":"libcamera-devel@lists.libcamera.org","Date":"Sun, 29 Jan 2023 14:58:29 +0100","Message-Id":"<20230129135830.27490-5-jacopo.mondi@ideasonboard.com>","X-Mailer":"git-send-email 2.39.0","In-Reply-To":"<20230129135830.27490-1-jacopo.mondi@ideasonboard.com>","References":"<20230129135830.27490-1-jacopo.mondi@ideasonboard.com>","MIME-Version":"1.0","Content-Transfer-Encoding":"8bit","Subject":"[libcamera-devel] [PATCH 4/5] libcamera: imx8-isi: Split Bayer/YUV\n\tconfig generation","X-BeenThere":"libcamera-devel@lists.libcamera.org","X-Mailman-Version":"2.1.29","Precedence":"list","List-Id":"<libcamera-devel.lists.libcamera.org>","List-Unsubscribe":"<https://lists.libcamera.org/options/libcamera-devel>,\n\t<mailto:libcamera-devel-request@lists.libcamera.org?subject=unsubscribe>","List-Archive":"<https://lists.libcamera.org/pipermail/libcamera-devel/>","List-Post":"<mailto:libcamera-devel@lists.libcamera.org>","List-Help":"<mailto:libcamera-devel-request@lists.libcamera.org?subject=help>","List-Subscribe":"<https://lists.libcamera.org/listinfo/libcamera-devel>,\n\t<mailto:libcamera-devel-request@lists.libcamera.org?subject=subscribe>","From":"Jacopo Mondi via libcamera-devel <libcamera-devel@lists.libcamera.org>","Reply-To":"Jacopo Mondi <jacopo.mondi@ideasonboard.com>","Cc":"Jacopo Mondi <jacopo.mondi@ideasonboard.com>","Errors-To":"libcamera-devel-bounces@lists.libcamera.org","Sender":"\"libcamera-devel\" <libcamera-devel-bounces@lists.libcamera.org>"},"content":"At generateConfiguration() a YUV/RGB pixel format is preferred for the\nStillCapture/VideoRecording/Viewfinder roles, but currently there are no\nguarantees in place that the sensor provides a non-Bayer bus format from\nwhich YUV/RGB can be generated.\n\nThis makes the default configuration generated for those roles not to\nwork if the sensor is a RAW-only one.\n\nTo improve the situation split the configuration generation in two,\none for YUV modes and one for Raw Bayer mode.\n\nStreamRoles assigned to a YUV mode will try to first generate a YUV\nconfiguration and then fallback to RAW if that's what the sensor can\nprovide.\n\nAs an additional requirement, for YUV streams, the generated mode has to\nbe validated with the sensor to confirm the desired sizes can be\ngenerated. In order to test a format on the sensor introduce\nCameraSensor::tryFormat().\n\nReported-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>\nSigned-off-by: Jacopo Mondi <jacopo.mondi@ideasonboard.com>\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(-)","diff":"diff --git a/include/libcamera/internal/camera_sensor.h b/include/libcamera/internal/camera_sensor.h\nindex 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<unsigned int> &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<uint32_t> &ids);\ndiff --git a/src/libcamera/camera_sensor.cpp b/src/libcamera/camera_sensor.cpp\nindex 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  *\ndiff --git a/src/libcamera/pipeline/imx8-isi/imx8-isi.cpp b/src/libcamera/pipeline/imx8-isi/imx8-isi.cpp\nindex 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<PixelFormat, std::vector<SizeRange>> 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<unsigned int, PixelFormat> 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<PixelFormat, std::vector<SizeRange>> 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<CameraConfiguration>\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<PixelFormat, std::vector<SizeRange>> 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","prefixes":["libcamera-devel","4/5"]}