[libcamera-devel,v4,05/11] libcamera: v4l2_videodevice: Support filtering formats by media bus code
diff mbox series

Message ID 20200404004438.17992-6-laurent.pinchart@ideasonboard.com
State Superseded
Headers show
Series
  • Simple pipeline handler
Related show

Commit Message

Laurent Pinchart April 4, 2020, 12:44 a.m. UTC 
Add support for the recent V4L2 extension to VIDIOC_ENUM_FMT that allows
filtering pixel formats by media bus codes.

Signed-off-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
---
 src/libcamera/include/v4l2_videodevice.h |  4 ++--
 src/libcamera/v4l2_videodevice.cpp       | 16 +++++++++++++---
 2 files changed, 15 insertions(+), 5 deletions(-)

Comments

Niklas Söderlund April 7, 2020, 11:38 p.m. UTC | #1 
Hi Laurent,

Thanks for your patch.

On 2020-04-04 03:44:32 +0300, Laurent Pinchart wrote:
> Add support for the recent V4L2 extension to VIDIOC_ENUM_FMT that allows
> filtering pixel formats by media bus codes.
> 
> Signed-off-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>

Is your intent to try and merge this ahead of V4L2_CAP_IO_MC upstream?  
If not I think we shall try and remember to update the commit message 
with which kernel version is needed make use of this feature.

Provided the patches on their way upstream stay the same,

Reviewed-by: Niklas Söderlund <niklas.soderlund@ragnatech.se>

