[libcamera-devel,4/5] libcamera: v4l2_device: Add formatsMap typedef

Message ID 20200529110335.620503-5-jacopo@jmondi.org
State Superseded
Headers show
Series
  • ImageFormats' not dead
Related show

Commit Message

Jacopo Mondi May 29, 2020, 11:03 a.m. UTC
Provide a type definition in the V4L2VideoDevice and V4L2Subdevice
classes to shorten the ImageFormatsMap<V4L2PixelFormat> and
ImageFormatsMap<uint32_t> spcialization names.

The new types reads as V4L2VideoDevice::formatsMap and
V4L2Subdevice::formatsMap respectively.

Signed-off-by: Jacopo Mondi <jacopo@jmondi.org>
---
 include/libcamera/internal/camera_sensor.h         |  6 ++----
 include/libcamera/internal/v4l2_subdevice.h        |  4 +++-
 include/libcamera/internal/v4l2_videodevice.h      |  4 +++-
 src/libcamera/pipeline/raspberrypi/raspberrypi.cpp | 11 ++++++-----
 src/libcamera/pipeline/simple/simple.cpp           |  2 +-
 src/libcamera/pipeline/uvcvideo/uvcvideo.cpp       |  2 +-
 src/libcamera/v4l2_subdevice.cpp                   |  9 +++++++--
 src/libcamera/v4l2_videodevice.cpp                 |  9 +++++++--
 test/v4l2_subdevice/list_formats.cpp               |  2 +-
 9 files changed, 31 insertions(+), 18 deletions(-)

Comments

Laurent Pinchart June 5, 2020, 11:31 p.m. UTC | #1
Hi Jacopo,

Thank you for the patch.

On Fri, May 29, 2020 at 01:03:34PM +0200, Jacopo Mondi wrote:
> Provide a type definition in the V4L2VideoDevice and V4L2Subdevice
> classes to shorten the ImageFormatsMap<V4L2PixelFormat> and
> ImageFormatsMap<uint32_t> spcialization names.
> 
> The new types reads as V4L2VideoDevice::formatsMap and
> V4L2Subdevice::formatsMap respectively.

That should be FormatsMap (or actually even Formats).

I would split this in two, with the V4L2VideoDevice part squashed with
2/5, as otherwise you essentially update the same code twice, oncee to
move from an explicit std::map to ImageFormats<V4L2PixelFormat>, and a
second time to move from ImageFormats<V4L2PixelFormat> to
V4L2VideoDevice::FormatsMap. The V4L2Subdevice part could then be moved
after patch 1/5 and before patch 2/5.

