[v2,4/6] libcamera: v4l2_videodevice: Update to the new kernel metadata API
diff mbox series

Message ID 20241108105117.137121-5-jacopo.mondi@ideasonboard.com
State Accepted
Headers show
Series
  • libcamera: Initial support for generic line based metadata
Related show

Commit Message

Jacopo Mondi Nov. 8, 2024, 10:51 a.m. UTC 
From: Laurent Pinchart <laurent.pinchart@ideasonboard.com>

With support for metadata in the streams API, the v4l2_meta_format
structure has been extended with width, height and bytesperline fields.

Support them in the V4L2VideoDevice getFormat() and setFormat()
functions is the video device is meta capture device and if the
pixel format is one of the generic line-based metadata formats.

Signed-off-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
Reviewed-by: Jacopo Mondi <jacopo.mondi@ideasonboard.com>
Signed-off-by: Jacopo Mondi <jacopo.mondi@ideasonboard.com>
---
 include/libcamera/internal/v4l2_pixelformat.h |  2 +
 src/libcamera/v4l2_pixelformat.cpp            | 34 +++++++++++++
 src/libcamera/v4l2_videodevice.cpp            | 51 ++++++++++++++-----
 3 files changed, 73 insertions(+), 14 deletions(-)

Comments

Kieran Bingham Jan. 7, 2025, 1:57 p.m. UTC | #1 
Quoting Jacopo Mondi (2024-11-08 10:51:14)
> From: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
> 
> With support for metadata in the streams API, the v4l2_meta_format
> structure has been extended with width, height and bytesperline fields.
> 
> Support them in the V4L2VideoDevice getFormat() and setFormat()
> functions is the video device is meta capture device and if the
> pixel format is one of the generic line-based metadata formats.
> 
> Signed-off-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
> Reviewed-by: Jacopo Mondi <jacopo.mondi@ideasonboard.com>
> Signed-off-by: Jacopo Mondi <jacopo.mondi@ideasonboard.com>
> ---
>  include/libcamera/internal/v4l2_pixelformat.h |  2 +
>  src/libcamera/v4l2_pixelformat.cpp            | 34 +++++++++++++
>  src/libcamera/v4l2_videodevice.cpp            | 51 ++++++++++++++-----
>  3 files changed, 73 insertions(+), 14 deletions(-)
> 
> diff --git a/include/libcamera/internal/v4l2_pixelformat.h b/include/libcamera/internal/v4l2_pixelformat.h
> index c836346bdc04..543eb21b3c07 100644
> --- a/include/libcamera/internal/v4l2_pixelformat.h
> +++ b/include/libcamera/internal/v4l2_pixelformat.h
> @@ -49,6 +49,8 @@ public:
>         static const std::vector<V4L2PixelFormat> &
>         fromPixelFormat(const PixelFormat &pixelFormat);
>  
> +       bool isGenericLineBasedMetadata() const;
> +
>  private:
>         uint32_t fourcc_;
>  };
> diff --git a/src/libcamera/v4l2_pixelformat.cpp b/src/libcamera/v4l2_pixelformat.cpp
> index eb9ac2224fd1..e8b3eb9c1394 100644
> --- a/src/libcamera/v4l2_pixelformat.cpp
> +++ b/src/libcamera/v4l2_pixelformat.cpp
> @@ -372,6 +372,40 @@ V4L2PixelFormat::fromPixelFormat(const PixelFormat &pixelFormat)
>         return info.v4l2Formats;
>  }
>  
> +/**
> + * \brief Test if a V4L2PixelFormat is one of the line based generic metadata
> + * formats
> + *
> + * A limited number of metadata formats, the ones that represents generic
> + * line-based metadata buffers, need to have their width, height and
> + * bytesperline set by userspace.
> + *
> + * This function tests if the current V4L2PixelFormat is one of those.
> + *
> + * Note: It would have been nicer to store this information in a
> + * V4L2PixelFormat::Info instance, but as metadata format are not exposed to
> + * applications, there are no PixelFormat and DRM fourcc codes associated to
> + * them.
> + *
> + * \return True if the V4L2PixelFormat() is a generic line based format, false
> + * otherwise
> + */
> +bool V4L2PixelFormat::isGenericLineBasedMetadata() const
> +{
> +       switch (fourcc_) {
> +       case V4L2_META_FMT_GENERIC_8:
> +       case V4L2_META_FMT_GENERIC_CSI2_10:
> +       case V4L2_META_FMT_GENERIC_CSI2_12:
> +       case V4L2_META_FMT_GENERIC_CSI2_14:
> +       case V4L2_META_FMT_GENERIC_CSI2_16:
> +       case V4L2_META_FMT_GENERIC_CSI2_20:
> +       case V4L2_META_FMT_GENERIC_CSI2_24:
> +               return true;
> +       default:
> +               return false;
> +       }

What happens with the non-generic formats ? Or does the 'specific'
nature of those embedded formats mean the sizes are 'known'
specifically?

> +}
> +
>  /**
>   * \brief Insert a text representation of a V4L2PixelFormat into an output
>   * stream
> diff --git a/src/libcamera/v4l2_videodevice.cpp b/src/libcamera/v4l2_videodevice.cpp
> index 14eba0561d6a..87cdbc3edf52 100644
> --- a/src/libcamera/v4l2_videodevice.cpp
> +++ b/src/libcamera/v4l2_videodevice.cpp
> @@ -888,7 +888,7 @@ int V4L2VideoDevice::setFormat(V4L2DeviceFormat *format)
>  int V4L2VideoDevice::getFormatMeta(V4L2DeviceFormat *format)
>  {
>         struct v4l2_format v4l2Format = {};
> -       struct v4l2_meta_format *pix = &v4l2Format.fmt.meta;
> +       struct v4l2_meta_format *meta = &v4l2Format.fmt.meta;
>         int ret;
>  
>         v4l2Format.type = bufferType_;
> @@ -898,25 +898,42 @@ int V4L2VideoDevice::getFormatMeta(V4L2DeviceFormat *format)
>                 return ret;
>         }
>  
> -       format->size.width = 0;
> -       format->size.height = 0;
> -       format->fourcc = V4L2PixelFormat(pix->dataformat);
> +       format->fourcc = V4L2PixelFormat(meta->dataformat);
> +       format->planes[0].size = meta->buffersize;
>         format->planesCount = 1;
> -       format->planes[0].bpl = pix->buffersize;
> -       format->planes[0].size = pix->buffersize;
> +
> +       bool genericLineBased = caps_.isMetaCapture() &&
> +                               format->fourcc.isGenericLineBasedMetadata();
> +
> +       if (genericLineBased) {
> +               format->size.width = meta->width;
> +               format->size.height = meta->height;
> +               format->planes[0].bpl = meta->bytesperline;
> +       } else {
> +               format->size.width = 0;
> +               format->size.height = 0;
> +               format->planes[0].bpl = meta->buffersize;

I guess perhaps it's here that it's just a full buffer size.

Anyway, I can see that could all be extended later when we /have/
specific formats to deal with.


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

> +       }
>  
>         return 0;
>  }
>  
>  int V4L2VideoDevice::trySetFormatMeta(V4L2DeviceFormat *format, bool set)
>  {
> +       bool genericLineBased = caps_.isMetaCapture() &&
> +                               format->fourcc.isGenericLineBasedMetadata();
>         struct v4l2_format v4l2Format = {};
> -       struct v4l2_meta_format *pix = &v4l2Format.fmt.meta;
> +       struct v4l2_meta_format *meta = &v4l2Format.fmt.meta;
>         int ret;
>  
>         v4l2Format.type = bufferType_;
> -       pix->dataformat = format->fourcc;
> -       pix->buffersize = format->planes[0].size;
> +       meta->dataformat = format->fourcc;
> +       meta->buffersize = format->planes[0].size;
> +       if (genericLineBased) {
> +               meta->width = format->size.width;
> +               meta->height = format->size.height;
> +               meta->bytesperline = format->planes[0].bpl;
> +       }
>         ret = ioctl(set ? VIDIOC_S_FMT : VIDIOC_TRY_FMT, &v4l2Format);
>         if (ret) {
>                 LOG(V4L2, Error)
> @@ -929,12 +946,18 @@ int V4L2VideoDevice::trySetFormatMeta(V4L2DeviceFormat *format, bool set)
>          * Return to caller the format actually applied on the video device,
>          * which might differ from the requested one.
>          */
> -       format->size.width = 0;
> -       format->size.height = 0;
> -       format->fourcc = V4L2PixelFormat(pix->dataformat);
> +       format->fourcc = V4L2PixelFormat(meta->dataformat);
>         format->planesCount = 1;
> -       format->planes[0].bpl = pix->buffersize;
> -       format->planes[0].size = pix->buffersize;
> +       format->planes[0].size = meta->buffersize;
> +       if (genericLineBased) {
> +               format->size.width = meta->width;
> +               format->size.height = meta->height;
> +               format->planes[0].bpl = meta->bytesperline;
> +       } else {
> +               format->size.width = 0;
> +               format->size.height = 0;
> +               format->planes[0].bpl = meta->buffersize;
> +       }
>  
>         return 0;
>  }
> -- 
> 2.47.0
>
Jacopo Mondi Jan. 8, 2025, 8:16 a.m. UTC | #2 
Hi Kieran

