[{"id":23409,"web_url":"https://patchwork.libcamera.org/comment/23409/","msgid":"<165533435080.2586493.6690601453915436163@Monstersaurus>","date":"2022-06-15T23:05:50","subject":"Re: [libcamera-devel] [PATCH v2 3/5] libcamera: pipeline: simple:\n\tFactor out format test to separate function","submitter":{"id":4,"url":"https://patchwork.libcamera.org/api/people/4/","name":"Kieran Bingham","email":"kieran.bingham@ideasonboard.com"},"content":"Quoting Laurent Pinchart via libcamera-devel (2022-06-12 16:23:09)\n> To prepare for the implementation of a more complex format discovery\n> method, factor out code that tries a pipeline configuration to a\n> separate function. No functional change intended.\n> \n> Signed-off-by: Laurent Pinchart \n> ---\n> Changes since v1:\n> \n> - Drop unused function declaration\n> - Document the tryPipeline() function\n\nLooks good to me.\n\n\nReviewed-by: Kieran Bingham \n\n> ---\n> src/libcamera/pipeline/simple/simple.cpp | 118 +++++++++++++----------\n> 1 file changed, 67 insertions(+), 51 deletions(-)\n> \n> diff --git a/src/libcamera/pipeline/simple/simple.cpp b/src/libcamera/pipeline/simple/simple.cpp\n> index b09368aee20b..3c90bdec60e0 100644\n> --- a/src/libcamera/pipeline/simple/simple.cpp\n> +++ b/src/libcamera/pipeline/simple/simple.cpp\n> @@ -250,6 +250,8 @@ public:\n> std::queue> converterQueue_;\n> \n> private:\n> + void tryPipeline(unsigned int code, const Size &size);\n> +\n> void converterInputDone(FrameBuffer *buffer);\n> void converterOutputDone(FrameBuffer *buffer);\n> };\n> @@ -466,58 +468,11 @@ int SimpleCameraData::init()\n> return ret;\n> \n> /*\n> - * Enumerate the possible pipeline configurations. For each media bus\n> - * format supported by the sensor, propagate the formats through the\n> - * pipeline, and enumerate the corresponding possible V4L2 pixel\n> - * formats on the video node.\n> + * Generate the list of possible pipeline configurations by trying each\n> + * media bus format supported by the sensor.\n> */\n> - for (unsigned int code : sensor_->mbusCodes()) {\n> - V4L2SubdeviceFormat format{};\n> - format.mbus_code = code;\n> - format.size = sensor_->resolution();\n> -\n> - ret = setupFormats(&format, V4L2Subdevice::TryFormat);\n> - if (ret < 0) {\n> - LOG(SimplePipeline, Debug)\n> - << \"Media bus code \" << utils::hex(code, 4)\n> - << \" not supported for this pipeline\";\n> - /* Try next mbus_code supported by the sensor */\n> - continue;\n> - }\n> -\n> - V4L2VideoDevice::Formats videoFormats =\n> - video_->formats(format.mbus_code);\n> -\n> - LOG(SimplePipeline, Debug)\n> - << \"Adding configuration for \" << format.size\n> - << \" in pixel formats [ \"\n> - << utils::join(videoFormats, \", \",\n> - [](const auto &f) {\n> - return f.first.toString();\n> - })\n> - << \" ]\";\n> -\n> - for (const auto &videoFormat : videoFormats) {\n> - PixelFormat pixelFormat = videoFormat.first.toPixelFormat();\n> - if (!pixelFormat)\n> - continue;\n> -\n> - Configuration config;\n> - config.code = code;\n> - config.captureFormat = pixelFormat;\n> - config.captureSize = format.size;\n> -\n> - if (!converter_) {\n> - config.outputFormats = { pixelFormat };\n> - config.outputSizes = config.captureSize;\n> - } else {\n> - config.outputFormats = converter_->formats(pixelFormat);\n> - config.outputSizes = converter_->sizes(format.size);\n> - }\n> -\n> - configs_.push_back(config);\n> - }\n> - }\n> + for (unsigned int code : sensor_->mbusCodes())\n> + tryPipeline(code, sensor_->resolution());\n> \n> if (configs_.empty()) {\n> LOG(SimplePipeline, Error) << \"No valid configuration found\";\n> @@ -541,6 +496,67 @@ int SimpleCameraData::init()\n> return 0;\n> }\n> \n> +/*\n> + * Generate a list of supported pipeline configurations for a sensor media bus\n> + * code and size.\n> + *\n> + * First propagate the media bus code and size through the pipeline from the\n> + * camera sensor to the video node. Then, query the video node for all supported\n> + * pixel formats compatible with the media bus code. For each pixel format, store\n> + * a full pipeline configuration in the configs_ vector.\n> + */\n> +void SimpleCameraData::tryPipeline(unsigned int code, const Size &size)\n> +{\n> + /*\n> + * Propagate the format through the pipeline, and enumerate the\n> + * corresponding possible V4L2 pixel formats on the video node.\n> + */\n> + V4L2SubdeviceFormat format{};\n> + format.mbus_code = code;\n> + format.size = size;\n> +\n> + int ret = setupFormats(&format, V4L2Subdevice::TryFormat);\n> + if (ret < 0) {\n> + /* Pipeline configuration failed, skip this configuration. */\n> + LOG(SimplePipeline, Debug)\n> + << \"Media bus code \" << utils::hex(code, 4)\n> + << \" not supported for this pipeline\";\n> + return;\n> + }\n> +\n> + V4L2VideoDevice::Formats videoFormats = video_->formats(format.mbus_code);\n> +\n> + LOG(SimplePipeline, Debug)\n> + << \"Adding configuration for \" << format.size\n> + << \" in pixel formats [ \"\n> + << utils::join(videoFormats, \", \",\n> + [](const auto &f) {\n> + return f.first.toString();\n> + })\n> + << \" ]\";\n> +\n> + for (const auto &videoFormat : videoFormats) {\n> + PixelFormat pixelFormat = videoFormat.first.toPixelFormat();\n> + if (!pixelFormat)\n> + continue;\n> +\n> + Configuration config;\n> + config.code = code;\n> + config.captureFormat = pixelFormat;\n> + config.captureSize = format.size;\n> +\n> + if (!converter_) {\n> + config.outputFormats = { pixelFormat };\n> + config.outputSizes = config.captureSize;\n> + } else {\n> + config.outputFormats = converter_->formats(pixelFormat);\n> + config.outputSizes = converter_->sizes(format.size);\n> + }\n> +\n> + configs_.push_back(config);\n> + }\n> +}\n> +\n> int SimpleCameraData::setupLinks()\n> {\n> int ret;\n> -- \n> Regards,\n> \n> Laurent Pinchart\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 06FAFBD808\n\tfor ;\n\tWed, 15 Jun 2022 23:05:56 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id 3199D65635;\n\tThu, 16 Jun 2022 01:05:55 +0200 (CEST)","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 D60CD601EF\n\tfor ;\n\tThu, 16 Jun 2022 01:05:53 +0200 (CEST)","from pendragon.ideasonboard.com\n\t(cpc89244-aztw30-2-0-cust3082.18-1.cable.virginm.net [86.31.172.11])\n\tby perceval.ideasonboard.com (Postfix) with ESMTPSA id 3F483415;\n\tThu, 16 Jun 2022 01:05:53 +0200 (CEST)"],"DKIM-Signature":["v=1; a=rsa-sha256; c=relaxed/simple; d=libcamera.org;\n\ts=mail; t=1655334355;\n\tbh=fg9IMa7ukxw6/qzO3VPYhozNiPTC/4Rulj0dm6uPIT8=;\n\th=In-Reply-To:References:To:Date:Subject:List-Id:List-Unsubscribe:\n\tList-Archive:List-Post:List-Help:List-Subscribe:From:Reply-To:Cc:\n\tFrom;\n\tb=11jdPZdfJl4F7LChKPyDeqtp7HECxWLAKmGnFkLRCPV8/n1ia9uE3DyjJow0nFSeX\n\tnhwg9VTMC//X57F4DeGKhT1C0iojWzBk68WmvI2aQXT0E9KsCf/iAQ6/UXotGBQCI6\n\tVD0ZlGkrooUQk9rbWCHpGQOSNxNLwWoE0VKvmzmbX7t9KXaeuivB6xgh3XjzQXXjXQ\n\tUKocSwJ2jIbP24habb/h/OW0S8FOXIcJYHiHo9E/2WoEKUr6jnNpsXLtHpD3tkr0sv\n\tj4KUN6ASgRJ9npmrV2ERWMem/UX/QpZYSUCai2J5LPwnIh+2RBIFCm+0pUdk9huaKt\n\tXfJ+zQkcAYhlg==","v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com;\n\ts=mail; t=1655334353;\n\tbh=fg9IMa7ukxw6/qzO3VPYhozNiPTC/4Rulj0dm6uPIT8=;\n\th=In-Reply-To:References:Subject:From:Cc:To:Date:From;\n\tb=TVPY+oPRgse8Xgm51Qw0VVNwkrPwg8rnEpWJD6uvu/jr8ka3q9W051zgvATjcmSUC\n\tr6mfVYUljwdvbiWUL9aA73jb3pzybqfdRVKGMedPVA6A4GkpvAL6Z2Pj7w5rXpJj8C\n\tHyU3j2vv+akA2aIlPKVdmeUTLLosON1IFkb77IOs="],"Authentication-Results":"lancelot.ideasonboard.com; dkim=pass (1024-bit key; \n\tunprotected) header.d=ideasonboard.com\n\theader.i=@ideasonboard.com\n\theader.b=\"TVPY+oPR\"; dkim-atps=neutral","Content-Type":"text/plain; charset=\"utf-8\"","MIME-Version":"1.0","Content-Transfer-Encoding":"quoted-printable","In-Reply-To":"<20220612152311.8408-4-laurent.pinchart@ideasonboard.com>","References":"<20220612152311.8408-1-laurent.pinchart@ideasonboard.com>\n\t<20220612152311.8408-4-laurent.pinchart@ideasonboard.com>","To":"Laurent Pinchart ,\n\tlibcamera-devel@lists.libcamera.org","Date":"Thu, 16 Jun 2022 00:05:50 +0100","Message-ID":"<165533435080.2586493.6690601453915436163@Monstersaurus>","User-Agent":"alot/0.10","Subject":"Re: [libcamera-devel] [PATCH v2 3/5] libcamera: pipeline: simple:\n\tFactor out format test to separate function","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":"Kieran Bingham via libcamera-devel\n\t","Reply-To":"Kieran Bingham ","Cc":"Dorota Czaplejewicz ,\n\tBenjamin Schaaf ","Errors-To":"libcamera-devel-bounces@lists.libcamera.org","Sender":"\"libcamera-devel\" "}}]