[libcamera-devel,v2,1/3] libcamera: v4l2_subdevice: Replace ImageFormats with a map
diff mbox series

Message ID 20200721135541.2931205-2-niklas.soderlund@ragnatech.se
State Accepted
Headers show
Series
  • libcamera: formats: Remove ImageFormat
Related show

Commit Message

Niklas Söderlund July 21, 2020, 1:55 p.m. UTC 
Replace the V4L2Subdevice usage of the ImageFormats class with a
std::map and the utils::map_keys() helper.

Signed-off-by: Niklas Söderlund <niklas.soderlund@ragnatech.se>
Reviewed-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
---
* Changes since RFC
- Fix spelling in commit message.
- Prevent double lookup in CameraSensor::getFormat().
- Preserve error message in V4L2Subdevice::formats().
---
 include/libcamera/internal/camera_sensor.h  |  6 ++----
 include/libcamera/internal/v4l2_subdevice.h |  4 +++-
 src/libcamera/camera_sensor.cpp             | 13 +++++++------
 src/libcamera/v4l2_subdevice.cpp            | 12 +++++++++---
 test/v4l2_subdevice/list_formats.cpp        | 16 ++++++++--------
 5 files changed, 29 insertions(+), 22 deletions(-)

Comments

Jacopo Mondi July 21, 2020, 3:20 p.m. UTC | #1 
Hi Niklas

