[libcamera-devel] [PATCH v2 08/10] pipeline: raspberrypi: Allow pipeline handler to always use the newest frame

[Naushir Patuck]

Hi Laurent,


On Mon, 5 Dec 2022 at 13:40, Laurent Pinchart <
laurent.pinchart at ideasonboard.com> wrote:

> Hi Naush,
>
> On Mon, Dec 05, 2022 at 10:58:40AM +0000, Naushir Patuck wrote:
> > On Fri, 2 Dec 2022 at 15:45, Laurent Pinchart wrote:
> > > On Tue, Nov 29, 2022 at 01:45:32PM +0000, Naushir Patuck via
> libcamera-devel wrote:
> > > > Add a pipeline config parameter "return_newest_frames" to always use
> the
> > > > most recently captured Unicam frame when processing a request. This
> effectively
> > > > stops the pipeline handler from queuing Unicam buffers and
> processing requests
> > > > using the buffer at the front of the queue.
> > > >
> > > > Note that setting this parameter might incur unnecessary frame drops
> during
> > > > times of high transient CPU loads where the application might not be
> able to
> > > > provide requests quick enough.
> > >
> > > You're explaining the downside, but not why an application may want to
> > > do this. What are the expected use cases ?
> >
> > This is one of those user requests that I don't necessarily agree with,
> but easy
> > enough to work around :-)
> >
> > Some users have expressed problems with their custom application where
> they
> > try to emulate timelapse captures by triggering Requests to the period of
> > the timelapse value.  The complaint is that the Request gets completed
> with one
> > of the Unicam buffers completed well before the Request was issued,
> which I suppose
> > is reasonable.
> >
> > Of course, there are other work-arounds (recycle requests as soon as
> possible
> > and only use the output at the timelapse interval), but the grumbling
> still continues,
> > so this gives an easy way to "fix" this.
>
> Fixing this in the pipeline handler seems reasonable. I don't think we
> have a written rule about this anywhere, but would it make sense to
> require pipeline handlers to never return frames with an earlier
> timestamp than the request queue time ?
>

Adding the logic for this is easy enough, but I am not sure it should be
enforced.
It might cause unnecessary frame drops because unexpected jitters in the
application/userland might delay the Request call ever so slightly.  The
unicam
(output) buffer queue smoothes such jitter away.  Maybe this is another
hint or
config parameter ;-)

Regards,
Naush



>
> > > > Signed-off-by: Naushir Patuck <naush at raspberrypi.com>
> > > > Reviewed-by: David Plowman <david.plowman at raspberrypi.com>
> > > > ---
> > > >  .../pipeline/raspberrypi/data/default.json    |  7 +++++-
> > > >  .../pipeline/raspberrypi/raspberrypi.cpp      | 23
> +++++++++++++++++++
> > > >  2 files changed, 29 insertions(+), 1 deletion(-)
> > > >
> > > > diff --git a/src/libcamera/pipeline/raspberrypi/data/default.json
> b/src/libcamera/pipeline/raspberrypi/data/default.json
> > > > index 707414bcc5c5..eda053258c67 100644
> > > > --- a/src/libcamera/pipeline/raspberrypi/data/default.json
> > > > +++ b/src/libcamera/pipeline/raspberrypi/data/default.json
> > > > @@ -18,6 +18,11 @@
> > > >                  "num_output0_buffers": 1,
> > > >
> > > >                  # Override any request from the IPA to drop a
> number of startup frames.
> > > > -                "disable_startup_frame_drops": false
> > > > +                "disable_startup_frame_drops": false,
> > > > +
> > > > +                # Always process a pending request with the last
> captured sensor frame.
> > > > +                # Note that this might lead to avoidable frame
> drops during periods
> > > > +                # of transient heavy CPU loading.
> > > > +                "return_newest_frames": false
> > > >          }
> > > >  }
> > > > diff --git a/src/libcamera/pipeline/raspberrypi/raspberrypi.cpp
> b/src/libcamera/pipeline/raspberrypi/raspberrypi.cpp
> > > > index ef49d32037af..a369fbd2a528 100644
> > > > --- a/src/libcamera/pipeline/raspberrypi/raspberrypi.cpp
> > > > +++ b/src/libcamera/pipeline/raspberrypi/raspberrypi.cpp
> > > > @@ -317,6 +317,11 @@ public:
> > > >                * frames.
> > > >                */
> > > >               bool disableStartupFrameDrops;
> > > > +             /*
> > > > +              * Always process a pending request with the last
> captured sensor
> > > > +              * frame.
> > > > +              */
> > > > +             bool returnNewestFrames;
> > > >       };
> > > >
> > > >       Config config_;
> > > > @@ -1457,6 +1462,7 @@ int
> PipelineHandlerRPi::configurePipelineHandler(RPiCameraData *data)
> > > >               .minTotalUnicamBuffers = 4,
> > > >               .numOutput0Buffers = 1,
> > > >               .disableStartupFrameDrops = false,
> > > > +             .returnNewestFrames = false,
> > > >       };
> > > >
> > > >       char const *configFromEnv =
> utils::secure_getenv("LIBCAMERA_RPI_CONFIG_FILE");
> > > > @@ -1493,6 +1499,8 @@ int
> PipelineHandlerRPi::configurePipelineHandler(RPiCameraData *data)
> > > >               phConfig["num_output0_buffers"].get<unsigned
> int>(config.numOutput0Buffers);
> > > >       config.disableStartupFrameDrops =
> > > >
> phConfig["disable_startup_frame_drops"].get<bool>(config.disableStartupFrameDrops);
> > > > +     config.returnNewestFrames =
> > > > +
> phConfig["return_newest_frames"].get<bool>(config.returnNewestFrames);
> > > >
> > > >       if (config.minTotalUnicamBuffers < config.minUnicamBuffers) {
> > > >               LOG(RPI, Error) << "Invalid configuration:
> min_total_unicam_buffers must be >= min_unicam_buffers!";
> > > > @@ -2349,6 +2357,21 @@ bool
> RPiCameraData::findMatchingBuffers(BayerFrame &bayerFrame, FrameBuffer *&em
> > > >       if (bayerQueue_.empty())
> > > >               return false;
> > > >
> > > > +     /*
> > > > +      * If the pipeline is configured to only ever return the most
> recently
> > > > +      * captured frame, empty the buffer queue until a single
> element is
> > > > +      * left, corresponding to the most recent buffer. Note that
> this will
> > > > +      * likely result in possibly avoidable dropped frames.
> > > > +      */
> > > > +     if (config_.returnNewestFrames &&
> !unicam_[Unicam::Image].isExternal()) {
> > > > +             while (bayerQueue_.size() > 1) {
> > > > +                     FrameBuffer *bayer =
> bayerQueue_.front().buffer;
> > > > +
> > > > +                     unicam_[Unicam::Image].returnBuffer(bayer);
> > > > +                     bayerQueue_.pop();
> > > > +             }
> > > > +     }
> > > > +
> > > >       /*
> > > >        * Find the embedded data buffer with a matching timestamp to
> pass to
> > > >        * the IPA. Any embedded buffers with a timestamp lower than
> the
>
> --
> Regards,
>
> Laurent Pinchart
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <https://lists.libcamera.org/pipermail/libcamera-devel/attachments/20221205/808e2e9c/attachment.htm>