[v2,2/6] ipa: rpi: Replace dropFrameCount in the IPA -> PH interface

23417 diff mbox series

Replace the dropFrameCount parameter returned from ipa::start() to the
pipeline handler by startupFrameCount and invalidFrameCount. The former
counts the number of frames required for AWB/AGC to converge, and the
latter counts the number of invalid frames produced by the sensor when
starting up.

In the pipeline handler, use the sum of these 2 values to replicate the
existing dropFrameCount behaviour.

Signed-off-by: Naushir Patuck <naush@raspberrypi.com>
Reviewed-by: David Plowman <david.plowman@raspberrypi.com>
---
 include/libcamera/ipa/raspberrypi.mojom            |  3 ++-
 src/ipa/rpi/common/ipa_base.cpp                    | 14 ++++++++------
 .../pipeline/rpi/common/pipeline_base.cpp          |  4 ++--
 3 files changed, 12 insertions(+), 9 deletions(-)

Hi Naush

On Thu, May 22, 2025 at 08:48:18AM +0100, Naushir Patuck wrote:
> Replace the dropFrameCount parameter returned from ipa::start() to the
> pipeline handler by startupFrameCount and invalidFrameCount. The former
> counts the number of frames required for AWB/AGC to converge, and the
> latter counts the number of invalid frames produced by the sensor when
> starting up.
>
> In the pipeline handler, use the sum of these 2 values to replicate the
> existing dropFrameCount behaviour.
>
> Signed-off-by: Naushir Patuck <naush@raspberrypi.com>
> Reviewed-by: David Plowman <david.plowman@raspberrypi.com>
> ---
>  include/libcamera/ipa/raspberrypi.mojom            |  3 ++-
>  src/ipa/rpi/common/ipa_base.cpp                    | 14 ++++++++------
>  .../pipeline/rpi/common/pipeline_base.cpp          |  4 ++--
>  3 files changed, 12 insertions(+), 9 deletions(-)
>
> diff --git a/include/libcamera/ipa/raspberrypi.mojom b/include/libcamera/ipa/raspberrypi.mojom
> index e30c70bde08b..12b083e9d04b 100644
> --- a/include/libcamera/ipa/raspberrypi.mojom
> +++ b/include/libcamera/ipa/raspberrypi.mojom
> @@ -52,7 +52,8 @@ struct ConfigResult {
>
>  struct StartResult {
>  	libcamera.ControlList controls;
> -	int32 dropFrameCount;
> +	int32 startupFrameCount;
> +	int32 invalidFrameCount;
>  };
>
>  struct PrepareParams {
> diff --git a/src/ipa/rpi/common/ipa_base.cpp b/src/ipa/rpi/common/ipa_base.cpp
> index e0a93daa9db2..c15f8a7bf71e 100644
> --- a/src/ipa/rpi/common/ipa_base.cpp
> +++ b/src/ipa/rpi/common/ipa_base.cpp
> @@ -324,6 +324,7 @@ void IpaBase::start(const ControlList &controls, StartResult *result)
>  	 * "mistrusted", which depends on whether this is a startup from cold,
>  	 * or merely a mode switch in a running system.
>  	 */
> +	unsigned int agcConvergenceFrames = 0, awbConvergenceFrames = 0;
>  	frameCount_ = 0;
>  	if (firstStart_) {
>  		dropFrameCount_ = helper_->hideFramesStartup();
> @@ -336,7 +337,6 @@ void IpaBase::start(const ControlList &controls, StartResult *result)
>  		 * (mistrustCount_) that they won't see. But if zero (i.e.
>  		 * no convergence necessary), no frames need to be dropped.
>  		 */
> -		unsigned int agcConvergenceFrames = 0;
>  		RPiController::AgcAlgorithm *agc = dynamic_cast<RPiController::AgcAlgorithm *>(
>  			controller_.getAlgorithm("agc"));
>  		if (agc) {
> @@ -345,7 +345,6 @@ void IpaBase::start(const ControlList &controls, StartResult *result)
>  				agcConvergenceFrames += mistrustCount_;
>  		}
>
> -		unsigned int awbConvergenceFrames = 0;
>  		RPiController::AwbAlgorithm *awb = dynamic_cast<RPiController::AwbAlgorithm *>(
>  			controller_.getAlgorithm("awb"));
>  		if (awb) {
> @@ -353,15 +352,18 @@ void IpaBase::start(const ControlList &controls, StartResult *result)
>  			if (awbConvergenceFrames)
>  				awbConvergenceFrames += mistrustCount_;
>  		}
> -
> -		dropFrameCount_ = std::max({ dropFrameCount_, agcConvergenceFrames, awbConvergenceFrames });
> -		LOG(IPARPI, Debug) << "Drop " << dropFrameCount_ << " frames on startup";
>  	} else {
>  		dropFrameCount_ = helper_->hideFramesModeSwitch();
>  		mistrustCount_ = helper_->mistrustFramesModeSwitch();
>  	}
>
> -	result->dropFrameCount = dropFrameCount_;
> +	result->startupFrameCount = std::max({ agcConvergenceFrames, awbConvergenceFrames });
> +	result->invalidFrameCount = dropFrameCount_;
> +
> +	dropFrameCount_ = std::max({ dropFrameCount_, agcConvergenceFrames, awbConvergenceFrames });
> +
> +	LOG(IPARPI, Debug) << "Startup frames: " << result->startupFrameCount
> +			   << " Invalid frames: " << result->invalidFrameCount;
>
>  	firstStart_ = false;
>  	lastRunTimestamp_ = 0;
> diff --git a/src/libcamera/pipeline/rpi/common/pipeline_base.cpp b/src/libcamera/pipeline/rpi/common/pipeline_base.cpp
> index 1f13e5230fae..5956485953a2 100644
> --- a/src/libcamera/pipeline/rpi/common/pipeline_base.cpp
> +++ b/src/libcamera/pipeline/rpi/common/pipeline_base.cpp
> @@ -660,8 +660,8 @@ int PipelineHandlerBase::start(Camera *camera, const ControlList *controls)
>  		data->setSensorControls(result.controls);
>
>  	/* Configure the number of dropped frames required on startup. */
> -	data->dropFrameCount_ = data->config_.disableStartupFrameDrops
> -			      ? 0 : result.dropFrameCount;
> +	data->dropFrameCount_ = data->config_.disableStartupFrameDrops ?
> +			0 : result.startupFrameCount + result.invalidFrameCount;

I would have kept the previous style. Can be adjusted when applying
Reviewed-by: Jacopo Mondi <jacopo.mondi@ideasonboard.com>

Thanks
  j

>
>  	for (auto const stream : data->streams_)
>  		stream->resetBuffers();
> --
> 2.43.0
>