[libcamera-devel] [PATCH v4 5/6] libcamera: v4l2_videodevice: Match formats supported by the device
Laurent Pinchart
laurent.pinchart at ideasonboard.com
Tue Aug 2 21:47:13 CEST 2022
Hi Jacopo,
Thank you for the patch.
On Tue, Aug 02, 2022 at 06:01:35PM +0200, Jacopo Mondi via libcamera-devel 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 the V4L2PixelFormat class to be
> associated with an hash function object and to support the comparison
s/an hash/a hash/
> operator.
>
> Signed-off-by: Jacopo Mondi <jacopo at jmondi.org>
> ---
> include/libcamera/internal/v4l2_pixelformat.h | 15 +++++
> include/libcamera/internal/v4l2_videodevice.h | 4 ++
> src/libcamera/v4l2_pixelformat.cpp | 29 ++++++++++
> src/libcamera/v4l2_videodevice.cpp | 58 ++++++++++++++-----
> 4 files changed, 92 insertions(+), 14 deletions(-)
>
> diff --git a/include/libcamera/internal/v4l2_pixelformat.h b/include/libcamera/internal/v4l2_pixelformat.h
> index d5400f90a67e..05069a30304d 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>
> @@ -22,6 +23,15 @@ namespace libcamera {
> class V4L2PixelFormat
> {
> public:
> + class Hasher
> + {
> + public:
> + std::size_t operator()(const V4L2PixelFormat &f) const
> + {
> + return f.fourcc();
> + }
> + };
It would be simpler to inject a specialization of std::hash<> in the std
namespace, that would allow declaring std::unordered_set and
std::unordered_map instances without having to pass a hasher explicitly.
I'll send a patch with this.
> +
> struct Info {
> PixelFormat format;
> const char *description;
> @@ -37,6 +47,11 @@ public:
> {
> }
>
> + bool operator==(const V4L2PixelFormat &other) const
> + {
> + return fourcc_ == other.fourcc_;
> + }
This isn't needed, the default operator==() implementation generated by
the compiler should be enough.
> +
> bool isValid() const { return fourcc_ != 0; }
> uint32_t fourcc() const { return fourcc_; }
> operator uint32_t() const { return fourcc_; }
> diff --git a/include/libcamera/internal/v4l2_videodevice.h b/include/libcamera/internal/v4l2_videodevice.h
> index 29fa0bbaf670..c9064291a669 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, V4L2PixelFormat::Hasher> pixelFormats_;
>
> enum v4l2_buf_type bufferType_;
> enum v4l2_memory memoryType_;
> diff --git a/src/libcamera/v4l2_pixelformat.cpp b/src/libcamera/v4l2_pixelformat.cpp
> index 90c8fa8d9aae..264ca80aab27 100644
> --- a/src/libcamera/v4l2_pixelformat.cpp
> +++ b/src/libcamera/v4l2_pixelformat.cpp
> @@ -187,6 +187,28 @@ const std::map<V4L2PixelFormat, V4L2PixelFormat::Info> vpf2pf{
>
> } /* namespace */
>
> +/**
> + * \class V4L2PixelFormat::Hasher
> + * \brief Function object class that implement hash function for the
> + * V4L2PixelFormat class
> + *
> + * The Hasher class is a function object class that that allows to use the
> + * V4L2PixelFormat class with C++ STL unordered associative containers.
> + *
> + * The hash for a V4L2PixelFormat is obtained by using the function call
> + * operator().
> + */
> +
> +/**
> + * \fn V4L2PixelFormat::Hasher::operator()(const V4L2PixelFormat &f) const
> + * \brief Function call operator that computes the V4L2PixelFormat hash value
> + * \param[in] f The V4L2PixelFormat to compute the hash on
> + *
> + * Compute the hash value of \a f, which simply is the numerical FourCC value.
> + *
> + * \return The numerical FourCC value for \a f
> + */
> +
> /**
> * \struct V4L2PixelFormat::Info
> * \brief Information about a V4L2 pixel format
> @@ -208,6 +230,13 @@ const std::map<V4L2PixelFormat, V4L2PixelFormat::Info> vpf2pf{
> * invalid, calling the isValid() function returns false.
> */
>
> +/**
> + * \fn V4L2PixelFormat::operator==()
> + * \brief Compare two V4L2PixelFormat by comparing their FourCC codes
> + * \param[in] other The other V4L2PixelFormat to compare with
> + * \return True if the FourCC codes are the same, false otherwise
> + */
> +
> /**
> * \fn V4L2PixelFormat::V4L2PixelFormat(uint32_t fourcc)
> * \brief Construct a V4L2PixelFormat from a FourCC value
> diff --git a/src/libcamera/v4l2_videodevice.cpp b/src/libcamera/v4l2_videodevice.cpp
> index 2ca22f485d45..c3343ca7078b 100644
> --- a/src/libcamera/v4l2_videodevice.cpp
> +++ b/src/libcamera/v4l2_videodevice.cpp
> @@ -629,18 +629,14 @@ int V4L2VideoDevice::open()
> fdBufferNotifier_->activated.connect(this, &V4L2VideoDevice::bufferAvailable);
> fdBufferNotifier_->setEnabled(false);
>
> + ret = initFormats();
> + if (ret)
> + return ret;
Could you move this below the debug message to avoid changing the
behaviour ? Same in the other open() function.
> +
> LOG(V4L2, Debug)
> << "Opened device " << caps_.bus_info() << ": "
> << caps_.driver() << ": " << caps_.card();
>
> - ret = getFormat(&format_);
> - if (ret) {
> - LOG(V4L2, Error) << "Failed to get format";
> - return ret;
> - }
> -
> - formatInfo_ = &PixelFormatInfo::info(format_.fourcc);
> -
> return 0;
> }
>
> @@ -722,11 +718,25 @@ int V4L2VideoDevice::open(SharedFD handle, enum v4l2_buf_type type)
> fdBufferNotifier_->activated.connect(this, &V4L2VideoDevice::bufferAvailable);
> fdBufferNotifier_->setEnabled(false);
>
> + ret = initFormats();
> + if (ret)
> + return ret;
> +
> LOG(V4L2, Debug)
> << "Opened device " << caps_.bus_info() << ": "
> << caps_.driver() << ": " << caps_.card();
>
> - ret = getFormat(&format_);
> + return 0;
> +}
> +
> +int V4L2VideoDevice::initFormats()
> +{
> + const std::vector<V4L2PixelFormat> &deviceFormats = enumPixelformats(0);
No error checking ?
> + pixelFormats_ = std::unordered_set<V4L2PixelFormat,
> + V4L2PixelFormat::Hasher>
> + (deviceFormats.begin(), deviceFormats.end());
The constructor you use here is not explicit, so you can write
pixelFormats_ = { deviceFormats.begin(), deviceFormats.end() };
With these small issues addressed, and the V4L2PixelFormat changes
dropped if you want to use the "[PATCH] libcamera: v4l2_pixelformat:
Implement std::hash specialization" patch I've sent (and the commit
message then updated accordingly),
Reviewed-by: Laurent Pinchart <laurent.pinchart at ideasonboard.com>
> +
> + int ret = getFormat(&format_);
> if (ret) {
> LOG(V4L2, Error) << "Failed to get format";
> return ret;
> @@ -1990,17 +2000,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
More information about the libcamera-devel
mailing list