[libcamera-devel,v3,10/16] libcamera: stream: Add StreamFormats

Message ID 20190616133402.21934-11-niklas.soderlund@ragnatech.se
State Superseded
Headers show
Series
  • libcamera: Add support for format information and validation
Related show

Commit Message

Niklas Söderlund June 16, 2019, 1:33 p.m. UTC
Add a StreamFormats class which describes all the formats supported by a
stream. The object does not collect any information  itself but can
simplify user interactions with formats as it's able to translate a
stream format range into discrete list and a discrete list to a range.

Signed-off-by: Niklas Söderlund <niklas.soderlund@ragnatech.se>
Reviewed-by: Jacopo Mondi <jacopo@jmondi.org>
---
 include/libcamera/stream.h |  16 +++
 src/libcamera/stream.cpp   | 226 +++++++++++++++++++++++++++++++++++++
 2 files changed, 242 insertions(+)

Comments

Laurent Pinchart June 18, 2019, 11:33 p.m. UTC | #1
Hi Niklas,

Thank you for the patch.

On Sun, Jun 16, 2019 at 03:33:56PM +0200, Niklas Söderlund wrote:
> Add a StreamFormats class which describes all the formats supported by a
> stream. The object does not collect any information  itself but can

s/  / /

> simplify user interactions with formats as it's able to translate a
> stream format range into discrete list and a discrete list to a range.

s/into discrete list/into a discrete list/

