[libcamera-devel] [PATCH 3/4] pipeline: ipu3: Set bytesused before queuing parameters buffer

[Jacopo Mondi]

On Mon, Oct 03, 2022 at 03:24:57PM +0300, Laurent Pinchart wrote:
> Hi Jacopo,
>
> On Mon, Oct 03, 2022 at 10:35:53AM +0200, Jacopo Mondi wrote:
> > On Sun, Oct 02, 2022 at 03:36:11AM +0300, Laurent Pinchart via libcamera-devel wrote:
> > > The bytesused value for the parameters buffer is initialized to 0 and
> > > never set. The V4L2 API specification indicates that, for an output
> > > video device, the driver will set the bytesused value to the size of the
> > > plane in that case. The videobuf2 framework does so, but considers this
> > > as deprecated and prints a warning:
> > >
> > > [   54.375534] use of bytesused == 0 is deprecated and will be removed in the future,
> > > [   54.388026] use the actual size instead.
> > >
> > > Fix it by setting bytesused to the correct value before queuing the
> > > parameters buffer.
> > >
> > > Signed-off-by: Laurent Pinchart <laurent.pinchart at ideasonboard.com>
> > > ---
> > >  src/libcamera/pipeline/ipu3/ipu3.cpp | 4 ++++
> > >  1 file changed, 4 insertions(+)
> > >
> > > diff --git a/src/libcamera/pipeline/ipu3/ipu3.cpp b/src/libcamera/pipeline/ipu3/ipu3.cpp
> > > index 93219a6c1134..3b892d9671c5 100644
> > > --- a/src/libcamera/pipeline/ipu3/ipu3.cpp
> > > +++ b/src/libcamera/pipeline/ipu3/ipu3.cpp
> > > @@ -11,6 +11,8 @@
> > >  #include <queue>
> > >  #include <vector>
> > >
> > > +#include <linux/intel-ipu3.h>
> > > +
> > >  #include <libcamera/base/log.h>
> > >  #include <libcamera/base/utils.h>
> > >
> > > @@ -1298,6 +1300,8 @@ void IPU3CameraData::paramsBufferReady(unsigned int id)
> > >  			imgu_->viewfinder_->queueBuffer(outbuffer);
> > >  	}
> > >
> > > +	info->paramBuffer->_d()->metadata().planes()[0].bytesused =
> > > +		sizeof(struct ipu3_uapi_params);
> >
> > Do you think we need and helper ?
> >
> >         FrameBuffer::Private::setBytesUsed(unsigned int plane,
> >                                            unsigned int bytesused) ?
>
> I don't really see what this would bring.

Less indirections for the callers ? If it's a repeated patter it might
be nice to have.

Not a strict requirement for this series if you think it's not worth
it.

For this and the RkISP equivalent
Reviewed-by: Jacopo Mondi <jacopo at jmondi.org>

Thanks
  j


>
> > >  	imgu_->param_->queueBuffer(info->paramBuffer);
> > >  	imgu_->stat_->queueBuffer(info->statBuffer);
> > >  	imgu_->input_->queueBuffer(info->rawBuffer);
>
> --
> Regards,
>
> Laurent Pinchart