[libcamera-devel,v1,05/10] pipeline: raspberrypi: Disable StreamOn for ISP Output0/1 when dropping frames

17601 diff mbox series

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@raspberrypi.com>
---
 .../pipeline/raspberrypi/data/default.json    |  2 +-
 .../pipeline/raspberrypi/raspberrypi.cpp      | 35 ++++++++++++++++---
 2 files changed, 32 insertions(+), 5 deletions(-)

Hi Naush

Thanks for the patch!

On Fri, 14 Oct 2022 at 14:19, Naushir Patuck via libcamera-devel
<libcamera-devel@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@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@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
>