From patchwork Thu Mar 2 13:54:27 2023 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Naushir Patuck X-Patchwork-Id: 18332 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 35DD8BE080 for ; Thu, 2 Mar 2023 13:54:34 +0000 (UTC) Received: from lancelot.ideasonboard.com (localhost [IPv6:::1]) by lancelot.ideasonboard.com (Postfix) with ESMTP id DCE61626DE; Thu, 2 Mar 2023 14:54:33 +0100 (CET) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=libcamera.org; s=mail; t=1677765273; bh=AzybjmvGnBPwvzyZhnNV47vkseJ3oOqdbm1re0zZiQk=; h=To:Date:In-Reply-To:References:List-Id:List-Post:From:Cc: List-Subscribe:List-Unsubscribe:List-Archive:Reply-To:List-Help: Subject:From; b=xrCNhrljiPszq8n2dhCJe2tuecH/Rv/JEFY+pBvSy7qYCCeSRiVPOy+6q74BOUU4c l2N4X/tG7dpeiR+kgapSh7txe5ifN9mKt4c3mo0OWNa/p6f9xtzaN2ISqcXK3ivUi4 XA07adT+yge8ffs+lHBU17J6tNiXWXWSBcMSuzXLTO3oyerD51ioj1hcfdMj8hqC6O WQtsoUk9pKbcNnB0zqbwAcS0VccV7+Twbb2K5IrSpBLtcZUroK1T9BjzYSHaRpeY7l E5j655HfJXXi3iBoKTEKHRLslu3uo90Xsu/jYQSCW+UC4WvNvwibfwv0Kz6L8NQjL8 eXZrKHNg7sMbA== To: libcamera-devel@lists.libcamera.org Date: Thu, 2 Mar 2023 13:54:27 +0000 In-Reply-To: <20230302135429.23821-1-naush@raspberrypi.com> References: <20230302135429.23821-1-naush@raspberrypi.com> MIME-Version: 1.0 Message-ID: List-Id: List-Post: X-Patchwork-Original-From: Naushir Patuck via libcamera-devel From: Naushir Patuck Precedence: list Cc: Jacopo Mondi X-Mailman-Version: 2.1.29 X-BeenThere: libcamera-devel@lists.libcamera.org List-Subscribe: , List-Unsubscribe: , List-Archive: Reply-To: Naushir Patuck List-Help: Subject: [libcamera-devel] [PATCH v2 1/3] pipeline: ipa: raspberrypi: Change Unicam timeout handling Content-Disposition: inline Errors-To: libcamera-devel-bounces@lists.libcamera.org Sender: "libcamera-devel" Add an explicit helper function setCameraTimeout() in the pipeline handler to set the Unicam timeout value. This function is signalled from the IPA to set up an appropriate timeout. This replaces the maxSensorFrameLengthMs value parameter returned back from IPARPi::start(). Adjust the timeout to be 5x the maximum frame duration reported by the IPA. Signed-off-by: Naushir Patuck Reviewed-by: David Plowman Reviewed-by: Jacopo Mondi Reviewed-by: Laurent Pinchart --- include/libcamera/ipa/raspberrypi.mojom | 2 +- src/ipa/raspberrypi/raspberrypi.cpp | 2 +- .../pipeline/raspberrypi/raspberrypi.cpp | 24 ++++++++++++------- 3 files changed, 18 insertions(+), 10 deletions(-) diff --git a/include/libcamera/ipa/raspberrypi.mojom b/include/libcamera/ipa/raspberrypi.mojom index 8e78f167f179..80e0126618c8 100644 --- a/include/libcamera/ipa/raspberrypi.mojom +++ b/include/libcamera/ipa/raspberrypi.mojom @@ -49,7 +49,6 @@ struct IPAConfigResult { struct StartConfig { libcamera.ControlList controls; int32 dropFrameCount; - uint32 maxSensorFrameLengthMs; }; interface IPARPiInterface { @@ -132,4 +131,5 @@ interface IPARPiEventInterface { setIspControls(libcamera.ControlList controls); setDelayedControls(libcamera.ControlList controls, uint32 delayContext); setLensControls(libcamera.ControlList controls); + setCameraTimeout(uint32 maxFrameLengthMs); }; diff --git a/src/ipa/raspberrypi/raspberrypi.cpp b/src/ipa/raspberrypi/raspberrypi.cpp index 9b08ae4ca622..f6826bf27fe1 100644 --- a/src/ipa/raspberrypi/raspberrypi.cpp +++ b/src/ipa/raspberrypi/raspberrypi.cpp @@ -341,7 +341,7 @@ void IPARPi::start(const ControlList &controls, StartConfig *startConfig) startConfig->dropFrameCount = dropFrameCount_; const Duration maxSensorFrameDuration = mode_.maxFrameLength * mode_.maxLineLength; - startConfig->maxSensorFrameLengthMs = maxSensorFrameDuration.get(); + setCameraTimeout.emit(maxSensorFrameDuration.get()); firstStart_ = false; lastRunTimestamp_ = 0; diff --git a/src/libcamera/pipeline/raspberrypi/raspberrypi.cpp b/src/libcamera/pipeline/raspberrypi/raspberrypi.cpp index 841209548350..3d04842a2440 100644 --- a/src/libcamera/pipeline/raspberrypi/raspberrypi.cpp +++ b/src/libcamera/pipeline/raspberrypi/raspberrypi.cpp @@ -212,6 +212,7 @@ public: void setIspControls(const ControlList &controls); void setDelayedControls(const ControlList &controls, uint32_t delayContext); void setLensControls(const ControlList &controls); + void setCameraTimeout(uint32_t maxExposureTimeMs); void setSensorControls(ControlList &controls); void unicamTimeout(); @@ -1166,14 +1167,6 @@ int PipelineHandlerRPi::start(Camera *camera, const ControlList *controls) } } - /* - * Set the dequeue timeout to the larger of 2x the maximum possible - * frame duration or 1 second. - */ - utils::Duration timeout = - std::max(1s, 2 * startConfig.maxSensorFrameLengthMs * 1ms); - data->unicam_[Unicam::Image].dev()->setDequeueTimeout(timeout); - return 0; } @@ -1645,6 +1638,7 @@ int RPiCameraData::loadIPA(ipa::RPi::IPAInitResult *result) ipa_->setIspControls.connect(this, &RPiCameraData::setIspControls); ipa_->setDelayedControls.connect(this, &RPiCameraData::setDelayedControls); ipa_->setLensControls.connect(this, &RPiCameraData::setLensControls); + ipa_->setCameraTimeout.connect(this, &RPiCameraData::setCameraTimeout); /* * The configuration (tuning file) is made from the sensor name unless @@ -1957,6 +1951,20 @@ void RPiCameraData::setLensControls(const ControlList &controls) } } +void RPiCameraData::setCameraTimeout(uint32_t maxFrameLengthMs) +{ + /* + * Set the dequeue timeout to the larger of 5x the maximum reported + * frame length advertised by the IPA over a number of frames. Allow + * a minimum timeout value of 1s. + */ + utils::Duration timeout = + std::max(1s, 5 * maxFrameLengthMs * 1ms); + + LOG(RPI, Debug) << "Setting Unicam timeout to " << timeout; + unicam_[Unicam::Image].dev()->setDequeueTimeout(timeout); +} + void RPiCameraData::setSensorControls(ControlList &controls) { /* From patchwork Thu Mar 2 13:54:28 2023 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Naushir Patuck X-Patchwork-Id: 18333 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 3B1C3C3261 for ; Thu, 2 Mar 2023 13:54:35 +0000 (UTC) Received: from lancelot.ideasonboard.com (localhost [IPv6:::1]) by lancelot.ideasonboard.com (Postfix) with ESMTP id 4DA56626E7; Thu, 2 Mar 2023 14:54:34 +0100 (CET) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=libcamera.org; s=mail; t=1677765274; bh=TVC6yxz98wuFiJN09PPjW07JmKiGUN0EP1/XbHV8y0M=; h=To:Date:In-Reply-To:References:List-Id:List-Post:From: List-Subscribe:List-Unsubscribe:List-Archive:Reply-To:List-Help: Subject:From; b=IXhTp4gwICfEISenBfUMOWo/LtPU9DkPYDbiA8g/3lqRZe44GfpSi8KdPpTaKT0Wt GL6ua+QxIJsO22SQv3iBakCta7hFBfL83dKp8MbnV3/tqYSU0TExlLZqFHdXIP89OK VWcsy5NnntqLvElxxo95YcAK0rJQ+DSK1d9vQ8vL1syIfmPlWZ+Fn1Fks8a+NJ270G vVdfibs8G8QVVtJcXUzTd/2mrnwuKuRv6dDK8klkvJB1Qd7dw0LC7d+AR6qda+gZEu UHuayaBPy9oOMuleeZ4hvoZHOZM3PxrOoM/RnE15bqLnFefnena0ujPVDrCa9Vs0yi +5LMGrUu1QNLQ== To: libcamera-devel@lists.libcamera.org Date: Thu, 2 Mar 2023 13:54:28 +0000 In-Reply-To: <20230302135429.23821-1-naush@raspberrypi.com> References: <20230302135429.23821-1-naush@raspberrypi.com> MIME-Version: 1.0 Message-ID: List-Id: List-Post: X-Patchwork-Original-From: Naushir Patuck via libcamera-devel From: Naushir Patuck Precedence: list X-Mailman-Version: 2.1.29 X-BeenThere: libcamera-devel@lists.libcamera.org List-Subscribe: , List-Unsubscribe: , List-Archive: Reply-To: Naushir Patuck List-Help: Subject: [libcamera-devel] [PATCH v2 2/3] ipa: raspberrypi: Better heruistics for calculating Unicam timeout Content-Disposition: inline Errors-To: libcamera-devel-bounces@lists.libcamera.org Sender: "libcamera-devel" The existing mechanism of setting a timeout value simply uses the maximum possible frame length advertised by the sensor mode. This can be problematic when, for example, the IMX477 sensor can use a frame length of over 600 seconds. However, for typical usage the frame length will never go over several 100s of milliseconds, making the timeout very impractical. Store a list of the last 10 frame length values requested by the AGC. On startup, and at the end of every frame, take the maximum frame length value from this list and return that to the pipeline handler through the setCameraTimeoutValue() signal. This allows the timeout value to better track the actual sensor usage. Signed-off-by: Naushir Patuck Reviewed-by: David Plowman Reviewed-by: Jacopo Mondi Reviewed-by: Laurent Pinchart --- src/ipa/raspberrypi/raspberrypi.cpp | 44 +++++++++++++++++++++++++++-- 1 file changed, 41 insertions(+), 3 deletions(-) diff --git a/src/ipa/raspberrypi/raspberrypi.cpp b/src/ipa/raspberrypi/raspberrypi.cpp index f6826bf27fe1..7b5aed644a67 100644 --- a/src/ipa/raspberrypi/raspberrypi.cpp +++ b/src/ipa/raspberrypi/raspberrypi.cpp @@ -8,6 +8,7 @@ #include #include #include +#include #include #include #include @@ -64,6 +65,9 @@ using utils::Duration; /* Number of metadata objects available in the context list. */ constexpr unsigned int numMetadataContexts = 16; +/* Number of frame length times to hold in the queue. */ +constexpr unsigned int FrameLengthsQueueSize = 10; + /* Configure the sensor with these values initially. */ constexpr double defaultAnalogueGain = 1.0; constexpr Duration defaultExposureTime = 20.0ms; @@ -121,7 +125,8 @@ class IPARPi : public IPARPiInterface public: IPARPi() : controller_(), frameCount_(0), checkCount_(0), mistrustCount_(0), - lastRunTimestamp_(0), lsTable_(nullptr), firstStart_(true) + lastRunTimestamp_(0), lsTable_(nullptr), firstStart_(true), + lastTimeout_(0s) { } @@ -155,6 +160,7 @@ private: void fillDeviceStatus(const ControlList &sensorControls, unsigned int ipaContext); RPiController::StatisticsPtr fillStatistics(bcm2835_isp_stats *stats) const; void processStats(unsigned int bufferId, unsigned int ipaContext); + void setCameraTimeoutValue(); void applyFrameDurations(Duration minFrameDuration, Duration maxFrameDuration); void applyAGC(const struct AgcStatus *agcStatus, ControlList &ctrls); void applyAWB(const struct AwbStatus *awbStatus, ControlList &ctrls); @@ -220,6 +226,10 @@ private: /* Maximum gain code for the sensor. */ uint32_t maxSensorGainCode_; + + /* Track the frame length times over FrameLengthsQueueSize frames. */ + std::deque frameLengths_; + Duration lastTimeout_; }; int IPARPi::init(const IPASettings &settings, bool lensPresent, IPAInitResult *result) @@ -284,6 +294,10 @@ void IPARPi::start(const ControlList &controls, StartConfig *startConfig) controller_.switchMode(mode_, &metadata); + /* Reset the frame lengths queue */ + frameLengths_.clear(); + frameLengths_.resize(FrameLengthsQueueSize, 0s); + /* SwitchMode may supply updated exposure/gain values to use. */ AgcStatus agcStatus; agcStatus.shutterTime = 0.0s; @@ -294,6 +308,7 @@ void IPARPi::start(const ControlList &controls, StartConfig *startConfig) ControlList ctrls(sensorCtrls_); applyAGC(&agcStatus, ctrls); startConfig->controls = std::move(ctrls); + setCameraTimeoutValue(); } /* @@ -340,8 +355,6 @@ void IPARPi::start(const ControlList &controls, StartConfig *startConfig) } startConfig->dropFrameCount = dropFrameCount_; - const Duration maxSensorFrameDuration = mode_.maxFrameLength * mode_.maxLineLength; - setCameraTimeout.emit(maxSensorFrameDuration.get()); firstStart_ = false; lastRunTimestamp_ = 0; @@ -1434,6 +1447,22 @@ void IPARPi::processStats(unsigned int bufferId, unsigned int ipaContext) applyAGC(&agcStatus, ctrls); setDelayedControls.emit(ctrls, ipaContext); + setCameraTimeoutValue(); + } +} + +void IPARPi::setCameraTimeoutValue() +{ + /* + * Take the maximum value of the exposure queue as the camera timeout + * value to pass back to the pipeline handler. Only signal if it has changed + * from the last set value. + */ + auto max = std::max_element(frameLengths_.begin(), frameLengths_.end()); + + if (*max != lastTimeout_) { + setCameraTimeout.emit(max->get()); + lastTimeout_ = *max; } } @@ -1522,6 +1551,15 @@ void IPARPi::applyAGC(const struct AgcStatus *agcStatus, ControlList &ctrls) */ if (mode_.minLineLength != mode_.maxLineLength) ctrls.set(V4L2_CID_HBLANK, static_cast(hblank)); + + /* + * Store the frame length times in a circular queue, up-to FrameLengthsQueueSize + * elements. This will be used to advertise a camera timeout value to the + * pipeline handler. + */ + frameLengths_.pop_front(); + frameLengths_.push_back(helper_->exposure(vblank + mode_.height, + helper_->hblankToLineLength(hblank))); } void IPARPi::applyDG(const struct AgcStatus *dgStatus, ControlList &ctrls) From patchwork Thu Mar 2 13:54:29 2023 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Naushir Patuck X-Patchwork-Id: 18334 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 A5BB3C329C for ; Thu, 2 Mar 2023 13:54:35 +0000 (UTC) Received: from lancelot.ideasonboard.com (localhost [IPv6:::1]) by lancelot.ideasonboard.com (Postfix) with ESMTP id BCD92626EB; Thu, 2 Mar 2023 14:54:34 +0100 (CET) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=libcamera.org; s=mail; t=1677765274; bh=jduccelxlH2i28khcmaLGCPGUXhx5ZPLJ0/L5zT9sus=; h=To:Date:In-Reply-To:References:List-Id:List-Post:From: List-Subscribe:List-Unsubscribe:List-Archive:Reply-To:List-Help: Subject:From; b=fyEhXgJVQi/3dhaKqeNWiy99pyFn8Kd6d35SY2f84z1pxpdanyhLBaOUb82TaF0+i pEoasqJGNwAwk9PL8r8DsmApKcCj7Xfbfm9PUzbUDLVMjPTHkzCmHtrX/Nrz5ePkwD pYAsU7+WfGGuXu4ZxrU60XewVb8w/EVFd8S2rFv6+ci+yFOO8TkMHjCsn9z3/IhE46 FLFQIJcJn9VK6nzxgCSoEjGcACD0Ztnk9IGL+nYn22uwL24sjg+pl5pRrQj1w3lAuu irjtvEDx/5D90RJqSvAFI2jUG//8zgy+9XJ1ZgTwq3gMIKO4lAv1vdnFyew0QVQiwv Nf4u1lSGp9QSw== To: libcamera-devel@lists.libcamera.org Date: Thu, 2 Mar 2023 13:54:29 +0000 In-Reply-To: <20230302135429.23821-1-naush@raspberrypi.com> References: <20230302135429.23821-1-naush@raspberrypi.com> MIME-Version: 1.0 Message-ID: List-Id: List-Post: X-Patchwork-Original-From: Naushir Patuck via libcamera-devel From: Naushir Patuck Precedence: list X-Mailman-Version: 2.1.29 X-BeenThere: libcamera-devel@lists.libcamera.org List-Subscribe: , List-Unsubscribe: , List-Archive: Reply-To: Naushir Patuck List-Help: Subject: [libcamera-devel] [PATCH v2 3/3] pipeline: raspberrypi: Add a Unicam timeout override config options Content-Disposition: inline Errors-To: libcamera-devel-bounces@lists.libcamera.org Sender: "libcamera-devel" Add a new parameter to the pipeline handler config file named "unicam_timeout_value_ms" to allow users to override the automiatically computed Unicam timeout value. This value is given in milliseconds, and setting a value of 0 (the default value) disables the override. Signed-off-by: Naushir Patuck Reviewed-by: David Plowman Reviewed-by: Jacopo Mondi --- .../pipeline/raspberrypi/data/example.yaml | 11 ++++++++++- src/libcamera/pipeline/raspberrypi/raspberrypi.cpp | 14 ++++++++++++++ 2 files changed, 24 insertions(+), 1 deletion(-) diff --git a/src/libcamera/pipeline/raspberrypi/data/example.yaml b/src/libcamera/pipeline/raspberrypi/data/example.yaml index ad5f2344384f..5d4ca997b7a0 100644 --- a/src/libcamera/pipeline/raspberrypi/data/example.yaml +++ b/src/libcamera/pipeline/raspberrypi/data/example.yaml @@ -32,6 +32,15 @@ # Override any request from the IPA to drop a number of startup # frames. # - # "disable_startup_frame_drops": false + # "disable_startup_frame_drops": false, + + # Custom timeout value (in ms) for Unicam to use. This overrides + # the value computed by the pipeline handler based on frame + # durations. + # + # Set this value to 0 to use the pipeline handler computed + # timeout value. + # + # "unicam_timeout_value_ms": 0 } } diff --git a/src/libcamera/pipeline/raspberrypi/raspberrypi.cpp b/src/libcamera/pipeline/raspberrypi/raspberrypi.cpp index 3d04842a2440..6b0880c579eb 100644 --- a/src/libcamera/pipeline/raspberrypi/raspberrypi.cpp +++ b/src/libcamera/pipeline/raspberrypi/raspberrypi.cpp @@ -319,6 +319,11 @@ public: * frames. */ bool disableStartupFrameDrops; + /* + * Override the Unicam timeout value calculated by the IPA based + * on frame durations. + */ + unsigned int unicamTimeoutValue; }; Config config_; @@ -1715,6 +1720,7 @@ int RPiCameraData::loadPipelineConfiguration() .minUnicamBuffers = 2, .minTotalUnicamBuffers = 4, .disableStartupFrameDrops = false, + .unicamTimeoutValue = 0, }; char const *configFromEnv = utils::secure_getenv("LIBCAMERA_RPI_CONFIG_FILE"); @@ -1750,6 +1756,14 @@ int RPiCameraData::loadPipelineConfiguration() phConfig["min_total_unicam_buffers"].get(config_.minTotalUnicamBuffers); config_.disableStartupFrameDrops = phConfig["disable_startup_frame_drops"].get(config_.disableStartupFrameDrops); + config_.unicamTimeoutValue = + phConfig["unicam_timeout_value_ms"].get(config_.unicamTimeoutValue); + + if (config_.unicamTimeoutValue) { + /* Disable the IPA signal to control timeout and set the user requested value. */ + unicam_[Unicam::Image].dev()->dequeueTimeout.disconnect(); + unicam_[Unicam::Image].dev()->setDequeueTimeout(config_.unicamTimeoutValue * 1ms); + } if (config_.minTotalUnicamBuffers < config_.minUnicamBuffers) { LOG(RPI, Error) << "Invalid configuration: min_total_unicam_buffers must be >= min_unicam_buffers";