Patch Detail
Show a patch.
GET /api/1.1/patches/18571/?format=api
{ "id": 18571, "url": "https://patchwork.libcamera.org/api/1.1/patches/18571/?format=api", "web_url": "https://patchwork.libcamera.org/patch/18571/", "project": { "id": 1, "url": "https://patchwork.libcamera.org/api/1.1/projects/1/?format=api", "name": "libcamera", "link_name": "libcamera", "list_id": "libcamera_core", "list_email": "libcamera-devel@lists.libcamera.org", "web_url": "", "scm_url": "", "webscm_url": "" }, "msgid": "<20230427144740.64075-6-jacopo.mondi@ideasonboard.com>", "date": "2023-04-27T14:47:39", "name": "[libcamera-devel,v3,5/6] libcamera: imx8-isi: Split Bayer/YUV config generation", "commit_ref": "680cde6005096677688ed976bdc758763774036c", "pull_url": null, "state": "accepted", "archived": false, "hash": "b6ec41d55f9e26483b38ab56217414e64de08163", "submitter": { "id": 143, "url": "https://patchwork.libcamera.org/api/1.1/people/143/?format=api", "name": "Jacopo Mondi", "email": "jacopo.mondi@ideasonboard.com" }, "delegate": null, "mbox": "https://patchwork.libcamera.org/patch/18571/mbox/", "series": [ { "id": 3852, "url": "https://patchwork.libcamera.org/api/1.1/series/3852/?format=api", "web_url": "https://patchwork.libcamera.org/project/libcamera/list/?series=3852", "date": "2023-04-27T14:47:34", "name": "libcamera: imx8-isi: Remove pixelformat-2-media-bus map", "version": 3, "mbox": "https://patchwork.libcamera.org/series/3852/mbox/" } ], "comments": "https://patchwork.libcamera.org/api/patches/18571/comments/", "check": "pending", "checks": "https://patchwork.libcamera.org/api/patches/18571/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 D20F1C0DA4\n\tfor <parsemail@patchwork.libcamera.org>;\n\tThu, 27 Apr 2023 14:48:06 +0000 (UTC)", "from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id EEAFF627F2;\n\tThu, 27 Apr 2023 16:48:05 +0200 (CEST)", "from perceval.ideasonboard.com (perceval.ideasonboard.com\n\t[213.167.242.64])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTPS id 536B0627E3\n\tfor <libcamera-devel@lists.libcamera.org>;\n\tThu, 27 Apr 2023 16:48:01 +0200 (CEST)", "from uno.lan (unknown\n\t[IPv6:2001:b07:5d2e:52c9:1cf0:b3bc:c785:4625])\n\tby perceval.ideasonboard.com (Postfix) with ESMTPSA id EC85CC7E;\n\tThu, 27 Apr 2023 16:47:48 +0200 (CEST)" ], "DKIM-Signature": [ "v=1; a=rsa-sha256; c=relaxed/simple; d=libcamera.org;\n\ts=mail; t=1682606885;\n\tbh=rifhwZru9G4otZhPGavLwbiy3J3h/TpVb/bc359dJv4=;\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=siF1CsdbyJJsJl86/JW62PKFJGla1dLc+1OTYjcylm+44mLo9RpoXyYOHA8IIhr10\n\teWOvn9uJa64vfipmlwJ92vmtimO3GVZZmp+XlNGtxh+igtBq2ARqzADflRt0bnLcBP\n\taeRel+OVTTHCwGEDi8ADhf+d7OoL2rlmUjvgojmkr/svSA+WxHbxGzCdL7VEFx2lY+\n\tGJM1vuC1+eHQKEVq7bgQyVLEkA8+yIg0IML8LUXceMNyJrQNL3tY0D8x2o8b6LVk6T\n\toXC6tGerS8tYNdIsoXoZnm3StrySzSAsqIN8a7T//vMNb2voAPopoC7nrk9nCihqOr\n\tuqUI9+OzzFRYg==", "v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com;\n\ts=mail; t=1682606869;\n\tbh=rifhwZru9G4otZhPGavLwbiy3J3h/TpVb/bc359dJv4=;\n\th=From:To:Cc:Subject:Date:In-Reply-To:References:From;\n\tb=DlJCjblbYY6um2hkJuVJDxkIBILJXr4se0VAvqa+dWRHhpLhjHXKHWE/sWHGzxk4U\n\tTeMhVE+ndMQbcGeGlUZwrvUEDmNSwlEtmHWeGcQwd+Ze2ZYuMGz25Q0H0iXNWqI938\n\tIuzTYUnLFINVyfc2eX8jKlHDnW69dWvUz2dsHe40=" ], "Authentication-Results": "lancelot.ideasonboard.com; dkim=pass (1024-bit key; \n\tunprotected) header.d=ideasonboard.com\n\theader.i=@ideasonboard.com\n\theader.b=\"DlJCjblb\"; dkim-atps=neutral", "To": "libcamera-devel@lists.libcamera.org", "Date": "Thu, 27 Apr 2023 16:47:39 +0200", "Message-Id": "<20230427144740.64075-6-jacopo.mondi@ideasonboard.com>", "X-Mailer": "git-send-email 2.40.0", "In-Reply-To": "<20230427144740.64075-1-jacopo.mondi@ideasonboard.com>", "References": "<20230427144740.64075-1-jacopo.mondi@ideasonboard.com>", "MIME-Version": "1.0", "Content-Transfer-Encoding": "8bit", "Subject": "[libcamera-devel] [PATCH v3 5/6] 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": "<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 as the ISI cannot up-scale. In order to test a format use the\nnewly introduced CameraSensor::tryFormat().\n\nReported-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>\nSigned-off-by: Jacopo Mondi <jacopo.mondi@ideasonboard.com>\nReviewed-by: Paul Elder <paul.elder@ideasonboard.com>\nReviewed-by: Daniel Scally <dan.scally@ideasonboard.com>\nTested-by: Daniel Scally <dan.scally@ideasonboard.com>\n---\n src/libcamera/pipeline/imx8-isi/imx8-isi.cpp | 243 ++++++++++++-------\n 1 file changed, 158 insertions(+), 85 deletions(-)", "diff": "diff --git a/src/libcamera/pipeline/imx8-isi/imx8-isi.cpp b/src/libcamera/pipeline/imx8-isi/imx8-isi.cpp\nindex ee3dc4ad4d3e..a8bb36e38c8a 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@@ -708,6 +712,131 @@ 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+\tV4L2SubdeviceFormat sensorFmt;\n+\tsensorFmt.mbus_code = mbusCode;\n+\tsensorFmt.size = size;\n+\n+\tint ret = data->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+\tstatic const 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\t{ MEDIA_BUS_FMT_SBGGR14_1X14, formats::SBGGR14 },\n+\t\t{ MEDIA_BUS_FMT_SGBRG14_1X14, formats::SGBRG14 },\n+\t\t{ MEDIA_BUS_FMT_SGRBG14_1X14, formats::SGRBG14 },\n+\t\t{ MEDIA_BUS_FMT_SRGGB14_1X14, formats::SRGGB14 },\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] = { { sensor->resolution(), 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@@ -725,107 +854,51 @@ 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-\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\tbreak;\n+\t\tStreamConfiguration cfg;\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+ switch (role) {\n+ case StreamRole::StillCapture:\n+ case StreamRole::Viewfinder:\n+ case StreamRole::VideoRecording: {\n+ Size size = role == StreamRole::StillCapture\n+ ? data->sensor_->resolution()\n+ : PipelineHandlerISI::kPreviewSize;\n+ cfg = generateYUVConfiguration(camera, size);\n+ if (cfg.pixelFormat.isValid())\n+ break;\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\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 \n-\t\t\tbreak;\n-\t\t}\n+ /*\n+ * Fallback to use a Bayer format if that's what the\n+ * sensor supports.\n+ */\n+ [[fallthrough]];\n+\n+\t\t }\n+\n+ case StreamRole::Raw: {\n+ cfg = generateRawConfiguration(camera);\n+ break;\n+ }\n \n \t\tdefault:\n \t\t\tLOG(ISI, Error) << \"Requested stream role not supported: \" << role;\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", "v3", "5/6" ] }