On Tue, Jul 21, 2020 at 03:55:39PM +0200, Niklas Söderlund wrote:
> Replace the V4L2Subdevice usage of the ImageFormats class with a
> std::map and the utils::map_keys() helper.
>
> Signed-off-by: Niklas Söderlund <niklas.soderlund@ragnatech.se>
> Reviewed-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
> ---
> * Changes since RFC
> - Fix spelling in commit message.
> - Prevent double lookup in CameraSensor::getFormat().
> - Preserve error message in V4L2Subdevice::formats().
> ---
>  include/libcamera/internal/camera_sensor.h  |  6 ++----
>  include/libcamera/internal/v4l2_subdevice.h |  4 +++-
>  src/libcamera/camera_sensor.cpp             | 13 +++++++------
>  src/libcamera/v4l2_subdevice.cpp            | 12 +++++++++---
>  test/v4l2_subdevice/list_formats.cpp        | 16 ++++++++--------
>  5 files changed, 29 insertions(+), 22 deletions(-)
>
> diff --git a/include/libcamera/internal/camera_sensor.h b/include/libcamera/internal/camera_sensor.h
> index 7f07413f95602881..06c8292ca30129de 100644
> --- a/include/libcamera/internal/camera_sensor.h
> +++ b/include/libcamera/internal/camera_sensor.h
> @@ -16,13 +16,11 @@
>
>  #include "libcamera/internal/formats.h"
>  #include "libcamera/internal/log.h"
> +#include "libcamera/internal/v4l2_subdevice.h"
>
>  namespace libcamera {
>
>  class MediaEntity;
> -class V4L2Subdevice;
> -
> -struct V4L2SubdeviceFormat;
>
>  struct CameraSensorInfo {
>  	std::string model;
> @@ -75,7 +73,7 @@ private:
>
>  	std::string model_;
>
> -	ImageFormats formats_;
> +	V4L2Subdevice::Formats formats_;
>  	Size resolution_;
>  	std::vector<unsigned int> mbusCodes_;
>  	std::vector<Size> sizes_;
> diff --git a/include/libcamera/internal/v4l2_subdevice.h b/include/libcamera/internal/v4l2_subdevice.h
> index a3ecf123f640dd54..02ee3e914a8b1d92 100644
> --- a/include/libcamera/internal/v4l2_subdevice.h
> +++ b/include/libcamera/internal/v4l2_subdevice.h
> @@ -32,6 +32,8 @@ struct V4L2SubdeviceFormat {
>  class V4L2Subdevice : public V4L2Device
>  {
>  public:
> +	using Formats = std::map<unsigned int, std::vector<SizeRange>>;
> +
>  	enum Whence {
>  		ActiveFormat,
>  		TryFormat,
> @@ -51,7 +53,7 @@ public:
>  	int setSelection(unsigned int pad, unsigned int target,
>  			 Rectangle *rect);
>
> -	ImageFormats formats(unsigned int pad);
> +	Formats formats(unsigned int pad);
>
>  	int getFormat(unsigned int pad, V4L2SubdeviceFormat *format,
>  		      Whence whence = ActiveFormat);
> diff --git a/src/libcamera/camera_sensor.cpp b/src/libcamera/camera_sensor.cpp
> index 6e93cc51155ba678..350f49accad99c7b 100644
> --- a/src/libcamera/camera_sensor.cpp
> +++ b/src/libcamera/camera_sensor.cpp
> @@ -18,7 +18,6 @@
>
>  #include "libcamera/internal/formats.h"
>  #include "libcamera/internal/utils.h"
> -#include "libcamera/internal/v4l2_subdevice.h"
>
>  /**
>   * \file camera_sensor.h
> @@ -245,15 +244,15 @@ int CameraSensor::init()
>
>  	/* Enumerate, sort and cache media bus codes and sizes. */
>  	formats_ = subdev_->formats(pad_);
> -	if (formats_.isEmpty()) {
> +	if (formats_.empty()) {
>  		LOG(CameraSensor, Error) << "No image format found";
>  		return -EINVAL;
>  	}
>
> -	mbusCodes_ = formats_.formats();
> +	mbusCodes_ = utils::map_keys(formats_);
>  	std::sort(mbusCodes_.begin(), mbusCodes_.end());
>
> -	for (const auto &format : formats_.data()) {
> +	for (const auto &format : formats_) {
>  		const std::vector<SizeRange> &ranges = format.second;
>  		std::transform(ranges.begin(), ranges.end(), std::back_inserter(sizes_),
>  			       [](const SizeRange &range) { return range.max; });
> @@ -359,9 +358,11 @@ V4L2SubdeviceFormat CameraSensor::getFormat(const std::vector<unsigned int> &mbu
>  	uint32_t bestCode = 0;
>
>  	for (unsigned int code : mbusCodes) {
> -		const std::vector<SizeRange> &ranges = formats_.sizes(code);
> +		const auto formats = formats_.find(code);
> +		if (formats == formats_.end())
> +			continue;
>
> -		for (const SizeRange &range : ranges) {
> +		for (const SizeRange &range : formats->second) {
>  			const Size &sz = range.max;
>
>  			if (sz.width < size.width || sz.height < size.height)
> diff --git a/src/libcamera/v4l2_subdevice.cpp b/src/libcamera/v4l2_subdevice.cpp
> index 32c6c7acc11a6a3f..85d00c246f5e5ee9 100644
> --- a/src/libcamera/v4l2_subdevice.cpp
> +++ b/src/libcamera/v4l2_subdevice.cpp
> @@ -217,6 +217,11 @@ uint8_t V4L2SubdeviceFormat::bitsPerPixel() const
>   * any device left open will be closed, and any resources released.
>   */
>
> +/**
> + * \typedef V4L2Subdevice::Formats
> + * \brief A map of supported media bus formats to frame sizes
> + */
> +
>  /**
>   * \enum V4L2Subdevice::Whence
>   * \brief Specify the type of format for getFormat() and setFormat() operations
> @@ -340,9 +345,9 @@ int V4L2Subdevice::setSelection(unsigned int pad, unsigned int target,
>   *
>   * \return A list of the supported device formats
>   */
> -ImageFormats V4L2Subdevice::formats(unsigned int pad)
> +V4L2Subdevice::Formats V4L2Subdevice::formats(unsigned int pad)
>  {
> -	ImageFormats formats;
> +	Formats formats;
>
>  	if (pad >= entity_->pads().size()) {
>  		LOG(V4L2, Error) << "Invalid pad: " << pad;
> @@ -354,7 +359,8 @@ ImageFormats V4L2Subdevice::formats(unsigned int pad)
>  		if (sizes.empty())
>  			return {};
>
> -		if (formats.addFormat(code, sizes)) {
> +		const auto inserted = formats.insert({ code, sizes });

Looking at the c++ reference example you could write
                const auto [it, ret] = formats.insert({ code, sizes });
                if (!ret)

Anway, using std::map::insert() is neat indeed!

Reviewed-by: Jacopo Mondi <jacopo@jmondi.org>

Thanks
  j

> +		if (!inserted.second) {
>  			LOG(V4L2, Error)
>  				<< "Could not add sizes for media bus code "
>  				<< code << " on pad " << pad;
> diff --git a/test/v4l2_subdevice/list_formats.cpp b/test/v4l2_subdevice/list_formats.cpp
> index a55af1100d9ab498..a6044c044036acd4 100644
> --- a/test/v4l2_subdevice/list_formats.cpp
> +++ b/test/v4l2_subdevice/list_formats.cpp
> @@ -47,29 +47,29 @@ void ListFormatsTest::printFormats(unsigned int pad,
>  int ListFormatsTest::run()
>  {
>  	/* List all formats available on existing "Scaler" pads. */
> -	ImageFormats formats;
> +	V4L2Subdevice::Formats formats;
>
>  	formats = scaler_->formats(0);
> -	if (formats.isEmpty()) {
> +	if (formats.empty()) {
>  		cerr << "Failed to list formats on pad 0 of subdevice "
>  		     << scaler_->entity()->name() << endl;
>  		return TestFail;
>  	}
> -	for (unsigned int code : formats.formats())
> -		printFormats(0, code, formats.sizes(code));
> +	for (unsigned int code : utils::map_keys(formats))
> +		printFormats(0, code, formats[code]);
>
>  	formats = scaler_->formats(1);
> -	if (formats.isEmpty()) {
> +	if (formats.empty()) {
>  		cerr << "Failed to list formats on pad 1 of subdevice "
>  		     << scaler_->entity()->name() << endl;
>  		return TestFail;
>  	}
> -	for (unsigned int code : formats.formats())
> -		printFormats(1, code, formats.sizes(code));
> +	for (unsigned int code : utils::map_keys(formats))
> +		printFormats(1, code, formats[code]);
>
>  	/* List format on a non-existing pad, format vector shall be empty. */
>  	formats = scaler_->formats(2);
> -	if (!formats.isEmpty()) {
> +	if (!formats.empty()) {
>  		cerr << "Listing formats on non-existing pad 2 of subdevice "
>  		     << scaler_->entity()->name()
>  		     << " should return an empty format list" << endl;
> --
> 2.27.0
>
> _______________________________________________
> libcamera-devel mailing list
> libcamera-devel@lists.libcamera.org
> https://lists.libcamera.org/listinfo/libcamera-devel
Laurent Pinchart July 23, 2020, 2:19 p.m. UTC | #2 
Hi Jacopo,

On Tue, Jul 21, 2020 at 05:20:22PM +0200, Jacopo Mondi wrote:
> On Tue, Jul 21, 2020 at 03:55:39PM +0200, Niklas Söderlund wrote:
> > Replace the V4L2Subdevice usage of the ImageFormats class with a
> > std::map and the utils::map_keys() helper.
> >
> > Signed-off-by: Niklas Söderlund <niklas.soderlund@ragnatech.se>
> > Reviewed-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
> > ---
> > * Changes since RFC
> > - Fix spelling in commit message.
> > - Prevent double lookup in CameraSensor::getFormat().
> > - Preserve error message in V4L2Subdevice::formats().
> > ---
> >  include/libcamera/internal/camera_sensor.h  |  6 ++----
> >  include/libcamera/internal/v4l2_subdevice.h |  4 +++-
> >  src/libcamera/camera_sensor.cpp             | 13 +++++++------
> >  src/libcamera/v4l2_subdevice.cpp            | 12 +++++++++---
> >  test/v4l2_subdevice/list_formats.cpp        | 16 ++++++++--------
> >  5 files changed, 29 insertions(+), 22 deletions(-)
> >
> > diff --git a/include/libcamera/internal/camera_sensor.h b/include/libcamera/internal/camera_sensor.h
> > index 7f07413f95602881..06c8292ca30129de 100644
> > --- a/include/libcamera/internal/camera_sensor.h
> > +++ b/include/libcamera/internal/camera_sensor.h
> > @@ -16,13 +16,11 @@
> >
> >  #include "libcamera/internal/formats.h"
> >  #include "libcamera/internal/log.h"
> > +#include "libcamera/internal/v4l2_subdevice.h"
> >
> >  namespace libcamera {
> >
> >  class MediaEntity;
> > -class V4L2Subdevice;
> > -
> > -struct V4L2SubdeviceFormat;
> >
> >  struct CameraSensorInfo {
> >  	std::string model;
> > @@ -75,7 +73,7 @@ private:
> >
> >  	std::string model_;
> >
> > -	ImageFormats formats_;
> > +	V4L2Subdevice::Formats formats_;
> >  	Size resolution_;
> >  	std::vector<unsigned int> mbusCodes_;
> >  	std::vector<Size> sizes_;
> > diff --git a/include/libcamera/internal/v4l2_subdevice.h b/include/libcamera/internal/v4l2_subdevice.h
> > index a3ecf123f640dd54..02ee3e914a8b1d92 100644
> > --- a/include/libcamera/internal/v4l2_subdevice.h
> > +++ b/include/libcamera/internal/v4l2_subdevice.h
> > @@ -32,6 +32,8 @@ struct V4L2SubdeviceFormat {
> >  class V4L2Subdevice : public V4L2Device
> >  {
> >  public:
> > +	using Formats = std::map<unsigned int, std::vector<SizeRange>>;
> > +
> >  	enum Whence {
> >  		ActiveFormat,
> >  		TryFormat,
> > @@ -51,7 +53,7 @@ public:
> >  	int setSelection(unsigned int pad, unsigned int target,
> >  			 Rectangle *rect);
> >
> > -	ImageFormats formats(unsigned int pad);
> > +	Formats formats(unsigned int pad);
> >
> >  	int getFormat(unsigned int pad, V4L2SubdeviceFormat *format,
> >  		      Whence whence = ActiveFormat);
> > diff --git a/src/libcamera/camera_sensor.cpp b/src/libcamera/camera_sensor.cpp
> > index 6e93cc51155ba678..350f49accad99c7b 100644
> > --- a/src/libcamera/camera_sensor.cpp
> > +++ b/src/libcamera/camera_sensor.cpp
> > @@ -18,7 +18,6 @@
> >
> >  #include "libcamera/internal/formats.h"
> >  #include "libcamera/internal/utils.h"
> > -#include "libcamera/internal/v4l2_subdevice.h"
> >
> >  /**
> >   * \file camera_sensor.h
> > @@ -245,15 +244,15 @@ int CameraSensor::init()
> >
> >  	/* Enumerate, sort and cache media bus codes and sizes. */
> >  	formats_ = subdev_->formats(pad_);
> > -	if (formats_.isEmpty()) {
> > +	if (formats_.empty()) {
> >  		LOG(CameraSensor, Error) << "No image format found";
> >  		return -EINVAL;
> >  	}
> >
> > -	mbusCodes_ = formats_.formats();
> > +	mbusCodes_ = utils::map_keys(formats_);
> >  	std::sort(mbusCodes_.begin(), mbusCodes_.end());
> >
> > -	for (const auto &format : formats_.data()) {
> > +	for (const auto &format : formats_) {
> >  		const std::vector<SizeRange> &ranges = format.second;
> >  		std::transform(ranges.begin(), ranges.end(), std::back_inserter(sizes_),
> >  			       [](const SizeRange &range) { return range.max; });
> > @@ -359,9 +358,11 @@ V4L2SubdeviceFormat CameraSensor::getFormat(const std::vector<unsigned int> &mbu
> >  	uint32_t bestCode = 0;
> >
> >  	for (unsigned int code : mbusCodes) {
> > -		const std::vector<SizeRange> &ranges = formats_.sizes(code);
> > +		const auto formats = formats_.find(code);
> > +		if (formats == formats_.end())
> > +			continue;
> >
> > -		for (const SizeRange &range : ranges) {
> > +		for (const SizeRange &range : formats->second) {
> >  			const Size &sz = range.max;
> >
> >  			if (sz.width < size.width || sz.height < size.height)
> > diff --git a/src/libcamera/v4l2_subdevice.cpp b/src/libcamera/v4l2_subdevice.cpp
> > index 32c6c7acc11a6a3f..85d00c246f5e5ee9 100644
> > --- a/src/libcamera/v4l2_subdevice.cpp
> > +++ b/src/libcamera/v4l2_subdevice.cpp
> > @@ -217,6 +217,11 @@ uint8_t V4L2SubdeviceFormat::bitsPerPixel() const
> >   * any device left open will be closed, and any resources released.
> >   */
> >
> > +/**
> > + * \typedef V4L2Subdevice::Formats
> > + * \brief A map of supported media bus formats to frame sizes
> > + */
> > +
> >  /**
> >   * \enum V4L2Subdevice::Whence
> >   * \brief Specify the type of format for getFormat() and setFormat() operations
> > @@ -340,9 +345,9 @@ int V4L2Subdevice::setSelection(unsigned int pad, unsigned int target,
> >   *
> >   * \return A list of the supported device formats
> >   */
> > -ImageFormats V4L2Subdevice::formats(unsigned int pad)
> > +V4L2Subdevice::Formats V4L2Subdevice::formats(unsigned int pad)
> >  {
> > -	ImageFormats formats;
> > +	Formats formats;
> >
> >  	if (pad >= entity_->pads().size()) {
> >  		LOG(V4L2, Error) << "Invalid pad: " << pad;
> > @@ -354,7 +359,8 @@ ImageFormats V4L2Subdevice::formats(unsigned int pad)
> >  		if (sizes.empty())
> >  			return {};
> >
> > -		if (formats.addFormat(code, sizes)) {
> > +		const auto inserted = formats.insert({ code, sizes });
> 
> Looking at the c++ reference example you could write
>                 const auto [it, ret] = formats.insert({ code, sizes });
>                 if (!ret)

Only in C++17 I'm afraid (which we may get soon).

> Anway, using std::map::insert() is neat indeed!
> 
> Reviewed-by: Jacopo Mondi <jacopo@jmondi.org>
> 
> > +		if (!inserted.second) {
> >  			LOG(V4L2, Error)
> >  				<< "Could not add sizes for media bus code "
> >  				<< code << " on pad " << pad;
> > diff --git a/test/v4l2_subdevice/list_formats.cpp b/test/v4l2_subdevice/list_formats.cpp
> > index a55af1100d9ab498..a6044c044036acd4 100644
> > --- a/test/v4l2_subdevice/list_formats.cpp
> > +++ b/test/v4l2_subdevice/list_formats.cpp
> > @@ -47,29 +47,29 @@ void ListFormatsTest::printFormats(unsigned int pad,
> >  int ListFormatsTest::run()
> >  {
> >  	/* List all formats available on existing "Scaler" pads. */
> > -	ImageFormats formats;
> > +	V4L2Subdevice::Formats formats;
> >
> >  	formats = scaler_->formats(0);
> > -	if (formats.isEmpty()) {
> > +	if (formats.empty()) {
> >  		cerr << "Failed to list formats on pad 0 of subdevice "
> >  		     << scaler_->entity()->name() << endl;
> >  		return TestFail;
> >  	}
> > -	for (unsigned int code : formats.formats())
> > -		printFormats(0, code, formats.sizes(code));
> > +	for (unsigned int code : utils::map_keys(formats))
> > +		printFormats(0, code, formats[code]);
> >
> >  	formats = scaler_->formats(1);
> > -	if (formats.isEmpty()) {
> > +	if (formats.empty()) {
> >  		cerr << "Failed to list formats on pad 1 of subdevice "
> >  		     << scaler_->entity()->name() << endl;
> >  		return TestFail;
> >  	}
> > -	for (unsigned int code : formats.formats())
> > -		printFormats(1, code, formats.sizes(code));
> > +	for (unsigned int code : utils::map_keys(formats))
> > +		printFormats(1, code, formats[code]);
> >
> >  	/* List format on a non-existing pad, format vector shall be empty. */
> >  	formats = scaler_->formats(2);
> > -	if (!formats.isEmpty()) {
> > +	if (!formats.empty()) {
> >  		cerr << "Listing formats on non-existing pad 2 of subdevice "
> >  		     << scaler_->entity()->name()
> >  		     << " should return an empty format list" << endl;

Patch
diff mbox series 

diff --git a/include/libcamera/internal/camera_sensor.h b/include/libcamera/internal/camera_sensor.h
index 7f07413f95602881..06c8292ca30129de 100644
--- a/include/libcamera/internal/camera_sensor.h
+++ b/include/libcamera/internal/camera_sensor.h
@@ -16,13 +16,11 @@ 
 
 #include "libcamera/internal/formats.h"
 #include "libcamera/internal/log.h"
+#include "libcamera/internal/v4l2_subdevice.h"
 
 namespace libcamera {
 
 class MediaEntity;
-class V4L2Subdevice;
-
-struct V4L2SubdeviceFormat;
 
 struct CameraSensorInfo {
 	std::string model;
@@ -75,7 +73,7 @@  private:
 
 	std::string model_;
 
-	ImageFormats formats_;
+	V4L2Subdevice::Formats formats_;
 	Size resolution_;
 	std::vector<unsigned int> mbusCodes_;
 	std::vector<Size> sizes_;
diff --git a/include/libcamera/internal/v4l2_subdevice.h b/include/libcamera/internal/v4l2_subdevice.h
index a3ecf123f640dd54..02ee3e914a8b1d92 100644
--- a/include/libcamera/internal/v4l2_subdevice.h
+++ b/include/libcamera/internal/v4l2_subdevice.h
@@ -32,6 +32,8 @@  struct V4L2SubdeviceFormat {
 class V4L2Subdevice : public V4L2Device
 {
 public:
+	using Formats = std::map<unsigned int, std::vector<SizeRange>>;
+
 	enum Whence {
 		ActiveFormat,
 		TryFormat,
@@ -51,7 +53,7 @@  public:
 	int setSelection(unsigned int pad, unsigned int target,
 			 Rectangle *rect);
 
-	ImageFormats formats(unsigned int pad);
+	Formats formats(unsigned int pad);
 
 	int getFormat(unsigned int pad, V4L2SubdeviceFormat *format,
 		      Whence whence = ActiveFormat);
diff --git a/src/libcamera/camera_sensor.cpp b/src/libcamera/camera_sensor.cpp
index 6e93cc51155ba678..350f49accad99c7b 100644
--- a/src/libcamera/camera_sensor.cpp
+++ b/src/libcamera/camera_sensor.cpp
@@ -18,7 +18,6 @@ 
 
 #include "libcamera/internal/formats.h"
 #include "libcamera/internal/utils.h"
-#include "libcamera/internal/v4l2_subdevice.h"
 
 /**
  * \file camera_sensor.h
@@ -245,15 +244,15 @@  int CameraSensor::init()
 
 	/* Enumerate, sort and cache media bus codes and sizes. */
 	formats_ = subdev_->formats(pad_);
-	if (formats_.isEmpty()) {
+	if (formats_.empty()) {
 		LOG(CameraSensor, Error) << "No image format found";
 		return -EINVAL;
 	}
 
-	mbusCodes_ = formats_.formats();
+	mbusCodes_ = utils::map_keys(formats_);
 	std::sort(mbusCodes_.begin(), mbusCodes_.end());
 
-	for (const auto &format : formats_.data()) {
+	for (const auto &format : formats_) {
 		const std::vector<SizeRange> &ranges = format.second;
 		std::transform(ranges.begin(), ranges.end(), std::back_inserter(sizes_),
 			       [](const SizeRange &range) { return range.max; });
@@ -359,9 +358,11 @@  V4L2SubdeviceFormat CameraSensor::getFormat(const std::vector<unsigned int> &mbu
 	uint32_t bestCode = 0;
 
 	for (unsigned int code : mbusCodes) {
-		const std::vector<SizeRange> &ranges = formats_.sizes(code);
+		const auto formats = formats_.find(code);
+		if (formats == formats_.end())
+			continue;
 
-		for (const SizeRange &range : ranges) {
+		for (const SizeRange &range : formats->second) {
 			const Size &sz = range.max;
 
 			if (sz.width < size.width || sz.height < size.height)
diff --git a/src/libcamera/v4l2_subdevice.cpp b/src/libcamera/v4l2_subdevice.cpp
index 32c6c7acc11a6a3f..85d00c246f5e5ee9 100644
--- a/src/libcamera/v4l2_subdevice.cpp
+++ b/src/libcamera/v4l2_subdevice.cpp
@@ -217,6 +217,11 @@  uint8_t V4L2SubdeviceFormat::bitsPerPixel() const
  * any device left open will be closed, and any resources released.
  */
 
+/**
+ * \typedef V4L2Subdevice::Formats
+ * \brief A map of supported media bus formats to frame sizes
+ */
+
 /**
  * \enum V4L2Subdevice::Whence
  * \brief Specify the type of format for getFormat() and setFormat() operations
@@ -340,9 +345,9 @@  int V4L2Subdevice::setSelection(unsigned int pad, unsigned int target,
  *
  * \return A list of the supported device formats
  */
-ImageFormats V4L2Subdevice::formats(unsigned int pad)
+V4L2Subdevice::Formats V4L2Subdevice::formats(unsigned int pad)
 {
-	ImageFormats formats;
+	Formats formats;
 
 	if (pad >= entity_->pads().size()) {
 		LOG(V4L2, Error) << "Invalid pad: " << pad;
@@ -354,7 +359,8 @@  ImageFormats V4L2Subdevice::formats(unsigned int pad)
 		if (sizes.empty())
 			return {};
 
-		if (formats.addFormat(code, sizes)) {
+		const auto inserted = formats.insert({ code, sizes });
+		if (!inserted.second) {
 			LOG(V4L2, Error)
 				<< "Could not add sizes for media bus code "
 				<< code << " on pad " << pad;
diff --git a/test/v4l2_subdevice/list_formats.cpp b/test/v4l2_subdevice/list_formats.cpp
index a55af1100d9ab498..a6044c044036acd4 100644
--- a/test/v4l2_subdevice/list_formats.cpp
+++ b/test/v4l2_subdevice/list_formats.cpp
@@ -47,29 +47,29 @@  void ListFormatsTest::printFormats(unsigned int pad,
 int ListFormatsTest::run()
 {
 	/* List all formats available on existing "Scaler" pads. */
-	ImageFormats formats;
+	V4L2Subdevice::Formats formats;
 
 	formats = scaler_->formats(0);
-	if (formats.isEmpty()) {
+	if (formats.empty()) {
 		cerr << "Failed to list formats on pad 0 of subdevice "
 		     << scaler_->entity()->name() << endl;
 		return TestFail;
 	}
-	for (unsigned int code : formats.formats())
-		printFormats(0, code, formats.sizes(code));
+	for (unsigned int code : utils::map_keys(formats))
+		printFormats(0, code, formats[code]);
 
 	formats = scaler_->formats(1);
-	if (formats.isEmpty()) {
+	if (formats.empty()) {
 		cerr << "Failed to list formats on pad 1 of subdevice "
 		     << scaler_->entity()->name() << endl;
 		return TestFail;
 	}
-	for (unsigned int code : formats.formats())
-		printFormats(1, code, formats.sizes(code));
+	for (unsigned int code : utils::map_keys(formats))
+		printFormats(1, code, formats[code]);
 
 	/* List format on a non-existing pad, format vector shall be empty. */
 	formats = scaler_->formats(2);
-	if (!formats.isEmpty()) {
+	if (!formats.empty()) {
 		cerr << "Listing formats on non-existing pad 2 of subdevice "
 		     << scaler_->entity()->name()
 		     << " should return an empty format list" << endl;