[libcamera-devel] [RFC PATCH 3/3] libcamera: pipeline: rkisp1: Make fixed amount of internal buffer allocation more explicit

[Nícolas F. R. A. Prado]

Em 2021-04-10 05:31, Jacopo Mondi escreveu:

> Hi Nicolas,
>
> On Fri, Apr 09, 2021 at 06:38:15PM -0300, Nícolas F. R. A. Prado wrote:
> > Now that bufferCount can be increased and there is a lc-compliance test
> > in place to check if the pipeline can handle more requests than it has
> > internal buffers, we need to be able to test it.
> >
> > Make the internal buffer allocation always constant, so that by
> > increasing bufferCount we can simulate this scenario. Scaling the
> > internal buffers amount with bufferCount only hides the issue, that
> > should be handled by queueing the overflowing requests.
> >
>
> Not a 100% sure I got it: is this patch meant to trigger an error just
> for the purpose of testing ?

More or less, yes. I think actually that this is something that would make sense
to merge only after other changes, but I sent it as a discussion starter.

Before this patch, a buffer overrun can happen on the internal buffer only if
the user allocates the buffers without using the FrameBufferAllocator. After the
patch, that can also happen using the FrameBufferAllocator if the bufferCount
value configured is above RKISP1_MIN_BUFFER_COUNT. So the way this is, it
actually increases the scenarios of failure, although in a way that is tested by
the new lc-compliance test.

>From my discussion with Laurent on IRC, my understanding is that the issue
should be fixed by queuing the requests until there are free internal buffers
rather than allocating more of them. Additionally, it seems the idea in the
future is to remove bufferCount. With that in mind it makes sense to me to have
a fixed amount of internal buffers and only rely on queuing the overflowing
requests internally. But that also probably means that this change should be
made after the internal request queueing is implemented.

I think a change like this would make a lot more sense right now on the IPU3
pipeline since that already has a patch for solving the issue in the works [1],
but I made it for the rkisp1 since it's what I have and can test on.

Thanks,
Nícolas

[1] https://lists.libcamera.org/pipermail/libcamera-devel/2021-April/019108.html