[libcamera-devel] [PATCH v3 10/16] libcamera: stream: Add StreamFormats
Laurent Pinchart
laurent.pinchart at ideasonboard.com
Wed Jun 19 01:33:55 CEST 2019
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 at ragnatech.se>
> Reviewed-by: Jacopo Mondi <jacopo at 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
--
Regards,
Laurent Pinchart
More information about the libcamera-devel
mailing list