libcamera: v4l2_videodevice: Clarify V4L2M2MDevice

22105 diff mbox series

The documentation seems to suggest that to create a new M2M
execution context it is expected users to call V4L2M2MDevice::open()
multiple times on the same video device path.

It is instead expected that multiple instances of the class are
created, one for each required execution context.

Clarify it in the documentation of the V4L2M2MDevice class.

Signed-off-by: Jacopo Mondi <jacopo.mondi@ideasonboard.com>
---
 src/libcamera/v4l2_videodevice.cpp | 9 +++++++--
 1 file changed, 7 insertions(+), 2 deletions(-)

Quoting Jacopo Mondi (2024-11-27 08:12:11)
> The documentation seems to suggest that to create a new M2M
> execution context it is expected users to call V4L2M2MDevice::open()
> multiple times on the same video device path.
> 
> It is instead expected that multiple instances of the class are
> created, one for each required execution context.
> 
> Clarify it in the documentation of the V4L2M2MDevice class.
> 
> Signed-off-by: Jacopo Mondi <jacopo.mondi@ideasonboard.com>
> ---
>  src/libcamera/v4l2_videodevice.cpp | 9 +++++++--
>  1 file changed, 7 insertions(+), 2 deletions(-)
> 
> diff --git a/src/libcamera/v4l2_videodevice.cpp b/src/libcamera/v4l2_videodevice.cpp
> index 14eba0561d6a..156601d08d0c 100644
> --- a/src/libcamera/v4l2_videodevice.cpp
> +++ b/src/libcamera/v4l2_videodevice.cpp
> @@ -2128,11 +2128,16 @@ V4L2PixelFormat V4L2VideoDevice::toV4L2PixelFormat(const PixelFormat &pixelForma
>   * deviceNode which operate together using two queues to implement the V4L2
>   * Memory to Memory API.
>   *
> - * The two devices should be opened by calling open() on the V4L2M2MDevice, and
> - * can be closed by calling close on the V4L2M2MDevice.

I would open this paragraph with a statement that it's possible to use
multiple contexts. I don't think that itself is clear...

"""
Memory to Memory devices in the kernel using the V4L2 M2M API can
operate with multiple contexts for parallel operations on a single
device. Each instance of a V4L2M2MDevice represents a single context.
"""

Then continue with your text with one small typo:

> + * User of this class should create a new instance of the V4L2M2MDevice for each

s/User/Users/

> + * desired execution context and then open it by calling open() on the
> + * V4L2M2MDevice and close it by calling close() on the V4L2M2MDevice.
>   *
>   * Calling V4L2VideoDevice::open() and V4L2VideoDevice::close() on the capture
>   * or output V4L2VideoDevice is not permitted.
> + *
> + * Once the M2M device is open users can operate on the output and capture queue
> + * represented by the V4L2VideoDevice returned by the output() and capture()
> + * functions.
>   */


Reviewed-by: Kieran Bingham <kieran.bingham@ideasonboard.com>

>  
>  /**
> -- 
> 2.47.0
>

Hi Kieran

On Thu, Nov 28, 2024 at 10:53:58AM +0000, Kieran Bingham wrote:
> Quoting Jacopo Mondi (2024-11-27 08:12:11)
> > The documentation seems to suggest that to create a new M2M
> > execution context it is expected users to call V4L2M2MDevice::open()
> > multiple times on the same video device path.
> >
> > It is instead expected that multiple instances of the class are
> > created, one for each required execution context.
> >
> > Clarify it in the documentation of the V4L2M2MDevice class.
> >
> > Signed-off-by: Jacopo Mondi <jacopo.mondi@ideasonboard.com>
> > ---
> >  src/libcamera/v4l2_videodevice.cpp | 9 +++++++--
> >  1 file changed, 7 insertions(+), 2 deletions(-)
> >
> > diff --git a/src/libcamera/v4l2_videodevice.cpp b/src/libcamera/v4l2_videodevice.cpp
> > index 14eba0561d6a..156601d08d0c 100644
> > --- a/src/libcamera/v4l2_videodevice.cpp
> > +++ b/src/libcamera/v4l2_videodevice.cpp
> > @@ -2128,11 +2128,16 @@ V4L2PixelFormat V4L2VideoDevice::toV4L2PixelFormat(const PixelFormat &pixelForma
> >   * deviceNode which operate together using two queues to implement the V4L2
> >   * Memory to Memory API.
> >   *
> > - * The two devices should be opened by calling open() on the V4L2M2MDevice, and
> > - * can be closed by calling close on the V4L2M2MDevice.
>
> I would open this paragraph with a statement that it's possible to use
> multiple contexts. I don't think that itself is clear...
>
> """
> Memory to Memory devices in the kernel using the V4L2 M2M API can
> operate with multiple contexts for parallel operations on a single
> device. Each instance of a V4L2M2MDevice represents a single context.
> """

Indeed, that's helpful!

>
> Then continue with your text with one small typo:
>
> > + * User of this class should create a new instance of the V4L2M2MDevice for each
>
> s/User/Users/
>
> > + * desired execution context and then open it by calling open() on the
> > + * V4L2M2MDevice and close it by calling close() on the V4L2M2MDevice.
> >   *
> >   * Calling V4L2VideoDevice::open() and V4L2VideoDevice::close() on the capture
> >   * or output V4L2VideoDevice is not permitted.
> > + *
> > + * Once the M2M device is open users can operate on the output and capture queue

and s/queue/queues/

> > + * represented by the V4L2VideoDevice returned by the output() and capture()
> > + * functions.
> >   */
>
>
> Reviewed-by: Kieran Bingham <kieran.bingham@ideasonboard.com>

Thanks
  j

>
> >
> >  /**
> > --
> > 2.47.0
> >