[libcamera-devel] [PATCH] libcamera: v4l2_videodevice: Explain multiplanar bytesused reasoning

[Kieran Bingham]

Quoting Laurent Pinchart (2021-10-14 21:12:18)
> Hi Kieran,
> 
> Thank you for the patch.
> 
> On Thu, Oct 14, 2021 at 09:54:05AM +0100, Kieran Bingham wrote:
> > The ternary operation used to get the total bytesused of a V4L2 single
> > planar format which is stored in a multiplanar buffer can easily be
> > mis-read to think it's a bug, and appears to be reading the value of the
> > first of N planes as the total.
> > 
> > Directly explain the reasoning for why it looks like the condition is
> > inverted, as it is correct that the total bytes used is stored in only
> > the first plane of the multiplanar buffer.
> > 
> > Signed-off-by: Kieran Bingham <kieran.bingham at ideasonboard.com>
> > ---
> >  src/libcamera/v4l2_videodevice.cpp | 1 +
> >  1 file changed, 1 insertion(+)
> > 
> > diff --git a/src/libcamera/v4l2_videodevice.cpp b/src/libcamera/v4l2_videodevice.cpp
> > index ba5f88cd41ed..f4752f8bb23f 100644
> > --- a/src/libcamera/v4l2_videodevice.cpp
> > +++ b/src/libcamera/v4l2_videodevice.cpp
> > @@ -1711,6 +1711,7 @@ FrameBuffer *V4L2VideoDevice::dequeueBuffer()
> >                       return buffer;
> >               }
> >  
> > +             /* Single planar format stored in a multiplanar buffer */
> 
> Now I find this a bit confusing :-) How about the following ?
> 
>                 /*
>                  * With a V4L2 single-planar format, all the data is stored in a
>                  * single memory plane. The number of bytes used is conveyed
>                  * through that plane when using the V4L2 multi-planar API, or
>                  * set directly in the buffer when using the V4L2 single-planar
>                  * API.
>                  */
> 
> It's a bit verbose perhaps ?

Perhaps, but I don't mind verbose there.
The line on it's own looks like there is a bug because of the inversion.
So it warrants an explanation. And if you've written one - we can add
it.

--
Kieran

> 
> >               unsigned int bytesused = multiPlanar ? planes[0].bytesused
> >                                      : buf.bytesused;
> >  
> 
> -- 
> Regards,
> 
> Laurent Pinchart