> Signed-off-by: Jacopo Mondi <jacopo@jmondi.org>
> ---
>  include/libcamera/internal/camera_sensor.h         |  6 ++----
>  include/libcamera/internal/v4l2_subdevice.h        |  4 +++-
>  include/libcamera/internal/v4l2_videodevice.h      |  4 +++-
>  src/libcamera/pipeline/raspberrypi/raspberrypi.cpp | 11 ++++++-----
>  src/libcamera/pipeline/simple/simple.cpp           |  2 +-
>  src/libcamera/pipeline/uvcvideo/uvcvideo.cpp       |  2 +-
>  src/libcamera/v4l2_subdevice.cpp                   |  9 +++++++--
>  src/libcamera/v4l2_videodevice.cpp                 |  9 +++++++--
>  test/v4l2_subdevice/list_formats.cpp               |  2 +-
>  9 files changed, 31 insertions(+), 18 deletions(-)
> 
> diff --git a/include/libcamera/internal/camera_sensor.h b/include/libcamera/internal/camera_sensor.h
> index 52eedb9ef880..1353edb1da28 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_;
>  
> -	ImageFormatsMap<uint32_t> formats_;
> +	V4L2Subdevice::formatsMap 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 c9aa3428f93c..06f10d7b4c5d 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 formatsMap = ImageFormatsMap<uint32_t>;
> +
>  	enum Whence {
>  		ActiveFormat,
>  		TryFormat,
> @@ -51,7 +53,7 @@ public:
>  	int setSelection(unsigned int pad, unsigned int target,
>  			 Rectangle *rect);
>  
> -	ImageFormatsMap<uint32_t> formats(unsigned int pad);
> +	formatsMap formats(unsigned int pad);
>  
>  	int getFormat(unsigned int pad, V4L2SubdeviceFormat *format,
>  		      Whence whence = ActiveFormat);
> diff --git a/include/libcamera/internal/v4l2_videodevice.h b/include/libcamera/internal/v4l2_videodevice.h
> index 02d101db0b2b..9632ad71b988 100644
> --- a/include/libcamera/internal/v4l2_videodevice.h
> +++ b/include/libcamera/internal/v4l2_videodevice.h
> @@ -168,6 +168,8 @@ public:
>  class V4L2VideoDevice : public V4L2Device
>  {
>  public:
> +	using formatsMap = ImageFormatsMap<V4L2PixelFormat>;
> +
>  	explicit V4L2VideoDevice(const std::string &deviceNode);
>  	explicit V4L2VideoDevice(const MediaEntity *entity);
>  	V4L2VideoDevice(const V4L2VideoDevice &) = delete;
> @@ -187,7 +189,7 @@ public:
>  
>  	int getFormat(V4L2DeviceFormat *format);
>  	int setFormat(V4L2DeviceFormat *format);
> -	ImageFormatsMap<V4L2PixelFormat> formats(uint32_t code = 0);
> +	formatsMap formats(uint32_t code = 0);
>  
>  	int setSelection(unsigned int target, Rectangle *rect);
>  
> diff --git a/src/libcamera/pipeline/raspberrypi/raspberrypi.cpp b/src/libcamera/pipeline/raspberrypi/raspberrypi.cpp
> index e0132113b072..7dd579090ec6 100644
> --- a/src/libcamera/pipeline/raspberrypi/raspberrypi.cpp
> +++ b/src/libcamera/pipeline/raspberrypi/raspberrypi.cpp
> @@ -65,7 +65,7 @@ double scoreFormat(double desired, double actual)
>  	return score;
>  }
>  
> -V4L2DeviceFormat findBestMode(ImageFormatsMap<V4L2PixelFormat> &formatsMap, const Size &req)
> +V4L2DeviceFormat findBestMode(V4L2VideoDevice::formatsMap &formatsMap, const Size &req)
>  {
>  	double bestScore = 9e9, score;
>  	V4L2DeviceFormat bestMode = {};
> @@ -424,7 +424,8 @@ CameraConfiguration::Status RPiCameraConfiguration::validate()
>  			 * Calculate the best sensor mode we can use based on
>  			 * the user request.
>  			 */
> -			ImageFormatsMap<V4L2PixelFormat> fmts = data_->unicam_[Unicam::Image].dev()->formats();
> +			V4L2VideoDevice::formatsMap fmts =
> +				data_->unicam_[Unicam::Image].dev()->formats();
>  			V4L2DeviceFormat sensorFormat = findBestMode(fmts, cfg.size);
>  			PixelFormat sensorPixFormat = sensorFormat.fourcc.toPixelFormat();
>  			if (cfg.size != sensorFormat.size ||
> @@ -478,7 +479,7 @@ CameraConfiguration::Status RPiCameraConfiguration::validate()
>  		 *
>  		 */
>  		PixelFormat &cfgPixFmt = config_.at(outSize[i].first).pixelFormat;
> -		ImageFormatsMap<V4L2PixelFormat> fmts;
> +		V4L2VideoDevice::formatsMap fmts;
>  
>  		if (i == maxIndex)
>  			fmts = data_->isp_[Isp::Output0].dev()->formats();
> @@ -515,7 +516,7 @@ CameraConfiguration *PipelineHandlerRPi::generateConfiguration(Camera *camera,
>  	RPiCameraData *data = cameraData(camera);
>  	CameraConfiguration *config = new RPiCameraConfiguration(data);
>  	V4L2DeviceFormat sensorFormat;
> -	ImageFormatsMap<V4L2PixelFormat> fmts;
> +	V4L2VideoDevice::formatsMap fmts;
>  
>  	if (roles.empty())
>  		return config;
> @@ -602,7 +603,7 @@ int PipelineHandlerRPi::configure(Camera *camera, CameraConfiguration *config)
>  	}
>  
>  	/* First calculate the best sensor mode we can use based on the user request. */
> -	ImageFormatsMap<V4L2PixelFormat> fmts = data->unicam_[Unicam::Image].dev()->formats();
> +	V4L2VideoDevice::formatsMap fmts = data->unicam_[Unicam::Image].dev()->formats();
>  	V4L2DeviceFormat sensorFormat = findBestMode(fmts, rawStream ? sensorSize : maxSize);
>  
>  	/*
> diff --git a/src/libcamera/pipeline/simple/simple.cpp b/src/libcamera/pipeline/simple/simple.cpp
> index 62906b560d6a..feaa2854f037 100644
> --- a/src/libcamera/pipeline/simple/simple.cpp
> +++ b/src/libcamera/pipeline/simple/simple.cpp
> @@ -275,7 +275,7 @@ int SimpleCameraData::init()
>  			return ret;
>  		}
>  
> -		ImageFormatsMap<V4L2PixelFormat> videoFormats = video_->formats(format.mbus_code);
> +		V4L2VideoDevice::formatsMap videoFormats = video_->formats(format.mbus_code);
>  
>  		LOG(SimplePipeline, Debug)
>  			<< "Adding configuration for " << format.size.toString()
> diff --git a/src/libcamera/pipeline/uvcvideo/uvcvideo.cpp b/src/libcamera/pipeline/uvcvideo/uvcvideo.cpp
> index e1b1ae32b821..6250186e9390 100644
> --- a/src/libcamera/pipeline/uvcvideo/uvcvideo.cpp
> +++ b/src/libcamera/pipeline/uvcvideo/uvcvideo.cpp
> @@ -159,7 +159,7 @@ CameraConfiguration *PipelineHandlerUVC::generateConfiguration(Camera *camera,
>  	if (roles.empty())
>  		return config;
>  
> -	ImageFormatsMap<V4L2PixelFormat> v4l2Formats = data->video_->formats();
> +	V4L2VideoDevice::formatsMap v4l2Formats = data->video_->formats();
>  	std::map<PixelFormat, std::vector<SizeRange>> deviceFormats;
>  	std::transform(v4l2Formats.begin(), v4l2Formats.end(),
>  		       std::inserter(deviceFormats, deviceFormats.begin()),
> diff --git a/src/libcamera/v4l2_subdevice.cpp b/src/libcamera/v4l2_subdevice.cpp
> index 80db4cda82bb..0dd8e8686967 100644
> --- a/src/libcamera/v4l2_subdevice.cpp
> +++ b/src/libcamera/v4l2_subdevice.cpp
> @@ -197,6 +197,11 @@ uint8_t V4L2SubdeviceFormat::bitsPerPixel() const
>   * any device left open will be closed, and any resources released.
>   */
>  
> +/**
> + * \typedef V4L2Subdevice::formatsMap
> + * \brief Map of media bus codes to associated image sizes
> + */
> +
>  /**
>   * \enum V4L2Subdevice::Whence
>   * \brief Specify the type of format for getFormat() and setFormat() operations
> @@ -320,9 +325,9 @@ int V4L2Subdevice::setSelection(unsigned int pad, unsigned int target,
>   *
>   * \return A list of the supported device formats
>   */
> -ImageFormatsMap<uint32_t> V4L2Subdevice::formats(unsigned int pad)
> +V4L2Subdevice::formatsMap V4L2Subdevice::formats(unsigned int pad)
>  {
> -	ImageFormatsMap<uint32_t> formats;
> +	formatsMap formats;
>  
>  	if (pad >= entity_->pads().size()) {
>  		LOG(V4L2, Error) << "Invalid pad: " << pad;
> diff --git a/src/libcamera/v4l2_videodevice.cpp b/src/libcamera/v4l2_videodevice.cpp
> index b264915cb73a..6d48018396c3 100644
> --- a/src/libcamera/v4l2_videodevice.cpp
> +++ b/src/libcamera/v4l2_videodevice.cpp
> @@ -461,6 +461,11 @@ const std::string V4L2DeviceFormat::toString() const
>   * \context This class is \threadbound.
>   */
>  
> +/**
> + * \typedef V4L2VideoDevice::formatsMap
> + * \brief Map of V4l2PixelFormat to associated image sizes
> + */
> +
>  /**
>   * \brief Construct a V4L2VideoDevice
>   * \param[in] deviceNode The file-system path to the video device node
> @@ -925,9 +930,9 @@ int V4L2VideoDevice::setFormatSingleplane(V4L2DeviceFormat *format)
>   *
>   * \return A list of the supported video device formats
>   */
> -ImageFormatsMap<V4L2PixelFormat> V4L2VideoDevice::formats(uint32_t code)
> +V4L2VideoDevice::formatsMap V4L2VideoDevice::formats(uint32_t code)
>  {
> -	ImageFormatsMap<V4L2PixelFormat> formats;
> +	formatsMap formats;
>  
>  	for (V4L2PixelFormat pixelFormat : enumPixelformats(code)) {
>  		std::vector<SizeRange> sizes = enumSizes(pixelFormat);
> diff --git a/test/v4l2_subdevice/list_formats.cpp b/test/v4l2_subdevice/list_formats.cpp
> index adcf5606c961..a32f5c833c28 100644
> --- a/test/v4l2_subdevice/list_formats.cpp
> +++ b/test/v4l2_subdevice/list_formats.cpp
> @@ -48,7 +48,7 @@ void ListFormatsTest::printFormats(unsigned int pad,
>  int ListFormatsTest::run()
>  {
>  	/* List all formats available on existing "Scaler" pads. */
> -	ImageFormatsMap<uint32_t> formats;
> +	V4L2Subdevice::formatsMap formats;
>  
>  	formats = scaler_->formats(0);
>  	if (formats.isEmpty()) {
Jacopo Mondi June 8, 2020, 8:02 a.m. UTC | #2
Hi Laurent,

On Sat, Jun 06, 2020 at 02:31:39AM +0300, Laurent Pinchart wrote:
> Hi Jacopo,
>
> Thank you for the patch.
>
> On Fri, May 29, 2020 at 01:03:34PM +0200, Jacopo Mondi wrote:
> > Provide a type definition in the V4L2VideoDevice and V4L2Subdevice
> > classes to shorten the ImageFormatsMap<V4L2PixelFormat> and
> > ImageFormatsMap<uint32_t> spcialization names.
> >
> > The new types reads as V4L2VideoDevice::formatsMap and
> > V4L2Subdevice::formatsMap respectively.
>
> That should be FormatsMap (or actually even Formats).

I'm still not satisfied with any name we tried for this construct. But
shorting it down to ::Format is better.

>
> I would split this in two, with the V4L2VideoDevice part squashed with
> 2/5, as otherwise you essentially update the same code twice, oncee to
> move from an explicit std::map to ImageFormats<V4L2PixelFormat>, and a
> second time to move from ImageFormats<V4L2PixelFormat> to
> V4L2VideoDevice::FormatsMap. The V4L2Subdevice part could then be moved
> after patch 1/5 and before patch 2/5.

This was intentional to ease review, but I can squash them and provide
typenames when moving users of V4L2VideoDevice from std::map to
ImageFormat

Thanks
  j

>
> > Signed-off-by: Jacopo Mondi <jacopo@jmondi.org>
> > ---
> >  include/libcamera/internal/camera_sensor.h         |  6 ++----
> >  include/libcamera/internal/v4l2_subdevice.h        |  4 +++-
> >  include/libcamera/internal/v4l2_videodevice.h      |  4 +++-
> >  src/libcamera/pipeline/raspberrypi/raspberrypi.cpp | 11 ++++++-----
> >  src/libcamera/pipeline/simple/simple.cpp           |  2 +-
> >  src/libcamera/pipeline/uvcvideo/uvcvideo.cpp       |  2 +-
> >  src/libcamera/v4l2_subdevice.cpp                   |  9 +++++++--
> >  src/libcamera/v4l2_videodevice.cpp                 |  9 +++++++--
> >  test/v4l2_subdevice/list_formats.cpp               |  2 +-
> >  9 files changed, 31 insertions(+), 18 deletions(-)
> >
> > diff --git a/include/libcamera/internal/camera_sensor.h b/include/libcamera/internal/camera_sensor.h
> > index 52eedb9ef880..1353edb1da28 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_;
> >
> > -	ImageFormatsMap<uint32_t> formats_;
> > +	V4L2Subdevice::formatsMap 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 c9aa3428f93c..06f10d7b4c5d 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 formatsMap = ImageFormatsMap<uint32_t>;
> > +
> >  	enum Whence {
> >  		ActiveFormat,
> >  		TryFormat,
> > @@ -51,7 +53,7 @@ public:
> >  	int setSelection(unsigned int pad, unsigned int target,
> >  			 Rectangle *rect);
> >
> > -	ImageFormatsMap<uint32_t> formats(unsigned int pad);
> > +	formatsMap formats(unsigned int pad);
> >
> >  	int getFormat(unsigned int pad, V4L2SubdeviceFormat *format,
> >  		      Whence whence = ActiveFormat);
> > diff --git a/include/libcamera/internal/v4l2_videodevice.h b/include/libcamera/internal/v4l2_videodevice.h
> > index 02d101db0b2b..9632ad71b988 100644
> > --- a/include/libcamera/internal/v4l2_videodevice.h
> > +++ b/include/libcamera/internal/v4l2_videodevice.h
> > @@ -168,6 +168,8 @@ public:
> >  class V4L2VideoDevice : public V4L2Device
> >  {
> >  public:
> > +	using formatsMap = ImageFormatsMap<V4L2PixelFormat>;
> > +
> >  	explicit V4L2VideoDevice(const std::string &deviceNode);
> >  	explicit V4L2VideoDevice(const MediaEntity *entity);
> >  	V4L2VideoDevice(const V4L2VideoDevice &) = delete;
> > @@ -187,7 +189,7 @@ public:
> >
> >  	int getFormat(V4L2DeviceFormat *format);
> >  	int setFormat(V4L2DeviceFormat *format);
> > -	ImageFormatsMap<V4L2PixelFormat> formats(uint32_t code = 0);
> > +	formatsMap formats(uint32_t code = 0);
> >
> >  	int setSelection(unsigned int target, Rectangle *rect);
> >
> > diff --git a/src/libcamera/pipeline/raspberrypi/raspberrypi.cpp b/src/libcamera/pipeline/raspberrypi/raspberrypi.cpp
> > index e0132113b072..7dd579090ec6 100644
> > --- a/src/libcamera/pipeline/raspberrypi/raspberrypi.cpp
> > +++ b/src/libcamera/pipeline/raspberrypi/raspberrypi.cpp
> > @@ -65,7 +65,7 @@ double scoreFormat(double desired, double actual)
> >  	return score;
> >  }
> >
> > -V4L2DeviceFormat findBestMode(ImageFormatsMap<V4L2PixelFormat> &formatsMap, const Size &req)
> > +V4L2DeviceFormat findBestMode(V4L2VideoDevice::formatsMap &formatsMap, const Size &req)
> >  {
> >  	double bestScore = 9e9, score;
> >  	V4L2DeviceFormat bestMode = {};
> > @@ -424,7 +424,8 @@ CameraConfiguration::Status RPiCameraConfiguration::validate()
> >  			 * Calculate the best sensor mode we can use based on
> >  			 * the user request.
> >  			 */
> > -			ImageFormatsMap<V4L2PixelFormat> fmts = data_->unicam_[Unicam::Image].dev()->formats();
> > +			V4L2VideoDevice::formatsMap fmts =
> > +				data_->unicam_[Unicam::Image].dev()->formats();
> >  			V4L2DeviceFormat sensorFormat = findBestMode(fmts, cfg.size);
> >  			PixelFormat sensorPixFormat = sensorFormat.fourcc.toPixelFormat();
> >  			if (cfg.size != sensorFormat.size ||
> > @@ -478,7 +479,7 @@ CameraConfiguration::Status RPiCameraConfiguration::validate()
> >  		 *
> >  		 */
> >  		PixelFormat &cfgPixFmt = config_.at(outSize[i].first).pixelFormat;
> > -		ImageFormatsMap<V4L2PixelFormat> fmts;
> > +		V4L2VideoDevice::formatsMap fmts;
> >
> >  		if (i == maxIndex)
> >  			fmts = data_->isp_[Isp::Output0].dev()->formats();
> > @@ -515,7 +516,7 @@ CameraConfiguration *PipelineHandlerRPi::generateConfiguration(Camera *camera,
> >  	RPiCameraData *data = cameraData(camera);
> >  	CameraConfiguration *config = new RPiCameraConfiguration(data);
> >  	V4L2DeviceFormat sensorFormat;
> > -	ImageFormatsMap<V4L2PixelFormat> fmts;
> > +	V4L2VideoDevice::formatsMap fmts;
> >
> >  	if (roles.empty())
> >  		return config;
> > @@ -602,7 +603,7 @@ int PipelineHandlerRPi::configure(Camera *camera, CameraConfiguration *config)
> >  	}
> >
> >  	/* First calculate the best sensor mode we can use based on the user request. */
> > -	ImageFormatsMap<V4L2PixelFormat> fmts = data->unicam_[Unicam::Image].dev()->formats();
> > +	V4L2VideoDevice::formatsMap fmts = data->unicam_[Unicam::Image].dev()->formats();
> >  	V4L2DeviceFormat sensorFormat = findBestMode(fmts, rawStream ? sensorSize : maxSize);
> >
> >  	/*
> > diff --git a/src/libcamera/pipeline/simple/simple.cpp b/src/libcamera/pipeline/simple/simple.cpp
> > index 62906b560d6a..feaa2854f037 100644
> > --- a/src/libcamera/pipeline/simple/simple.cpp
> > +++ b/src/libcamera/pipeline/simple/simple.cpp
> > @@ -275,7 +275,7 @@ int SimpleCameraData::init()
> >  			return ret;
> >  		}
> >
> > -		ImageFormatsMap<V4L2PixelFormat> videoFormats = video_->formats(format.mbus_code);
> > +		V4L2VideoDevice::formatsMap videoFormats = video_->formats(format.mbus_code);
> >
> >  		LOG(SimplePipeline, Debug)
> >  			<< "Adding configuration for " << format.size.toString()
> > diff --git a/src/libcamera/pipeline/uvcvideo/uvcvideo.cpp b/src/libcamera/pipeline/uvcvideo/uvcvideo.cpp
> > index e1b1ae32b821..6250186e9390 100644
> > --- a/src/libcamera/pipeline/uvcvideo/uvcvideo.cpp
> > +++ b/src/libcamera/pipeline/uvcvideo/uvcvideo.cpp
> > @@ -159,7 +159,7 @@ CameraConfiguration *PipelineHandlerUVC::generateConfiguration(Camera *camera,
> >  	if (roles.empty())
> >  		return config;
> >
> > -	ImageFormatsMap<V4L2PixelFormat> v4l2Formats = data->video_->formats();
> > +	V4L2VideoDevice::formatsMap v4l2Formats = data->video_->formats();
> >  	std::map<PixelFormat, std::vector<SizeRange>> deviceFormats;
> >  	std::transform(v4l2Formats.begin(), v4l2Formats.end(),
> >  		       std::inserter(deviceFormats, deviceFormats.begin()),
> > diff --git a/src/libcamera/v4l2_subdevice.cpp b/src/libcamera/v4l2_subdevice.cpp
> > index 80db4cda82bb..0dd8e8686967 100644
> > --- a/src/libcamera/v4l2_subdevice.cpp
> > +++ b/src/libcamera/v4l2_subdevice.cpp
> > @@ -197,6 +197,11 @@ uint8_t V4L2SubdeviceFormat::bitsPerPixel() const
> >   * any device left open will be closed, and any resources released.
> >   */
> >
> > +/**
> > + * \typedef V4L2Subdevice::formatsMap
> > + * \brief Map of media bus codes to associated image sizes
> > + */
> > +
> >  /**
> >   * \enum V4L2Subdevice::Whence
> >   * \brief Specify the type of format for getFormat() and setFormat() operations
> > @@ -320,9 +325,9 @@ int V4L2Subdevice::setSelection(unsigned int pad, unsigned int target,
> >   *
> >   * \return A list of the supported device formats
> >   */
> > -ImageFormatsMap<uint32_t> V4L2Subdevice::formats(unsigned int pad)
> > +V4L2Subdevice::formatsMap V4L2Subdevice::formats(unsigned int pad)
> >  {
> > -	ImageFormatsMap<uint32_t> formats;
> > +	formatsMap formats;
> >
> >  	if (pad >= entity_->pads().size()) {
> >  		LOG(V4L2, Error) << "Invalid pad: " << pad;
> > diff --git a/src/libcamera/v4l2_videodevice.cpp b/src/libcamera/v4l2_videodevice.cpp
> > index b264915cb73a..6d48018396c3 100644
> > --- a/src/libcamera/v4l2_videodevice.cpp
> > +++ b/src/libcamera/v4l2_videodevice.cpp
> > @@ -461,6 +461,11 @@ const std::string V4L2DeviceFormat::toString() const
> >   * \context This class is \threadbound.
> >   */
> >
> > +/**
> > + * \typedef V4L2VideoDevice::formatsMap
> > + * \brief Map of V4l2PixelFormat to associated image sizes
> > + */
> > +
> >  /**
> >   * \brief Construct a V4L2VideoDevice
> >   * \param[in] deviceNode The file-system path to the video device node
> > @@ -925,9 +930,9 @@ int V4L2VideoDevice::setFormatSingleplane(V4L2DeviceFormat *format)
> >   *
> >   * \return A list of the supported video device formats
> >   */
> > -ImageFormatsMap<V4L2PixelFormat> V4L2VideoDevice::formats(uint32_t code)
> > +V4L2VideoDevice::formatsMap V4L2VideoDevice::formats(uint32_t code)
> >  {
> > -	ImageFormatsMap<V4L2PixelFormat> formats;
> > +	formatsMap formats;
> >
> >  	for (V4L2PixelFormat pixelFormat : enumPixelformats(code)) {
> >  		std::vector<SizeRange> sizes = enumSizes(pixelFormat);
> > diff --git a/test/v4l2_subdevice/list_formats.cpp b/test/v4l2_subdevice/list_formats.cpp
> > index adcf5606c961..a32f5c833c28 100644
> > --- a/test/v4l2_subdevice/list_formats.cpp
> > +++ b/test/v4l2_subdevice/list_formats.cpp
> > @@ -48,7 +48,7 @@ void ListFormatsTest::printFormats(unsigned int pad,
> >  int ListFormatsTest::run()
> >  {
> >  	/* List all formats available on existing "Scaler" pads. */
> > -	ImageFormatsMap<uint32_t> formats;
> > +	V4L2Subdevice::formatsMap formats;
> >
> >  	formats = scaler_->formats(0);
> >  	if (formats.isEmpty()) {
>
> --
> Regards,
>
> Laurent Pinchart
Laurent Pinchart June 8, 2020, 1:08 p.m. UTC | #3
Hi Jacopo,

On Mon, Jun 08, 2020 at 10:02:58AM +0200, Jacopo Mondi wrote:
> On Sat, Jun 06, 2020 at 02:31:39AM +0300, Laurent Pinchart wrote:
> > On Fri, May 29, 2020 at 01:03:34PM +0200, Jacopo Mondi wrote:
> > > Provide a type definition in the V4L2VideoDevice and V4L2Subdevice
> > > classes to shorten the ImageFormatsMap<V4L2PixelFormat> and
> > > ImageFormatsMap<uint32_t> spcialization names.
> > >
> > > The new types reads as V4L2VideoDevice::formatsMap and
> > > V4L2Subdevice::formatsMap respectively.
> >
> > That should be FormatsMap (or actually even Formats).
> 
> I'm still not satisfied with any name we tried for this construct. But
> shorting it down to ::Format is better.

Do you mean Formats ?

> > I would split this in two, with the V4L2VideoDevice part squashed with
> > 2/5, as otherwise you essentially update the same code twice, oncee to
> > move from an explicit std::map to ImageFormats<V4L2PixelFormat>, and a
> > second time to move from ImageFormats<V4L2PixelFormat> to
> > V4L2VideoDevice::FormatsMap. The V4L2Subdevice part could then be moved
> > after patch 1/5 and before patch 2/5.
> 
> This was intentional to ease review, but I can squash them and provide
> typenames when moving users of V4L2VideoDevice from std::map to
> ImageFormat

I think that will actually ease review for me :-) The
V4L2Subdevice::Formats should of course still be introduced in a
separate patch.

> > > Signed-off-by: Jacopo Mondi <jacopo@jmondi.org>
> > > ---
> > >  include/libcamera/internal/camera_sensor.h         |  6 ++----
> > >  include/libcamera/internal/v4l2_subdevice.h        |  4 +++-
> > >  include/libcamera/internal/v4l2_videodevice.h      |  4 +++-
> > >  src/libcamera/pipeline/raspberrypi/raspberrypi.cpp | 11 ++++++-----
> > >  src/libcamera/pipeline/simple/simple.cpp           |  2 +-
> > >  src/libcamera/pipeline/uvcvideo/uvcvideo.cpp       |  2 +-
> > >  src/libcamera/v4l2_subdevice.cpp                   |  9 +++++++--
> > >  src/libcamera/v4l2_videodevice.cpp                 |  9 +++++++--
> > >  test/v4l2_subdevice/list_formats.cpp               |  2 +-
> > >  9 files changed, 31 insertions(+), 18 deletions(-)
> > >
> > > diff --git a/include/libcamera/internal/camera_sensor.h b/include/libcamera/internal/camera_sensor.h
> > > index 52eedb9ef880..1353edb1da28 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_;
> > >
> > > -	ImageFormatsMap<uint32_t> formats_;
> > > +	V4L2Subdevice::formatsMap 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 c9aa3428f93c..06f10d7b4c5d 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 formatsMap = ImageFormatsMap<uint32_t>;
> > > +
> > >  	enum Whence {
> > >  		ActiveFormat,
> > >  		TryFormat,
> > > @@ -51,7 +53,7 @@ public:
> > >  	int setSelection(unsigned int pad, unsigned int target,
> > >  			 Rectangle *rect);
> > >
> > > -	ImageFormatsMap<uint32_t> formats(unsigned int pad);
> > > +	formatsMap formats(unsigned int pad);
> > >
> > >  	int getFormat(unsigned int pad, V4L2SubdeviceFormat *format,
> > >  		      Whence whence = ActiveFormat);
> > > diff --git a/include/libcamera/internal/v4l2_videodevice.h b/include/libcamera/internal/v4l2_videodevice.h
> > > index 02d101db0b2b..9632ad71b988 100644
> > > --- a/include/libcamera/internal/v4l2_videodevice.h
> > > +++ b/include/libcamera/internal/v4l2_videodevice.h
> > > @@ -168,6 +168,8 @@ public:
> > >  class V4L2VideoDevice : public V4L2Device
> > >  {
> > >  public:
> > > +	using formatsMap = ImageFormatsMap<V4L2PixelFormat>;
> > > +
> > >  	explicit V4L2VideoDevice(const std::string &deviceNode);
> > >  	explicit V4L2VideoDevice(const MediaEntity *entity);
> > >  	V4L2VideoDevice(const V4L2VideoDevice &) = delete;
> > > @@ -187,7 +189,7 @@ public:
> > >
> > >  	int getFormat(V4L2DeviceFormat *format);
> > >  	int setFormat(V4L2DeviceFormat *format);
> > > -	ImageFormatsMap<V4L2PixelFormat> formats(uint32_t code = 0);
> > > +	formatsMap formats(uint32_t code = 0);
> > >
> > >  	int setSelection(unsigned int target, Rectangle *rect);
> > >
> > > diff --git a/src/libcamera/pipeline/raspberrypi/raspberrypi.cpp b/src/libcamera/pipeline/raspberrypi/raspberrypi.cpp
> > > index e0132113b072..7dd579090ec6 100644
> > > --- a/src/libcamera/pipeline/raspberrypi/raspberrypi.cpp
> > > +++ b/src/libcamera/pipeline/raspberrypi/raspberrypi.cpp
> > > @@ -65,7 +65,7 @@ double scoreFormat(double desired, double actual)
> > >  	return score;
> > >  }
> > >
> > > -V4L2DeviceFormat findBestMode(ImageFormatsMap<V4L2PixelFormat> &formatsMap, const Size &req)
> > > +V4L2DeviceFormat findBestMode(V4L2VideoDevice::formatsMap &formatsMap, const Size &req)
> > >  {
> > >  	double bestScore = 9e9, score;
> > >  	V4L2DeviceFormat bestMode = {};
> > > @@ -424,7 +424,8 @@ CameraConfiguration::Status RPiCameraConfiguration::validate()
> > >  			 * Calculate the best sensor mode we can use based on
> > >  			 * the user request.
> > >  			 */
> > > -			ImageFormatsMap<V4L2PixelFormat> fmts = data_->unicam_[Unicam::Image].dev()->formats();
> > > +			V4L2VideoDevice::formatsMap fmts =
> > > +				data_->unicam_[Unicam::Image].dev()->formats();
> > >  			V4L2DeviceFormat sensorFormat = findBestMode(fmts, cfg.size);
> > >  			PixelFormat sensorPixFormat = sensorFormat.fourcc.toPixelFormat();
> > >  			if (cfg.size != sensorFormat.size ||
> > > @@ -478,7 +479,7 @@ CameraConfiguration::Status RPiCameraConfiguration::validate()
> > >  		 *
> > >  		 */
> > >  		PixelFormat &cfgPixFmt = config_.at(outSize[i].first).pixelFormat;
> > > -		ImageFormatsMap<V4L2PixelFormat> fmts;
> > > +		V4L2VideoDevice::formatsMap fmts;
> > >
> > >  		if (i == maxIndex)
> > >  			fmts = data_->isp_[Isp::Output0].dev()->formats();
> > > @@ -515,7 +516,7 @@ CameraConfiguration *PipelineHandlerRPi::generateConfiguration(Camera *camera,
> > >  	RPiCameraData *data = cameraData(camera);
> > >  	CameraConfiguration *config = new RPiCameraConfiguration(data);
> > >  	V4L2DeviceFormat sensorFormat;
> > > -	ImageFormatsMap<V4L2PixelFormat> fmts;
> > > +	V4L2VideoDevice::formatsMap fmts;
> > >
> > >  	if (roles.empty())
> > >  		return config;
> > > @@ -602,7 +603,7 @@ int PipelineHandlerRPi::configure(Camera *camera, CameraConfiguration *config)
> > >  	}
> > >
> > >  	/* First calculate the best sensor mode we can use based on the user request. */
> > > -	ImageFormatsMap<V4L2PixelFormat> fmts = data->unicam_[Unicam::Image].dev()->formats();
> > > +	V4L2VideoDevice::formatsMap fmts = data->unicam_[Unicam::Image].dev()->formats();
> > >  	V4L2DeviceFormat sensorFormat = findBestMode(fmts, rawStream ? sensorSize : maxSize);
> > >
> > >  	/*
> > > diff --git a/src/libcamera/pipeline/simple/simple.cpp b/src/libcamera/pipeline/simple/simple.cpp
> > > index 62906b560d6a..feaa2854f037 100644
> > > --- a/src/libcamera/pipeline/simple/simple.cpp
> > > +++ b/src/libcamera/pipeline/simple/simple.cpp
> > > @@ -275,7 +275,7 @@ int SimpleCameraData::init()
> > >  			return ret;
> > >  		}
> > >
> > > -		ImageFormatsMap<V4L2PixelFormat> videoFormats = video_->formats(format.mbus_code);
> > > +		V4L2VideoDevice::formatsMap videoFormats = video_->formats(format.mbus_code);
> > >
> > >  		LOG(SimplePipeline, Debug)
> > >  			<< "Adding configuration for " << format.size.toString()
> > > diff --git a/src/libcamera/pipeline/uvcvideo/uvcvideo.cpp b/src/libcamera/pipeline/uvcvideo/uvcvideo.cpp
> > > index e1b1ae32b821..6250186e9390 100644
> > > --- a/src/libcamera/pipeline/uvcvideo/uvcvideo.cpp
> > > +++ b/src/libcamera/pipeline/uvcvideo/uvcvideo.cpp
> > > @@ -159,7 +159,7 @@ CameraConfiguration *PipelineHandlerUVC::generateConfiguration(Camera *camera,
> > >  	if (roles.empty())
> > >  		return config;
> > >
> > > -	ImageFormatsMap<V4L2PixelFormat> v4l2Formats = data->video_->formats();
> > > +	V4L2VideoDevice::formatsMap v4l2Formats = data->video_->formats();
> > >  	std::map<PixelFormat, std::vector<SizeRange>> deviceFormats;
> > >  	std::transform(v4l2Formats.begin(), v4l2Formats.end(),
> > >  		       std::inserter(deviceFormats, deviceFormats.begin()),
> > > diff --git a/src/libcamera/v4l2_subdevice.cpp b/src/libcamera/v4l2_subdevice.cpp
> > > index 80db4cda82bb..0dd8e8686967 100644
> > > --- a/src/libcamera/v4l2_subdevice.cpp
> > > +++ b/src/libcamera/v4l2_subdevice.cpp
> > > @@ -197,6 +197,11 @@ uint8_t V4L2SubdeviceFormat::bitsPerPixel() const
> > >   * any device left open will be closed, and any resources released.
> > >   */
> > >
> > > +/**
> > > + * \typedef V4L2Subdevice::formatsMap
> > > + * \brief Map of media bus codes to associated image sizes
> > > + */
> > > +
> > >  /**
> > >   * \enum V4L2Subdevice::Whence
> > >   * \brief Specify the type of format for getFormat() and setFormat() operations
> > > @@ -320,9 +325,9 @@ int V4L2Subdevice::setSelection(unsigned int pad, unsigned int target,
> > >   *
> > >   * \return A list of the supported device formats
> > >   */
> > > -ImageFormatsMap<uint32_t> V4L2Subdevice::formats(unsigned int pad)
> > > +V4L2Subdevice::formatsMap V4L2Subdevice::formats(unsigned int pad)
> > >  {
> > > -	ImageFormatsMap<uint32_t> formats;
> > > +	formatsMap formats;
> > >
> > >  	if (pad >= entity_->pads().size()) {
> > >  		LOG(V4L2, Error) << "Invalid pad: " << pad;
> > > diff --git a/src/libcamera/v4l2_videodevice.cpp b/src/libcamera/v4l2_videodevice.cpp
> > > index b264915cb73a..6d48018396c3 100644
> > > --- a/src/libcamera/v4l2_videodevice.cpp
> > > +++ b/src/libcamera/v4l2_videodevice.cpp
> > > @@ -461,6 +461,11 @@ const std::string V4L2DeviceFormat::toString() const
> > >   * \context This class is \threadbound.
> > >   */
> > >
> > > +/**
> > > + * \typedef V4L2VideoDevice::formatsMap
> > > + * \brief Map of V4l2PixelFormat to associated image sizes
> > > + */
> > > +
> > >  /**
> > >   * \brief Construct a V4L2VideoDevice
> > >   * \param[in] deviceNode The file-system path to the video device node
> > > @@ -925,9 +930,9 @@ int V4L2VideoDevice::setFormatSingleplane(V4L2DeviceFormat *format)
> > >   *
> > >   * \return A list of the supported video device formats
> > >   */
> > > -ImageFormatsMap<V4L2PixelFormat> V4L2VideoDevice::formats(uint32_t code)
> > > +V4L2VideoDevice::formatsMap V4L2VideoDevice::formats(uint32_t code)
> > >  {
> > > -	ImageFormatsMap<V4L2PixelFormat> formats;
> > > +	formatsMap formats;
> > >
> > >  	for (V4L2PixelFormat pixelFormat : enumPixelformats(code)) {
> > >  		std::vector<SizeRange> sizes = enumSizes(pixelFormat);
> > > diff --git a/test/v4l2_subdevice/list_formats.cpp b/test/v4l2_subdevice/list_formats.cpp
> > > index adcf5606c961..a32f5c833c28 100644
> > > --- a/test/v4l2_subdevice/list_formats.cpp
> > > +++ b/test/v4l2_subdevice/list_formats.cpp
> > > @@ -48,7 +48,7 @@ void ListFormatsTest::printFormats(unsigned int pad,
> > >  int ListFormatsTest::run()
> > >  {
> > >  	/* List all formats available on existing "Scaler" pads. */
> > > -	ImageFormatsMap<uint32_t> formats;
> > > +	V4L2Subdevice::formatsMap formats;
> > >
> > >  	formats = scaler_->formats(0);
> > >  	if (formats.isEmpty()) {
> >
> > --
> > Regards,
> >
> > Laurent Pinchart

Patch

diff --git a/include/libcamera/internal/camera_sensor.h b/include/libcamera/internal/camera_sensor.h
index 52eedb9ef880..1353edb1da28 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_;
 
-	ImageFormatsMap<uint32_t> formats_;
+	V4L2Subdevice::formatsMap 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 c9aa3428f93c..06f10d7b4c5d 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 formatsMap = ImageFormatsMap<uint32_t>;
+
 	enum Whence {
 		ActiveFormat,
 		TryFormat,
@@ -51,7 +53,7 @@  public:
 	int setSelection(unsigned int pad, unsigned int target,
 			 Rectangle *rect);
 
-	ImageFormatsMap<uint32_t> formats(unsigned int pad);
+	formatsMap formats(unsigned int pad);
 
 	int getFormat(unsigned int pad, V4L2SubdeviceFormat *format,
 		      Whence whence = ActiveFormat);
diff --git a/include/libcamera/internal/v4l2_videodevice.h b/include/libcamera/internal/v4l2_videodevice.h
index 02d101db0b2b..9632ad71b988 100644
--- a/include/libcamera/internal/v4l2_videodevice.h
+++ b/include/libcamera/internal/v4l2_videodevice.h
@@ -168,6 +168,8 @@  public:
 class V4L2VideoDevice : public V4L2Device
 {
 public:
+	using formatsMap = ImageFormatsMap<V4L2PixelFormat>;
+
 	explicit V4L2VideoDevice(const std::string &deviceNode);
 	explicit V4L2VideoDevice(const MediaEntity *entity);
 	V4L2VideoDevice(const V4L2VideoDevice &) = delete;
@@ -187,7 +189,7 @@  public:
 
 	int getFormat(V4L2DeviceFormat *format);
 	int setFormat(V4L2DeviceFormat *format);
-	ImageFormatsMap<V4L2PixelFormat> formats(uint32_t code = 0);
+	formatsMap formats(uint32_t code = 0);
 
 	int setSelection(unsigned int target, Rectangle *rect);
 
diff --git a/src/libcamera/pipeline/raspberrypi/raspberrypi.cpp b/src/libcamera/pipeline/raspberrypi/raspberrypi.cpp
index e0132113b072..7dd579090ec6 100644
--- a/src/libcamera/pipeline/raspberrypi/raspberrypi.cpp
+++ b/src/libcamera/pipeline/raspberrypi/raspberrypi.cpp
@@ -65,7 +65,7 @@  double scoreFormat(double desired, double actual)
 	return score;
 }
 
-V4L2DeviceFormat findBestMode(ImageFormatsMap<V4L2PixelFormat> &formatsMap, const Size &req)
+V4L2DeviceFormat findBestMode(V4L2VideoDevice::formatsMap &formatsMap, const Size &req)
 {
 	double bestScore = 9e9, score;
 	V4L2DeviceFormat bestMode = {};
@@ -424,7 +424,8 @@  CameraConfiguration::Status RPiCameraConfiguration::validate()
 			 * Calculate the best sensor mode we can use based on
 			 * the user request.
 			 */
-			ImageFormatsMap<V4L2PixelFormat> fmts = data_->unicam_[Unicam::Image].dev()->formats();
+			V4L2VideoDevice::formatsMap fmts =
+				data_->unicam_[Unicam::Image].dev()->formats();
 			V4L2DeviceFormat sensorFormat = findBestMode(fmts, cfg.size);
 			PixelFormat sensorPixFormat = sensorFormat.fourcc.toPixelFormat();
 			if (cfg.size != sensorFormat.size ||
@@ -478,7 +479,7 @@  CameraConfiguration::Status RPiCameraConfiguration::validate()
 		 *
 		 */
 		PixelFormat &cfgPixFmt = config_.at(outSize[i].first).pixelFormat;
-		ImageFormatsMap<V4L2PixelFormat> fmts;
+		V4L2VideoDevice::formatsMap fmts;
 
 		if (i == maxIndex)
 			fmts = data_->isp_[Isp::Output0].dev()->formats();
@@ -515,7 +516,7 @@  CameraConfiguration *PipelineHandlerRPi::generateConfiguration(Camera *camera,
 	RPiCameraData *data = cameraData(camera);
 	CameraConfiguration *config = new RPiCameraConfiguration(data);
 	V4L2DeviceFormat sensorFormat;
-	ImageFormatsMap<V4L2PixelFormat> fmts;
+	V4L2VideoDevice::formatsMap fmts;
 
 	if (roles.empty())
 		return config;
@@ -602,7 +603,7 @@  int PipelineHandlerRPi::configure(Camera *camera, CameraConfiguration *config)
 	}
 
 	/* First calculate the best sensor mode we can use based on the user request. */
-	ImageFormatsMap<V4L2PixelFormat> fmts = data->unicam_[Unicam::Image].dev()->formats();
+	V4L2VideoDevice::formatsMap fmts = data->unicam_[Unicam::Image].dev()->formats();
 	V4L2DeviceFormat sensorFormat = findBestMode(fmts, rawStream ? sensorSize : maxSize);
 
 	/*
diff --git a/src/libcamera/pipeline/simple/simple.cpp b/src/libcamera/pipeline/simple/simple.cpp
index 62906b560d6a..feaa2854f037 100644
--- a/src/libcamera/pipeline/simple/simple.cpp
+++ b/src/libcamera/pipeline/simple/simple.cpp
@@ -275,7 +275,7 @@  int SimpleCameraData::init()
 			return ret;
 		}
 
-		ImageFormatsMap<V4L2PixelFormat> videoFormats = video_->formats(format.mbus_code);
+		V4L2VideoDevice::formatsMap videoFormats = video_->formats(format.mbus_code);
 
 		LOG(SimplePipeline, Debug)
 			<< "Adding configuration for " << format.size.toString()
diff --git a/src/libcamera/pipeline/uvcvideo/uvcvideo.cpp b/src/libcamera/pipeline/uvcvideo/uvcvideo.cpp
index e1b1ae32b821..6250186e9390 100644
--- a/src/libcamera/pipeline/uvcvideo/uvcvideo.cpp
+++ b/src/libcamera/pipeline/uvcvideo/uvcvideo.cpp
@@ -159,7 +159,7 @@  CameraConfiguration *PipelineHandlerUVC::generateConfiguration(Camera *camera,
 	if (roles.empty())
 		return config;
 
-	ImageFormatsMap<V4L2PixelFormat> v4l2Formats = data->video_->formats();
+	V4L2VideoDevice::formatsMap v4l2Formats = data->video_->formats();
 	std::map<PixelFormat, std::vector<SizeRange>> deviceFormats;
 	std::transform(v4l2Formats.begin(), v4l2Formats.end(),
 		       std::inserter(deviceFormats, deviceFormats.begin()),
diff --git a/src/libcamera/v4l2_subdevice.cpp b/src/libcamera/v4l2_subdevice.cpp
index 80db4cda82bb..0dd8e8686967 100644
--- a/src/libcamera/v4l2_subdevice.cpp
+++ b/src/libcamera/v4l2_subdevice.cpp
@@ -197,6 +197,11 @@  uint8_t V4L2SubdeviceFormat::bitsPerPixel() const
  * any device left open will be closed, and any resources released.
  */
 
+/**
+ * \typedef V4L2Subdevice::formatsMap
+ * \brief Map of media bus codes to associated image sizes
+ */
+
 /**
  * \enum V4L2Subdevice::Whence
  * \brief Specify the type of format for getFormat() and setFormat() operations
@@ -320,9 +325,9 @@  int V4L2Subdevice::setSelection(unsigned int pad, unsigned int target,
  *
  * \return A list of the supported device formats
  */
-ImageFormatsMap<uint32_t> V4L2Subdevice::formats(unsigned int pad)
+V4L2Subdevice::formatsMap V4L2Subdevice::formats(unsigned int pad)
 {
-	ImageFormatsMap<uint32_t> formats;
+	formatsMap formats;
 
 	if (pad >= entity_->pads().size()) {
 		LOG(V4L2, Error) << "Invalid pad: " << pad;
diff --git a/src/libcamera/v4l2_videodevice.cpp b/src/libcamera/v4l2_videodevice.cpp
index b264915cb73a..6d48018396c3 100644
--- a/src/libcamera/v4l2_videodevice.cpp
+++ b/src/libcamera/v4l2_videodevice.cpp
@@ -461,6 +461,11 @@  const std::string V4L2DeviceFormat::toString() const
  * \context This class is \threadbound.
  */
 
+/**
+ * \typedef V4L2VideoDevice::formatsMap
+ * \brief Map of V4l2PixelFormat to associated image sizes
+ */
+
 /**
  * \brief Construct a V4L2VideoDevice
  * \param[in] deviceNode The file-system path to the video device node
@@ -925,9 +930,9 @@  int V4L2VideoDevice::setFormatSingleplane(V4L2DeviceFormat *format)
  *
  * \return A list of the supported video device formats
  */
-ImageFormatsMap<V4L2PixelFormat> V4L2VideoDevice::formats(uint32_t code)
+V4L2VideoDevice::formatsMap V4L2VideoDevice::formats(uint32_t code)
 {
-	ImageFormatsMap<V4L2PixelFormat> formats;
+	formatsMap formats;
 
 	for (V4L2PixelFormat pixelFormat : enumPixelformats(code)) {
 		std::vector<SizeRange> sizes = enumSizes(pixelFormat);
diff --git a/test/v4l2_subdevice/list_formats.cpp b/test/v4l2_subdevice/list_formats.cpp
index adcf5606c961..a32f5c833c28 100644
--- a/test/v4l2_subdevice/list_formats.cpp
+++ b/test/v4l2_subdevice/list_formats.cpp
@@ -48,7 +48,7 @@  void ListFormatsTest::printFormats(unsigned int pad,
 int ListFormatsTest::run()
 {
 	/* List all formats available on existing "Scaler" pads. */
-	ImageFormatsMap<uint32_t> formats;
+	V4L2Subdevice::formatsMap formats;
 
 	formats = scaler_->formats(0);
 	if (formats.isEmpty()) {