[libcamera-devel] [PATCH v3 08/16] libcamera: v4l2_subdevice: Replace FormatEnum with ImageFormats

Laurent Pinchart laurent.pinchart at ideasonboard.com
Wed Jun 19 00:59:42 CEST 2019


Hi Niklas,

Thank you for the patch.

On Sun, Jun 16, 2019 at 03:33:54PM +0200, Niklas Söderlund wrote:
> Replace all usage of FormatEnum with ImageFormats and completely
> remove FormatEnum which is no longer needed.
> 
> Signed-off-by: Niklas Söderlund <niklas.soderlund at ragnatech.se>
> ---
>  src/libcamera/camera_sensor.cpp        | 14 ++++++--------
>  src/libcamera/formats.cpp              | 10 ----------
>  src/libcamera/include/formats.h        |  2 --
>  src/libcamera/include/v4l2_subdevice.h |  2 +-
>  src/libcamera/v4l2_subdevice.cpp       | 26 +++++++++++++-------------
>  test/v4l2_subdevice/list_formats.cpp   | 16 ++++++++--------
>  6 files changed, 28 insertions(+), 42 deletions(-)
> 
> diff --git a/src/libcamera/camera_sensor.cpp b/src/libcamera/camera_sensor.cpp
> index cb6649efac3ff794..a804a68c9d91cda6 100644
> --- a/src/libcamera/camera_sensor.cpp
> +++ b/src/libcamera/camera_sensor.cpp
> @@ -90,27 +90,25 @@ int CameraSensor::init()
>  		return ret;
>  
>  	/* Enumerate and cache media bus codes and sizes. */
> -	const FormatEnum formats = subdev_->formats(0);
> -	if (formats.empty()) {
> +	const ImageFormats formats = subdev_->formats(0);
> +	if (formats.isEmpty()) {
>  		LOG(CameraSensor, Error) << "No image format found";
>  		return -EINVAL;
>  	}
>  
> -	std::transform(formats.begin(), formats.end(),
> -		       std::back_inserter(mbusCodes_),
> -		       [](decltype(*formats.begin()) f) { return f.first; });
> +	mbusCodes_ = formats.formats();
>  
>  	/*
>  	 * Extract the supported sizes from the first format as we only support
>  	 * sensors that offer the same frame sizes for all media bus codes.
>  	 * Verify this assumption and reject the sensor if it isn't true.
>  	 */
> -	const std::vector<SizeRange> &sizes = formats.begin()->second;
> +	const std::vector<SizeRange> &sizes = formats.sizes(mbusCodes_[0]);
>  	std::transform(sizes.begin(), sizes.end(), std::back_inserter(sizes_),
>  		       [](const SizeRange &range) { return range.max; });
>  
> -	for (auto it = ++formats.begin(); it != formats.end(); ++it) {
> -		if (it->second != sizes) {
> +	for (unsigned int code : mbusCodes_) {
> +		if (formats.sizes(code) != sizes) {
>  			LOG(CameraSensor, Error)
>  				<< "Frame sizes differ between media bus codes";
>  			return -EINVAL;
> diff --git a/src/libcamera/formats.cpp b/src/libcamera/formats.cpp
> index 2fd0c5480324ce33..0d196ee3a83e9c0d 100644
> --- a/src/libcamera/formats.cpp
> +++ b/src/libcamera/formats.cpp
> @@ -14,16 +14,6 @@
>  
>  namespace libcamera {
>  
> -/**
> - * \typedef FormatEnum
> - * \brief Type definition for the map of image formats and sizes
> - *
> - * Type definition used to enumerate the supported pixel formats and image
> - * frame sizes. The type associates in a map a pixel format (for memory
> - * formats) or a media bus code (for bus formats), to a vector of image
> - * resolutions represented by SizeRange items.
> - */
> -
>  /**
>   * \class ImageFormats
>   * \brief Describe V4L2Device and V4L2SubDevice image formats
> diff --git a/src/libcamera/include/formats.h b/src/libcamera/include/formats.h
> index a49f83f3d8d60621..de63ba557e045585 100644
> --- a/src/libcamera/include/formats.h
> +++ b/src/libcamera/include/formats.h
> @@ -15,8 +15,6 @@
>  
>  namespace libcamera {
>  
> -typedef std::map<unsigned int, std::vector<SizeRange>> FormatEnum;
> -
>  class ImageFormats
>  {
>  public:
> diff --git a/src/libcamera/include/v4l2_subdevice.h b/src/libcamera/include/v4l2_subdevice.h
> index c6fdf417b43c0423..9afd28b6db023ddf 100644
> --- a/src/libcamera/include/v4l2_subdevice.h
> +++ b/src/libcamera/include/v4l2_subdevice.h
> @@ -45,7 +45,7 @@ public:
>  	int setCrop(unsigned int pad, Rectangle *rect);
>  	int setCompose(unsigned int pad, Rectangle *rect);
>  
> -	FormatEnum formats(unsigned int pad);
> +	ImageFormats formats(unsigned int pad);
>  
>  	int getFormat(unsigned int pad, V4L2SubdeviceFormat *format);
>  	int setFormat(unsigned int pad, V4L2SubdeviceFormat *format);
> diff --git a/src/libcamera/v4l2_subdevice.cpp b/src/libcamera/v4l2_subdevice.cpp
> index 35052b4aa45d3e42..e109a00bcd3bdd46 100644
> --- a/src/libcamera/v4l2_subdevice.cpp
> +++ b/src/libcamera/v4l2_subdevice.cpp
> @@ -187,22 +187,17 @@ int V4L2Subdevice::setCompose(unsigned int pad, Rectangle *rect)
>  }
>  
>  /**
> - * \brief List the sub-device image resolutions and formats on \a pad
> + * \brief Enumerate all media bus codes and frame sizes on a \a pad
>   * \param[in] pad The 0-indexed pad number to enumerate formats on
>   *
> - * Retrieve a list of image formats and sizes on the \a pad of a video
> - * subdevice. Subdevices can report either a list of discrete sizes they
> - * support or a list of intervals expressed as a [min-max] sizes range.
> + * Enumerate all media bus codes  and frame sizes supported by the subdevice on

s/  / /

> + * a \a pad.
>   *
> - * Each image size list is associated with a media bus pixel code for which
> - * the reported resolutions are supported.
> - *
> - * \return A map of image formats associated with a list of image sizes, or
> - * an empty map on error or if the pad does not exist
> + * \return A list of the supported device formats
>   */
> -FormatEnum V4L2Subdevice::formats(unsigned int pad)
> +ImageFormats V4L2Subdevice::formats(unsigned int pad)
>  {
> -	FormatEnum formatMap = {};
> +	ImageFormats formats;
>  
>  	if (pad >= entity_->pads().size()) {
>  		LOG(V4L2Subdev, Error) << "Invalid pad: " << pad;
> @@ -215,10 +210,15 @@ FormatEnum V4L2Subdevice::formats(unsigned int pad)
>  		if (sizes.empty())
>  			return {};
>  
> -		formatMap[code] = sizes;
> +		if (formats.addFormat(code, sizes)) {
> +			LOG(V4L2Subdev, Error)
> +				<< "Could not add sizes for media bus code "
> +				<< code << "on pad " << pad;

s/"on pad "/" on pad "/

Reviewed-by: Laurent Pinchart <laurent.pinchart at ideasonboard.com>

> +			return {};
> +		}
>  	}
>  
> -	return formatMap;
> +	return formats;
>  }
>  
>  /**
> diff --git a/test/v4l2_subdevice/list_formats.cpp b/test/v4l2_subdevice/list_formats.cpp
> index 3f0edafcdcd72d6b..067dc5ed30f4edd9 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. */
> -	std::map<unsigned int, std::vector<SizeRange>> formats;
> +	ImageFormats formats;
>  
>  	formats = scaler_->formats(0);
> -	if (formats.empty()) {
> +	if (formats.isEmpty()) {
>  		cerr << "Failed to list formats on pad 0 of subdevice "
>  		     << scaler_->entity()->name() << endl;
>  		return TestFail;
>  	}
> -	for (auto it = formats.begin(); it != formats.end(); ++it)
> -		printFormats(0, it->first, it->second);
> +	for (unsigned int code : formats.formats())
> +		printFormats(0, code, formats.sizes(code));
>  
>  	formats = scaler_->formats(1);
> -	if (formats.empty()) {
> +	if (formats.isEmpty()) {
>  		cerr << "Failed to list formats on pad 1 of subdevice "
>  		     << scaler_->entity()->name() << endl;
>  		return TestFail;
>  	}
> -	for (auto it = formats.begin(); it != formats.end(); ++it)
> -		printFormats(1, it->first, it->second);
> +	for (unsigned int code : formats.formats())
> +		printFormats(1, code, formats.sizes(code));
>  
>  	/* List format on a non-existing pad, format vector shall be empty. */
>  	formats = scaler_->formats(2);
> -	if (!formats.empty()) {
> +	if (!formats.isEmpty()) {
>  		cerr << "Listing formats on non-existing pad 2 of subdevice "
>  		     << scaler_->entity()->name()
>  		     << " should return an empty format list" << endl;

-- 
Regards,

Laurent Pinchart


More information about the libcamera-devel mailing list