<div dir="ltr"><div dir="ltr"><div>Hi Jacopo,</div><div><br></div><div>Thank you for the review for this series!</div><div><br></div></div><br><div class="gmail_quote gmail_quote_container"><div dir="ltr" class="gmail_attr">On Thu, 5 Jun 2025 at 08:24, Jacopo Mondi <<a href="mailto:jacopo.mondi@ideasonboard.com">jacopo.mondi@ideasonboard.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>
On Thu, May 22, 2025 at 08:48:20AM +0100, Naushir Patuck wrote:<br>
> With the previous change to not drop frames in the pipeline handler,<br>
> the "disable_startup_frame_drops" pipeline config option is not used.<br>
> Remove it, and throw a warning if the option is present in the YAML<br>
> config file.<br>
><br>
> Signed-off-by: Naushir Patuck <<a href="mailto:naush@raspberrypi.com" target="_blank">naush@raspberrypi.com</a>><br>
> Reviewed-by: David Plowman <<a href="mailto:david.plowman@raspberrypi.com" target="_blank">david.plowman@raspberrypi.com</a>><br>
> ---<br>
> src/libcamera/pipeline/rpi/common/pipeline_base.cpp | 7 ++++---<br>
> src/libcamera/pipeline/rpi/common/pipeline_base.h | 5 -----<br>
> src/libcamera/pipeline/rpi/pisp/data/example.yaml | 5 -----<br>
> src/libcamera/pipeline/rpi/vc4/data/example.yaml | 5 -----<br>
> 4 files changed, 4 insertions(+), 18 deletions(-)<br>
><br>
> diff --git a/src/libcamera/pipeline/rpi/common/pipeline_base.cpp b/src/libcamera/pipeline/rpi/common/pipeline_base.cpp<br>
> index 3f0b7abdc59a..bef057a70353 100644<br>
> --- a/src/libcamera/pipeline/rpi/common/pipeline_base.cpp<br>
> +++ b/src/libcamera/pipeline/rpi/common/pipeline_base.cpp<br>
> @@ -1078,7 +1078,6 @@ void CameraData::enumerateVideoDevices(MediaLink *link, const std::string &front<br>
> int CameraData::loadPipelineConfiguration()<br>
> {<br>
> config_ = {<br>
> - .disableStartupFrameDrops = false,<br>
> .cameraTimeoutValue = 0,<br>
> };<br>
><br>
> @@ -1115,8 +1114,10 @@ int CameraData::loadPipelineConfiguration()<br>
><br>
> const YamlObject &phConfig = (*root)["pipeline_handler"];<br>
><br>
> - config_.disableStartupFrameDrops =<br>
> - phConfig["disable_startup_frame_drops"].get<bool>(config_.disableStartupFrameDrops);<br>
> + if (phConfig.contains("disable_startup_frame_drops"))<br>
> + LOG(RPI, Warning)<br>
> + << "The disable_startup_frame_drops key is now deprecated, "<br>
> + << "please use FrameMetadata::Status::FrameStartup instead.";<br>
<br>
I wonder if the "please use .." might mis-lead people that have to<br>
update their config file. Maybe<br>
<br>
<< "The disable_startup_frame_drops key is now deprecated, "<br>
<< "startup frames are not identified by the FrameMetadata::Status::FrameStartup flag";<br>
<br>
A minor indeed<br>
<br>
Reviewed-by: Jacopo Mondi <<a href="mailto:jacopo.mondi@ideasonboard.com" target="_blank">jacopo.mondi@ideasonboard.com</a>><br></blockquote><div><br></div><div>Ack, I'll update the message and post a new revision of the series.</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>
Thanks<br>
j<br>
<br>
><br>
> config_.cameraTimeoutValue =<br>
> phConfig["camera_timeout_value_ms"].get<unsigned int>(config_.cameraTimeoutValue);<br>
> diff --git a/src/libcamera/pipeline/rpi/common/pipeline_base.h b/src/libcamera/pipeline/rpi/common/pipeline_base.h<br>
> index 6023f9f9d6b3..e27c4f860d1a 100644<br>
> --- a/src/libcamera/pipeline/rpi/common/pipeline_base.h<br>
> +++ b/src/libcamera/pipeline/rpi/common/pipeline_base.h<br>
> @@ -164,11 +164,6 @@ public:<br>
> bool buffersAllocated_;<br>
><br>
> struct Config {<br>
> - /*<br>
> - * Override any request from the IPA to drop a number of startup<br>
> - * frames.<br>
> - */<br>
> - bool disableStartupFrameDrops;<br>
> /*<br>
> * Override the camera timeout value calculated by the IPA based<br>
> * on frame durations.<br>
> diff --git a/src/libcamera/pipeline/rpi/pisp/data/example.yaml b/src/libcamera/pipeline/rpi/pisp/data/example.yaml<br>
> index d67e654a8b9e..baf03be79bb3 100644<br>
> --- a/src/libcamera/pipeline/rpi/pisp/data/example.yaml<br>
> +++ b/src/libcamera/pipeline/rpi/pisp/data/example.yaml<br>
> @@ -16,11 +16,6 @@<br>
> #<br>
> # "num_cfe_config_queue": 2,<br>
><br>
> - # Override any request from the IPA to drop a number of startup<br>
> - # frames.<br>
> - #<br>
> - # "disable_startup_frame_drops": false,<br>
> -<br>
> # Custom timeout value (in ms) for camera to use. This overrides<br>
> # the value computed by the pipeline handler based on frame<br>
> # durations.<br>
> diff --git a/src/libcamera/pipeline/rpi/vc4/data/example.yaml b/src/libcamera/pipeline/rpi/vc4/data/example.yaml<br>
> index b8e01adeaf40..27e543488d48 100644<br>
> --- a/src/libcamera/pipeline/rpi/vc4/data/example.yaml<br>
> +++ b/src/libcamera/pipeline/rpi/vc4/data/example.yaml<br>
> @@ -29,11 +29,6 @@<br>
> #<br>
> # "min_total_unicam_buffers": 4,<br>
><br>
> - # Override any request from the IPA to drop a number of startup<br>
> - # frames.<br>
> - #<br>
> - # "disable_startup_frame_drops": false,<br>
> -<br>
> # Custom timeout value (in ms) for camera to use. This overrides<br>
> # the value computed by the pipeline handler based on frame<br>
> # durations.<br>
> --<br>
> 2.43.0<br>
><br>
</blockquote></div></div>