On Tue, Jan 07, 2025 at 01:57:32PM +0000, Kieran Bingham wrote:
> Quoting Jacopo Mondi (2024-11-08 10:51:14)
> > From: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
> >
> > With support for metadata in the streams API, the v4l2_meta_format
> > structure has been extended with width, height and bytesperline fields.
> >
> > Support them in the V4L2VideoDevice getFormat() and setFormat()
> > functions is the video device is meta capture device and if the
> > pixel format is one of the generic line-based metadata formats.
> >
> > Signed-off-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
> > Reviewed-by: Jacopo Mondi <jacopo.mondi@ideasonboard.com>
> > Signed-off-by: Jacopo Mondi <jacopo.mondi@ideasonboard.com>
> > ---
> >  include/libcamera/internal/v4l2_pixelformat.h |  2 +
> >  src/libcamera/v4l2_pixelformat.cpp            | 34 +++++++++++++
> >  src/libcamera/v4l2_videodevice.cpp            | 51 ++++++++++++++-----
> >  3 files changed, 73 insertions(+), 14 deletions(-)
> >
> > diff --git a/include/libcamera/internal/v4l2_pixelformat.h b/include/libcamera/internal/v4l2_pixelformat.h
> > index c836346bdc04..543eb21b3c07 100644
> > --- a/include/libcamera/internal/v4l2_pixelformat.h
> > +++ b/include/libcamera/internal/v4l2_pixelformat.h
> > @@ -49,6 +49,8 @@ public:
> >         static const std::vector<V4L2PixelFormat> &
> >         fromPixelFormat(const PixelFormat &pixelFormat);
> >
> > +       bool isGenericLineBasedMetadata() const;
> > +
> >  private:
> >         uint32_t fourcc_;
> >  };
> > diff --git a/src/libcamera/v4l2_pixelformat.cpp b/src/libcamera/v4l2_pixelformat.cpp
> > index eb9ac2224fd1..e8b3eb9c1394 100644
> > --- a/src/libcamera/v4l2_pixelformat.cpp
> > +++ b/src/libcamera/v4l2_pixelformat.cpp
> > @@ -372,6 +372,40 @@ V4L2PixelFormat::fromPixelFormat(const PixelFormat &pixelFormat)
> >         return info.v4l2Formats;
> >  }
> >
> > +/**
> > + * \brief Test if a V4L2PixelFormat is one of the line based generic metadata
> > + * formats
> > + *
> > + * A limited number of metadata formats, the ones that represents generic
> > + * line-based metadata buffers, need to have their width, height and
> > + * bytesperline set by userspace.
> > + *
> > + * This function tests if the current V4L2PixelFormat is one of those.
> > + *
> > + * Note: It would have been nicer to store this information in a
> > + * V4L2PixelFormat::Info instance, but as metadata format are not exposed to
> > + * applications, there are no PixelFormat and DRM fourcc codes associated to
> > + * them.
> > + *
> > + * \return True if the V4L2PixelFormat() is a generic line based format, false
> > + * otherwise
> > + */
> > +bool V4L2PixelFormat::isGenericLineBasedMetadata() const
> > +{
> > +       switch (fourcc_) {
> > +       case V4L2_META_FMT_GENERIC_8:
> > +       case V4L2_META_FMT_GENERIC_CSI2_10:
> > +       case V4L2_META_FMT_GENERIC_CSI2_12:
> > +       case V4L2_META_FMT_GENERIC_CSI2_14:
> > +       case V4L2_META_FMT_GENERIC_CSI2_16:
> > +       case V4L2_META_FMT_GENERIC_CSI2_20:
> > +       case V4L2_META_FMT_GENERIC_CSI2_24:
> > +               return true;
> > +       default:
> > +               return false;
> > +       }
>
> What happens with the non-generic formats ? Or does the 'specific'

When talking about sensor embedded data, device-specific meta formats
are only visibile on the internal sink pad. Along the rest of the
pipeline only generic formats are used, including on the Meta Capture
video device through which embedded data are captured.


> nature of those embedded formats mean the sizes are 'known'
> specifically?
>

The other "specific" formats are not line based embedded data from a
sensor, but in example, stats from an ISP for which width and height
are 0 by convention and the only relevant information is the buffer
size.

> > +}
> > +
> >  /**
> >   * \brief Insert a text representation of a V4L2PixelFormat into an output
> >   * stream
> > diff --git a/src/libcamera/v4l2_videodevice.cpp b/src/libcamera/v4l2_videodevice.cpp
> > index 14eba0561d6a..87cdbc3edf52 100644
> > --- a/src/libcamera/v4l2_videodevice.cpp
> > +++ b/src/libcamera/v4l2_videodevice.cpp
> > @@ -888,7 +888,7 @@ int V4L2VideoDevice::setFormat(V4L2DeviceFormat *format)
> >  int V4L2VideoDevice::getFormatMeta(V4L2DeviceFormat *format)
> >  {
> >         struct v4l2_format v4l2Format = {};
> > -       struct v4l2_meta_format *pix = &v4l2Format.fmt.meta;
> > +       struct v4l2_meta_format *meta = &v4l2Format.fmt.meta;
> >         int ret;
> >
> >         v4l2Format.type = bufferType_;
> > @@ -898,25 +898,42 @@ int V4L2VideoDevice::getFormatMeta(V4L2DeviceFormat *format)
> >                 return ret;
> >         }
> >
> > -       format->size.width = 0;
> > -       format->size.height = 0;
> > -       format->fourcc = V4L2PixelFormat(pix->dataformat);
> > +       format->fourcc = V4L2PixelFormat(meta->dataformat);
> > +       format->planes[0].size = meta->buffersize;
> >         format->planesCount = 1;
> > -       format->planes[0].bpl = pix->buffersize;
> > -       format->planes[0].size = pix->buffersize;
> > +
> > +       bool genericLineBased = caps_.isMetaCapture() &&
> > +                               format->fourcc.isGenericLineBasedMetadata();
> > +
> > +       if (genericLineBased) {
> > +               format->size.width = meta->width;
> > +               format->size.height = meta->height;
> > +               format->planes[0].bpl = meta->bytesperline;
> > +       } else {
> > +               format->size.width = 0;
> > +               format->size.height = 0;
> > +               format->planes[0].bpl = meta->buffersize;
>
> I guess perhaps it's here that it's just a full buffer size.
>
> Anyway, I can see that could all be extended later when we /have/
> specific formats to deal with.

I don't think this will need extensions, but I might be missing
something.

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

Thanks
  j

>
> > +       }
> >
> >         return 0;
> >  }
> >
> >  int V4L2VideoDevice::trySetFormatMeta(V4L2DeviceFormat *format, bool set)
> >  {
> > +       bool genericLineBased = caps_.isMetaCapture() &&
> > +                               format->fourcc.isGenericLineBasedMetadata();
> >         struct v4l2_format v4l2Format = {};
> > -       struct v4l2_meta_format *pix = &v4l2Format.fmt.meta;
> > +       struct v4l2_meta_format *meta = &v4l2Format.fmt.meta;
> >         int ret;
> >
> >         v4l2Format.type = bufferType_;
> > -       pix->dataformat = format->fourcc;
> > -       pix->buffersize = format->planes[0].size;
> > +       meta->dataformat = format->fourcc;
> > +       meta->buffersize = format->planes[0].size;
> > +       if (genericLineBased) {
> > +               meta->width = format->size.width;
> > +               meta->height = format->size.height;
> > +               meta->bytesperline = format->planes[0].bpl;
> > +       }
> >         ret = ioctl(set ? VIDIOC_S_FMT : VIDIOC_TRY_FMT, &v4l2Format);
> >         if (ret) {
> >                 LOG(V4L2, Error)
> > @@ -929,12 +946,18 @@ int V4L2VideoDevice::trySetFormatMeta(V4L2DeviceFormat *format, bool set)
> >          * Return to caller the format actually applied on the video device,
> >          * which might differ from the requested one.
> >          */
> > -       format->size.width = 0;
> > -       format->size.height = 0;
> > -       format->fourcc = V4L2PixelFormat(pix->dataformat);
> > +       format->fourcc = V4L2PixelFormat(meta->dataformat);
> >         format->planesCount = 1;
> > -       format->planes[0].bpl = pix->buffersize;
> > -       format->planes[0].size = pix->buffersize;
> > +       format->planes[0].size = meta->buffersize;
> > +       if (genericLineBased) {
> > +               format->size.width = meta->width;
> > +               format->size.height = meta->height;
> > +               format->planes[0].bpl = meta->bytesperline;
> > +       } else {
> > +               format->size.width = 0;
> > +               format->size.height = 0;
> > +               format->planes[0].bpl = meta->buffersize;
> > +       }
> >
> >         return 0;
> >  }
> > --
> > 2.47.0
> >

