[libcamera-devel,v2,05/10] libcamera: v4l2_videodevice: Expose the device capabilities

3122 diff mbox series

Add a caps() function that exposes the V4L2 capabilities for the device.
This is useful for generic code that can't hardcode any a priori
knowledge of the device, such as in a simple pipeline handler.

Signed-off-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
Reviewed-by: Niklas Söderlund <niklas.soderlund@ragnatech.se>
---
 src/libcamera/include/v4l2_videodevice.h | 2 ++
 src/libcamera/v4l2_videodevice.cpp       | 6 ++++++
 2 files changed, 8 insertions(+)

HI Laurent,

On Mon, Mar 16, 2020 at 11:43:05PM +0200, Laurent Pinchart wrote:
> Add a caps() function that exposes the V4L2 capabilities for the device.
> This is useful for generic code that can't hardcode any a priori
> knowledge of the device, such as in a simple pipeline handler.
>

I'm not thrilled by this as it again exposes another piece of V4L2
information to pipeline handlers. THey're anyway dealing with a V4L2
device, so I think it's fine. Also I cannot propose anything better
than "let's define libcamera video device caps", but that's probably
not the right time.

Reviewed-by: Jacopo Mondi <jacopo@jmondi.org>

Thanks
  j
> Signed-off-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
> Reviewed-by: Niklas Söderlund <niklas.soderlund@ragnatech.se>
> ---
>  src/libcamera/include/v4l2_videodevice.h | 2 ++
>  src/libcamera/v4l2_videodevice.cpp       | 6 ++++++
>  2 files changed, 8 insertions(+)
>
> diff --git a/src/libcamera/include/v4l2_videodevice.h b/src/libcamera/include/v4l2_videodevice.h
> index 7196cabd09c7..16fc6b60de02 100644
> --- a/src/libcamera/include/v4l2_videodevice.h
> +++ b/src/libcamera/include/v4l2_videodevice.h
> @@ -182,6 +182,8 @@ public:
>  	const char *deviceName() const { return caps_.card(); }
>  	const char *busName() const { return caps_.bus_info(); }
>
> +	const V4L2Capability &caps() const { return caps_; }
> +
>  	int getFormat(V4L2DeviceFormat *format);
>  	int setFormat(V4L2DeviceFormat *format);
>  	ImageFormats formats(uint32_t code = 0);
> diff --git a/src/libcamera/v4l2_videodevice.cpp b/src/libcamera/v4l2_videodevice.cpp
> index 084394fa5380..f5a925d97b63 100644
> --- a/src/libcamera/v4l2_videodevice.cpp
> +++ b/src/libcamera/v4l2_videodevice.cpp
> @@ -682,6 +682,12 @@ void V4L2VideoDevice::close()
>   * \return The string containing the device location
>   */
>
> +/**
> + * \fn V4L2VideoDevice::caps()
> + * \brief Retrieve the device V4L2 capabilities
> + * \return The device V4L2 capabilities
> + */
> +
>  std::string V4L2VideoDevice::logPrefix() const
>  {
>  	return deviceNode() + (V4L2_TYPE_IS_OUTPUT(bufferType_) ? "[out]" : "[cap]");
> --
> Regards,
>
> Laurent Pinchart
>
> _______________________________________________
> libcamera-devel mailing list
> libcamera-devel@lists.libcamera.org
> https://lists.libcamera.org/listinfo/libcamera-devel

Hi Jacopo,

On Tue, Mar 17, 2020 at 01:29:01PM +0100, Jacopo Mondi wrote:
> On Mon, Mar 16, 2020 at 11:43:05PM +0200, Laurent Pinchart wrote:
> > Add a caps() function that exposes the V4L2 capabilities for the device.
> > This is useful for generic code that can't hardcode any a priori
> > knowledge of the device, such as in a simple pipeline handler.
> 
> I'm not thrilled by this as it again exposes another piece of V4L2
> information to pipeline handlers. THey're anyway dealing with a V4L2
> device, so I think it's fine. Also I cannot propose anything better
> than "let's define libcamera video device caps", but that's probably
> not the right time.

I don't think we could hide V4L2 completely from pipeline handlers
anyway, so it's fine in my opinion to expose V4L2 API elements. The
purpose of the V4L2 helper classes is to simplify usage of the V4L2 API,
we don't need to wrap everything when not needed.

> Reviewed-by: Jacopo Mondi <jacopo@jmondi.org>
> 
> > Signed-off-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
> > Reviewed-by: Niklas Söderlund <niklas.soderlund@ragnatech.se>
> > ---
> >  src/libcamera/include/v4l2_videodevice.h | 2 ++
> >  src/libcamera/v4l2_videodevice.cpp       | 6 ++++++
> >  2 files changed, 8 insertions(+)
> >
> > diff --git a/src/libcamera/include/v4l2_videodevice.h b/src/libcamera/include/v4l2_videodevice.h
> > index 7196cabd09c7..16fc6b60de02 100644
> > --- a/src/libcamera/include/v4l2_videodevice.h
> > +++ b/src/libcamera/include/v4l2_videodevice.h
> > @@ -182,6 +182,8 @@ public:
> >  	const char *deviceName() const { return caps_.card(); }
> >  	const char *busName() const { return caps_.bus_info(); }
> >
> > +	const V4L2Capability &caps() const { return caps_; }
> > +
> >  	int getFormat(V4L2DeviceFormat *format);
> >  	int setFormat(V4L2DeviceFormat *format);
> >  	ImageFormats formats(uint32_t code = 0);
> > diff --git a/src/libcamera/v4l2_videodevice.cpp b/src/libcamera/v4l2_videodevice.cpp
> > index 084394fa5380..f5a925d97b63 100644
> > --- a/src/libcamera/v4l2_videodevice.cpp
> > +++ b/src/libcamera/v4l2_videodevice.cpp
> > @@ -682,6 +682,12 @@ void V4L2VideoDevice::close()
> >   * \return The string containing the device location
> >   */
> >
> > +/**
> > + * \fn V4L2VideoDevice::caps()
> > + * \brief Retrieve the device V4L2 capabilities
> > + * \return The device V4L2 capabilities
> > + */
> > +
> >  std::string V4L2VideoDevice::logPrefix() const
> >  {
> >  	return deviceNode() + (V4L2_TYPE_IS_OUTPUT(bufferType_) ? "[out]" : "[cap]");