[libcamera-devel] [PATCH v5 6/7] libcamera: v4l2_videodevice: Match formats supported by the device

[Laurent Pinchart]

Hi Jacopo,

On Wed, Aug 03, 2022 at 12:38:48PM +0200, Jacopo Mondi wrote:
> Now that V4L2PixelFormat::fromPixelFormat() returns a list of formats
> to chose from, select the one supported by the video device by matching
> against the list of supported pixel formats.
> 
> The first format found to match one of the device supported ones is
> returned.
> 
> As the list of pixel formats supported by the video device does not
> change at run-time, cache it at device open() time. To maximize the
> lookup efficiency store the list of supported V4L2PixelFormat in an
> std::unordered_set<> which requires a specialization of
> std::hash<V4L2PixelFormat> to be injected in the std namespace.

Would you mind including the patch I sent to add
std::hash<V4L2PixelFormat> in this series and rebasing this on top ?
It's a separate function change, and if we later want to hash
PixelFormat too, it will show how to do so (with a commit message
template) without unrelated changes.

The commit message here should then drop " ... which requires " unwards,
and you can also drop my SoB line.

> Signed-off-by: Jacopo Mondi <jacopo at jmondi.org>
> Signed-off-by: Laurent Pinchart <laurent.pinchart at ideasonboard.com>
> Reviewed-by: Laurent Pinchart <laurent.pinchart at ideasonboard.com>
> ---
>  include/libcamera/internal/v4l2_pixelformat.h | 13 +++++
>  include/libcamera/internal/v4l2_videodevice.h |  4 ++
>  src/libcamera/v4l2_videodevice.cpp            | 57 +++++++++++++++----
>  3 files changed, 62 insertions(+), 12 deletions(-)
> 
> diff --git a/include/libcamera/internal/v4l2_pixelformat.h b/include/libcamera/internal/v4l2_pixelformat.h
> index d5400f90a67e..34d283db44f4 100644
> --- a/include/libcamera/internal/v4l2_pixelformat.h
> +++ b/include/libcamera/internal/v4l2_pixelformat.h
> @@ -8,6 +8,7 @@
>  
>  #pragma once
>  
> +#include <functional>
>  #include <ostream>
>  #include <stdint.h>
>  #include <string>
> @@ -55,3 +56,15 @@ private:
>  std::ostream &operator<<(std::ostream &out, const V4L2PixelFormat &f);
>  
>  } /* namespace libcamera */
> +
> +namespace std {
> +
> +template<>
> +struct hash<libcamera::V4L2PixelFormat> {
> +	size_t operator()(libcamera::V4L2PixelFormat const &format) const noexcept
> +	{
> +		return format.fourcc();
> +	}
> +};
> +
> +} /* namespace std */
> diff --git a/include/libcamera/internal/v4l2_videodevice.h b/include/libcamera/internal/v4l2_videodevice.h
> index 29fa0bbaf670..ed98a284de16 100644
> --- a/include/libcamera/internal/v4l2_videodevice.h
> +++ b/include/libcamera/internal/v4l2_videodevice.h
> @@ -14,6 +14,7 @@
>  #include <ostream>
>  #include <stdint.h>
>  #include <string>
> +#include <unordered_set>
>  #include <vector>
>  
>  #include <linux/videodev2.h>
> @@ -242,6 +243,8 @@ private:
>  		Stopped,
>  	};
>  
> +	int initFormats();
> +
>  	int getFormatMeta(V4L2DeviceFormat *format);
>  	int trySetFormatMeta(V4L2DeviceFormat *format, bool set);
>  
> @@ -268,6 +271,7 @@ private:
>  	V4L2Capability caps_;
>  	V4L2DeviceFormat format_;
>  	const PixelFormatInfo *formatInfo_;
> +	std::unordered_set<V4L2PixelFormat> pixelFormats_;
>  
>  	enum v4l2_buf_type bufferType_;
>  	enum v4l2_memory memoryType_;
> diff --git a/src/libcamera/v4l2_videodevice.cpp b/src/libcamera/v4l2_videodevice.cpp
> index 2ca22f485d45..b80ee1cdbcca 100644
> --- a/src/libcamera/v4l2_videodevice.cpp
> +++ b/src/libcamera/v4l2_videodevice.cpp
> @@ -633,13 +633,9 @@ int V4L2VideoDevice::open()
>  		<< "Opened device " << caps_.bus_info() << ": "
>  		<< caps_.driver() << ": " << caps_.card();
>  
> -	ret = getFormat(&format_);
> -	if (ret) {
> -		LOG(V4L2, Error) << "Failed to get format";
> +	ret = initFormats();
> +	if (ret)
>  		return ret;
> -	}
> -
> -	formatInfo_ = &PixelFormatInfo::info(format_.fourcc);
>  
>  	return 0;
>  }
> @@ -726,7 +722,24 @@ int V4L2VideoDevice::open(SharedFD handle, enum v4l2_buf_type type)
>  		<< "Opened device " << caps_.bus_info() << ": "
>  		<< caps_.driver() << ": " << caps_.card();
>  
> -	ret = getFormat(&format_);
> +	ret = initFormats();
> +	if (ret)
> +		return ret;
> +
> +	return 0;
> +}
> +
> +int V4L2VideoDevice::initFormats()
> +{
> +	const std::vector<V4L2PixelFormat> &deviceFormats = enumPixelformats(0);
> +	if (deviceFormats.empty()) {
> +		LOG(V4L2, Error) << "Failed to initialize device formats";
> +		return -EINVAL;
> +	}
> +
> +	pixelFormats_ = { deviceFormats.begin(), deviceFormats.end() };
> +
> +	int ret = getFormat(&format_);
>  	if (ret) {
>  		LOG(V4L2, Error) << "Failed to get format";
>  		return ret;
> @@ -1990,17 +2003,37 @@ V4L2VideoDevice::fromEntityName(const MediaDevice *media,
>  }
>  
>  /**
> - * \brief Convert \a PixelFormat to its corresponding V4L2 FourCC
> + * \brief Convert \a PixelFormat to a V4L2PixelFormat supported by the device
>   * \param[in] pixelFormat The PixelFormat to convert
>   *
> - * The V4L2 format variant the function returns the contiguous version
> - * unconditionally.
> + * Convert \a pixelformat to a V4L2 FourCC that is known to be supported by
> + * the video device.
>   *
> - * \return The V4L2_PIX_FMT_* pixel format code corresponding to \a pixelFormat
> + * A V4L2VideoDevice may support different V4L2 pixel formats that map the same
> + * PixelFormat. This is the case of the contiguous and non-contiguous variants
> + * of multiplanar formats, and with the V4L2 MJPEG and JPEG pixel formats.
> + * Converting a PixelFormat to a V4L2PixelFormat may thus have multiple answers.
> + *
> + * This function converts the \a pixelFormat using the list of V4L2 pixel
> + * formats that the V4L2VideoDevice supports. This guarantees that the returned
> + * V4L2PixelFormat will be valid for the device. If multiple matches are still
> + * possible, contiguous variants are preferred. If the \a pixelFormat is not
> + * supported by the device, the function returns an invalid V4L2PixelFormat.
> + *
> + * \return The V4L2PixelFormat corresponding to \a pixelFormat if supported by
> + * the device, or an invalid V4L2PixelFormat otherwise
>   */
>  V4L2PixelFormat V4L2VideoDevice::toV4L2PixelFormat(const PixelFormat &pixelFormat) const
>  {
> -	return V4L2PixelFormat::fromPixelFormat(pixelFormat)[0];
> +	const std::vector<V4L2PixelFormat> &v4l2PixelFormats =
> +		V4L2PixelFormat::fromPixelFormat(pixelFormat);
> +
> +	for (const V4L2PixelFormat &v4l2Format : v4l2PixelFormats) {
> +		if (pixelFormats_.count(v4l2Format))
> +			return v4l2Format;
> +	}
> +
> +	return {};
>  }
>  
>  /**

-- 
Regards,

Laurent Pinchart