[libcamera-devel] [PATCH v3 4/4] libcamera: stream: Remove bufferCount

[David Plowman]

Hi everyone

On Mon, 26 Apr 2021 at 11:17, Jean-Michel Hautbois
<jeanmichel.hautbois at ideasonboard.com> wrote:
>
> Hi,
>
> On 26/04/2021 11:28, Niklas Söderlund wrote:
> > Hello,
> >
> > On 2021-04-26 06:19:57 +0300, Laurent Pinchart wrote:
> >
> > <snip>
> >
> >>> @@ -668,15 +661,9 @@ int PipelineHandlerIPU3::allocateBuffers(Camera
> >>> *camera)
> >>>  {
> >>>     IPU3CameraData *data = cameraData(camera);
> >>>     ImgUDevice *imgu = data->imgu_;
> >>> -   unsigned int bufferCount;
> >>> +   unsigned int bufferCount = data->properties_.get(properties::QueueDepth);
> >>>     int ret;
> >>>
> >>> -   bufferCount = std::max({
> >>> -           data->outStream_.configuration().bufferCount,
> >>> -           data->vfStream_.configuration().bufferCount,
> >>> -           data->rawStream_.configuration().bufferCount,
> >>> -   });
> >>
> >> I'm not sure properties::QueueDepth is the right value here. This deals
> >> with both allocation of stats and params buffers, which are internal,
> >> and the number of V4L2 buffer slots for the ImgU input and output. For
> >> the latter, we probably should overallocate, as underallocation would
> >> mean trashing dmabuf mappings. For the former, we shouldn't allocate too
> >> much to avoid wasting memory, so we should take into account how long
> >> the IPA would need to hold on the params and stats buffers.
> >>
> >> Jacopo, Jean-Michel, Niklas, any comment ?
> >
> > I agree that QueueDepth is likely not the best fit here, but neither was
> > bufferCount :-) IIRC this was done very early on where bufferCount was
> > not really defined and added as a first steeping stone. I'm very happy
> > to see this being thought about.
> >
> > Maybe the best solution for this series is to add some const numerical
> > values to the IPU3 pipeline handler to replace the usage of bufferCount
> > but not making it depending on QueueDepth?
> >
> > Maybe even separating it in two values as Laurent outlines, one for the
> > output and viewfinder slots allocation and one for the raw stream? Maybe
> > it make sens to link the raw streams count with the value used for the
> > stats and param buffer counts?
>
> Indeed IPA may need some buffers, and even now it is not satisfying :-).
> Overallocating is a need, and we should not go lower than the highest
> number of frame delay in the delayedControls list. For IPU3, EXPOSURE
> has a delay of 2, so we should not have less than 3 buffers to be
> "comfortable".
> My 2 cents :-).

I've been following this discussion and there seem to be a lot of
queues and delays flying around! Maybe someone can help me to
understand - at the moment I believe there is:

* A notion of how many buffers need to be allocated for each
stream.The more you have, the more resilient processing tends to be to
latencies and delays.

* This number can be different for different streams. You could
imagine an application requiring fewer raw stream buffers, for
example.

* There's a minimum number of buffers that are required for the
pipeline to work. For the Raspberry Pi this is 1.

* I don't understand how delayed controls are related to this. They're
purely to cope with the fact that the sensor has delays before
settings that have been applied take effect, and that this delay may
vary between controls. They also vary with the sensor, they're not
fixed for a particular pipeline handler.

Best regards
David

>
> --
> _______________________________________________
> libcamera-devel mailing list
> libcamera-devel at lists.libcamera.org
> https://lists.libcamera.org/listinfo/libcamera-devel