[libcamera-devel] [PATCH v2 06/10] pipeline: raspberrypi: Reorder startup drop frame initialisation

[Naushir Patuck]

Hi Laurent,

On Fri, 2 Dec 2022 at 13:53, Laurent Pinchart <
laurent.pinchart at ideasonboard.com> wrote:

> Hi Naush,
>
> Thank you for the patch.
>
> On Tue, Nov 29, 2022 at 01:45:30PM +0000, Naushir Patuck via
> libcamera-devel wrote:
> > Reorder the code such that the IPA requested startup drop frames count is
> > available before the pipeline handler allocates any stream buffers.
> >
> > This will be used in a subsequent change to stop Unicam buffer
> allocations if
> > there are no startup drop frames required.
>
> Do you mean "if there are no startup drop frames required and the
> application has configured a raw stream and always provides buffers for
> it" ?
>

Yes!


>
> Why is that related ? Can't you avoid allocating internal raw buffers
> even without startup frames being dropped ? Can't you use the raw buffer
> from the first request to capture all the dropped frames and the first
> useful frame ?
>

There are a couple of reasons for this:

1) I'm working under the assumption that I cannot enqueue the same buffer
multiple times in v4l2  Because of this, I can only ever queue 1 buffer,
have it
dequeued, processed by the ISP, and requeue it back to Unicam.  This
effectively halves my overall framerate because of the added latency of
running
the ISP.

2) Even if V4L2 does allow enquing the same framebuffer multiple times,
there is
a possible (probable?) HW race condition where the ISP could be processing
the
buffer for frame N, while Unicam is writing out to the buffer for frame N +
1.

 It would be interesting to know if (1) is actually possible.  I don't
recall exactly, but
I think I did try it, and might have got complaints from the v4l2 framework.

Regards,
Naush


> > Signed-off-by: Naushir Patuck <naush at raspberrypi.com>
> > Reviewed-by: David Plowman <david.plowman at raspberrypi.com>
> > ---
> >  .../pipeline/raspberrypi/raspberrypi.cpp      | 30 +++++++++----------
> >  1 file changed, 15 insertions(+), 15 deletions(-)
> >
> > diff --git a/src/libcamera/pipeline/raspberrypi/raspberrypi.cpp
> b/src/libcamera/pipeline/raspberrypi/raspberrypi.cpp
> > index 86eb43a3a3c5..742521927780 100644
> > --- a/src/libcamera/pipeline/raspberrypi/raspberrypi.cpp
> > +++ b/src/libcamera/pipeline/raspberrypi/raspberrypi.cpp
> > @@ -1044,21 +1044,6 @@ int PipelineHandlerRPi::start(Camera *camera,
> const ControlList *controls)
> >       RPiCameraData *data = cameraData(camera);
> >       int ret;
> >
> > -     for (auto const stream : data->streams_)
> > -             stream->resetBuffers();
> > -
> > -     if (!data->buffersAllocated_) {
> > -             /* Allocate buffers for internal pipeline usage. */
> > -             ret = prepareBuffers(camera);
> > -             if (ret) {
> > -                     LOG(RPI, Error) << "Failed to allocate buffers";
> > -                     data->freeBuffers();
> > -                     stop(camera);
> > -                     return ret;
> > -             }
> > -             data->buffersAllocated_ = true;
> > -     }
> > -
> >       /* Check if a ScalerCrop control was specified. */
> >       if (controls)
> >               data->applyScalerCrop(*controls);
> > @@ -1075,6 +1060,21 @@ int PipelineHandlerRPi::start(Camera *camera,
> const ControlList *controls)
> >       /* Configure the number of dropped frames required on startup. */
> >       data->dropFrameCount_ = startConfig.dropFrameCount;
> >
> > +     for (auto const stream : data->streams_)
> > +             stream->resetBuffers();
> > +
> > +     if (!data->buffersAllocated_) {
> > +             /* Allocate buffers for internal pipeline usage. */
> > +             ret = prepareBuffers(camera);
> > +             if (ret) {
> > +                     LOG(RPI, Error) << "Failed to allocate buffers";
> > +                     data->freeBuffers();
> > +                     stop(camera);
> > +                     return ret;
> > +             }
> > +             data->buffersAllocated_ = true;
> > +     }
> > +
> >       /* We need to set the dropFrameCount_ before queueing buffers. */
> >       ret = queueAllBuffers(camera);
> >       if (ret) {
>
> --
> Regards,
>
> Laurent Pinchart
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <https://lists.libcamera.org/pipermail/libcamera-devel/attachments/20221202/71417666/attachment.htm>