Message ID | 20221129134534.2933-8-naush@raspberrypi.com |
---|---|
State | Superseded |
Headers | show |
Series |
|
Related | show |
Hi Naush, Thank you for the patch. On Tue, Nov 29, 2022 at 01:45:31PM +0000, Naushir Patuck via libcamera-devel wrote: > Add a new pipeline config parameter "disable_startup_frame_drops" to disable > any startup drop frames, overriding the IPA request. > > When this parameter is set, it allows the pipeline handler to run with no > internally allocated Unicam buffers ("min_unicam_buffers"). > > Add a validation to ensure if "disable_startup_frame_drops" is false, at least > one internal Unicam buffer is allocated, possibly overriding the > "min_unicam_buffers" parameter. Is this meant to support applications that want to capture all frames, or only as a side effect of the memory allocation optimizations to lower memory consumption ? In the latter case, as mentioned in the review of 06/10, I don't think this would be required. > Signed-off-by: Naushir Patuck <naush@raspberrypi.com> > Reviewed-by: David Plowman <david.plowman@raspberrypi.com> > --- > .../pipeline/raspberrypi/data/default.json | 7 +++++-- > .../pipeline/raspberrypi/raspberrypi.cpp | 19 ++++++++++++++++++- > 2 files changed, 23 insertions(+), 3 deletions(-) > > diff --git a/src/libcamera/pipeline/raspberrypi/data/default.json b/src/libcamera/pipeline/raspberrypi/data/default.json > index a7ea735c87f4..707414bcc5c5 100644 > --- a/src/libcamera/pipeline/raspberrypi/data/default.json > +++ b/src/libcamera/pipeline/raspberrypi/data/default.json > @@ -5,7 +5,7 @@ > "pipeline_handler": > { > # The minimum number of internal buffers to be allocated for Unicam. > - # This value must be greater than 0, but less than or equal to min_total_unicam_buffers. > + # This value must less than or equal to min_total_unicam_buffers. > "min_unicam_buffers": 2, > > # The minimum total (internal + external) buffer count used for Unicam. > @@ -15,6 +15,9 @@ > "min_total_unicam_buffers": 4, > > # The number of internal buffers used for ISP Output0. > - "num_output0_buffers": 1 > + "num_output0_buffers": 1, > + > + # Override any request from the IPA to drop a number of startup frames. > + "disable_startup_frame_drops": false > } > } > diff --git a/src/libcamera/pipeline/raspberrypi/raspberrypi.cpp b/src/libcamera/pipeline/raspberrypi/raspberrypi.cpp > index 742521927780..ef49d32037af 100644 > --- a/src/libcamera/pipeline/raspberrypi/raspberrypi.cpp > +++ b/src/libcamera/pipeline/raspberrypi/raspberrypi.cpp > @@ -312,6 +312,11 @@ public: > * stream. > */ > unsigned int numOutput0Buffers; > + /* > + * Override any request from the IPA to drop a number of startup > + * frames. > + */ > + bool disableStartupFrameDrops; > }; > > Config config_; > @@ -1058,7 +1063,7 @@ int PipelineHandlerRPi::start(Camera *camera, const ControlList *controls) > data->setSensorControls(startConfig.controls); > > /* Configure the number of dropped frames required on startup. */ > - data->dropFrameCount_ = startConfig.dropFrameCount; > + data->dropFrameCount_ = data->config_.disableStartupFrameDrops ? 0 : startConfig.dropFrameCount; > > for (auto const stream : data->streams_) > stream->resetBuffers(); > @@ -1451,6 +1456,7 @@ int PipelineHandlerRPi::configurePipelineHandler(RPiCameraData *data) > .minUnicamBuffers = 2, > .minTotalUnicamBuffers = 4, > .numOutput0Buffers = 1, > + .disableStartupFrameDrops = false, > }; > > char const *configFromEnv = utils::secure_getenv("LIBCAMERA_RPI_CONFIG_FILE"); > @@ -1485,6 +1491,8 @@ int PipelineHandlerRPi::configurePipelineHandler(RPiCameraData *data) > phConfig["min_total_unicam_buffers"].get<unsigned int>(config.minTotalUnicamBuffers); > config.numOutput0Buffers = > phConfig["num_output0_buffers"].get<unsigned int>(config.numOutput0Buffers); > + config.disableStartupFrameDrops = > + phConfig["disable_startup_frame_drops"].get<bool>(config.disableStartupFrameDrops); > > if (config.minTotalUnicamBuffers < config.minUnicamBuffers) { > LOG(RPI, Error) << "Invalid configuration: min_total_unicam_buffers must be >= min_unicam_buffers!"; > @@ -1569,6 +1577,15 @@ int PipelineHandlerRPi::prepareBuffers(Camera *camera) > */ > numBuffers = std::max<int>(data->config_.minUnicamBuffers, > minBuffers - numRawBuffers); > + > + if (numBuffers == 0 && data->dropFrameCount_) { > + LOG(RPI, Warning) > + << "Configured with no Unicam buffers," > + " but the IPA requested startup frame drops." > + " Increasing to one buffer."; > + numBuffers = 1; > + } > + > data->numUnicamBuffers = numBuffers; > } else if (stream == &data->isp_[Isp::Input]) { > /*
diff --git a/src/libcamera/pipeline/raspberrypi/data/default.json b/src/libcamera/pipeline/raspberrypi/data/default.json index a7ea735c87f4..707414bcc5c5 100644 --- a/src/libcamera/pipeline/raspberrypi/data/default.json +++ b/src/libcamera/pipeline/raspberrypi/data/default.json @@ -5,7 +5,7 @@ "pipeline_handler": { # The minimum number of internal buffers to be allocated for Unicam. - # This value must be greater than 0, but less than or equal to min_total_unicam_buffers. + # This value must less than or equal to min_total_unicam_buffers. "min_unicam_buffers": 2, # The minimum total (internal + external) buffer count used for Unicam. @@ -15,6 +15,9 @@ "min_total_unicam_buffers": 4, # The number of internal buffers used for ISP Output0. - "num_output0_buffers": 1 + "num_output0_buffers": 1, + + # Override any request from the IPA to drop a number of startup frames. + "disable_startup_frame_drops": false } } diff --git a/src/libcamera/pipeline/raspberrypi/raspberrypi.cpp b/src/libcamera/pipeline/raspberrypi/raspberrypi.cpp index 742521927780..ef49d32037af 100644 --- a/src/libcamera/pipeline/raspberrypi/raspberrypi.cpp +++ b/src/libcamera/pipeline/raspberrypi/raspberrypi.cpp @@ -312,6 +312,11 @@ public: * stream. */ unsigned int numOutput0Buffers; + /* + * Override any request from the IPA to drop a number of startup + * frames. + */ + bool disableStartupFrameDrops; }; Config config_; @@ -1058,7 +1063,7 @@ int PipelineHandlerRPi::start(Camera *camera, const ControlList *controls) data->setSensorControls(startConfig.controls); /* Configure the number of dropped frames required on startup. */ - data->dropFrameCount_ = startConfig.dropFrameCount; + data->dropFrameCount_ = data->config_.disableStartupFrameDrops ? 0 : startConfig.dropFrameCount; for (auto const stream : data->streams_) stream->resetBuffers(); @@ -1451,6 +1456,7 @@ int PipelineHandlerRPi::configurePipelineHandler(RPiCameraData *data) .minUnicamBuffers = 2, .minTotalUnicamBuffers = 4, .numOutput0Buffers = 1, + .disableStartupFrameDrops = false, }; char const *configFromEnv = utils::secure_getenv("LIBCAMERA_RPI_CONFIG_FILE"); @@ -1485,6 +1491,8 @@ int PipelineHandlerRPi::configurePipelineHandler(RPiCameraData *data) phConfig["min_total_unicam_buffers"].get<unsigned int>(config.minTotalUnicamBuffers); config.numOutput0Buffers = phConfig["num_output0_buffers"].get<unsigned int>(config.numOutput0Buffers); + config.disableStartupFrameDrops = + phConfig["disable_startup_frame_drops"].get<bool>(config.disableStartupFrameDrops); if (config.minTotalUnicamBuffers < config.minUnicamBuffers) { LOG(RPI, Error) << "Invalid configuration: min_total_unicam_buffers must be >= min_unicam_buffers!"; @@ -1569,6 +1577,15 @@ int PipelineHandlerRPi::prepareBuffers(Camera *camera) */ numBuffers = std::max<int>(data->config_.minUnicamBuffers, minBuffers - numRawBuffers); + + if (numBuffers == 0 && data->dropFrameCount_) { + LOG(RPI, Warning) + << "Configured with no Unicam buffers," + " but the IPA requested startup frame drops." + " Increasing to one buffer."; + numBuffers = 1; + } + data->numUnicamBuffers = numBuffers; } else if (stream == &data->isp_[Isp::Input]) { /*