[{"id":25438,"web_url":"https://patchwork.libcamera.org/comment/25438/","msgid":"","date":"2022-10-17T23:34:39","subject":"Re: [libcamera-devel] [PATCH] camera_sensor: Add minimum and\n\tmaximum line length to IPACameraSensorInfo","submitter":{"id":2,"url":"https://patchwork.libcamera.org/api/people/2/","name":"Laurent Pinchart","email":"laurent.pinchart@ideasonboard.com"},"content":"Hi Naush,\n\nThank you for the patch.\n\nOn Mon, Oct 10, 2022 at 08:42:32AM +0100, Naushir Patuck via libcamera-devel wrote:\n> Add fields for minimum and maximum line length (in units of pixels) to the\n> IPACameraSensorInfo structure. This replaces the existing lineLength field.\n> \n> Update the ipu3, raspberrypi and rkisp1 IPAs to use IPACameraSensorInfo::minLineLength\n> instead of IPACameraSensorInfo::lineLength, as logically we will always want to\n> use the fastest sensor readout by default.\n> \n> Since the IPAs now use minLineLength for their calculations, set the starting\n> value of the V4L2_CID_HBLANK control to its minimum in CameraSensor::init().\n> \n> Signed-off-by: Naushir Patuck \n> Tested-by: Dave Stevenson \n\nReviewed-by: Laurent Pinchart \n\n> ---\n> include/libcamera/ipa/core.mojom | 21 +++++++++++++------\n> src/ipa/ipu3/ipu3.cpp | 6 ++++--\n> src/ipa/raspberrypi/raspberrypi.cpp | 2 +-\n> src/ipa/rkisp1/rkisp1.cpp | 2 +-\n> src/libcamera/camera_sensor.cpp | 32 +++++++++++++++++++++++++++--\n> 5 files changed, 51 insertions(+), 12 deletions(-)\n> \n> diff --git a/include/libcamera/ipa/core.mojom b/include/libcamera/ipa/core.mojom\n> index 74f3339e56f2..2bc3028c22d6 100644\n> --- a/include/libcamera/ipa/core.mojom\n> +++ b/include/libcamera/ipa/core.mojom\n> @@ -172,10 +172,17 @@ module libcamera;\n> */\n> \n> /**\n> - * \\var IPACameraSensorInfo::lineLength\n> - * \\brief Total line length in pixels\n> + * \\var IPACameraSensorInfo::minLineLength\n> + * \\brief The minimum line length in pixels\n> *\n> - * The total line length in pixel clock periods, including blanking.\n> + * The minimum allowable line length in pixel clock periods, including blanking.\n> + */\n> +\n> +/**\n> + * \\var IPACameraSensorInfo::maxLineLength\n> + * \\brief The maximum line length in pixels\n> + *\n> + * The maximum allowable line length in pixel clock periods, including blanking.\n> */\n> \n> /**\n> @@ -189,7 +196,7 @@ module libcamera;\n> * To obtain the minimum frame duration:\n> *\n> * \\verbatim\n> - frameDuration(s) = minFrameLength(lines) * lineLength(pixels) / pixelRate(pixels per second)\n> + frameDuration(s) = minFrameLength(lines) * minLineLength(pixels) / pixelRate(pixels per second)\n> \\endverbatim\n> */\n> \n> @@ -204,7 +211,7 @@ module libcamera;\n> * To obtain the maximum frame duration:\n> *\n> * \\verbatim\n> - frameDuration(s) = maxFrameLength(lines) * lineLength(pixels) / pixelRate(pixels per second)\n> + frameDuration(s) = maxFrameLength(lines) * maxLineLength(pixels) / pixelRate(pixels per second)\n> \\endverbatim\n> */\n> struct IPACameraSensorInfo {\n> @@ -217,7 +224,9 @@ struct IPACameraSensorInfo {\n> \tSize outputSize;\n> \n> \tuint64 pixelRate;\n> -\tuint32 lineLength;\n> +\n> +\tuint32 minLineLength;\n> +\tuint32 maxLineLength;\n> \n> \tuint32 minFrameLength;\n> \tuint32 maxFrameLength;\n> diff --git a/src/ipa/ipu3/ipu3.cpp b/src/ipa/ipu3/ipu3.cpp\n> index b93a09d40c39..c89f76c56ee3 100644\n> --- a/src/ipa/ipu3/ipu3.cpp\n> +++ b/src/ipa/ipu3/ipu3.cpp\n> @@ -336,7 +336,8 @@ int IPAIPU3::init(const IPASettings &settings,\n> \n> \t/* Clean context */\n> \tcontext_.configuration = {};\n> -\tcontext_.configuration.sensor.lineDuration = sensorInfo.lineLength * 1.0s / sensorInfo.pixelRate;\n> +\tcontext_.configuration.sensor.lineDuration = sensorInfo.minLineLength\n> +\t\t\t\t\t\t * 1.0s / sensorInfo.pixelRate;\n> \n> \t/* Load the tuning data file. */\n> \tFile file(settings.configurationFile);\n> @@ -499,7 +500,8 @@ int IPAIPU3::configure(const IPAConfigInfo &configInfo,\n> \tcontext_.frameContexts.clear();\n> \n> \t/* Initialise the sensor configuration. */\n> -\tcontext_.configuration.sensor.lineDuration = sensorInfo_.lineLength * 1.0s / sensorInfo_.pixelRate;\n> +\tcontext_.configuration.sensor.lineDuration = sensorInfo_.minLineLength\n> +\t\t\t\t\t\t * 1.0s / sensorInfo_.pixelRate;\n> \n> \t/*\n> \t * Compute the sensor V4L2 controls to be used by the algorithms and\n> diff --git a/src/ipa/raspberrypi/raspberrypi.cpp b/src/ipa/raspberrypi/raspberrypi.cpp\n> index 8d731435764e..358a119da222 100644\n> --- a/src/ipa/raspberrypi/raspberrypi.cpp\n> +++ b/src/ipa/raspberrypi/raspberrypi.cpp\n> @@ -356,7 +356,7 @@ void IPARPi::setMode(const IPACameraSensorInfo &sensorInfo)\n> \t * Calculate the line length as the ratio between the line length in\n> \t * pixels and the pixel rate.\n> \t */\n> -\tmode_.lineLength = sensorInfo.lineLength * (1.0s / sensorInfo.pixelRate);\n> +\tmode_.lineLength = sensorInfo.minLineLength * (1.0s / sensorInfo.pixelRate);\n> \n> \t/*\n> \t * Set the frame length limits for the mode to ensure exposure and\n> diff --git a/src/ipa/rkisp1/rkisp1.cpp b/src/ipa/rkisp1/rkisp1.cpp\n> index 32feb1682749..ddb22d98eb41 100644\n> --- a/src/ipa/rkisp1/rkisp1.cpp\n> +++ b/src/ipa/rkisp1/rkisp1.cpp\n> @@ -246,7 +246,7 @@ int IPARkISP1::configure([[maybe_unused]] const IPACameraSensorInfo &info,\n> \tcontext_.configuration.hw.revision = hwRevision_;\n> \n> \tcontext_.configuration.sensor.size = info.outputSize;\n> -\tcontext_.configuration.sensor.lineDuration = info.lineLength * 1.0s / info.pixelRate;\n> +\tcontext_.configuration.sensor.lineDuration = info.minLineLength * 1.0s / info.pixelRate;\n> \n> \t/*\n> \t * When the AGC computes the new exposure values for a frame, it needs\n> diff --git a/src/libcamera/camera_sensor.cpp b/src/libcamera/camera_sensor.cpp\n> index 911fd0beae4e..572a313a8f99 100644\n> --- a/src/libcamera/camera_sensor.cpp\n> +++ b/src/libcamera/camera_sensor.cpp\n> @@ -176,6 +176,32 @@ int CameraSensor::init()\n> \tif (ret)\n> \t\treturn ret;\n> \n> +\t/*\n> +\t * Set HBLANK to the minimum to start with a well-defined line length,\n> +\t * allowing IPA modules that do not modify HBLANK to use the sensor\n> +\t * minimum line length in their calculations.\n> +\t *\n> +\t * At present, there is no way of knowing if a control is read-only.\n> +\t * As a workaround, assume that if the minimum and maximum values of\n> +\t * the V4L2_CID_HBLANK control are the same, it implies the control\n> +\t * is read-only.\n> +\t *\n> +\t * \\todo The control API ought to have a flag to specify if a control\n> +\t * is read-only which could be used below.\n> +\t */\n> +\tconst ControlInfo hblank = ctrls.infoMap()->at(V4L2_CID_HBLANK);\n> +\tconst int32_t hblankMin = hblank.min().get();\n> +\tconst int32_t hblankMax = hblank.max().get();\n> +\n> +\tif (hblankMin != hblankMax) {\n> +\t\tControlList ctrl(subdev_->controls());\n> +\n> +\t\tctrl.set(V4L2_CID_HBLANK, hblankMin);\n> +\t\tret = subdev_->setControls(&ctrl);\n> +\t\tif (ret)\n> +\t\t\treturn ret;\n> +\t}\n> +\n> \treturn applyTestPatternMode(controls::draft::TestPatternModeEnum::TestPatternModeOff);\n> }\n> \n> @@ -883,10 +909,12 @@ int CameraSensor::sensorInfo(IPACameraSensorInfo *info) const\n> \t\treturn -EINVAL;\n> \t}\n> \n> -\tint32_t hblank = ctrls.get(V4L2_CID_HBLANK).get();\n> -\tinfo->lineLength = info->outputSize.width + hblank;\n> \tinfo->pixelRate = ctrls.get(V4L2_CID_PIXEL_RATE).get();\n> \n> +\tconst ControlInfo hblank = ctrls.infoMap()->at(V4L2_CID_HBLANK);\n> +\tinfo->minLineLength = info->outputSize.width + hblank.min().get();\n> +\tinfo->maxLineLength = info->outputSize.width + hblank.max().get();\n> +\n> \tconst ControlInfo vblank = ctrls.infoMap()->at(V4L2_CID_VBLANK);\n> \tinfo->minFrameLength = info->outputSize.height + vblank.min().get();\n> \tinfo->maxFrameLength = info->outputSize.height + vblank.max().get();","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 73B37C0DA4\n\tfor ;\n\tMon, 17 Oct 2022 23:35:07 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id B03DD62E05;\n\tTue, 18 Oct 2022 01:35:06 +0200 (CEST)","from perceval.ideasonboard.com (perceval.ideasonboard.com\n\t[213.167.242.64])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTPS id 50EB961F55\n\tfor ;\n\tTue, 18 Oct 2022 01:35:04 +0200 (CEST)","from pendragon.ideasonboard.com (62-78-145-57.bb.dnainternet.fi\n\t[62.78.145.57])\n\tby perceval.ideasonboard.com (Postfix) with ESMTPSA id 8F02A8CC;\n\tTue, 18 Oct 2022 01:35:03 +0200 (CEST)"],"DKIM-Signature":["v=1; a=rsa-sha256; c=relaxed/simple; d=libcamera.org;\n\ts=mail; t=1666049706;\n\tbh=83bklqwH60NRDkJgufngvOo9vfVEvlFMNS2Wir5nZ7c=;\n\th=Date:To:References:In-Reply-To:Subject:List-Id:List-Unsubscribe:\n\tList-Archive:List-Post:List-Help:List-Subscribe:From:Reply-To:Cc:\n\tFrom;\n\tb=P2LYfUwbCl0khNJ1svS0Eme6a0jIk3Tj26Exw3HaTu5SEIbFTi7EvVUvRBzEDrdsa\n\tVrMy+8QsKpmXJT81SnEmaYUdK4/oFN9Ob+nIB5jSCc0JVxUVprrnSW7L7fobHjw2eL\n\tIH9HdS3mQaatVDvURzf6INcKMJkyS0Dtc70dnQ/K+xz3y2ogmGi3X2w3K+kW3kBqq0\n\tooHXjB+ZEeV16yrPVIGJZqo+YcYDS+nfJVBu8xhQJ9Q5BjHULoRnm9IUsqLwe7Vl3B\n\tA1+UIH+F+wo/CVQCmaSCHfTRjdlvCGVAx1YSezNnx/NTaSrg1uDxum2X24Qa6cPdIE\n\tv3tH1fIfb4H5A==","v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com;\n\ts=mail; t=1666049703;\n\tbh=83bklqwH60NRDkJgufngvOo9vfVEvlFMNS2Wir5nZ7c=;\n\th=Date:From:To:Cc:Subject:References:In-Reply-To:From;\n\tb=RKe+oi3H+27wda1PJcO+TSHI/5HuGRWdhy3E1N80jLaN4q2Ha6+7m4C7GS+CoASdX\n\thb9dW1JzeW4AxQOhO/WVtEEK04+X4BX0iSSuSFakfYUTUamzTkZA6O6MRSJn2hRyIL\n\tIC5oGca03bPm1Cj4GtG4hLIm2oWgffopeZwEKsig="],"Authentication-Results":"lancelot.ideasonboard.com; dkim=pass (1024-bit key; \n\tunprotected) header.d=ideasonboard.com\n\theader.i=@ideasonboard.com\n\theader.b=\"RKe+oi3H\"; dkim-atps=neutral","Date":"Tue, 18 Oct 2022 02:34:39 +0300","To":"Naushir Patuck ","Message-ID":"","References":"<20221006131744.5179-2-naush@raspberrypi.com>\n\t<20221010074232.2404-1-naush@raspberrypi.com>","MIME-Version":"1.0","Content-Type":"text/plain; charset=utf-8","Content-Disposition":"inline","In-Reply-To":"<20221010074232.2404-1-naush@raspberrypi.com>","Subject":"Re: [libcamera-devel] [PATCH] camera_sensor: Add minimum and\n\tmaximum line length to IPACameraSensorInfo","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":"Laurent Pinchart via libcamera-devel\n\t","Reply-To":"Laurent Pinchart ","Cc":"libcamera-devel@lists.libcamera.org","Errors-To":"libcamera-devel-bounces@lists.libcamera.org","Sender":"\"libcamera-devel\" "}}]