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

[Laurent Pinchart]

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 ?

> > > 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