[libcamera-devel] [PATCH v1 05/10] pipeline: raspberrypi: Disable StreamOn for ISP Output0/1 when dropping 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:
>
> If the pipeline handler is required to drop startup frames by the IPA, do not
> call StreamOn on the ISP Output0 and Output1 device nodes from
> PipelineHandlerRPi::start. This stops the ISP from generating output on those
> channels giving improving latency, and more crucially does not require internal
> buffers to be allocated to deal with this condition.
>
> Signed-off-by: Naushir Patuck <naush at raspberrypi.com>
> ---
>  .../pipeline/raspberrypi/data/default.json    |  2 +-
>  .../pipeline/raspberrypi/raspberrypi.cpp      | 35 ++++++++++++++++---
>  2 files changed, 32 insertions(+), 5 deletions(-)
>
> diff --git a/src/libcamera/pipeline/raspberrypi/data/default.json b/src/libcamera/pipeline/raspberrypi/data/default.json
> index d709e31850af..a7ea735c87f4 100644
> --- a/src/libcamera/pipeline/raspberrypi/data/default.json
> +++ b/src/libcamera/pipeline/raspberrypi/data/default.json
> @@ -14,7 +14,7 @@
>                  #                             min_total_unicam_buffers - external buffer count)
>                  "min_total_unicam_buffers": 4,
>
> -                # The number of internal buffers used for ISP Output0. This must be set to 1.
> +                # The number of internal buffers used for ISP Output0.
>                  "num_output0_buffers": 1
>          }
>  }
> diff --git a/src/libcamera/pipeline/raspberrypi/raspberrypi.cpp b/src/libcamera/pipeline/raspberrypi/raspberrypi.cpp
> index bac7a66ba900..fc674e4f5bdd 100644
> --- a/src/libcamera/pipeline/raspberrypi/raspberrypi.cpp
> +++ b/src/libcamera/pipeline/raspberrypi/raspberrypi.cpp
> @@ -1080,8 +1080,18 @@ int PipelineHandlerRPi::start(Camera *camera, const ControlList *controls)
>
>         data->state_ = RPiCameraData::State::Idle;
>
> -       /* Start all streams. */
> +       /*
> +        * Start all streams with the exception of ISP Output0 and Output1 if
> +        * we are dropping some start-up frames. This saves a tiny bit of latency
> +        * and avoids the need for allocating an Output0 buffer only to handle
> +        * startup drop frame conditions.
> +        */
>         for (auto const stream : data->streams_) {
> +               if (data->dropFrameCount_ &&
> +                   (stream == &data->isp_[Isp::Output0] ||
> +                    stream == &data->isp_[Isp::Output1]))
> +                       continue;
> +
>                 ret = stream->dev()->streamOn();
>                 if (ret) {
>                         stop(camera);
> @@ -1474,6 +1484,13 @@ int PipelineHandlerRPi::queueAllBuffers(Camera *camera)
>                         if (ret < 0)
>                                 return ret;
>                 } else {
> +                       /*
> +                        * We don't enable streaming for Output0 and Output1 for
> +                        * startup frame drops, so don't queue any buffers.
> +                        */
> +                       if (stream == &data->isp_[Isp::Output0] ||
> +                           stream == &data->isp_[Isp::Output1])
> +                               continue;
>                         /*
>                          * For external streams, we must queue up a set of internal
>                          * buffers to handle the number of drop frames requested by
> @@ -2143,16 +2160,26 @@ void RPiCameraData::checkRequestCompleted()
>         }
>
>         /*
> -        * Make sure we have three outputs completed in the case of a dropped
> -        * frame.
> +        * Only the ISP statistics output is generated when we are dropping
> +        * frames on startup.
>          */
>         if (state_ == State::IpaComplete &&
> -           ((ispOutputCount_ == 3 && dropFrameCount_) || requestCompleted)) {
> +           ((ispOutputCount_ == 1 && dropFrameCount_) || requestCompleted)) {
>                 state_ = State::Idle;
>                 if (dropFrameCount_) {
>                         dropFrameCount_--;
>                         LOG(RPI, Debug) << "Dropping frame at the request of the IPA ("
>                                         << dropFrameCount_ << " left)";
> +
> +                       /*
> +                        * If we ahve finished dropping startup frames, start

s/ahve/have     (sorry!)

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

Thanks!
David

> +                        * streaming on the ISP Output0 and Output1 nodes for
> +                        * normal operation.
> +                        */
> +                       if (!dropFrameCount_) {
> +                               isp_[Isp::Output0].dev()->streamOn();
> +                               isp_[Isp::Output1].dev()->streamOn();
> +                       }
>                 }
>         }
>  }
> --
> 2.25.1
>