[{"id":10878,"web_url":"https://patchwork.libcamera.org/comment/10878/","msgid":"<20200626024130.GZ5865@pendragon.ideasonboard.com>","date":"2020-06-26T02:41:30","subject":"Re: [libcamera-devel] [PATCH v3 7/8] libcamera: camera_sensor:\n\tInitialize frame durations","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 Fri, Jun 05, 2020 at 04:10:01PM +0200, Jacopo Mondi wrote:\n> Calculate the camera minimum and maximum frame durations, and register\n> the corresponding FrameDurationLimits property with those values.\n> \n> Signed-off-by: Jacopo Mondi \n> ---\n> src/libcamera/camera_sensor.cpp | 74 +++++++++++++++++++++++++++++++++\n> 1 file changed, 74 insertions(+)\n> \n> diff --git a/src/libcamera/camera_sensor.cpp b/src/libcamera/camera_sensor.cpp\n> index b9428175c55e..b144940f047d 100644\n> --- a/src/libcamera/camera_sensor.cpp\n> +++ b/src/libcamera/camera_sensor.cpp\n> @@ -283,6 +283,80 @@ int CameraSensor::initProperties()\n> \t\tpropertyValue = 0;\n> \tproperties_.set(properties::Rotation, propertyValue);\n> \n> +\t/* Frame durations. */\n> +\tstd::map controlLimits;\n> +\tstd::array controlIds = {\n> +\t\tV4L2_CID_VBLANK,\n> +\t\tV4L2_CID_HBLANK,\n> +\t\tV4L2_CID_PIXEL_RATE\n> +\t};\n\nThis won't work well with existing kernel drivers, for two reasons. One\nof them is that the pixel rate is not guaranteed to be constant, it can\ndepend on the selected mode, or, for sensor drivers that are not\nmode-based, controlled by userspace through the link frequency control.\nThe second reason is that lost of sensor drivers change the limits of\nthe h/v blank controls based on the selected mode (or sensor\nconfiguration in general).\n\nThe latter is likely something we need to fix in V4L2, I think the\nmaximum h/v blank values should not be mode-dependent. We don't have to\nfix all sensor drivers in one go, but we need to at least update the\nspec (and thus first reach an agreement). The former isn't a bug, so it\ncan't be \"fixed\".\n\nWe will very likely need something quite more elaborate than the code\nbelow. I'm available for a brainstorming session on this topic.\n\n> +\tfor (uint32_t controlId : controlIds) {\n> +\t\t/*\n> +\t\t * Make sure the subdevice reports VBLANK, HBLANK and PIXEL_RATE\n> +\t\t * controls and collect the control information.\n> +\t\t */\n> +\t\tauto it = controls.find(controlId);\n> +\t\tif (it == controls.end()) {\n> +\t\t\tLOG(CameraSensor, Error)\n> +\t\t\t\t<< \"Camera sensor does not support control \"\n> +\t\t\t\t<< controlId;\n> +\t\t\treturn -EINVAL;\n> +\t\t}\n> +\n> +\t\tcontrolLimits[controlId] = &it->second;\n> +\t}\n> +\n> +\t/*\n> +\t * While the maximum line and frame sizes can be calculated by adding\n> +\t * each control's maximum supported value to the current configuration,\n> +\t * the minimum sizes shall be calculated by using the minimum available\n> +\t * resolution. Get the current and smalles available size then calculate\n> +\t * the frame durations.\n> +\t */\n> +\tV4L2SubdeviceFormat fmt{};\n> +\tint ret = subdev_->getFormat(0, &fmt);\n> +\tif (ret)\n> +\t\treturn ret;\n> +\tconst Size ¤tSize = fmt.size;\n> +\tconst Size &minSize = *sizes_.begin();\n> +\n> +\tstd::vector durations;\n> +\tint32_t duration;\n> +\n> +\t/*\n> +\t * Pixel rate is reported in Hz while frame duration is expressed in\n> +\t * nanoseconds. Adjust it and calculate the minimum and maximum\n> +\t * durations.\n> +\t */\n> +\tint64_t pixelRate = controlLimits[V4L2_CID_PIXEL_RATE]->max().get();\n> +\tfloat rate = static_cast(pixelRate) / 1e9F;\n> +\n> +\t/* Minimum frame duration: higher frame rate. */\n> +\tduration = (minSize.width +\n> +\t\t controlLimits[V4L2_CID_HBLANK]->min().get())\n> +\t\t * (minSize.height +\n> +\t\t controlLimits[V4L2_CID_VBLANK]->min().get());\n> +\tduration = static_cast(static_cast(duration) / rate);\n> +\tdurations.push_back(duration);\n> +\n> +\tpixelRate = controlLimits[V4L2_CID_PIXEL_RATE]->min().get();\n> +\trate = static_cast(pixelRate) / 1e9F;\n> +\n> +\t/* Maximum frame duration: lower frame rate. */\n> +\tduration = (currentSize.width +\n> +\t\t controlLimits[V4L2_CID_HBLANK]->max().get())\n> +\t\t * (currentSize.height +\n> +\t\t controlLimits[V4L2_CID_VBLANK]->max().get());\n> +\tduration = static_cast(static_cast(duration) / rate);\n> +\tdurations.push_back(duration);\n> +\n> +\tproperties_.set(properties::FrameDurationLimits, durations);\n> +\n> +\tLOG(CameraSensor, Debug) << \"Frame durations interval = [\"\n> +\t\t\t\t << durations[0] << \" - \"\n> +\t\t\t\t << durations[1] << \"]\";\n> +\n> +\treturn 0;\n> }\n> \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 6B1C6C2E65\n\tfor ;\n\tFri, 26 Jun 2020 02:41:34 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id DB71B609C8;\n\tFri, 26 Jun 2020 04:41:33 +0200 (CEST)","from perceval.ideasonboard.com (perceval.ideasonboard.com\n\t[213.167.242.64])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTPS id A891E603BA\n\tfor ;\n\tFri, 26 Jun 2020 04:41:32 +0200 (CEST)","from pendragon.ideasonboard.com (81-175-216-236.bb.dnainternet.fi\n\t[81.175.216.236])\n\tby perceval.ideasonboard.com (Postfix) with ESMTPSA id 1376D72E;\n\tFri, 26 Jun 2020 04:41:32 +0200 (CEST)"],"Authentication-Results":"lancelot.ideasonboard.com;\n\tdkim=fail reason=\"signature verification failed\" (1024-bit key;\n\tunprotected) header.d=ideasonboard.com header.i=@ideasonboard.com\n\theader.b=\"rLATnEqm\"; dkim-atps=neutral","DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com;\n\ts=mail; t=1593139292;\n\tbh=9Bvy83xv4ylaGsJATf96MNPpsEAvo9m0c2zeQ/u0tUM=;\n\th=Date:From:To:Cc:Subject:References:In-Reply-To:From;\n\tb=rLATnEqmSziX0bbER51yISi/Q5gSu8hyUR22tOxS4PPjOK3GyHv99qfqAAvXXmqdi\n\tx/1AmrmXQT687fPpwHiTwI/7lBBFAs+LNCugaxBbzB6+Zst/vcOnxI4PbrRw0iaRWJ\n\t64E/zX3S584oy4Q1sN4sY1d+0P+zn31f4ASPLQUk=","Date":"Fri, 26 Jun 2020 05:41:30 +0300","From":"Laurent Pinchart ","To":"Jacopo Mondi ","Message-ID":"<20200626024130.GZ5865@pendragon.ideasonboard.com>","References":"<20200605141002.49119-1-jacopo@jmondi.org>\n\t<20200605141002.49119-8-jacopo@jmondi.org>","MIME-Version":"1.0","Content-Disposition":"inline","In-Reply-To":"<20200605141002.49119-8-jacopo@jmondi.org>","Subject":"Re: [libcamera-devel] [PATCH v3 7/8] libcamera: camera_sensor:\n\tInitialize frame durations","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","Content-Type":"text/plain; charset=\"us-ascii\"","Content-Transfer-Encoding":"7bit","Errors-To":"libcamera-devel-bounces@lists.libcamera.org","Sender":"\"libcamera-devel\" "}}]