From patchwork Fri Jan 29 11:16:14 2021 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Naushir Patuck X-Patchwork-Id: 11053 Return-Path: X-Original-To: parsemail@patchwork.libcamera.org Delivered-To: parsemail@patchwork.libcamera.org Received: from lancelot.ideasonboard.com (lancelot.ideasonboard.com [92.243.16.209]) by patchwork.libcamera.org (Postfix) with ESMTPS id 49E41BD808 for ; Fri, 29 Jan 2021 11:16:33 +0000 (UTC) Received: from lancelot.ideasonboard.com (localhost [IPv6:::1]) by lancelot.ideasonboard.com (Postfix) with ESMTP id 1769C683B2; Fri, 29 Jan 2021 12:16:33 +0100 (CET) Authentication-Results: lancelot.ideasonboard.com; dkim=fail reason="signature verification failed" (2048-bit key; unprotected) header.d=raspberrypi.com header.i=@raspberrypi.com header.b="qpHzXIAK"; dkim-atps=neutral Received: from mail-wr1-x42d.google.com (mail-wr1-x42d.google.com [IPv6:2a00:1450:4864:20::42d]) by lancelot.ideasonboard.com (Postfix) with ESMTPS id D4F7C683A2 for ; Fri, 29 Jan 2021 12:16:30 +0100 (CET) Received: by mail-wr1-x42d.google.com with SMTP id a1so8464748wrq.6 for ; Fri, 29 Jan 2021 03:16:30 -0800 (PST) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=raspberrypi.com; s=google; h=from:to:cc:subject:date:message-id:in-reply-to:references :mime-version:content-transfer-encoding; bh=JTTOH31m7p+bu3QZbZYeF2LaEMpxFjrTVsaqweEBDIo=; b=qpHzXIAKqHbxIEOukqmcnYJf96XztdOh2iYAeWN+Oh7pzDVSe8gmIBFMAzi9GrrZDh IfzrQHhsAcGF0ru0isgp4dLsAuyIl6xYDmh9JVOdrQTVaX36YfMYrmpmzG9B7ayBzA43 UOZoNfWoPeVm2wmsncDj0SFkGfnUeihAMM46+sqfHFiqkMBcuyekd3dNk9Yi/PcpCzHk 0gw9EIuaCwm0fWkgGQmJLpY4RQPUFCnImXIL4lToETPokXoRr7yzuoVk6fzV6CxKWXu7 5k5mtV0XQqn5Q/D8vMyBaLnmTRQabNIoiM86aXu4rbciwYrKC6gxOcF1SnKe8JPZeQWK sY3g== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20161025; h=x-gm-message-state:from:to:cc:subject:date:message-id:in-reply-to :references:mime-version:content-transfer-encoding; bh=JTTOH31m7p+bu3QZbZYeF2LaEMpxFjrTVsaqweEBDIo=; b=sV9xE2+BfzxIyynURpvDfSAAomID/PfiYTtrDLMzA/OB2K5zNQ1qzQyGN/YTUVCSOh ciJfyBvuIc0Z2pbh2nlsIZ3R8juy1HjulNctaBBpKShtvmHY7cD1I8atozlr3Y5r/1ss S3WSaDitVGlgFPusSOEGwujzOWysPO/d5+vfkNqDvRjIe4R4fdrlS+rb/u/CVsFxPLsC 4KD9JPLoAR0BqVHaSxf99lS7hvtcHzELedlBRlBasPNM/q1tzVQA0/OHCeOKY3dLs/Ao 7Q8doHgpK412oZ/2hGjqs5p0S/LoMATvnT+BglCd0K/b99XzKGamWnIh118Ls8Q3GIlv jO2A== X-Gm-Message-State: AOAM532wnq+xMOB7XEUsibY9NaAoApE9lS1Z7X9F7WbvdnciWfEmIayp 2wp7/6bFhxA7rtU/WQNkQHMg67rdKyC0nRBt X-Google-Smtp-Source: ABdhPJz1aDqKWFWXKqeGtv8+dJ4jlBJADZCpPsKDNbA+hyQED4EQFMVz1rmDpJjprZcLAtopoMLklg== X-Received: by 2002:a5d:58c2:: with SMTP id o2mr3956475wrf.31.1611918990196; Fri, 29 Jan 2021 03:16:30 -0800 (PST) Received: from naushir-VirtualBox.patuck.local ([88.97.76.4]) by smtp.gmail.com with ESMTPSA id f14sm11324007wre.69.2021.01.29.03.16.27 (version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256); Fri, 29 Jan 2021 03:16:28 -0800 (PST) From: Naushir Patuck To: libcamera-devel@lists.libcamera.org Date: Fri, 29 Jan 2021 11:16:14 +0000 Message-Id: <20210129111616.1047483-4-naush@raspberrypi.com> X-Mailer: git-send-email 2.25.1 In-Reply-To: <20210129111616.1047483-1-naush@raspberrypi.com> References: <20210129111616.1047483-1-naush@raspberrypi.com> MIME-Version: 1.0 Subject: [libcamera-devel] [PATCH v4 3/5] ipa: raspberrypi: Limit the calculated 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: , List-Archive: List-Post: List-Help: List-Subscribe: , Errors-To: libcamera-devel-bounces@lists.libcamera.org Sender: "libcamera-devel" The existing framerate/vblank calculations did not account for the sensor mode limits. This commit extracts vblank limits from the sensor v4l2 controls, and stores it in the camera modes structure. Exposure and vblank value calculations now use values clamped to the sensor mode limits. The libcamera::controls::FrameDurations metadata return values now reflect the minimum and maximum after clamping. Signed-off-by: Naushir Patuck Reviewed-by: David Plowman Reviewed-by: Jacopo Mondi Reviewed-by: Laurent Pinchart --- src/ipa/raspberrypi/cam_helper.cpp | 16 +++---- src/ipa/raspberrypi/cam_helper.hpp | 5 +- src/ipa/raspberrypi/cam_helper_imx219.cpp | 6 +-- src/ipa/raspberrypi/cam_helper_imx477.cpp | 4 +- src/ipa/raspberrypi/cam_helper_ov5647.cpp | 4 +- src/ipa/raspberrypi/controller/camera_mode.h | 2 + src/ipa/raspberrypi/raspberrypi.cpp | 49 +++++++++++++------- 7 files changed, 47 insertions(+), 39 deletions(-) diff --git a/src/ipa/raspberrypi/cam_helper.cpp b/src/ipa/raspberrypi/cam_helper.cpp index b7b8faf09c69..93d1b7b0296a 100644 --- a/src/ipa/raspberrypi/cam_helper.cpp +++ b/src/ipa/raspberrypi/cam_helper.cpp @@ -34,9 +34,8 @@ CamHelper *CamHelper::Create(std::string const &cam_name) return nullptr; } -CamHelper::CamHelper(MdParser *parser, unsigned int maxFrameLength, - unsigned int frameIntegrationDiff) - : parser_(parser), initialized_(false), maxFrameLength_(maxFrameLength), +CamHelper::CamHelper(MdParser *parser, unsigned int frameIntegrationDiff) + : parser_(parser), initialized_(false), frameIntegrationDiff_(frameIntegrationDiff) { } @@ -67,13 +66,12 @@ uint32_t CamHelper::GetVBlanking(double &exposure, double minFrameDuration, assert(initialized_); /* - * Clamp frame length by the frame duration range and the maximum allowable - * value in the sensor, given by maxFrameLength_. + * minFrameDuration and maxFrameDuration are clamped by the caller + * based on the limits for the active sensor mode. */ - frameLengthMin = std::clamp(1e3 * minFrameDuration / mode_.line_length, - mode_.height, maxFrameLength_); - frameLengthMax = std::clamp(1e3 * maxFrameDuration / mode_.line_length, - mode_.height, maxFrameLength_); + frameLengthMin = 1e3 * minFrameDuration / mode_.line_length; + frameLengthMax = 1e3 * maxFrameDuration / mode_.line_length; + /* * Limit the exposure to the maximum frame duration requested, and * re-calculate if it has been clipped. diff --git a/src/ipa/raspberrypi/cam_helper.hpp b/src/ipa/raspberrypi/cam_helper.hpp index b1739a57e342..14d70112cb62 100644 --- a/src/ipa/raspberrypi/cam_helper.hpp +++ b/src/ipa/raspberrypi/cam_helper.hpp @@ -62,8 +62,7 @@ class CamHelper { public: static CamHelper *Create(std::string const &cam_name); - CamHelper(MdParser *parser, unsigned int maxFrameLength, - unsigned int frameIntegrationDiff); + CamHelper(MdParser *parser, unsigned int frameIntegrationDiff); virtual ~CamHelper(); void SetCameraMode(const CameraMode &mode); MdParser &Parser() const { return *parser_; } @@ -86,8 +85,6 @@ protected: private: bool initialized_; - /* Largest possible frame length, in units of lines. */ - unsigned int maxFrameLength_; /* * Smallest difference between the frame length and integration time, * in units of lines. diff --git a/src/ipa/raspberrypi/cam_helper_imx219.cpp b/src/ipa/raspberrypi/cam_helper_imx219.cpp index 8688ec091c44..95b8e698fe3b 100644 --- a/src/ipa/raspberrypi/cam_helper_imx219.cpp +++ b/src/ipa/raspberrypi/cam_helper_imx219.cpp @@ -56,15 +56,13 @@ private: * in units of lines. */ static constexpr int frameIntegrationDiff = 4; - /* Largest possible frame length, in units of lines. */ - static constexpr int maxFrameLength = 0xffff; }; CamHelperImx219::CamHelperImx219() #if ENABLE_EMBEDDED_DATA - : CamHelper(new MdParserImx219(), maxFrameLength, frameIntegrationDiff) + : CamHelper(new MdParserImx219(), frameIntegrationDiff) #else - : CamHelper(new MdParserRPi(), maxFrameLength, frameIntegrationDiff) + : CamHelper(new MdParserRPi(), frameIntegrationDiff) #endif { } diff --git a/src/ipa/raspberrypi/cam_helper_imx477.cpp b/src/ipa/raspberrypi/cam_helper_imx477.cpp index 5396131003f1..eaa7be16d20e 100644 --- a/src/ipa/raspberrypi/cam_helper_imx477.cpp +++ b/src/ipa/raspberrypi/cam_helper_imx477.cpp @@ -45,12 +45,10 @@ private: * in units of lines. */ static constexpr int frameIntegrationDiff = 22; - /* Largest possible frame length, in units of lines. */ - static constexpr int maxFrameLength = 0xffdc; }; CamHelperImx477::CamHelperImx477() - : CamHelper(new MdParserImx477(), maxFrameLength, frameIntegrationDiff) + : CamHelper(new MdParserImx477(), frameIntegrationDiff) { } diff --git a/src/ipa/raspberrypi/cam_helper_ov5647.cpp b/src/ipa/raspberrypi/cam_helper_ov5647.cpp index 2bd8a754f133..a7f417324048 100644 --- a/src/ipa/raspberrypi/cam_helper_ov5647.cpp +++ b/src/ipa/raspberrypi/cam_helper_ov5647.cpp @@ -30,8 +30,6 @@ private: * in units of lines. */ static constexpr int frameIntegrationDiff = 4; - /* Largest possible frame length, in units of lines. */ - static constexpr int maxFrameLength = 0xffff; }; /* @@ -40,7 +38,7 @@ private: */ CamHelperOv5647::CamHelperOv5647() - : CamHelper(new MdParserRPi(), maxFrameLength, frameIntegrationDiff) + : CamHelper(new MdParserRPi(), frameIntegrationDiff) { } diff --git a/src/ipa/raspberrypi/controller/camera_mode.h b/src/ipa/raspberrypi/controller/camera_mode.h index 920f11beb0b0..256438c931d9 100644 --- a/src/ipa/raspberrypi/controller/camera_mode.h +++ b/src/ipa/raspberrypi/controller/camera_mode.h @@ -37,6 +37,8 @@ struct CameraMode { double line_length; // any camera transform *not* reflected already in the camera tuning libcamera::Transform transform; + // minimum and maximum fame lengths in units of lines + uint32_t min_frame_length, max_frame_length; }; #ifdef __cplusplus diff --git a/src/ipa/raspberrypi/raspberrypi.cpp b/src/ipa/raspberrypi/raspberrypi.cpp index 5ccc7a6551f5..e4911b734e20 100644 --- a/src/ipa/raspberrypi/raspberrypi.cpp +++ b/src/ipa/raspberrypi/raspberrypi.cpp @@ -102,6 +102,7 @@ private: void reportMetadata(); bool parseEmbeddedData(unsigned int bufferId, struct DeviceStatus &deviceStatus); void processStats(unsigned int bufferId); + void applyFrameDurations(double minFrameDuration, double maxFrameDuration); void applyAGC(const struct AgcStatus *agcStatus, ControlList &ctrls); void applyAWB(const struct AwbStatus *awbStatus, ControlList &ctrls); void applyDG(const struct AgcStatus *dgStatus, ControlList &ctrls); @@ -279,6 +280,13 @@ void IPARPi::setMode(const CameraSensorInfo &sensorInfo) * the line length in pixels and the pixel rate. */ mode_.line_length = 1e9 * sensorInfo.lineLength / sensorInfo.pixelRate; + + /* + * Set the frame length limits for the mode to ensure exposure and + * framerate calculations are clipped appropriately. + */ + mode_.min_frame_length = sensorInfo.minFrameLength; + mode_.max_frame_length = sensorInfo.maxFrameLength; } void IPARPi::configure(const CameraSensorInfo &sensorInfo, @@ -384,8 +392,8 @@ void IPARPi::configure(const CameraSensorInfo &sensorInfo, controller_.Initialise(); controllerInit_ = true; - minFrameDuration_ = defaultMinFrameDuration; - maxFrameDuration_ = defaultMaxFrameDuration; + /* Supply initial values for frame durations. */ + applyFrameDurations(defaultMinFrameDuration, defaultMaxFrameDuration); /* Supply initial values for gain and exposure. */ ControlList ctrls(sensorCtrls_); @@ -819,20 +827,7 @@ void IPARPi::queueRequest(const ControlList &controls) case controls::FRAME_DURATIONS: { auto frameDurations = ctrl.second.get>(); - - /* This will be applied once AGC recalculations occur. */ - minFrameDuration_ = frameDurations[0] ? frameDurations[0] : defaultMinFrameDuration; - maxFrameDuration_ = frameDurations[1] ? frameDurations[1] : defaultMaxFrameDuration; - maxFrameDuration_ = std::max(maxFrameDuration_, minFrameDuration_); - - /* - * \todo The values returned in the metadata below must be - * correctly clipped by what the sensor mode supports and - * what the AGC exposure mode or manual shutter speed limits - */ - libcameraMetadata_.set(controls::FrameDurations, - { static_cast(minFrameDuration_), - static_cast(maxFrameDuration_) }); + applyFrameDurations(frameDurations[0], frameDurations[1]); break; } @@ -991,6 +986,28 @@ void IPARPi::applyAWB(const struct AwbStatus *awbStatus, ControlList &ctrls) static_cast(awbStatus->gain_b * 1000)); } +void IPARPi::applyFrameDurations(double minFrameDuration, double maxFrameDuration) +{ + const double minSensorFrameDuration = 1e-3 * mode_.min_frame_length * mode_.line_length; + const double maxSensorFrameDuration = 1e-3 * mode_.max_frame_length * mode_.line_length; + + /* + * This will only be applied once AGC recalculations occur. + * The values may be clamped based on the sensor mode capabilities as well. + */ + minFrameDuration_ = minFrameDuration ? minFrameDuration : defaultMaxFrameDuration; + maxFrameDuration_ = maxFrameDuration ? maxFrameDuration : defaultMinFrameDuration; + minFrameDuration_ = std::clamp(minFrameDuration_, + minSensorFrameDuration, maxSensorFrameDuration); + maxFrameDuration_ = std::clamp(maxFrameDuration_, + minSensorFrameDuration, maxSensorFrameDuration); + + /* Return the validated limits out though metadata. */ + libcameraMetadata_.set(controls::FrameDurations, + { static_cast(minFrameDuration_), + static_cast(maxFrameDuration_) }); +} + void IPARPi::applyAGC(const struct AgcStatus *agcStatus, ControlList &ctrls) { int32_t gainCode = helper_->GainCode(agcStatus->analogue_gain);