<div dir="ltr"><div dir="ltr">Hi David,<div><br></div><div>Thank you for your feedback.</div></div><br><div class="gmail_quote"><div dir="ltr" class="gmail_attr">On Tue, 1 Nov 2022 at 12:12, David Plowman <<a href="mailto:david.plowman@raspberrypi.com">david.plowman@raspberrypi.com</a>> wrote:<br></div><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left:1px solid rgb(204,204,204);padding-left:1ex">Hi Naush<br>
<br>
Thanks for the patch!<br>
<br>
On Fri, 14 Oct 2022 at 14:19, Naushir Patuck via libcamera-devel<br>
<<a href="mailto:libcamera-devel@lists.libcamera.org" target="_blank">libcamera-devel@lists.libcamera.org</a>> wrote:<br>
><br>
> Add a new pipeline config parameter "disable_startup_frame_drops" to disable<br>
> any startup drop frames, overriding the IPA request.<br>
><br>
> When this parameter is set, it allows the pipeline handler to run with no<br>
> internally allocated Unicam buffers ("min_unicam_buffers").<br>
><br>
> Add a validation to ensure if "disable_startup_frame_drops" is false, at least<br>
> one internal Unicam buffer is allocated, possibly overriding the<br>
> "min_unicam_buffers" parameter.<br>
><br>
> Signed-off-by: Naushir Patuck <<a href="mailto:naush@raspberrypi.com" target="_blank">naush@raspberrypi.com</a>><br>
> ---<br>
> .../pipeline/raspberrypi/data/default.json | 7 +++++--<br>
> .../pipeline/raspberrypi/raspberrypi.cpp | 15 ++++++++++++++-<br>
> 2 files changed, 19 insertions(+), 3 deletions(-)<br>
><br>
> diff --git a/src/libcamera/pipeline/raspberrypi/data/default.json b/src/libcamera/pipeline/raspberrypi/data/default.json<br>
> index a7ea735c87f4..707414bcc5c5 100644<br>
> --- a/src/libcamera/pipeline/raspberrypi/data/default.json<br>
> +++ b/src/libcamera/pipeline/raspberrypi/data/default.json<br>
> @@ -5,7 +5,7 @@<br>
> "pipeline_handler":<br>
> {<br>
> # The minimum number of internal buffers to be allocated for Unicam.<br>
> - # This value must be greater than 0, but less than or equal to min_total_unicam_buffers.<br>
> + # This value must less than or equal to min_total_unicam_buffers.<br>
> "min_unicam_buffers": 2,<br>
><br>
> # The minimum total (internal + external) buffer count used for Unicam.<br>
> @@ -15,6 +15,9 @@<br>
> "min_total_unicam_buffers": 4,<br>
><br>
> # The number of internal buffers used for ISP Output0.<br>
> - "num_output0_buffers": 1<br>
> + "num_output0_buffers": 1,<br>
> +<br>
> + # Override any request from the IPA to drop a number of startup frames.<br>
> + "disable_startup_frame_drops": false<br>
> }<br>
> }<br>
> diff --git a/src/libcamera/pipeline/raspberrypi/raspberrypi.cpp b/src/libcamera/pipeline/raspberrypi/raspberrypi.cpp<br>
> index 2aba0430c02e..135948d82f41 100644<br>
> --- a/src/libcamera/pipeline/raspberrypi/raspberrypi.cpp<br>
> +++ b/src/libcamera/pipeline/raspberrypi/raspberrypi.cpp<br>
> @@ -300,6 +300,7 @@ public:<br>
> unsigned int minUnicamBuffers;<br>
> unsigned int minTotalUnicamBuffers;<br>
> unsigned int numOutput0Buffers;<br>
> + bool disableStartupFrameDrops;<br>
> };<br>
><br>
> Config config_;<br>
> @@ -1044,7 +1045,7 @@ int PipelineHandlerRPi::start(Camera *camera, const ControlList *controls)<br>
> data->setSensorControls(startConfig.controls);<br>
><br>
> /* Configure the number of dropped frames required on startup. */<br>
> - data->dropFrameCount_ = startConfig.dropFrameCount;<br>
> + data->dropFrameCount_ = data->config_.disableStartupFrameDrops ? 0 : startConfig.dropFrameCount;<br>
<br>
Might this warrant an INFO message, just in case folks are confused by<br>
the changing AGC/AWB when things start? Or maybe that would be<br>
annoying, for example if an application wants to run like this and<br>
handles the startup frames for itself?<br></blockquote><div><br></div><div>There are already DEBUG level messages for dropping convergence frames,</div><div>do you think that is enough?</div><div><br></div><div>Regards,</div><div>Naush</div><div><br></div><div> </div><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left:1px solid rgb(204,204,204);padding-left:1ex">
<br>
Reviewed-by: David Plowman <<a href="mailto:david.plowman@raspberrypi.com" target="_blank">david.plowman@raspberrypi.com</a>><br>
<br>
Thanks!<br>
David<br>
<br>
><br>
> for (auto const stream : data->streams_)<br>
> stream->resetBuffers();<br>
> @@ -1430,6 +1431,7 @@ int PipelineHandlerRPi::configurePipelineHandler(RPiCameraData *data)<br>
> .minUnicamBuffers = 2,<br>
> .minTotalUnicamBuffers = 4,<br>
> .numOutput0Buffers = 1,<br>
> + .disableStartupFrameDrops = false,<br>
> };<br>
><br>
> char const *configFromEnv = utils::secure_getenv("LIBCAMERA_RPI_CONFIG_FILE");<br>
> @@ -1464,6 +1466,8 @@ int PipelineHandlerRPi::configurePipelineHandler(RPiCameraData *data)<br>
> phConfig["min_total_unicam_buffers"].get<unsigned int>(config.minTotalUnicamBuffers);<br>
> config.numOutput0Buffers =<br>
> phConfig["num_output0_buffers"].get<unsigned int>(config.numOutput0Buffers);<br>
> + config.disableStartupFrameDrops =<br>
> + phConfig["disable_startup_frame_drops"].get<bool>(config.disableStartupFrameDrops);<br>
><br>
> if (config.minTotalUnicamBuffers < config.minUnicamBuffers || config.minTotalUnicamBuffers < 1) {<br>
> LOG(RPI, Error) << "Invalid Unicam buffer configuration used!";<br>
> @@ -1543,6 +1547,15 @@ int PipelineHandlerRPi::prepareBuffers(Camera *camera)<br>
> */<br>
> numBuffers = std::max<int>(data->config_.minUnicamBuffers,<br>
> minBuffers - numRawBuffers);<br>
> +<br>
> + if (numBuffers == 0 && data->dropFrameCount_) {<br>
> + LOG(RPI, Warning)<br>
> + << "Configured with no Unicam buffers,"<br>
> + " but the IPA requested startup frame drops."<br>
> + " Increasing to one buffer.";<br>
> + numBuffers = 1;<br>
> + }<br>
> +<br>
> data->numUnicamBuffers = numBuffers;<br>
> } else if (stream == &data->isp_[Isp::Input]) {<br>
> /*<br>
> --<br>
> 2.25.1<br>
><br>
</blockquote></div></div>