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