Patch
diff mbox series 

diff --git a/include/libcamera/internal/v4l2_pixelformat.h b/include/libcamera/internal/v4l2_pixelformat.h
index c836346bdc04..543eb21b3c07 100644
--- a/include/libcamera/internal/v4l2_pixelformat.h
+++ b/include/libcamera/internal/v4l2_pixelformat.h
@@ -49,6 +49,8 @@  public:
 	static const std::vector<V4L2PixelFormat> &
 	fromPixelFormat(const PixelFormat &pixelFormat);
 
+	bool isGenericLineBasedMetadata() const;
+
 private:
 	uint32_t fourcc_;
 };
diff --git a/src/libcamera/v4l2_pixelformat.cpp b/src/libcamera/v4l2_pixelformat.cpp
index eb9ac2224fd1..e8b3eb9c1394 100644
--- a/src/libcamera/v4l2_pixelformat.cpp
+++ b/src/libcamera/v4l2_pixelformat.cpp
@@ -372,6 +372,40 @@  V4L2PixelFormat::fromPixelFormat(const PixelFormat &pixelFormat)
 	return info.v4l2Formats;
 }
 
+/**
+ * \brief Test if a V4L2PixelFormat is one of the line based generic metadata
+ * formats
+ *
+ * A limited number of metadata formats, the ones that represents generic
+ * line-based metadata buffers, need to have their width, height and
+ * bytesperline set by userspace.
+ *
+ * This function tests if the current V4L2PixelFormat is one of those.
+ *
+ * Note: It would have been nicer to store this information in a
+ * V4L2PixelFormat::Info instance, but as metadata format are not exposed to
+ * applications, there are no PixelFormat and DRM fourcc codes associated to
+ * them.
+ *
+ * \return True if the V4L2PixelFormat() is a generic line based format, false
+ * otherwise
+ */
+bool V4L2PixelFormat::isGenericLineBasedMetadata() const
+{
+	switch (fourcc_) {
+	case V4L2_META_FMT_GENERIC_8:
+	case V4L2_META_FMT_GENERIC_CSI2_10:
+	case V4L2_META_FMT_GENERIC_CSI2_12:
+	case V4L2_META_FMT_GENERIC_CSI2_14:
+	case V4L2_META_FMT_GENERIC_CSI2_16:
+	case V4L2_META_FMT_GENERIC_CSI2_20:
+	case V4L2_META_FMT_GENERIC_CSI2_24:
+		return true;
+	default:
+		return false;
+	}
+}
+
 /**
  * \brief Insert a text representation of a V4L2PixelFormat into an output
  * stream
diff --git a/src/libcamera/v4l2_videodevice.cpp b/src/libcamera/v4l2_videodevice.cpp
index 14eba0561d6a..87cdbc3edf52 100644
--- a/src/libcamera/v4l2_videodevice.cpp
+++ b/src/libcamera/v4l2_videodevice.cpp
@@ -888,7 +888,7 @@  int V4L2VideoDevice::setFormat(V4L2DeviceFormat *format)
 int V4L2VideoDevice::getFormatMeta(V4L2DeviceFormat *format)
 {
 	struct v4l2_format v4l2Format = {};
-	struct v4l2_meta_format *pix = &v4l2Format.fmt.meta;
+	struct v4l2_meta_format *meta = &v4l2Format.fmt.meta;
 	int ret;
 
 	v4l2Format.type = bufferType_;
@@ -898,25 +898,42 @@  int V4L2VideoDevice::getFormatMeta(V4L2DeviceFormat *format)
 		return ret;
 	}
 
-	format->size.width = 0;
-	format->size.height = 0;
-	format->fourcc = V4L2PixelFormat(pix->dataformat);
+	format->fourcc = V4L2PixelFormat(meta->dataformat);
+	format->planes[0].size = meta->buffersize;
 	format->planesCount = 1;
-	format->planes[0].bpl = pix->buffersize;
-	format->planes[0].size = pix->buffersize;
+
+	bool genericLineBased = caps_.isMetaCapture() &&
+				format->fourcc.isGenericLineBasedMetadata();
+
+	if (genericLineBased) {
+		format->size.width = meta->width;
+		format->size.height = meta->height;
+		format->planes[0].bpl = meta->bytesperline;
+	} else {
+		format->size.width = 0;
+		format->size.height = 0;
+		format->planes[0].bpl = meta->buffersize;
+	}
 
 	return 0;
 }
 
 int V4L2VideoDevice::trySetFormatMeta(V4L2DeviceFormat *format, bool set)
 {
+	bool genericLineBased = caps_.isMetaCapture() &&
+				format->fourcc.isGenericLineBasedMetadata();
 	struct v4l2_format v4l2Format = {};
-	struct v4l2_meta_format *pix = &v4l2Format.fmt.meta;
+	struct v4l2_meta_format *meta = &v4l2Format.fmt.meta;
 	int ret;
 
 	v4l2Format.type = bufferType_;
-	pix->dataformat = format->fourcc;
-	pix->buffersize = format->planes[0].size;
+	meta->dataformat = format->fourcc;
+	meta->buffersize = format->planes[0].size;
+	if (genericLineBased) {
+		meta->width = format->size.width;
+		meta->height = format->size.height;
+		meta->bytesperline = format->planes[0].bpl;
+	}
 	ret = ioctl(set ? VIDIOC_S_FMT : VIDIOC_TRY_FMT, &v4l2Format);
 	if (ret) {
 		LOG(V4L2, Error)
@@ -929,12 +946,18 @@  int V4L2VideoDevice::trySetFormatMeta(V4L2DeviceFormat *format, bool set)
 	 * Return to caller the format actually applied on the video device,
 	 * which might differ from the requested one.
 	 */
-	format->size.width = 0;
-	format->size.height = 0;
-	format->fourcc = V4L2PixelFormat(pix->dataformat);
+	format->fourcc = V4L2PixelFormat(meta->dataformat);
 	format->planesCount = 1;
-	format->planes[0].bpl = pix->buffersize;
-	format->planes[0].size = pix->buffersize;
+	format->planes[0].size = meta->buffersize;
+	if (genericLineBased) {
+		format->size.width = meta->width;
+		format->size.height = meta->height;
+		format->planes[0].bpl = meta->bytesperline;
+	} else {
+		format->size.width = 0;
+		format->size.height = 0;
+		format->planes[0].bpl = meta->buffersize;
+	}
 
 	return 0;
 }