{"id":11040,"url":"https://patchwork.libcamera.org/api/patches/11040/?format=json","web_url":"https://patchwork.libcamera.org/patch/11040/","project":{"id":1,"url":"https://patchwork.libcamera.org/api/projects/1/?format=json","name":"libcamera","link_name":"libcamera","list_id":"libcamera_core","list_email":"libcamera-devel@lists.libcamera.org","web_url":"","scm_url":"","webscm_url":""},"msgid":"<20210128091050.881815-4-naush@raspberrypi.com>","date":"2021-01-28T09:10:48","name":"[libcamera-devel,v3,3/5] ipa: raspberrypi: Limit the calculated vblank based on the sensor mode","commit_ref":null,"pull_url":null,"state":"superseded","archived":false,"hash":"dfa08b7ba81d84818206797784c02c27399da86c","submitter":{"id":34,"url":"https://patchwork.libcamera.org/api/people/34/?format=json","name":"Naushir Patuck","email":"naush@raspberrypi.com"},"delegate":null,"mbox":"https://patchwork.libcamera.org/patch/11040/mbox/","series":[{"id":1619,"url":"https://patchwork.libcamera.org/api/series/1619/?format=json","web_url":"https://patchwork.libcamera.org/project/libcamera/list/?series=1619","date":"2021-01-28T09:10:45","name":"Raspberry Pi: FrameDurations control refinements","version":3,"mbox":"https://patchwork.libcamera.org/series/1619/mbox/"}],"comments":"https://patchwork.libcamera.org/api/patches/11040/comments/","check":"pending","checks":"https://patchwork.libcamera.org/api/patches/11040/checks/","tags":{},"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 B7088BD808\n\tfor ;\n\tThu, 28 Jan 2021 09:11:01 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id 879D768390;\n\tThu, 28 Jan 2021 10:11:01 +0100 (CET)","from mail-wm1-x32c.google.com (mail-wm1-x32c.google.com\n\t[IPv6:2a00:1450:4864:20::32c])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTPS id E1E8F68383\n\tfor ;\n\tThu, 28 Jan 2021 10:10:57 +0100 (CET)","by mail-wm1-x32c.google.com with SMTP id i9so3958888wmq.1\n\tfor ;\n\tThu, 28 Jan 2021 01:10:57 -0800 (PST)","from naushir-VirtualBox.patuck.local ([88.97.76.4])\n\tby smtp.gmail.com with ESMTPSA id\n\tf17sm6517056wrv.0.2021.01.28.01.10.55\n\t(version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256);\n\tThu, 28 Jan 2021 01:10:56 -0800 (PST)"],"Authentication-Results":"lancelot.ideasonboard.com;\n\tdkim=fail reason=\"signature verification failed\" (2048-bit key;\n\tunprotected) header.d=raspberrypi.com header.i=@raspberrypi.com\n\theader.b=\"UzntPPn6\"; dkim-atps=neutral","DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/relaxed;\n\td=raspberrypi.com; s=google;\n\th=from:to:cc:subject:date:message-id:in-reply-to:references\n\t:mime-version:content-transfer-encoding;\n\tbh=vLoV02H7yVVWHeI2CE7xbapt2Jd5AA/m9VmsfsTEGDg=;\n\tb=UzntPPn6gxvzXTZCcZR2idty+q6NWIiLUwO6PBEiVW26WfiStHRTAsiUIhVocqtbhS\n\tlblj2oYF99aANk5xE84T1z8+EH3NjrWGLbZwrAZ1wC0FHJhIbvBCBVxX6z9ZpMAqS68F\n\twBFWZedwtddB5ypChCQQUWannEra473YxLKCKiamRfw5hlzEvNWvMOd8LAhYi/Rw4e2F\n\tfUfiAQ2hoKDBc7aTF06Wpa01Z09yqD7daTuWYYucClb/DqSyAzj5SBnAcLeg3Js5ROff\n\tYknlbMRjafM644CexFQRPV/voWp9Wgn19rEEi/75In3PYMB65iDnzkfeUMoFHwxLTzcx\n\tD77Q==","X-Google-DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/relaxed;\n\td=1e100.net; s=20161025;\n\th=x-gm-message-state:from:to:cc:subject:date:message-id:in-reply-to\n\t:references:mime-version:content-transfer-encoding;\n\tbh=vLoV02H7yVVWHeI2CE7xbapt2Jd5AA/m9VmsfsTEGDg=;\n\tb=JW+7SePBTo0UGZd+bVz6iJgv8Ga1LknpKBqCwPiTug8gwZfZ2WMgy9d+5ntnfob1HC\n\tDubewIjt18ezeyFMhb0RDpjKKJP1GXsDHGkelPJY++Qusm34Aks8wU4WyrFzuks1i41p\n\t95heUOvAgge8gyR9pYG9wE5bIW1KXQgs8RtXeblTQVs7oqoXmoGUDGeC42Xk72UaEUPM\n\tFoBI7LQWcJpHBy1/JqWlGCSw//5Q47+VmKXZXw62DfWiTDTxk/wNo3REVQOSvgEA91U+\n\t+MVLIE5Sbn4wX4SplnUQPwAYmWuK/XeAFSWywrPgqbXOije2OiIVfLQESoa5Uq80n7/L\n\tlm2g==","X-Gm-Message-State":"AOAM5320lfWT40OHDUspBseNAzgnixuIyZSMTd4r9An5kR2g9bEyyea5\n\tNq5tnxDLoBikGDI3L0g0pzDkb3eOkKS3Yfjz","X-Google-Smtp-Source":"ABdhPJxLZZ9agbXfclSKIgWVgK3+Vbg3ofn/XeC5nAMZgVAPEbl9/K+v6WEXx+fJUoIdHj/54u72IQ==","X-Received":"by 2002:a1c:a593:: with SMTP id\n\to141mr7855987wme.92.1611825056893; \n\tThu, 28 Jan 2021 01:10:56 -0800 (PST)","From":"Naushir Patuck ","To":"libcamera-devel@lists.libcamera.org","Date":"Thu, 28 Jan 2021 09:10:48 +0000","Message-Id":"<20210128091050.881815-4-naush@raspberrypi.com>","X-Mailer":"git-send-email 2.25.1","In-Reply-To":"<20210128091050.881815-1-naush@raspberrypi.com>","References":"<20210128091050.881815-1-naush@raspberrypi.com>","MIME-Version":"1.0","Subject":"[libcamera-devel] [PATCH v3 3/5] ipa: raspberrypi: Limit the\n\tcalculated vblank based on the sensor mode","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","Content-Type":"text/plain; charset=\"us-ascii\"","Content-Transfer-Encoding":"7bit","Errors-To":"libcamera-devel-bounces@lists.libcamera.org","Sender":"\"libcamera-devel\" "},"content":"The existing framerate/vblank calculations did not account for the\nsensor mode limits. This commit extracts vblank limits from the sensor\nv4l2 controls, and stores it in the camera modes structure.\n\nExposure and vblank value calculations now use values clamped to the\nsensor mode limits. The libcamera::controls::FrameDurations metadata\nreturn values now reflect the minimum and maximum after clamping.\n\nSigned-off-by: Naushir Patuck \nReviewed-by: David Plowman \n---\n src/ipa/raspberrypi/cam_helper.cpp | 16 +++----\n src/ipa/raspberrypi/cam_helper.hpp | 5 +-\n src/ipa/raspberrypi/cam_helper_imx219.cpp | 6 +--\n src/ipa/raspberrypi/cam_helper_imx477.cpp | 4 +-\n src/ipa/raspberrypi/cam_helper_ov5647.cpp | 4 +-\n src/ipa/raspberrypi/controller/camera_mode.h | 2 +\n src/ipa/raspberrypi/raspberrypi.cpp | 49 +++++++++++++-------\n 7 files changed, 47 insertions(+), 39 deletions(-)","diff":"diff --git a/src/ipa/raspberrypi/cam_helper.cpp b/src/ipa/raspberrypi/cam_helper.cpp\nindex b7b8faf09c69..3e17255737b2 100644\n--- a/src/ipa/raspberrypi/cam_helper.cpp\n+++ b/src/ipa/raspberrypi/cam_helper.cpp\n@@ -34,9 +34,8 @@ CamHelper *CamHelper::Create(std::string const &cam_name)\n \treturn nullptr;\n }\n \n-CamHelper::CamHelper(MdParser *parser, unsigned int maxFrameLength,\n-\t\t unsigned int frameIntegrationDiff)\n-\t: parser_(parser), initialized_(false), maxFrameLength_(maxFrameLength),\n+CamHelper::CamHelper(MdParser *parser, unsigned int frameIntegrationDiff)\n+\t: parser_(parser), initialized_(false),\n \t frameIntegrationDiff_(frameIntegrationDiff)\n {\n }\n@@ -67,13 +66,12 @@ uint32_t CamHelper::GetVBlanking(double &exposure, double minFrameDuration,\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 * minFrameDuration and maxFrameDuration will have been clamped to the\n+\t * maximum allowable values in the sensor for this mode.\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+\tframeLengthMin = 1e3 * minFrameDuration / mode_.line_length;\n+\tframeLengthMax = 1e3 * maxFrameDuration / mode_.line_length;\n+\n \t/*\n \t * Limit the exposure to the maximum frame duration requested, and\n \t * re-calculate if it has been clipped.\ndiff --git a/src/ipa/raspberrypi/cam_helper.hpp b/src/ipa/raspberrypi/cam_helper.hpp\nindex b1739a57e342..14d70112cb62 100644\n--- a/src/ipa/raspberrypi/cam_helper.hpp\n+++ b/src/ipa/raspberrypi/cam_helper.hpp\n@@ -62,8 +62,7 @@ class CamHelper\n {\n public:\n \tstatic CamHelper *Create(std::string const &cam_name);\n-\tCamHelper(MdParser *parser, unsigned int maxFrameLength,\n-\t\t unsigned int frameIntegrationDiff);\n+\tCamHelper(MdParser *parser, unsigned int frameIntegrationDiff);\n \tvirtual ~CamHelper();\n \tvoid SetCameraMode(const CameraMode &mode);\n \tMdParser &Parser() const { return *parser_; }\n@@ -86,8 +85,6 @@ protected:\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.\ndiff --git a/src/ipa/raspberrypi/cam_helper_imx219.cpp b/src/ipa/raspberrypi/cam_helper_imx219.cpp\nindex 8688ec091c44..95b8e698fe3b 100644\n--- a/src/ipa/raspberrypi/cam_helper_imx219.cpp\n+++ b/src/ipa/raspberrypi/cam_helper_imx219.cpp\n@@ -56,15 +56,13 @@ private:\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(), maxFrameLength, frameIntegrationDiff)\n+\t: CamHelper(new MdParserImx219(), frameIntegrationDiff)\n #else\n-\t: CamHelper(new MdParserRPi(), maxFrameLength, frameIntegrationDiff)\n+\t: CamHelper(new MdParserRPi(), frameIntegrationDiff)\n #endif\n {\n }\ndiff --git a/src/ipa/raspberrypi/cam_helper_imx477.cpp b/src/ipa/raspberrypi/cam_helper_imx477.cpp\nindex 5396131003f1..eaa7be16d20e 100644\n--- a/src/ipa/raspberrypi/cam_helper_imx477.cpp\n+++ b/src/ipa/raspberrypi/cam_helper_imx477.cpp\n@@ -45,12 +45,10 @@ private:\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(), maxFrameLength, frameIntegrationDiff)\n+\t: CamHelper(new MdParserImx477(), frameIntegrationDiff)\n {\n }\n \ndiff --git a/src/ipa/raspberrypi/cam_helper_ov5647.cpp b/src/ipa/raspberrypi/cam_helper_ov5647.cpp\nindex 2bd8a754f133..a7f417324048 100644\n--- a/src/ipa/raspberrypi/cam_helper_ov5647.cpp\n+++ b/src/ipa/raspberrypi/cam_helper_ov5647.cpp\n@@ -30,8 +30,6 @@ private:\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@@ -40,7 +38,7 @@ private:\n */\n \n CamHelperOv5647::CamHelperOv5647()\n-\t: CamHelper(new MdParserRPi(), maxFrameLength, frameIntegrationDiff)\n+\t: CamHelper(new MdParserRPi(), frameIntegrationDiff)\n {\n }\n \ndiff --git a/src/ipa/raspberrypi/controller/camera_mode.h b/src/ipa/raspberrypi/controller/camera_mode.h\nindex 920f11beb0b0..256438c931d9 100644\n--- a/src/ipa/raspberrypi/controller/camera_mode.h\n+++ b/src/ipa/raspberrypi/controller/camera_mode.h\n@@ -37,6 +37,8 @@ struct CameraMode {\n \tdouble line_length;\n \t// any camera transform *not* reflected already in the camera tuning\n \tlibcamera::Transform transform;\n+\t// minimum and maximum fame lengths in units of lines\n+\tuint32_t min_frame_length, max_frame_length;\n };\n \n #ifdef __cplusplus\ndiff --git a/src/ipa/raspberrypi/raspberrypi.cpp b/src/ipa/raspberrypi/raspberrypi.cpp\nindex 5ccc7a6551f5..e4911b734e20 100644\n--- a/src/ipa/raspberrypi/raspberrypi.cpp\n+++ b/src/ipa/raspberrypi/raspberrypi.cpp\n@@ -102,6 +102,7 @@ private:\n \tvoid reportMetadata();\n \tbool parseEmbeddedData(unsigned int bufferId, struct DeviceStatus &deviceStatus);\n \tvoid processStats(unsigned int bufferId);\n+\tvoid applyFrameDurations(double minFrameDuration, double maxFrameDuration);\n \tvoid applyAGC(const struct AgcStatus *agcStatus, ControlList &ctrls);\n \tvoid applyAWB(const struct AwbStatus *awbStatus, ControlList &ctrls);\n \tvoid applyDG(const struct AgcStatus *dgStatus, ControlList &ctrls);\n@@ -279,6 +280,13 @@ void IPARPi::setMode(const CameraSensorInfo &sensorInfo)\n \t * the line length in pixels and the pixel rate.\n \t */\n \tmode_.line_length = 1e9 * sensorInfo.lineLength / sensorInfo.pixelRate;\n+\n+\t/*\n+\t * Set the frame length limits for the mode to ensure exposure and\n+\t * framerate calculations are clipped appropriately.\n+\t */\n+\tmode_.min_frame_length = sensorInfo.minFrameLength;\n+\tmode_.max_frame_length = sensorInfo.maxFrameLength;\n }\n \n void IPARPi::configure(const CameraSensorInfo &sensorInfo,\n@@ -384,8 +392,8 @@ void IPARPi::configure(const CameraSensorInfo &sensorInfo,\n \t\tcontroller_.Initialise();\n \t\tcontrollerInit_ = true;\n \n-\t\tminFrameDuration_ = defaultMinFrameDuration;\n-\t\tmaxFrameDuration_ = defaultMaxFrameDuration;\n+\t\t/* Supply initial values for frame durations. */\n+\t\tapplyFrameDurations(defaultMinFrameDuration, defaultMaxFrameDuration);\n \n \t\t/* Supply initial values for gain and exposure. */\n \t\tControlList ctrls(sensorCtrls_);\n@@ -819,20 +827,7 @@ void IPARPi::queueRequest(const ControlList &controls)\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 supports and\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\tapplyFrameDurations(frameDurations[0], frameDurations[1]);\n \t\t\tbreak;\n \t\t}\n \n@@ -991,6 +986,28 @@ void IPARPi::applyAWB(const struct AwbStatus *awbStatus, ControlList &ctrls)\n \t\t static_cast(awbStatus->gain_b * 1000));\n }\n \n+void IPARPi::applyFrameDurations(double minFrameDuration, double maxFrameDuration)\n+{\n+\tconst double minSensorFrameDuration = 1e-3 * mode_.min_frame_length * mode_.line_length;\n+\tconst double maxSensorFrameDuration = 1e-3 * mode_.max_frame_length * mode_.line_length;\n+\n+\t/*\n+\t * This will only be applied once AGC recalculations occur.\n+\t * The values may be clamped based on the sensor mode capabilities as well.\n+\t */\n+\tminFrameDuration_ = minFrameDuration ? minFrameDuration : defaultMaxFrameDuration;\n+\tmaxFrameDuration_ = maxFrameDuration ? maxFrameDuration : defaultMinFrameDuration;\n+\tminFrameDuration_ = std::clamp(minFrameDuration_,\n+\t\t\t\t minSensorFrameDuration, maxSensorFrameDuration);\n+\tmaxFrameDuration_ = std::clamp(maxFrameDuration_,\n+\t\t\t\t minSensorFrameDuration, maxSensorFrameDuration);\n+\n+\t/* Return the validated limits out though metadata. */\n+\tlibcameraMetadata_.set(controls::FrameDurations,\n+\t\t\t { static_cast(minFrameDuration_),\n+\t\t\t\t static_cast(maxFrameDuration_) });\n+}\n+\n void IPARPi::applyAGC(const struct AgcStatus *agcStatus, ControlList &ctrls)\n {\n \tint32_t gainCode = helper_->GainCode(agcStatus->analogue_gain);\n","prefixes":["libcamera-devel","v3","3/5"]}