> ---
>  src/libcamera/include/v4l2_videodevice.h |  4 ++--
>  src/libcamera/v4l2_videodevice.cpp       | 16 +++++++++++++---
>  2 files changed, 15 insertions(+), 5 deletions(-)
> 
> diff --git a/src/libcamera/include/v4l2_videodevice.h b/src/libcamera/include/v4l2_videodevice.h
> index 7d7c4a9e6ebd..7b011d3b4e87 100644
> --- a/src/libcamera/include/v4l2_videodevice.h
> +++ b/src/libcamera/include/v4l2_videodevice.h
> @@ -207,7 +207,7 @@ public:
>  
>  	int getFormat(V4L2DeviceFormat *format);
>  	int setFormat(V4L2DeviceFormat *format);
> -	std::map<V4L2PixelFormat, std::vector<SizeRange>> formats();
> +	std::map<V4L2PixelFormat, std::vector<SizeRange>> formats(uint32_t code = 0);
>  
>  	int setCrop(Rectangle *rect);
>  	int setCompose(Rectangle *rect);
> @@ -246,7 +246,7 @@ private:
>  	int getFormatSingleplane(V4L2DeviceFormat *format);
>  	int setFormatSingleplane(V4L2DeviceFormat *format);
>  
> -	std::vector<V4L2PixelFormat> enumPixelformats();
> +	std::vector<V4L2PixelFormat> enumPixelformats(uint32_t code);
>  	std::vector<SizeRange> enumSizes(V4L2PixelFormat pixelFormat);
>  
>  	int setSelection(unsigned int target, Rectangle *rect);
> diff --git a/src/libcamera/v4l2_videodevice.cpp b/src/libcamera/v4l2_videodevice.cpp
> index eb33a68e50d6..f7b98272d247 100644
> --- a/src/libcamera/v4l2_videodevice.cpp
> +++ b/src/libcamera/v4l2_videodevice.cpp
> @@ -973,16 +973,19 @@ int V4L2VideoDevice::setFormatSingleplane(V4L2DeviceFormat *format)
>  
>  /**
>   * \brief Enumerate all pixel formats and frame sizes
> + * \param[in] code Restrict formats to this media bus code.
>   *
>   * Enumerate all pixel formats and frame sizes supported by the video device.
> + * If the \a code argument is not zero, only formats compatible with that media
> + * bus code will be enumerated.
>   *
>   * \return A list of the supported video device formats
>   */
> -std::map<V4L2PixelFormat, std::vector<SizeRange>> V4L2VideoDevice::formats()
> +std::map<V4L2PixelFormat, std::vector<SizeRange>> V4L2VideoDevice::formats(uint32_t code)
>  {
>  	std::map<V4L2PixelFormat, std::vector<SizeRange>> formats;
>  
> -	for (V4L2PixelFormat pixelFormat : enumPixelformats()) {
> +	for (V4L2PixelFormat pixelFormat : enumPixelformats(code)) {
>  		std::vector<SizeRange> sizes = enumSizes(pixelFormat);
>  		if (sizes.empty())
>  			return {};
> @@ -1000,15 +1003,22 @@ std::map<V4L2PixelFormat, std::vector<SizeRange>> V4L2VideoDevice::formats()
>  	return formats;
>  }
>  
> -std::vector<V4L2PixelFormat> V4L2VideoDevice::enumPixelformats()
> +std::vector<V4L2PixelFormat> V4L2VideoDevice::enumPixelformats(uint32_t code)
>  {
>  	std::vector<V4L2PixelFormat> formats;
>  	int ret;
>  
> +	if (code && !(caps_.device_caps() & V4L2_CAP_IO_MC)) {
> +		LOG(V4L2, Error)
> +			<< "Media bus code filtering not supported by the device";
> +		return {};
> +	}
> +
>  	for (unsigned int index = 0; ; index++) {
>  		struct v4l2_fmtdesc pixelformatEnum = {};
>  		pixelformatEnum.index = index;
>  		pixelformatEnum.type = bufferType_;
> +		pixelformatEnum.mbus_code = code;
>  
>  		ret = ioctl(VIDIOC_ENUM_FMT, &pixelformatEnum);
>  		if (ret)
> -- 
> Regards,
> 
> Laurent Pinchart
> 
> _______________________________________________
> libcamera-devel mailing list
> libcamera-devel@lists.libcamera.org
> https://lists.libcamera.org/listinfo/libcamera-devel
Laurent Pinchart April 8, 2020, 12:29 a.m. UTC | #2 
Hi Niklas,

On Wed, Apr 08, 2020 at 01:38:34AM +0200, Niklas Söderlund wrote:
> On 2020-04-04 03:44:32 +0300, Laurent Pinchart wrote:
> > Add support for the recent V4L2 extension to VIDIOC_ENUM_FMT that allows
> > filtering pixel formats by media bus codes.
> > 
> > Signed-off-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
> 
> Is your intent to try and merge this ahead of V4L2_CAP_IO_MC upstream?  
> If not I think we shall try and remember to update the commit message 
> with which kernel version is needed make use of this feature.

I'd prefer getting the kernel patches merged first if possible. I don't
think they're very far from being ready. If more work is needed on the
kernel side I would possibly consider merging this series already, as
it's easier to develop it further with the code in master, but I won't
push too much for that (for now at least :-)).

> Provided the patches on their way upstream stay the same,
> 
> Reviewed-by: Niklas Söderlund <niklas.soderlund@ragnatech.se>
> 
> > ---
> >  src/libcamera/include/v4l2_videodevice.h |  4 ++--
> >  src/libcamera/v4l2_videodevice.cpp       | 16 +++++++++++++---
> >  2 files changed, 15 insertions(+), 5 deletions(-)
> > 
> > diff --git a/src/libcamera/include/v4l2_videodevice.h b/src/libcamera/include/v4l2_videodevice.h
> > index 7d7c4a9e6ebd..7b011d3b4e87 100644
> > --- a/src/libcamera/include/v4l2_videodevice.h
> > +++ b/src/libcamera/include/v4l2_videodevice.h
> > @@ -207,7 +207,7 @@ public:
> >  
> >  	int getFormat(V4L2DeviceFormat *format);
> >  	int setFormat(V4L2DeviceFormat *format);
> > -	std::map<V4L2PixelFormat, std::vector<SizeRange>> formats();
> > +	std::map<V4L2PixelFormat, std::vector<SizeRange>> formats(uint32_t code = 0);
> >  
> >  	int setCrop(Rectangle *rect);
> >  	int setCompose(Rectangle *rect);
> > @@ -246,7 +246,7 @@ private:
> >  	int getFormatSingleplane(V4L2DeviceFormat *format);
> >  	int setFormatSingleplane(V4L2DeviceFormat *format);
> >  
> > -	std::vector<V4L2PixelFormat> enumPixelformats();
> > +	std::vector<V4L2PixelFormat> enumPixelformats(uint32_t code);
> >  	std::vector<SizeRange> enumSizes(V4L2PixelFormat pixelFormat);
> >  
> >  	int setSelection(unsigned int target, Rectangle *rect);
> > diff --git a/src/libcamera/v4l2_videodevice.cpp b/src/libcamera/v4l2_videodevice.cpp
> > index eb33a68e50d6..f7b98272d247 100644
> > --- a/src/libcamera/v4l2_videodevice.cpp
> > +++ b/src/libcamera/v4l2_videodevice.cpp
> > @@ -973,16 +973,19 @@ int V4L2VideoDevice::setFormatSingleplane(V4L2DeviceFormat *format)
> >  
> >  /**
> >   * \brief Enumerate all pixel formats and frame sizes
> > + * \param[in] code Restrict formats to this media bus code.
> >   *
> >   * Enumerate all pixel formats and frame sizes supported by the video device.
> > + * If the \a code argument is not zero, only formats compatible with that media
> > + * bus code will be enumerated.
> >   *
> >   * \return A list of the supported video device formats
> >   */
> > -std::map<V4L2PixelFormat, std::vector<SizeRange>> V4L2VideoDevice::formats()
> > +std::map<V4L2PixelFormat, std::vector<SizeRange>> V4L2VideoDevice::formats(uint32_t code)
> >  {
> >  	std::map<V4L2PixelFormat, std::vector<SizeRange>> formats;
> >  
> > -	for (V4L2PixelFormat pixelFormat : enumPixelformats()) {
> > +	for (V4L2PixelFormat pixelFormat : enumPixelformats(code)) {
> >  		std::vector<SizeRange> sizes = enumSizes(pixelFormat);
> >  		if (sizes.empty())
> >  			return {};
> > @@ -1000,15 +1003,22 @@ std::map<V4L2PixelFormat, std::vector<SizeRange>> V4L2VideoDevice::formats()
> >  	return formats;
> >  }
> >  
> > -std::vector<V4L2PixelFormat> V4L2VideoDevice::enumPixelformats()
> > +std::vector<V4L2PixelFormat> V4L2VideoDevice::enumPixelformats(uint32_t code)
> >  {
> >  	std::vector<V4L2PixelFormat> formats;
> >  	int ret;
> >  
> > +	if (code && !(caps_.device_caps() & V4L2_CAP_IO_MC)) {
> > +		LOG(V4L2, Error)
> > +			<< "Media bus code filtering not supported by the device";
> > +		return {};
> > +	}
> > +
> >  	for (unsigned int index = 0; ; index++) {
> >  		struct v4l2_fmtdesc pixelformatEnum = {};
> >  		pixelformatEnum.index = index;
> >  		pixelformatEnum.type = bufferType_;
> > +		pixelformatEnum.mbus_code = code;
> >  
> >  		ret = ioctl(VIDIOC_ENUM_FMT, &pixelformatEnum);
> >  		if (ret)

Patch
diff mbox series 

diff --git a/src/libcamera/include/v4l2_videodevice.h b/src/libcamera/include/v4l2_videodevice.h
index 7d7c4a9e6ebd..7b011d3b4e87 100644
--- a/src/libcamera/include/v4l2_videodevice.h
+++ b/src/libcamera/include/v4l2_videodevice.h
@@ -207,7 +207,7 @@  public:
 
 	int getFormat(V4L2DeviceFormat *format);
 	int setFormat(V4L2DeviceFormat *format);
-	std::map<V4L2PixelFormat, std::vector<SizeRange>> formats();
+	std::map<V4L2PixelFormat, std::vector<SizeRange>> formats(uint32_t code = 0);
 
 	int setCrop(Rectangle *rect);
 	int setCompose(Rectangle *rect);
@@ -246,7 +246,7 @@  private:
 	int getFormatSingleplane(V4L2DeviceFormat *format);
 	int setFormatSingleplane(V4L2DeviceFormat *format);
 
-	std::vector<V4L2PixelFormat> enumPixelformats();
+	std::vector<V4L2PixelFormat> enumPixelformats(uint32_t code);
 	std::vector<SizeRange> enumSizes(V4L2PixelFormat pixelFormat);
 
 	int setSelection(unsigned int target, Rectangle *rect);
diff --git a/src/libcamera/v4l2_videodevice.cpp b/src/libcamera/v4l2_videodevice.cpp
index eb33a68e50d6..f7b98272d247 100644
--- a/src/libcamera/v4l2_videodevice.cpp
+++ b/src/libcamera/v4l2_videodevice.cpp
@@ -973,16 +973,19 @@  int V4L2VideoDevice::setFormatSingleplane(V4L2DeviceFormat *format)
 
 /**
  * \brief Enumerate all pixel formats and frame sizes
+ * \param[in] code Restrict formats to this media bus code.
  *
  * Enumerate all pixel formats and frame sizes supported by the video device.
+ * If the \a code argument is not zero, only formats compatible with that media
+ * bus code will be enumerated.
  *
  * \return A list of the supported video device formats
  */
-std::map<V4L2PixelFormat, std::vector<SizeRange>> V4L2VideoDevice::formats()
+std::map<V4L2PixelFormat, std::vector<SizeRange>> V4L2VideoDevice::formats(uint32_t code)
 {
 	std::map<V4L2PixelFormat, std::vector<SizeRange>> formats;
 
-	for (V4L2PixelFormat pixelFormat : enumPixelformats()) {
+	for (V4L2PixelFormat pixelFormat : enumPixelformats(code)) {
 		std::vector<SizeRange> sizes = enumSizes(pixelFormat);
 		if (sizes.empty())
 			return {};
@@ -1000,15 +1003,22 @@  std::map<V4L2PixelFormat, std::vector<SizeRange>> V4L2VideoDevice::formats()
 	return formats;
 }
 
-std::vector<V4L2PixelFormat> V4L2VideoDevice::enumPixelformats()
+std::vector<V4L2PixelFormat> V4L2VideoDevice::enumPixelformats(uint32_t code)
 {
 	std::vector<V4L2PixelFormat> formats;
 	int ret;
 
+	if (code && !(caps_.device_caps() & V4L2_CAP_IO_MC)) {
+		LOG(V4L2, Error)
+			<< "Media bus code filtering not supported by the device";
+		return {};
+	}
+
 	for (unsigned int index = 0; ; index++) {
 		struct v4l2_fmtdesc pixelformatEnum = {};
 		pixelformatEnum.index = index;
 		pixelformatEnum.type = bufferType_;
+		pixelformatEnum.mbus_code = code;
 
 		ret = ioctl(VIDIOC_ENUM_FMT, &pixelformatEnum);
 		if (ret)