[{"id":15794,"web_url":"https://patchwork.libcamera.org/comment/15794/","msgid":"<20210322182432.sh6weburwhy7k2cq@uno.localdomain>","date":"2021-03-22T18:24:32","subject":"Re: [libcamera-devel] [PATCH 2/3] pipeline: simple: Add\n\tFrameDurations to stream configuration","submitter":{"id":3,"url":"https://patchwork.libcamera.org/api/people/3/","name":"Jacopo Mondi","email":"jacopo@jmondi.org"},"content":"Hi Marian,\n\nOn Mon, Mar 22, 2021 at 11:42:41AM +0100, Marian Cichy wrote:\n> Add a instance of CameraSensorInfo to the simple pipeline and populate\n> it with the sensorInfo from the sensor. With the informations from the\n> sensor, compute the frame duration by using lineLength, minFrameLength\n> and pixelRate and save it in the controlList of the stream.\n\nI would more simply phrase it as:\n\nUse the camera sensor frame lenght an pixel rate to calculate the\nper-stream FrameDurations in the simple pipeline handler.\n\n>\n> Signed-off-by: Marian Cichy \n> ---\n> src/libcamera/pipeline/simple/simple.cpp | 9 +++++++++\n> 1 file changed, 9 insertions(+)\n>\n> diff --git a/src/libcamera/pipeline/simple/simple.cpp b/src/libcamera/pipeline/simple/simple.cpp\n> index 24d940fc..2df83302 100644\n> --- a/src/libcamera/pipeline/simple/simple.cpp\n> +++ b/src/libcamera/pipeline/simple/simple.cpp\n> @@ -22,6 +22,8 @@\n> #include \n>\n> #include \n> +#include \n> +#include \n> #include \n> #include \n>\n> @@ -176,6 +178,7 @@ public:\n>\n> \tstd::vector streams_;\n> \tstd::unique_ptr sensor_;\n> +\tCameraSensorInfo sensorInfo_;\n> \tstd::list entities_;\n> \tV4L2VideoDevice *video_;\n>\n> @@ -339,6 +342,8 @@ SimpleCameraData::SimpleCameraData(SimplePipelineHandler *pipe,\n> \tret = sensor_->init();\n> \tif (ret)\n> \t\tsensor_.reset();\n> +\n> +\tsensor_->sensorInfo(&sensorInfo_);\n\nIf I'm not mistaken the simple pipeline handler is meant to work with\nYUV sensors too, for which it is not possible to retrieve the\nCameraSensorInfo so you should check the return error and not try to\naccess it if this call fails.\n\nAlso, the content of CameraSensorInfo depends on the current sensor\nconfiguration so you shall fetch it at configure() time otherwise you\nwould work with a stale representation of the sensor configuration.\n\n> }\n>\n> int SimpleCameraData::init()\n> @@ -679,6 +684,10 @@ CameraConfiguration *SimplePipelineHandler::generateConfiguration(Camera *camera\n> \t\tStreamConfiguration cfg{ StreamFormats{ formats } };\n> \t\tcfg.pixelFormat = formats.begin()->first;\n> \t\tcfg.size = formats.begin()->second[0].max;\n> +\t\tdouble frameDuration = (data->sensorInfo_.minFrameLength * data->sensorInfo_.lineLength) /\n> +\t\t\t(data->sensorInfo_.pixelRate / 1'000'000);\n\nUsing the sensor information at generateConfiguration() time is\nunfortunately a no-go in my opinion. As the information there reported\ndepend on the configuration applied to the sensor, you would here get\neither the ones relative to the default sensor configuration (the\nfirst time you generate a configuration) or even worse the information\nrelative to the configuration that was applied the last time\nCamera::configure() was called. IOW do not use CameraSensorInfo if not\nafter a setFormat() has been called on the sensor.\n\nIf you intend to calculate the absolute limits of the Camera that's\ndifferent, and you can read as an example the IPU3 pipeline handler\ninitControls() how its done (and we had to made assumptions there\ntoo).\n\nAs you can see minFrameLength and well as maxFrameLenght represent the\nlower and upper frame lenght bounds, not the current frame lenght :/\nIf you where doing this at configure() time I would have rather used\nthe frame sizes and get the VBLANK control value to compute the frame\nlenght, but that's a problem for you as you want this information to\nbe reported at generateConfiguration() time not at configure() time\nand at this very time no configuration has been applied to the sensor\n:/\n\nIOW at 'generateConfiguration()' time you can only have a (probably\nmisleading) approximation of the current frame duration, as you would\nneed to apply a configuration to the sensor to have that information\ncomputed properly. And as you can only here estimate the absolute\nlimits by using the min/max frame lenghtes, I would rather compute\nthem at camera creation time as we do in the other pipeline handlers\n:/\n\nI'm sorry but I don't see an easy way out here as we really need to\nkick the driver with a SUBDEV_S_FMT to have at least VBLANK and pixel\nrate updated. Maybe Laurent has some ideas ?\n\nThanks\n j\n\n> +\t\tcfg.controls.set(controls::FrameDurations, { static_cast(frameDuration),\n> +\t\t\t\t\t\t\t static_cast(frameDuration) });\n>\n> \t\tconfig->addConfiguration(cfg);\n> \t}\n> --\n> 2.29.2\n>\n> _______________________________________________\n> libcamera-devel mailing list\n> libcamera-devel@lists.libcamera.org\n> https://lists.libcamera.org/listinfo/libcamera-devel","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 31ECBBD80C\n\tfor ;\n\tMon, 22 Mar 2021 18:24:04 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id 842AD68D62;\n\tMon, 22 Mar 2021 19:24:03 +0100 (CET)","from relay10.mail.gandi.net (relay10.mail.gandi.net\n\t[217.70.178.230])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTPS id 74ACF68D50\n\tfor ;\n\tMon, 22 Mar 2021 19:24:01 +0100 (CET)","from uno.localdomain (host-82-63-7-72.business.telecomitalia.it\n\t[82.63.7.72]) (Authenticated sender: jacopo@jmondi.org)\n\tby relay10.mail.gandi.net (Postfix) with ESMTPSA id 4F927240010;\n\tMon, 22 Mar 2021 18:23:59 +0000 (UTC)"],"Date":"Mon, 22 Mar 2021 19:24:32 +0100","From":"Jacopo Mondi ","To":"Marian Cichy ","Message-ID":"<20210322182432.sh6weburwhy7k2cq@uno.localdomain>","References":"<20210322104242.31107-1-m.cichy@pengutronix.de>\n\t<20210322104242.31107-3-m.cichy@pengutronix.de>","MIME-Version":"1.0","Content-Disposition":"inline","In-Reply-To":"<20210322104242.31107-3-m.cichy@pengutronix.de>","Subject":"Re: [libcamera-devel] [PATCH 2/3] pipeline: simple: Add\n\tFrameDurations to stream configuration","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","Cc":"libcamera-devel@lists.libcamera.org, graphics@pengutronix.de","Content-Type":"text/plain; charset=\"us-ascii\"","Content-Transfer-Encoding":"7bit","Errors-To":"libcamera-devel-bounces@lists.libcamera.org","Sender":"\"libcamera-devel\" "}}]