[{"id":14530,"web_url":"https://patchwork.libcamera.org/comment/14530/","msgid":"","date":"2021-01-11T23:14:36","subject":"Re: [libcamera-devel] [PATCH v11 2/3] libcamera: raspberrypi: Add\n\tcontrol of sensor vblanking","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 Wed, Jan 06, 2021 at 10:06:58AM +0000, Naushir Patuck wrote:\n> Add support for setting V4L2_CID_VBLANK appropriately when setting\n> V4L2_CID_EXPOSURE. This will allow adaptive framerates during\n> viewfinder use cases (e.g. when the exposure time goes above 33ms, we\n> can reduce the framerate to lower than 30fps).\n> \n> The minimum and maximum frame durations are provided via libcamera\n> controls, and will prioritise exposure time limits over any AGC request.\n> \n> V4L2_CID_VBLANK is controlled through the staggered writer, just like\n> the exposure and gain controls.\n> \n> Signed-off-by: Naushir Patuck \n> Reviewed-by: David Plowman \n> Tested-by: David Plowman \n> Reviewed-by: Jacopo Mondi \n> ---\n> include/libcamera/ipa/raspberrypi.h | 1 +\n> src/ipa/raspberrypi/cam_helper.cpp | 35 +++++++++++-\n> src/ipa/raspberrypi/cam_helper.hpp | 15 ++++-\n> src/ipa/raspberrypi/cam_helper_imx219.cpp | 13 ++++-\n> src/ipa/raspberrypi/cam_helper_imx477.cpp | 11 +++-\n> src/ipa/raspberrypi/cam_helper_ov5647.cpp | 11 +++-\n> src/ipa/raspberrypi/raspberrypi.cpp | 56 ++++++++++++++++---\n> .../pipeline/raspberrypi/raspberrypi.cpp | 3 +-\n> 8 files changed, 130 insertions(+), 15 deletions(-)\n> \n> diff --git a/include/libcamera/ipa/raspberrypi.h b/include/libcamera/ipa/raspberrypi.h\n> index 01fe5abc..1de36039 100644\n> --- a/include/libcamera/ipa/raspberrypi.h\n> +++ b/include/libcamera/ipa/raspberrypi.h\n> @@ -65,6 +65,7 @@ static const ControlInfoMap Controls = {\n> \t{ &controls::Sharpness, ControlInfo(0.0f, 16.0f, 1.0f) },\n> \t{ &controls::ColourCorrectionMatrix, ControlInfo(-16.0f, 16.0f) },\n> \t{ &controls::ScalerCrop, ControlInfo(Rectangle{}, Rectangle(65535, 65535, 65535, 65535), Rectangle{}) },\n> +\t{ &controls::FrameDurations, ControlInfo(1000, 1000000000) },\n> };\n> \n> } /* namespace RPi */\n> diff --git a/src/ipa/raspberrypi/cam_helper.cpp b/src/ipa/raspberrypi/cam_helper.cpp\n> index 6efa0d7f..b7b8faf0 100644\n> --- a/src/ipa/raspberrypi/cam_helper.cpp\n> +++ b/src/ipa/raspberrypi/cam_helper.cpp\n> @@ -34,8 +34,10 @@ CamHelper *CamHelper::Create(std::string const &cam_name)\n> \treturn nullptr;\n> }\n> \n> -CamHelper::CamHelper(MdParser *parser)\n> -\t: parser_(parser), initialized_(false)\n> +CamHelper::CamHelper(MdParser *parser, unsigned int maxFrameLength,\n> +\t\t unsigned int frameIntegrationDiff)\n> +\t: parser_(parser), initialized_(false), maxFrameLength_(maxFrameLength),\n> +\t frameIntegrationDiff_(frameIntegrationDiff)\n> {\n> }\n> \n> @@ -56,6 +58,35 @@ double CamHelper::Exposure(uint32_t exposure_lines) const\n> \treturn exposure_lines * mode_.line_length / 1000.0;\n> }\n> \n> +uint32_t CamHelper::GetVBlanking(double &exposure, double minFrameDuration,\n> +\t\t\t\t double maxFrameDuration) const\n> +{\n> +\tuint32_t frameLengthMin, frameLengthMax, vblank;\n> +\tuint32_t exposureLines = ExposureLines(exposure);\n> +\n> +\tassert(initialized_);\n> +\n> +\t/*\n> +\t * Clamp frame length by the frame duration range and the maximum allowable\n> +\t * value in the sensor, given by maxFrameLength_.\n> +\t */\n> +\tframeLengthMin = std::clamp(1e3 * minFrameDuration / mode_.line_length,\n> +\t\t\t\t\t mode_.height, maxFrameLength_);\n> +\tframeLengthMax = std::clamp(1e3 * maxFrameDuration / mode_.line_length,\n> +\t\t\t\t\t mode_.height, maxFrameLength_);\n> +\t/*\n> +\t * Limit the exposure to the maximum frame duration requested, and\n> +\t * re-calculate if it has been clipped.\n> +\t */\n> +\texposureLines = std::min(frameLengthMax - frameIntegrationDiff_, exposureLines);\n> +\texposure = Exposure(exposureLines);\n> +\n> +\t/* Limit the vblank to the range allowed by the frame length limits. */\n> +\tvblank = std::clamp(exposureLines + frameIntegrationDiff_,\n> +\t\t\t frameLengthMin, frameLengthMax) - mode_.height;\n> +\treturn vblank;\n> +}\n> +\n> void CamHelper::SetCameraMode(const CameraMode &mode)\n> {\n> \tmode_ = mode;\n> diff --git a/src/ipa/raspberrypi/cam_helper.hpp b/src/ipa/raspberrypi/cam_helper.hpp\n> index 044c2866..b1739a57 100644\n> --- a/src/ipa/raspberrypi/cam_helper.hpp\n> +++ b/src/ipa/raspberrypi/cam_helper.hpp\n> @@ -62,12 +62,15 @@ class CamHelper\n> {\n> public:\n> \tstatic CamHelper *Create(std::string const &cam_name);\n> -\tCamHelper(MdParser *parser);\n> +\tCamHelper(MdParser *parser, unsigned int maxFrameLength,\n> +\t\t unsigned int frameIntegrationDiff);\n> \tvirtual ~CamHelper();\n> \tvoid SetCameraMode(const CameraMode &mode);\n> \tMdParser &Parser() const { return *parser_; }\n> \tuint32_t ExposureLines(double exposure_us) const;\n> \tdouble Exposure(uint32_t exposure_lines) const; // in us\n> +\tvirtual uint32_t GetVBlanking(double &exposure_us, double minFrameDuration,\n> +\t\t\t\t double maxFrameDuration) const;\n> \tvirtual uint32_t GainCode(double gain) const = 0;\n> \tvirtual double Gain(uint32_t gain_code) const = 0;\n> \tvirtual void GetDelays(int &exposure_delay, int &gain_delay) const;\n> @@ -76,10 +79,20 @@ public:\n> \tvirtual unsigned int HideFramesModeSwitch() const;\n> \tvirtual unsigned int MistrustFramesStartup() const;\n> \tvirtual unsigned int MistrustFramesModeSwitch() const;\n> +\n> protected:\n> \tMdParser *parser_;\n> \tCameraMode mode_;\n> +\n> +private:\n> \tbool initialized_;\n> +\t/* Largest possible frame length, in units of lines. */\n> +\tunsigned int maxFrameLength_;\n> +\t/*\n> +\t * Smallest difference between the frame length and integration time,\n> +\t * in units of lines.\n> +\t */\n> +\tunsigned int frameIntegrationDiff_;\n> };\n> \n> // This is for registering camera helpers with the system, so that the\n> diff --git a/src/ipa/raspberrypi/cam_helper_imx219.cpp b/src/ipa/raspberrypi/cam_helper_imx219.cpp\n> index db8ab879..8688ec09 100644\n> --- a/src/ipa/raspberrypi/cam_helper_imx219.cpp\n> +++ b/src/ipa/raspberrypi/cam_helper_imx219.cpp\n> @@ -49,13 +49,22 @@ public:\n> \tdouble Gain(uint32_t gain_code) const override;\n> \tunsigned int MistrustFramesModeSwitch() const override;\n> \tbool SensorEmbeddedDataPresent() const override;\n> +\n> +private:\n> +\t/*\n> +\t * Smallest difference between the frame length and integration time,\n> +\t * in units of lines.\n> +\t */\n> +\tstatic constexpr int frameIntegrationDiff = 4;\n> +\t/* Largest possible frame length, in units of lines. */\n> +\tstatic constexpr int maxFrameLength = 0xffff;\n> };\n> \n> CamHelperImx219::CamHelperImx219()\n> #if ENABLE_EMBEDDED_DATA\n> -\t: CamHelper(new MdParserImx219())\n> +\t: CamHelper(new MdParserImx219(), maxFrameLength, frameIntegrationDiff)\n> #else\n> -\t: CamHelper(new MdParserRPi())\n> +\t: CamHelper(new MdParserRPi(), maxFrameLength, frameIntegrationDiff)\n> #endif\n> {\n> }\n> diff --git a/src/ipa/raspberrypi/cam_helper_imx477.cpp b/src/ipa/raspberrypi/cam_helper_imx477.cpp\n> index 0e896ac7..53961310 100644\n> --- a/src/ipa/raspberrypi/cam_helper_imx477.cpp\n> +++ b/src/ipa/raspberrypi/cam_helper_imx477.cpp\n> @@ -38,10 +38,19 @@ public:\n> \tuint32_t GainCode(double gain) const override;\n> \tdouble Gain(uint32_t gain_code) const override;\n> \tbool SensorEmbeddedDataPresent() const override;\n> +\n> +private:\n> +\t/*\n> +\t * Smallest difference between the frame length and integration time,\n> +\t * in units of lines.\n> +\t */\n> +\tstatic constexpr int frameIntegrationDiff = 22;\n> +\t/* Largest possible frame length, in units of lines. */\n> +\tstatic constexpr int maxFrameLength = 0xffdc;\n> };\n> \n> CamHelperImx477::CamHelperImx477()\n> -\t: CamHelper(new MdParserImx477())\n> +\t: CamHelper(new MdParserImx477(), maxFrameLength, frameIntegrationDiff)\n> {\n> }\n> \n> diff --git a/src/ipa/raspberrypi/cam_helper_ov5647.cpp b/src/ipa/raspberrypi/cam_helper_ov5647.cpp\n> index 0b841cd1..2bd8a754 100644\n> --- a/src/ipa/raspberrypi/cam_helper_ov5647.cpp\n> +++ b/src/ipa/raspberrypi/cam_helper_ov5647.cpp\n> @@ -23,6 +23,15 @@ public:\n> \tunsigned int HideFramesModeSwitch() const override;\n> \tunsigned int MistrustFramesStartup() const override;\n> \tunsigned int MistrustFramesModeSwitch() const override;\n> +\n> +private:\n> +\t/*\n> +\t * Smallest difference between the frame length and integration time,\n> +\t * in units of lines.\n> +\t */\n> +\tstatic constexpr int frameIntegrationDiff = 4;\n> +\t/* Largest possible frame length, in units of lines. */\n> +\tstatic constexpr int maxFrameLength = 0xffff;\n> };\n> \n> /*\n> @@ -31,7 +40,7 @@ public:\n> */\n> \n> CamHelperOv5647::CamHelperOv5647()\n> -\t: CamHelper(new MdParserRPi())\n> +\t: CamHelper(new MdParserRPi(), maxFrameLength, frameIntegrationDiff)\n> {\n> }\n> \n> diff --git a/src/ipa/raspberrypi/raspberrypi.cpp b/src/ipa/raspberrypi/raspberrypi.cpp\n> index d087b07e..ba9bc398 100644\n> --- a/src/ipa/raspberrypi/raspberrypi.cpp\n> +++ b/src/ipa/raspberrypi/raspberrypi.cpp\n> @@ -58,6 +58,8 @@ namespace libcamera {\n> /* Configure the sensor with these values initially. */\n> constexpr double DefaultAnalogueGain = 1.0;\n> constexpr unsigned int DefaultExposureTime = 20000;\n> +constexpr double defaultMinFrameDuration = 1e6 / 30.0;\n> +constexpr double defaultMaxFrameDuration = 1e6 / 0.01;\n> \n> LOG_DEFINE_CATEGORY(IPARPI)\n> \n> @@ -150,6 +152,10 @@ private:\n> \n> \t/* Distinguish the first camera start from others. */\n> \tbool firstStart_;\n> +\n> +\t/* Frame duration (1/fps) limits, given in microseconds. */\n> +\tdouble minFrameDuration_;\n> +\tdouble maxFrameDuration_;\n> };\n> \n> int IPARPi::init(const IPASettings &settings)\n> @@ -332,7 +338,8 @@ void IPARPi::configure(const CameraSensorInfo &sensorInfo,\n> \t\tsensorMetadata = helper_->SensorEmbeddedDataPresent();\n> \n> \t\tresult->data.push_back(gainDelay);\n> -\t\tresult->data.push_back(exposureDelay);\n> +\t\tresult->data.push_back(exposureDelay); /* For EXPOSURE ctrl */\n> +\t\tresult->data.push_back(exposureDelay); /* For VBLANK ctrl */\n> \t\tresult->data.push_back(sensorMetadata);\n> \n> \t\tresult->operation |= RPi::IPA_CONFIG_STAGGERED_WRITE;\n> @@ -377,6 +384,9 @@ void IPARPi::configure(const CameraSensorInfo &sensorInfo,\n> \t\tcontroller_.Initialise();\n> \t\tcontrollerInit_ = true;\n> \n> +\t\tminFrameDuration_ = defaultMinFrameDuration;\n> +\t\tmaxFrameDuration_ = defaultMaxFrameDuration;\n> +\n> \t\t/* Supply initial values for gain and exposure. */\n> \t\tControlList ctrls(sensorCtrls_);\n> \t\tAgcStatus agcStatus;\n> @@ -524,7 +534,8 @@ bool IPARPi::validateSensorControls()\n> {\n> \tstatic const uint32_t ctrls[] = {\n> \t\tV4L2_CID_ANALOGUE_GAIN,\n> -\t\tV4L2_CID_EXPOSURE\n> +\t\tV4L2_CID_EXPOSURE,\n> +\t\tV4L2_CID_VBLANK,\n> \t};\n> \n> \tfor (auto c : ctrls) {\n> @@ -551,7 +562,7 @@ bool IPARPi::validateIspControls()\n> \t\tV4L2_CID_USER_BCM2835_ISP_DENOISE,\n> \t\tV4L2_CID_USER_BCM2835_ISP_SHARPEN,\n> \t\tV4L2_CID_USER_BCM2835_ISP_DPC,\n> -\t\tV4L2_CID_USER_BCM2835_ISP_LENS_SHADING\n> +\t\tV4L2_CID_USER_BCM2835_ISP_LENS_SHADING,\n> \t};\n> \n> \tfor (auto c : ctrls) {\n> @@ -804,6 +815,25 @@ void IPARPi::queueRequest(const ControlList &controls)\n> \t\t\tbreak;\n> \t\t}\n> \n> +\t\tcase controls::FRAME_DURATIONS: {\n> +\t\t\tauto frameDurations = ctrl.second.get>();\n> +\n> +\t\t\t/* This will be applied once AGC recalculations occur. */\n> +\t\t\tminFrameDuration_ = frameDurations[0] ? frameDurations[0] : defaultMinFrameDuration;\n> +\t\t\tmaxFrameDuration_ = frameDurations[1] ? frameDurations[1] : defaultMaxFrameDuration;\n> +\t\t\tmaxFrameDuration_ = std::max(maxFrameDuration_, minFrameDuration_);\n> +\n> +\t\t\t/*\n> +\t\t\t * \\todo The values returned in the metadata below must be\n> +\t\t\t * correctly clipped by what the sensor mode suppots and\n\ns/suppots/supports/\n\nReviewed-by: Laurent Pinchart \n\n> +\t\t\t * what the AGC exposure mode or manual shutter speed limits\n> +\t\t\t */\n> +\t\t\tlibcameraMetadata_.set(controls::FrameDurations,\n> +\t\t\t\t\t { static_cast(minFrameDuration_),\n> +\t\t\t\t\t\t static_cast(maxFrameDuration_) });\n> +\t\t\tbreak;\n> +\t\t}\n> +\n> \t\tdefault:\n> \t\t\tLOG(IPARPI, Warning)\n> \t\t\t\t<< \"Ctrl \" << controls::controls.at(ctrl.first)->name()\n> @@ -962,15 +992,27 @@ void IPARPi::applyAWB(const struct AwbStatus *awbStatus, ControlList &ctrls)\n> void IPARPi::applyAGC(const struct AgcStatus *agcStatus, ControlList &ctrls)\n> {\n> \tint32_t gainCode = helper_->GainCode(agcStatus->analogue_gain);\n> -\tint32_t exposureLines = helper_->ExposureLines(agcStatus->shutter_time);\n> \n> -\tLOG(IPARPI, Debug) << \"Applying AGC Exposure: \" << agcStatus->shutter_time\n> -\t\t\t << \" (Shutter lines: \" << exposureLines << \") Gain: \"\n> +\t/* GetVBlanking might clip exposure time to the fps limits. */\n> +\tdouble exposure = agcStatus->shutter_time;\n> +\tint32_t vblanking = helper_->GetVBlanking(exposure, minFrameDuration_,\n> +\t\t\t\t\t\t maxFrameDuration_);\n> +\tint32_t exposureLines = helper_->ExposureLines(exposure);\n> +\n> +\tLOG(IPARPI, Debug) << \"Applying AGC Exposure: \" << exposure\n> +\t\t\t << \" (Shutter lines: \" << exposureLines << \", AGC requested \"\n> +\t\t\t << agcStatus->shutter_time << \") Gain: \"\n> \t\t\t << agcStatus->analogue_gain << \" (Gain Code: \"\n> \t\t\t << gainCode << \")\";\n> \n> -\tctrls.set(V4L2_CID_ANALOGUE_GAIN, gainCode);\n> +\t/*\n> +\t * Due to the behavior of V4L2, the current value of VBLANK could clip the\n> +\t * exposure time without us knowing. The next time though this function should\n> +\t * clip exposure correctly.\n> +\t */\n> +\tctrls.set(V4L2_CID_VBLANK, vblanking);\n> \tctrls.set(V4L2_CID_EXPOSURE, exposureLines);\n> +\tctrls.set(V4L2_CID_ANALOGUE_GAIN, gainCode);\n> }\n> \n> void IPARPi::applyDG(const struct AgcStatus *dgStatus, ControlList &ctrls)\n> diff --git a/src/libcamera/pipeline/raspberrypi/raspberrypi.cpp b/src/libcamera/pipeline/raspberrypi/raspberrypi.cpp\n> index 7a5f5881..252cab64 100644\n> --- a/src/libcamera/pipeline/raspberrypi/raspberrypi.cpp\n> +++ b/src/libcamera/pipeline/raspberrypi/raspberrypi.cpp\n> @@ -1233,7 +1233,8 @@ int RPiCameraData::configureIPA(const CameraConfiguration *config)\n> \t\tif (!staggeredCtrl_) {\n> \t\t\tstaggeredCtrl_.init(unicam_[Unicam::Image].dev(),\n> \t\t\t\t\t { { V4L2_CID_ANALOGUE_GAIN, result.data[resultIdx++] },\n> -\t\t\t\t\t { V4L2_CID_EXPOSURE, result.data[resultIdx++] } });\n> +\t\t\t\t\t { V4L2_CID_EXPOSURE, result.data[resultIdx++] },\n> +\t\t\t\t\t { V4L2_CID_VBLANK, result.data[resultIdx++] } });\n> \t\t\tsensorMetadata_ = result.data[resultIdx++];\n> \t\t}\n> \t}","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 F2F11BD808\n\tfor ;\n\tMon, 11 Jan 2021 23:14:51 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id 8483A680C4;\n\tTue, 12 Jan 2021 00:14:51 +0100 (CET)","from perceval.ideasonboard.com (perceval.ideasonboard.com\n\t[213.167.242.64])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTPS id 881F160523\n\tfor ;\n\tTue, 12 Jan 2021 00:14:50 +0100 (CET)","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 EC4863E;\n\tTue, 12 Jan 2021 00:14:49 +0100 (CET)"],"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=\"EnP+VbRs\"; dkim-atps=neutral","DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com;\n\ts=mail; t=1610406890;\n\tbh=llKqVkWmwfOeb2DqzN2A/LgQl0LQC2aLaFXPziPG4As=;\n\th=Date:From:To:Cc:Subject:References:In-Reply-To:From;\n\tb=EnP+VbRs/HFN952EEg+IZukSU2r62J59yuIfVxv7FFGf3l+pt7tMnjUagaFR4xICn\n\ttfv4BPJgju0aqHmt6QPS5JtqS4ZGX1BrfhORG6vjzL4mDqEuSl6aRKXyqmHRdIsr0h\n\tVfHim2GUJFlPdwSnYNqfeLrnb1rK/KHQibWAaL/Y=","Date":"Tue, 12 Jan 2021 01:14:36 +0200","From":"Laurent Pinchart ","To":"Naushir Patuck ","Message-ID":"","References":"<20210106100659.8363-1-naush@raspberrypi.com>\n\t<20210106100659.8363-3-naush@raspberrypi.com>","MIME-Version":"1.0","Content-Disposition":"inline","In-Reply-To":"<20210106100659.8363-3-naush@raspberrypi.com>","Subject":"Re: [libcamera-devel] [PATCH v11 2/3] libcamera: raspberrypi: Add\n\tcontrol of sensor vblanking","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\" "}}]