[libcamera-devel] [PATCH v1 07/10] pipeline: raspberrypi: Add a parameter to disable startup drop frames

[David Plowman]

Hi Naush

Thanks for the patch!

On Fri, 14 Oct 2022 at 14:19, Naushir Patuck via libcamera-devel
<libcamera-devel at lists.libcamera.org> 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.
>
> Signed-off-by: Naushir Patuck <naush at raspberrypi.com>
> ---
>  .../pipeline/raspberrypi/data/default.json        |  7 +++++--
>  .../pipeline/raspberrypi/raspberrypi.cpp          | 15 ++++++++++++++-
>  2 files changed, 19 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 2aba0430c02e..135948d82f41 100644
> --- a/src/libcamera/pipeline/raspberrypi/raspberrypi.cpp
> +++ b/src/libcamera/pipeline/raspberrypi/raspberrypi.cpp
> @@ -300,6 +300,7 @@ public:
>                 unsigned int minUnicamBuffers;
>                 unsigned int minTotalUnicamBuffers;
>                 unsigned int numOutput0Buffers;
> +               bool disableStartupFrameDrops;
>         };
>
>         Config config_;
> @@ -1044,7 +1045,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;

Might this warrant an INFO message, just in case folks are confused by
the changing AGC/AWB when things start? Or maybe that would be
annoying, for example if an application wants to run like this and
handles the startup frames for itself?

Reviewed-by: David Plowman <david.plowman at raspberrypi.com>

Thanks!
David

>
>         for (auto const stream : data->streams_)
>                 stream->resetBuffers();
> @@ -1430,6 +1431,7 @@ int PipelineHandlerRPi::configurePipelineHandler(RPiCameraData *data)
>                 .minUnicamBuffers = 2,
>                 .minTotalUnicamBuffers = 4,
>                 .numOutput0Buffers = 1,
> +               .disableStartupFrameDrops = false,
>         };
>
>         char const *configFromEnv = utils::secure_getenv("LIBCAMERA_RPI_CONFIG_FILE");
> @@ -1464,6 +1466,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 || config.minTotalUnicamBuffers < 1) {
>                 LOG(RPI, Error) << "Invalid Unicam buffer configuration used!";
> @@ -1543,6 +1547,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]) {
>                         /*
> --
> 2.25.1
>