> 
> Signed-off-by: Niklas Söderlund <niklas.soderlund@ragnatech.se>
> Reviewed-by: Jacopo Mondi <jacopo@jmondi.org>
> ---
>  include/libcamera/stream.h |  16 +++
>  src/libcamera/stream.cpp   | 226 +++++++++++++++++++++++++++++++++++++
>  2 files changed, 242 insertions(+)
> 
> diff --git a/include/libcamera/stream.h b/include/libcamera/stream.h
> index e38c0e7e827d5888..48daf5ac23f55d85 100644
> --- a/include/libcamera/stream.h
> +++ b/include/libcamera/stream.h
> @@ -7,6 +7,7 @@
>  #ifndef __LIBCAMERA_STREAM_H__
>  #define __LIBCAMERA_STREAM_H__
>  
> +#include <map>
>  #include <string>
>  #include <vector>
>  
> @@ -18,6 +19,21 @@ namespace libcamera {
>  class Camera;
>  class Stream;
>  
> +class StreamFormats
> +{
> +public:
> +	StreamFormats();
> +	StreamFormats(const std::map<unsigned int, std::vector<SizeRange>> &formats);
> +
> +	std::vector<unsigned int> pixelformats() const;

Repeating the comments I made on v1,

For efficiency reasons, do you think it would make sense to cache the
pixel formats vector the first time it is computed and return a const
reference to the cached copy ? Or do you expect applications to
typically retrieve the pixel formats once only ? Same question for the
sizes and ranges.

> +	std::vector<Size> sizes(unsigned int pixelformat) const;
> +
> +	SizeRange range(unsigned int pixelformat) const;
> +
> +private:
> +	std::map<unsigned int, std::vector<SizeRange>> formats_;
> +};
> +
>  struct StreamConfiguration {
>  	StreamConfiguration()
>  		: stream_(nullptr)
> diff --git a/src/libcamera/stream.cpp b/src/libcamera/stream.cpp
> index 0c59a31a3a0501d3..27fde5fe29eeb340 100644
> --- a/src/libcamera/stream.cpp
> +++ b/src/libcamera/stream.cpp
> @@ -7,9 +7,13 @@
>  
>  #include <libcamera/stream.h>
>  
> +#include <algorithm>
> +#include <climits>
>  #include <iomanip>
>  #include <sstream>
>  
> +#include "log.h"
> +
>  /**
>   * \file stream.h
>   * \brief Video stream for a Camera
> @@ -32,6 +36,228 @@
>  
>  namespace libcamera {
>  
> +LOG_DEFINE_CATEGORY(Stream)
> +
> +/**
> + * \class StreamFormats
> + * \brief Hold information about supported stream formats
> + *
> + * The StreamFormats class holds information about the pixel formats and frame
> + * sizes a stream supports. The class groups size information by the pixel
> + * format which can produce it.
> + *
> + * There are two ways to examine the size information, as a range or as a list
> + * of discrete sizes. When sizes are viewed as a range it describes the minimum
> + * and maximum width and height values. There is a possibility to supplement the
> + * range description with a horizontal and vertical step. The step describes the
> + * size in pixels starting from the minimum with/height.

"The range description can include horizontal and vertical steps."

and I would remove the last sentence, as it's explained neatly in the
SizeRange class.

> + *
> + * When sizes are viewed as a list of discrete sizes they describe the exact
> + * dimensions which can be selected and used.
> + *
> + * Pipeline handlers can create StreamFormats describing each pixel format using
> + * either a range or a list of discrete sizes. The StreamFormats class attempts
> + * to translate between the two different ways to view them. The translations
> + * are performed as:
> + *
> + *  - If the StreamFormat is constructed using a list of discrete image sizes
> + *    and a range is requested, it gets created by taking the minimum and
> + *    maximum width/height in the list. The step information is not recreated
> + *    and is set to 0 to indicate the range is generated.

It can make sense to set the step to 0, but this invalidates some of my
comments on earlier patches, so I'll get back to them.

> + *
> + *  - If the image sizes used to construct a StreamFormat are expressed as a
> + *    range and a list of discrete sizes is requested, one which fits inside
> + *    that range are selected from a list of common sizes. The step information
> + *    is taken into consideration when generating the sizes.
> + *
> + * Applications examining sizes as a range with step values of 0 shall be
> + * aware that the range are generated from a list of discrete sizes and there
> + * could be a large quantity of possible Size combinations which may not

s/quantity/number/
s/which/that/

> + * be supported by the Stream.
> + *
> + * All sizes retrieved from StreamFormats shall be treated as advisory and no
> + * size shall be considered to be supported until its been verified using

s/its/it has/

> + * CameraConfiguration::validate().
> + */
> +
> +StreamFormats::StreamFormats()
> +{
> +}
> +
> +/**
> + * \brief Construct a StreamFormats object with a map of image formats
> + * \param[in] formats A map of pixel formats to a sizes description
> + */
> +StreamFormats::StreamFormats(const std::map<unsigned int, std::vector<SizeRange>> &formats)
> +	: formats_(formats)
> +{
> +}
> +
> +/**
> + * \brief Retrieve the list of supported pixel formats
> + * \return The list of supported pixel formats
> + */
> +std::vector<unsigned int> StreamFormats::pixelformats() const
> +{
> +	std::vector<unsigned int> formats;
> +
> +	for (auto const &it : formats_)
> +		formats.push_back(it.first);
> +
> +	return formats;
> +}
> +
> +/**
> + * \brief Retrieve the list of frame sizes supported by \a pixelformat

s/by/for/

> + * \param[in] pixelformat Pixel format to retrieve sizes for
> + *
> + * If the sizes described for \a pixelformat are discrete they are returned
> + * directly. If the size described is a range a list of discrete sizes are

"If the sizes are described as a range, a list of ..."

> + * computed from a set list of common resolutions which fits inside the

s/set list/set/ ? Or did you mean "fixed list" or "fixed set" ?
s/which fits/that fit/

> + * described range. When computing the discrete list step values are considered
> + * but there are no guarantees that all sizes computed are supported.
> + *
> + * \return A list of frame sizes or an empty list on error
> + */
> +std::vector<Size> StreamFormats::sizes(unsigned int pixelformat) const
> +{
> +	/*
> +	 * Sizes to try and extract from ranges.
> +	 * \todo Verify list of resolutions are good, current list compiled
> +	 * from v4l2 documentation and source code as well as lists of
> +	 * common frame sizes.
> +	 */
> +	static const std::vector<Size> rangeDiscreteSizes = {

Would it work if you used std::array instead of std::vector ?

> +		Size(160, 120),
> +		Size(240, 160),
> +		Size(320, 240),
> +		Size(400, 240),
> +		Size(480, 320),
> +		Size(640, 360),
> +		Size(640, 480),
> +		Size(720, 480),
> +		Size(720, 576),
> +		Size(768, 480),
> +		Size(800, 600),
> +		Size(854, 480),
> +		Size(960, 540),
> +		Size(960, 640),
> +		Size(1024, 576),
> +		Size(1024, 600),
> +		Size(1024, 768),
> +		Size(1152, 864),
> +		Size(1280, 1024),
> +		Size(1280, 1080),
> +		Size(1280, 720),
> +		Size(1280, 800),
> +		Size(1360, 768),
> +		Size(1366, 768),
> +		Size(1400, 1050),
> +		Size(1440, 900),
> +		Size(1536, 864),
> +		Size(1600, 1200),
> +		Size(1600, 900),
> +		Size(1680, 1050),
> +		Size(1920, 1080),
> +		Size(1920, 1200),
> +		Size(2048, 1080),
> +		Size(2048, 1152),
> +		Size(2048, 1536),
> +		Size(2160, 1080),
> +		Size(2560, 1080),
> +		Size(2560, 1440),
> +		Size(2560, 1600),
> +		Size(2560, 2048),
> +		Size(2960, 1440),
> +		Size(3200, 1800),
> +		Size(3200, 2048),
> +		Size(3200, 2400),
> +		Size(3440, 1440),
> +		Size(3840, 1080),
> +		Size(3840, 1600),
> +		Size(3840, 2160),
> +		Size(3840, 2400),
> +		Size(4096, 2160),
> +		Size(5120, 2160),
> +		Size(5120, 2880),
> +		Size(7680, 4320),
> +	};
> +	std::vector<Size> sizes;
> +
> +	/* Make sure pixel format exists. */
> +	auto const &it = formats_.find(pixelformat);
> +	if (it == formats_.end())
> +		return {};
> +
> +	/* Try creating a list of discrete sizes. */
> +	const std::vector<SizeRange> &ranges = it->second;
> +	bool discrete = true;
> +	for (const SizeRange &range : ranges) {
> +		if (range.min != range.max) {
> +			discrete = false;
> +			break;
> +		}
> +		sizes.emplace_back(range.min);
> +	}
> +
> +	/* If discrete not possible generate from range. */
> +	if (!discrete) {
> +		if (ranges.size() != 1) {
> +			LOG(Stream, Error) << "Range format is ambiguous";
> +			return {};
> +		}
> +
> +		const SizeRange &limit = ranges.front();
> +		sizes.clear();
> +
> +		for (const Size &size : rangeDiscreteSizes)
> +			if (limit.contains(size))
> +				sizes.push_back(size);
> +	}
> +
> +	std::sort(sizes.begin(), sizes.end());
> +
> +	return sizes;
> +}
> +
> +/**
> + * \brief Retrieve the range of minimum and maximu sizes

s/maximu/maximum/

> + * \param[in] pixelformat Pixel format to retrieve range for
> + *
> + * If the size described for \a pixelformat is a range that range is returned

s/is a range/is a range,/

> + * directly. If the sizes described are a list of discrete sizes a range is

s/a range/, a range/

> + * computed from the minimum and maxim sizes in the list. The step values of

s/computed/created/
s/maxim/maximum/

> + * a computed range is set to 0 to indicate that the range is generated and

s/a computed range is set/the range are set/

> + * that not all image sizes contained in the range might be supported.
> + *
> + * \return A range of valid image sizes or an empty range on error
> + */
> +SizeRange StreamFormats::range(unsigned int pixelformat) const
> +{
> +	auto const it = formats_.find(pixelformat);
> +	if (it == formats_.end())
> +		return {};
> +
> +	const std::vector<SizeRange> &ranges = it->second;
> +	if (ranges.size() == 1)
> +		return ranges[0];
> +
> +	LOG(Stream, Debug) << "Building range from discrete";

s/discrete/discrete sizes/

> +	SizeRange range(UINT_MAX, UINT_MAX, 0, 0);
> +	for (const SizeRange &limit : ranges) {
> +		if (limit.min < range.min)
> +			range.min = limit.min;
> +
> +		if (limit.max > range.max)
> +			range.max = limit.max;
> +	}
> +
> +	range.hStep = 0;
> +	range.vStep = 0;
> +
> +	return range;
> +}
> +
>  /**
>   * \struct StreamConfiguration
>   * \brief Configuration parameters for a stream
Niklas Söderlund June 19, 2019, 12:07 a.m. UTC | #2
Hi Laurent,

Thanks for your feedback.

On 2019-06-19 02:33:55 +0300, Laurent Pinchart wrote:
> Hi Niklas,
> 
> Thank you for the patch.
> 
> On Sun, Jun 16, 2019 at 03:33:56PM +0200, Niklas Söderlund wrote:
> > Add a StreamFormats class which describes all the formats supported by a
> > stream. The object does not collect any information  itself but can
> 
> s/  / /
> 
> > simplify user interactions with formats as it's able to translate a
> > stream format range into discrete list and a discrete list to a range.
> 
> s/into discrete list/into a discrete list/
> 
> > 
> > Signed-off-by: Niklas Söderlund <niklas.soderlund@ragnatech.se>
> > Reviewed-by: Jacopo Mondi <jacopo@jmondi.org>
> > ---
> >  include/libcamera/stream.h |  16 +++
> >  src/libcamera/stream.cpp   | 226 +++++++++++++++++++++++++++++++++++++
> >  2 files changed, 242 insertions(+)
> > 
> > diff --git a/include/libcamera/stream.h b/include/libcamera/stream.h
> > index e38c0e7e827d5888..48daf5ac23f55d85 100644
> > --- a/include/libcamera/stream.h
> > +++ b/include/libcamera/stream.h
> > @@ -7,6 +7,7 @@
> >  #ifndef __LIBCAMERA_STREAM_H__
> >  #define __LIBCAMERA_STREAM_H__
> >  
> > +#include <map>
> >  #include <string>
> >  #include <vector>
> >  
> > @@ -18,6 +19,21 @@ namespace libcamera {
> >  class Camera;
> >  class Stream;
> >  
> > +class StreamFormats
> > +{
> > +public:
> > +	StreamFormats();
> > +	StreamFormats(const std::map<unsigned int, std::vector<SizeRange>> &formats);
> > +
> > +	std::vector<unsigned int> pixelformats() const;
> 
> Repeating the comments I made on v1,
> 
> For efficiency reasons, do you think it would make sense to cache the
> pixel formats vector the first time it is computed and return a const
> reference to the cached copy ? Or do you expect applications to
> typically retrieve the pixel formats once only ? Same question for the
> sizes and ranges.

Short answer is I'm not sure how often a typical application will 
retrieve this information. It would certainly be possible to cache the 
information, but I wonder if it would be a premature optimization?

I have no strong feeling one way or the other. We could always add 
caching on top later if this turns out to be called multiple times in 
out example applications. If you suspect already that they will be 
called often I might as well add caching now. Let me know what you 
think.

> 
> > +	std::vector<Size> sizes(unsigned int pixelformat) const;
> > +
> > +	SizeRange range(unsigned int pixelformat) const;
> > +
> > +private:
> > +	std::map<unsigned int, std::vector<SizeRange>> formats_;
> > +};
> > +
> >  struct StreamConfiguration {
> >  	StreamConfiguration()
> >  		: stream_(nullptr)
> > diff --git a/src/libcamera/stream.cpp b/src/libcamera/stream.cpp
> > index 0c59a31a3a0501d3..27fde5fe29eeb340 100644
> > --- a/src/libcamera/stream.cpp
> > +++ b/src/libcamera/stream.cpp
> > @@ -7,9 +7,13 @@
> >  
> >  #include <libcamera/stream.h>
> >  
> > +#include <algorithm>
> > +#include <climits>
> >  #include <iomanip>
> >  #include <sstream>
> >  
> > +#include "log.h"
> > +
> >  /**
> >   * \file stream.h
> >   * \brief Video stream for a Camera
> > @@ -32,6 +36,228 @@
> >  
> >  namespace libcamera {
> >  
> > +LOG_DEFINE_CATEGORY(Stream)
> > +
> > +/**
> > + * \class StreamFormats
> > + * \brief Hold information about supported stream formats
> > + *
> > + * The StreamFormats class holds information about the pixel formats and frame
> > + * sizes a stream supports. The class groups size information by the pixel
> > + * format which can produce it.
> > + *
> > + * There are two ways to examine the size information, as a range or as a list
> > + * of discrete sizes. When sizes are viewed as a range it describes the minimum
> > + * and maximum width and height values. There is a possibility to supplement the
> > + * range description with a horizontal and vertical step. The step describes the
> > + * size in pixels starting from the minimum with/height.
> 
> "The range description can include horizontal and vertical steps."
> 
> and I would remove the last sentence, as it's explained neatly in the
> SizeRange class.
> 
> > + *
> > + * When sizes are viewed as a list of discrete sizes they describe the exact
> > + * dimensions which can be selected and used.
> > + *
> > + * Pipeline handlers can create StreamFormats describing each pixel format using
> > + * either a range or a list of discrete sizes. The StreamFormats class attempts
> > + * to translate between the two different ways to view them. The translations
> > + * are performed as:
> > + *
> > + *  - If the StreamFormat is constructed using a list of discrete image sizes
> > + *    and a range is requested, it gets created by taking the minimum and
> > + *    maximum width/height in the list. The step information is not recreated
> > + *    and is set to 0 to indicate the range is generated.
> 
> It can make sense to set the step to 0, but this invalidates some of my
> comments on earlier patches, so I'll get back to them.
> 
> > + *
> > + *  - If the image sizes used to construct a StreamFormat are expressed as a
> > + *    range and a list of discrete sizes is requested, one which fits inside
> > + *    that range are selected from a list of common sizes. The step information
> > + *    is taken into consideration when generating the sizes.
> > + *
> > + * Applications examining sizes as a range with step values of 0 shall be
> > + * aware that the range are generated from a list of discrete sizes and there
> > + * could be a large quantity of possible Size combinations which may not
> 
> s/quantity/number/
> s/which/that/
> 
> > + * be supported by the Stream.
> > + *
> > + * All sizes retrieved from StreamFormats shall be treated as advisory and no
> > + * size shall be considered to be supported until its been verified using
> 
> s/its/it has/
> 
> > + * CameraConfiguration::validate().
> > + */
> > +
> > +StreamFormats::StreamFormats()
> > +{
> > +}
> > +
> > +/**
> > + * \brief Construct a StreamFormats object with a map of image formats
> > + * \param[in] formats A map of pixel formats to a sizes description
> > + */
> > +StreamFormats::StreamFormats(const std::map<unsigned int, std::vector<SizeRange>> &formats)
> > +	: formats_(formats)
> > +{
> > +}
> > +
> > +/**
> > + * \brief Retrieve the list of supported pixel formats
> > + * \return The list of supported pixel formats
> > + */
> > +std::vector<unsigned int> StreamFormats::pixelformats() const
> > +{
> > +	std::vector<unsigned int> formats;
> > +
> > +	for (auto const &it : formats_)
> > +		formats.push_back(it.first);
> > +
> > +	return formats;
> > +}
> > +
> > +/**
> > + * \brief Retrieve the list of frame sizes supported by \a pixelformat
> 
> s/by/for/
> 
> > + * \param[in] pixelformat Pixel format to retrieve sizes for
> > + *
> > + * If the sizes described for \a pixelformat are discrete they are returned
> > + * directly. If the size described is a range a list of discrete sizes are
> 
> "If the sizes are described as a range, a list of ..."
> 
> > + * computed from a set list of common resolutions which fits inside the
> 
> s/set list/set/ ? Or did you mean "fixed list" or "fixed set" ?
> s/which fits/that fit/
> 
> > + * described range. When computing the discrete list step values are considered
> > + * but there are no guarantees that all sizes computed are supported.
> > + *
> > + * \return A list of frame sizes or an empty list on error
> > + */
> > +std::vector<Size> StreamFormats::sizes(unsigned int pixelformat) const
> > +{
> > +	/*
> > +	 * Sizes to try and extract from ranges.
> > +	 * \todo Verify list of resolutions are good, current list compiled
> > +	 * from v4l2 documentation and source code as well as lists of
> > +	 * common frame sizes.
> > +	 */
> > +	static const std::vector<Size> rangeDiscreteSizes = {
> 
> Would it work if you used std::array instead of std::vector ?

I suspect it will, will give it a try for next version.

> 
> > +		Size(160, 120),
> > +		Size(240, 160),
> > +		Size(320, 240),
> > +		Size(400, 240),
> > +		Size(480, 320),
> > +		Size(640, 360),
> > +		Size(640, 480),
> > +		Size(720, 480),
> > +		Size(720, 576),
> > +		Size(768, 480),
> > +		Size(800, 600),
> > +		Size(854, 480),
> > +		Size(960, 540),
> > +		Size(960, 640),
> > +		Size(1024, 576),
> > +		Size(1024, 600),
> > +		Size(1024, 768),
> > +		Size(1152, 864),
> > +		Size(1280, 1024),
> > +		Size(1280, 1080),
> > +		Size(1280, 720),
> > +		Size(1280, 800),
> > +		Size(1360, 768),
> > +		Size(1366, 768),
> > +		Size(1400, 1050),
> > +		Size(1440, 900),
> > +		Size(1536, 864),
> > +		Size(1600, 1200),
> > +		Size(1600, 900),
> > +		Size(1680, 1050),
> > +		Size(1920, 1080),
> > +		Size(1920, 1200),
> > +		Size(2048, 1080),
> > +		Size(2048, 1152),
> > +		Size(2048, 1536),
> > +		Size(2160, 1080),
> > +		Size(2560, 1080),
> > +		Size(2560, 1440),
> > +		Size(2560, 1600),
> > +		Size(2560, 2048),
> > +		Size(2960, 1440),
> > +		Size(3200, 1800),
> > +		Size(3200, 2048),
> > +		Size(3200, 2400),
> > +		Size(3440, 1440),
> > +		Size(3840, 1080),
> > +		Size(3840, 1600),
> > +		Size(3840, 2160),
> > +		Size(3840, 2400),
> > +		Size(4096, 2160),
> > +		Size(5120, 2160),
> > +		Size(5120, 2880),
> > +		Size(7680, 4320),
> > +	};
> > +	std::vector<Size> sizes;
> > +
> > +	/* Make sure pixel format exists. */
> > +	auto const &it = formats_.find(pixelformat);
> > +	if (it == formats_.end())
> > +		return {};
> > +
> > +	/* Try creating a list of discrete sizes. */
> > +	const std::vector<SizeRange> &ranges = it->second;
> > +	bool discrete = true;
> > +	for (const SizeRange &range : ranges) {
> > +		if (range.min != range.max) {
> > +			discrete = false;
> > +			break;
> > +		}
> > +		sizes.emplace_back(range.min);
> > +	}
> > +
> > +	/* If discrete not possible generate from range. */
> > +	if (!discrete) {
> > +		if (ranges.size() != 1) {
> > +			LOG(Stream, Error) << "Range format is ambiguous";
> > +			return {};
> > +		}
> > +
> > +		const SizeRange &limit = ranges.front();
> > +		sizes.clear();
> > +
> > +		for (const Size &size : rangeDiscreteSizes)
> > +			if (limit.contains(size))
> > +				sizes.push_back(size);
> > +	}
> > +
> > +	std::sort(sizes.begin(), sizes.end());
> > +
> > +	return sizes;
> > +}
> > +
> > +/**
> > + * \brief Retrieve the range of minimum and maximu sizes
> 
> s/maximu/maximum/
> 
> > + * \param[in] pixelformat Pixel format to retrieve range for
> > + *
> > + * If the size described for \a pixelformat is a range that range is returned
> 
> s/is a range/is a range,/
> 
> > + * directly. If the sizes described are a list of discrete sizes a range is
> 
> s/a range/, a range/
> 
> > + * computed from the minimum and maxim sizes in the list. The step values of
> 
> s/computed/created/
> s/maxim/maximum/
> 
> > + * a computed range is set to 0 to indicate that the range is generated and
> 
> s/a computed range is set/the range are set/
> 
> > + * that not all image sizes contained in the range might be supported.
> > + *
> > + * \return A range of valid image sizes or an empty range on error
> > + */
> > +SizeRange StreamFormats::range(unsigned int pixelformat) const
> > +{
> > +	auto const it = formats_.find(pixelformat);
> > +	if (it == formats_.end())
> > +		return {};
> > +
> > +	const std::vector<SizeRange> &ranges = it->second;
> > +	if (ranges.size() == 1)
> > +		return ranges[0];
> > +
> > +	LOG(Stream, Debug) << "Building range from discrete";
> 
> s/discrete/discrete sizes/
> 
> > +	SizeRange range(UINT_MAX, UINT_MAX, 0, 0);
> > +	for (const SizeRange &limit : ranges) {
> > +		if (limit.min < range.min)
> > +			range.min = limit.min;
> > +
> > +		if (limit.max > range.max)
> > +			range.max = limit.max;
> > +	}
> > +
> > +	range.hStep = 0;
> > +	range.vStep = 0;
> > +
> > +	return range;
> > +}
> > +
> >  /**
> >   * \struct StreamConfiguration
> >   * \brief Configuration parameters for a stream
> 
> -- 
> Regards,
> 
> Laurent Pinchart

Patch

diff --git a/include/libcamera/stream.h b/include/libcamera/stream.h
index e38c0e7e827d5888..48daf5ac23f55d85 100644
--- a/include/libcamera/stream.h
+++ b/include/libcamera/stream.h
@@ -7,6 +7,7 @@ 
 #ifndef __LIBCAMERA_STREAM_H__
 #define __LIBCAMERA_STREAM_H__
 
+#include <map>
 #include <string>
 #include <vector>
 
@@ -18,6 +19,21 @@  namespace libcamera {
 class Camera;
 class Stream;
 
+class StreamFormats
+{
+public:
+	StreamFormats();
+	StreamFormats(const std::map<unsigned int, std::vector<SizeRange>> &formats);
+
+	std::vector<unsigned int> pixelformats() const;
+	std::vector<Size> sizes(unsigned int pixelformat) const;
+
+	SizeRange range(unsigned int pixelformat) const;
+
+private:
+	std::map<unsigned int, std::vector<SizeRange>> formats_;
+};
+
 struct StreamConfiguration {
 	StreamConfiguration()
 		: stream_(nullptr)
diff --git a/src/libcamera/stream.cpp b/src/libcamera/stream.cpp
index 0c59a31a3a0501d3..27fde5fe29eeb340 100644
--- a/src/libcamera/stream.cpp
+++ b/src/libcamera/stream.cpp
@@ -7,9 +7,13 @@ 
 
 #include <libcamera/stream.h>
 
+#include <algorithm>
+#include <climits>
 #include <iomanip>
 #include <sstream>
 
+#include "log.h"
+
 /**
  * \file stream.h
  * \brief Video stream for a Camera
@@ -32,6 +36,228 @@ 
 
 namespace libcamera {
 
+LOG_DEFINE_CATEGORY(Stream)
+
+/**
+ * \class StreamFormats
+ * \brief Hold information about supported stream formats
+ *
+ * The StreamFormats class holds information about the pixel formats and frame
+ * sizes a stream supports. The class groups size information by the pixel
+ * format which can produce it.
+ *
+ * There are two ways to examine the size information, as a range or as a list
+ * of discrete sizes. When sizes are viewed as a range it describes the minimum
+ * and maximum width and height values. There is a possibility to supplement the
+ * range description with a horizontal and vertical step. The step describes the
+ * size in pixels starting from the minimum with/height.
+ *
+ * When sizes are viewed as a list of discrete sizes they describe the exact
+ * dimensions which can be selected and used.
+ *
+ * Pipeline handlers can create StreamFormats describing each pixel format using
+ * either a range or a list of discrete sizes. The StreamFormats class attempts
+ * to translate between the two different ways to view them. The translations
+ * are performed as:
+ *
+ *  - If the StreamFormat is constructed using a list of discrete image sizes
+ *    and a range is requested, it gets created by taking the minimum and
+ *    maximum width/height in the list. The step information is not recreated
+ *    and is set to 0 to indicate the range is generated.
+ *
+ *  - If the image sizes used to construct a StreamFormat are expressed as a
+ *    range and a list of discrete sizes is requested, one which fits inside
+ *    that range are selected from a list of common sizes. The step information
+ *    is taken into consideration when generating the sizes.
+ *
+ * Applications examining sizes as a range with step values of 0 shall be
+ * aware that the range are generated from a list of discrete sizes and there
+ * could be a large quantity of possible Size combinations which may not
+ * be supported by the Stream.
+ *
+ * All sizes retrieved from StreamFormats shall be treated as advisory and no
+ * size shall be considered to be supported until its been verified using
+ * CameraConfiguration::validate().
+ */
+
+StreamFormats::StreamFormats()
+{
+}
+
+/**
+ * \brief Construct a StreamFormats object with a map of image formats
+ * \param[in] formats A map of pixel formats to a sizes description
+ */
+StreamFormats::StreamFormats(const std::map<unsigned int, std::vector<SizeRange>> &formats)
+	: formats_(formats)
+{
+}
+
+/**
+ * \brief Retrieve the list of supported pixel formats
+ * \return The list of supported pixel formats
+ */
+std::vector<unsigned int> StreamFormats::pixelformats() const
+{
+	std::vector<unsigned int> formats;
+
+	for (auto const &it : formats_)
+		formats.push_back(it.first);
+
+	return formats;
+}
+
+/**
+ * \brief Retrieve the list of frame sizes supported by \a pixelformat
+ * \param[in] pixelformat Pixel format to retrieve sizes for
+ *
+ * If the sizes described for \a pixelformat are discrete they are returned
+ * directly. If the size described is a range a list of discrete sizes are
+ * computed from a set list of common resolutions which fits inside the
+ * described range. When computing the discrete list step values are considered
+ * but there are no guarantees that all sizes computed are supported.
+ *
+ * \return A list of frame sizes or an empty list on error
+ */
+std::vector<Size> StreamFormats::sizes(unsigned int pixelformat) const
+{
+	/*
+	 * Sizes to try and extract from ranges.
+	 * \todo Verify list of resolutions are good, current list compiled
+	 * from v4l2 documentation and source code as well as lists of
+	 * common frame sizes.
+	 */
+	static const std::vector<Size> rangeDiscreteSizes = {
+		Size(160, 120),
+		Size(240, 160),
+		Size(320, 240),
+		Size(400, 240),
+		Size(480, 320),
+		Size(640, 360),
+		Size(640, 480),
+		Size(720, 480),
+		Size(720, 576),
+		Size(768, 480),
+		Size(800, 600),
+		Size(854, 480),
+		Size(960, 540),
+		Size(960, 640),
+		Size(1024, 576),
+		Size(1024, 600),
+		Size(1024, 768),
+		Size(1152, 864),
+		Size(1280, 1024),
+		Size(1280, 1080),
+		Size(1280, 720),
+		Size(1280, 800),
+		Size(1360, 768),
+		Size(1366, 768),
+		Size(1400, 1050),
+		Size(1440, 900),
+		Size(1536, 864),
+		Size(1600, 1200),
+		Size(1600, 900),
+		Size(1680, 1050),
+		Size(1920, 1080),
+		Size(1920, 1200),
+		Size(2048, 1080),
+		Size(2048, 1152),
+		Size(2048, 1536),
+		Size(2160, 1080),
+		Size(2560, 1080),
+		Size(2560, 1440),
+		Size(2560, 1600),
+		Size(2560, 2048),
+		Size(2960, 1440),
+		Size(3200, 1800),
+		Size(3200, 2048),
+		Size(3200, 2400),
+		Size(3440, 1440),
+		Size(3840, 1080),
+		Size(3840, 1600),
+		Size(3840, 2160),
+		Size(3840, 2400),
+		Size(4096, 2160),
+		Size(5120, 2160),
+		Size(5120, 2880),
+		Size(7680, 4320),
+	};
+	std::vector<Size> sizes;
+
+	/* Make sure pixel format exists. */
+	auto const &it = formats_.find(pixelformat);
+	if (it == formats_.end())
+		return {};
+
+	/* Try creating a list of discrete sizes. */
+	const std::vector<SizeRange> &ranges = it->second;
+	bool discrete = true;
+	for (const SizeRange &range : ranges) {
+		if (range.min != range.max) {
+			discrete = false;
+			break;
+		}
+		sizes.emplace_back(range.min);
+	}
+
+	/* If discrete not possible generate from range. */
+	if (!discrete) {
+		if (ranges.size() != 1) {
+			LOG(Stream, Error) << "Range format is ambiguous";
+			return {};
+		}
+
+		const SizeRange &limit = ranges.front();
+		sizes.clear();
+
+		for (const Size &size : rangeDiscreteSizes)
+			if (limit.contains(size))
+				sizes.push_back(size);
+	}
+
+	std::sort(sizes.begin(), sizes.end());
+
+	return sizes;
+}
+
+/**
+ * \brief Retrieve the range of minimum and maximu sizes
+ * \param[in] pixelformat Pixel format to retrieve range for
+ *
+ * If the size described for \a pixelformat is a range that range is returned
+ * directly. If the sizes described are a list of discrete sizes a range is
+ * computed from the minimum and maxim sizes in the list. The step values of
+ * a computed range is set to 0 to indicate that the range is generated and
+ * that not all image sizes contained in the range might be supported.
+ *
+ * \return A range of valid image sizes or an empty range on error
+ */
+SizeRange StreamFormats::range(unsigned int pixelformat) const
+{
+	auto const it = formats_.find(pixelformat);
+	if (it == formats_.end())
+		return {};
+
+	const std::vector<SizeRange> &ranges = it->second;
+	if (ranges.size() == 1)
+		return ranges[0];
+
+	LOG(Stream, Debug) << "Building range from discrete";
+	SizeRange range(UINT_MAX, UINT_MAX, 0, 0);
+	for (const SizeRange &limit : ranges) {
+		if (limit.min < range.min)
+			range.min = limit.min;
+
+		if (limit.max > range.max)
+			range.max = limit.max;
+	}
+
+	range.hStep = 0;
+	range.vStep = 0;
+
+	return range;
+}
+
 /**
  * \struct StreamConfiguration
  * \brief Configuration parameters for a stream