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

[Niklas Söderlund]

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?

-- 
Regards,
Niklas Söderlund