[libcamera-devel] [PATCH v2 1/6] libcamera: formats: Make ImageFormats a templated class
Laurent Pinchart
laurent.pinchart at ideasonboard.com
Fri Jun 26 03:25:41 CEST 2020
Hi Jacopo,
Thank you for the patch.
On Tue, Jun 09, 2020 at 01:28:39AM +0200, Jacopo Mondi wrote:
> The ImageFormats class was originally designed to be used by both
> V4L2VideoDevices and V4L2Subdevices. As video devices enumerates their
> image formats using V4L2PixelFormat instances, the ImageFormats class
> cannot be used there anymore and it has been replaced by raw maps.
>
> Prepare to re-introduce usage of ImageFormats in the V4L2VideoDevice
> class and its users by making ImageFormats a templated class that
> indexes the image sizes using on keys of variadic type.
>
> Signed-off-by: Jacopo Mondi <jacopo at jmondi.org>
> ---
> include/libcamera/internal/camera_sensor.h | 2 +-
> include/libcamera/internal/formats.h | 11 ++++-----
> include/libcamera/internal/v4l2_subdevice.h | 2 +-
> src/libcamera/formats.cpp | 25 +++++++++++++--------
> src/libcamera/v4l2_subdevice.cpp | 6 ++---
> test/v4l2_subdevice/list_formats.cpp | 2 +-
> 6 files changed, 28 insertions(+), 20 deletions(-)
>
> diff --git a/include/libcamera/internal/camera_sensor.h b/include/libcamera/internal/camera_sensor.h
> index 7f07413f9560..d5814a26a121 100644
> --- a/include/libcamera/internal/camera_sensor.h
> +++ b/include/libcamera/internal/camera_sensor.h
> @@ -75,7 +75,7 @@ private:
>
> std::string model_;
>
> - ImageFormats formats_;
> + ImageFormats<uint32_t> formats_;
> Size resolution_;
> std::vector<unsigned int> mbusCodes_;
> std::vector<Size> sizes_;
> diff --git a/include/libcamera/internal/formats.h b/include/libcamera/internal/formats.h
> index 4b172efc6588..5668f3744c5d 100644
> --- a/include/libcamera/internal/formats.h
> +++ b/include/libcamera/internal/formats.h
> @@ -18,18 +18,19 @@
>
> namespace libcamera {
>
> +template<typename T>
> class ImageFormats
> {
> public:
> - int addFormat(unsigned int format, const std::vector<SizeRange> &sizes);
> + int addFormat(T format, const std::vector<SizeRange> &sizes);
>
> bool isEmpty() const;
> - std::vector<unsigned int> formats() const;
> - const std::vector<SizeRange> &sizes(unsigned int format) const;
> - const std::map<unsigned int, std::vector<SizeRange>> &data() const;
> + std::vector<T> formats() const;
> + const std::vector<SizeRange> &sizes(T format) const;
> + const std::map<T, std::vector<SizeRange>> &data() const;
>
> private:
> - std::map<unsigned int, std::vector<SizeRange>> data_;
> + std::map<T, std::vector<SizeRange>> data_;
> };
>
> class PixelFormatInfo
> diff --git a/include/libcamera/internal/v4l2_subdevice.h b/include/libcamera/internal/v4l2_subdevice.h
> index a3ecf123f640..f811d316dada 100644
> --- a/include/libcamera/internal/v4l2_subdevice.h
> +++ b/include/libcamera/internal/v4l2_subdevice.h
> @@ -51,7 +51,7 @@ public:
> int setSelection(unsigned int pad, unsigned int target,
> Rectangle *rect);
>
> - ImageFormats formats(unsigned int pad);
> + ImageFormats<uint32_t> formats(unsigned int pad);
>
> int getFormat(unsigned int pad, V4L2SubdeviceFormat *format,
> Whence whence = ActiveFormat);
> diff --git a/src/libcamera/formats.cpp b/src/libcamera/formats.cpp
> index 2ac3b412ecdb..62fd46686d7d 100644
> --- a/src/libcamera/formats.cpp
> +++ b/src/libcamera/formats.cpp
> @@ -28,9 +28,8 @@ LOG_DEFINE_CATEGORY(Formats)
> * corresponding set of image sizes. It is used to describe the formats and
> * sizes supported by a V4L2Device or V4L2Subdevice.
> *
> - * Formats are stored as an integer. When used for a V4L2Device, the image
> - * formats are fourcc pixel formats. When used for a V4L2Subdevice they are
> - * media bus codes. Both are defined by the V4L2 specification.
> + * When used for a V4L2Device, the image formats are V4L2PixelFormat instances.
> + * When used for a V4L2Subdevice formats are uint32_t media bus codes.
> *
> * Sizes are stored as a list of SizeRange.
> */
> @@ -43,7 +42,8 @@ LOG_DEFINE_CATEGORY(Formats)
> * \return 0 on success or a negative error code otherwise
> * \retval -EEXIST The format is already described
> */
> -int ImageFormats::addFormat(unsigned int format, const std::vector<SizeRange> &sizes)
> +template<typename T>
> +int ImageFormats<T>::addFormat(T format, const std::vector<SizeRange> &sizes)
> {
> if (data_.find(format) != data_.end())
> return -EEXIST;
> @@ -57,7 +57,8 @@ int ImageFormats::addFormat(unsigned int format, const std::vector<SizeRange> &s
> * \brief Check if the list of devices supported formats is empty
> * \return True if the list of supported formats is empty
> */
> -bool ImageFormats::isEmpty() const
> +template<typename T>
> +bool ImageFormats<T>::isEmpty() const
> {
> return data_.empty();
> }
> @@ -66,9 +67,10 @@ bool ImageFormats::isEmpty() const
> * \brief Retrieve a list of all supported image formats
> * \return List of pixel formats or media bus codes
> */
> -std::vector<unsigned int> ImageFormats::formats() const
> +template<typename T>
> +std::vector<T> ImageFormats<T>::formats() const
> {
> - std::vector<unsigned int> formats;
> + std::vector<T> formats;
> formats.reserve(data_.size());
>
> /* \todo: Should this be cached instead of computed each time? */
> @@ -88,7 +90,8 @@ std::vector<unsigned int> ImageFormats::formats() const
> * \return The list of image sizes supported for \a format, or an empty list if
> * the format is not supported
> */
> -const std::vector<SizeRange> &ImageFormats::sizes(unsigned int format) const
> +template<typename T>
> +const std::vector<SizeRange> &ImageFormats<T>::sizes(T format) const
> {
> static const std::vector<SizeRange> empty;
>
> @@ -103,11 +106,15 @@ const std::vector<SizeRange> &ImageFormats::sizes(unsigned int format) const
> * \brief Retrieve the map that associates formats to image sizes
> * \return The map that associates formats to image sizes
> */
> -const std::map<unsigned int, std::vector<SizeRange>> &ImageFormats::data() const
> +template<typename T>
> +const std::map<T, std::vector<SizeRange>> &ImageFormats<T>::data() const
> {
> return data_;
> }
>
> +template class ImageFormats<uint32_t>;
> +template class ImageFormats<V4L2PixelFormat>;
Should we include libcamera/internal/v4l2_pixelformat.h at the top ?
It's pulled in by formats.h, but that's for PixelFormatInfo, and it
could change in the future.
Reviewed-by: Laurent Pinchart <laurent.pinchart at ideasonboard.com>
> +
> /**
> * \class PixelFormatInfo
> * \brief Information about pixel formats
> diff --git a/src/libcamera/v4l2_subdevice.cpp b/src/libcamera/v4l2_subdevice.cpp
> index 7aefc1be032d..9fa20e84a904 100644
> --- a/src/libcamera/v4l2_subdevice.cpp
> +++ b/src/libcamera/v4l2_subdevice.cpp
> @@ -320,16 +320,16 @@ int V4L2Subdevice::setSelection(unsigned int pad, unsigned int target,
> *
> * \return A list of the supported device formats
> */
> -ImageFormats V4L2Subdevice::formats(unsigned int pad)
> +ImageFormats<uint32_t> V4L2Subdevice::formats(unsigned int pad)
> {
> - ImageFormats formats;
> + ImageFormats<uint32_t> formats;
>
> if (pad >= entity_->pads().size()) {
> LOG(V4L2, Error) << "Invalid pad: " << pad;
> return {};
> }
>
> - for (unsigned int code : enumPadCodes(pad)) {
> + for (uint32_t code : enumPadCodes(pad)) {
> std::vector<SizeRange> sizes = enumPadSizes(pad, code);
> if (sizes.empty())
> return {};
> diff --git a/test/v4l2_subdevice/list_formats.cpp b/test/v4l2_subdevice/list_formats.cpp
> index a55af1100d9a..3e8d4cdba045 100644
> --- a/test/v4l2_subdevice/list_formats.cpp
> +++ b/test/v4l2_subdevice/list_formats.cpp
> @@ -47,7 +47,7 @@ void ListFormatsTest::printFormats(unsigned int pad,
> int ListFormatsTest::run()
> {
> /* List all formats available on existing "Scaler" pads. */
> - ImageFormats formats;
> + ImageFormats<uint32_t> formats;
>
> formats = scaler_->formats(0);
> if (formats.isEmpty()) {
--
Regards,
Laurent Pinchart
More information about the libcamera-devel
mailing list