[PATCH v3 09/17] libcamera: converter: Add functions to adjust config
Stefan Klug
stefan.klug at ideasonboard.com
Wed Dec 11 17:55:22 CET 2024
Hi Jacopo,
Thank you for the patch :-)
On Fri, Dec 06, 2024 at 11:13:31AM +0100, Stefan Klug wrote:
> From: Jacopo Mondi <jacopo.mondi at ideasonboard.com>
>
> Add to the Converter interface two functions used by pipeline handlers
> to validate and adjust the converter input and output configurations
> by specifying the desired alignment for the adjustment.
>
> Add the adjustInputSize() and adjustOutputSize() functions that allows
> to adjust the converter input/output sizes with the desired alignment.
>
> Add a validateOutput() function meant to be used by the pipeline
> handler implementations of validate(). The function adjusts a
> StreamConfiguration to a valid configuration produced by the Converter.
>
> Signed-off-by: Jacopo Mondi <jacopo.mondi at ideasonboard.com>
>
> ---
> Changes in v3:
> - Added this patch
> ---
> include/libcamera/internal/converter.h | 17 ++
> .../internal/converter/converter_v4l2_m2m.h | 11 ++
> src/libcamera/converter.cpp | 43 +++++
> .../converter/converter_v4l2_m2m.cpp | 169 ++++++++++++++++++
> 4 files changed, 240 insertions(+)
>
> diff --git a/include/libcamera/internal/converter.h b/include/libcamera/internal/converter.h
> index ffbb6f345cd5..9213ae5b9c33 100644
> --- a/include/libcamera/internal/converter.h
> +++ b/include/libcamera/internal/converter.h
> @@ -41,6 +41,13 @@ public:
>
> using Features = Flags<Feature>;
>
> + enum class Alignment {
> + Down = 0,
> + Up = (1 << 0),
> + };
> +
> + using Alignments = Flags<Alignment>;
> +
> Converter(MediaDevice *media, Features features = Feature::None);
> virtual ~Converter();
>
> @@ -51,9 +58,19 @@ public:
> virtual std::vector<PixelFormat> formats(PixelFormat input) = 0;
> virtual SizeRange sizes(const Size &input) = 0;
>
> + virtual Size adjustInputSize(const PixelFormat &pixFmt,
> + const Size &size,
> + Alignments align = Alignment::Down) = 0;
> + virtual Size adjustOutputSize(const PixelFormat &pixFmt,
> + const Size &size,
> + Alignments align = Alignment::Down) = 0;
> +
> virtual std::tuple<unsigned int, unsigned int>
> strideAndFrameSize(const PixelFormat &pixelFormat, const Size &size) = 0;
>
> + virtual int validateOutput(StreamConfiguration *cfg, bool *adjusted,
> + Alignments align = Alignment::Down) = 0;
> +
I'm wondering when to use non-const references and when a pointer. Is
there a reason cfg is a pointer?
> virtual int configure(const StreamConfiguration &inputCfg,
> const std::vector<std::reference_wrapper<StreamConfiguration>> &outputCfgs) = 0;
> virtual int exportBuffers(const Stream *stream, unsigned int count,
> diff --git a/include/libcamera/internal/converter/converter_v4l2_m2m.h b/include/libcamera/internal/converter/converter_v4l2_m2m.h
> index a5286166f574..89bd2878b190 100644
> --- a/include/libcamera/internal/converter/converter_v4l2_m2m.h
> +++ b/include/libcamera/internal/converter/converter_v4l2_m2m.h
> @@ -47,6 +47,11 @@ public:
> std::tuple<unsigned int, unsigned int>
> strideAndFrameSize(const PixelFormat &pixelFormat, const Size &size);
>
> + Size adjustInputSize(const PixelFormat &pixFmt,
> + const Size &size, Alignments align = Alignment::Down) override;
> + Size adjustOutputSize(const PixelFormat &pixFmt,
> + const Size &size, Alignments align = Alignment::Down) override;
> +
> int configure(const StreamConfiguration &inputCfg,
> const std::vector<std::reference_wrapper<StreamConfiguration>> &outputCfg);
> int exportBuffers(const Stream *stream, unsigned int count,
> @@ -55,6 +60,9 @@ public:
> int start();
> void stop();
>
> + int validateOutput(StreamConfiguration *cfg, bool *adjusted,
> + Alignments align = Alignment::Down) override;
> +
> int queueBuffers(FrameBuffer *input,
> const std::map<const Stream *, FrameBuffer *> &outputs);
>
> @@ -101,6 +109,9 @@ private:
> std::pair<Rectangle, Rectangle> inputCropBounds_;
> };
>
> + Size adjustSizes(const Size &size, const std::vector<SizeRange> &ranges,
> + Alignments align);
> +
> std::unique_ptr<V4L2M2MDevice> m2m_;
>
> std::map<const Stream *, std::unique_ptr<V4L2M2MStream>> streams_;
> diff --git a/src/libcamera/converter.cpp b/src/libcamera/converter.cpp
> index aa16970cd446..c3da162b7de7 100644
> --- a/src/libcamera/converter.cpp
> +++ b/src/libcamera/converter.cpp
> @@ -50,6 +50,21 @@ LOG_DEFINE_CATEGORY(Converter)
> * \brief A bitwise combination of features supported by the converter
> */
>
> +/**
> + * \enum Converter::Alignment
> + * \brief The alignment mode specified when adjusting the converter input or
> + * output sizes
> + * \var Converter::Alignment::Down
> + * \brief Adjust the Converter sizes to a smaller valid size
> + * \var Converter::Alignment::Up
> + * \brief Adjust the Converter sizes to a larger valid size
> + */
> +
> +/**
> + * \typedef Converter::Alignments
> + * \brief A bitwise combination of alignments supported by the converter
> + */
> +
> /**
> * \brief Construct a Converter instance
> * \param[in] media The media device implementing the converter
> @@ -110,6 +125,24 @@ Converter::~Converter()
> * \return A range of output image sizes
> */
>
> +/**
> + * \fn Converter::adjustInputSize()
> + * \brief Adjust the converter input \a size to a valid value
> + * \param[in] pixFmt The pixel format of the converter input stream
> + * \param[in] size The converter input size to adjust to a valid value
> + * \param[in] align The desired alignment
> + * \return The adjusted converter input size
> + */
What gets returned if there is no valid side for the request?
My comments where all nits. So
Reviewed-by: Stefan Klug <stefan.klug at ideasonboard.com>
> +
> +/**
> + * \fn Converter::adjustOutputSize()
> + * \brief Adjust the converter output \a size to a valid value
> + * \param[in] pixFmt The pixel format of the converter output stream
> + * \param[in] size The converter output size to adjust to a valid value
> + * \param[in] align The desired alignment
> + * \return The adjusted converter output size
> + */
> +
> /**
> * \fn Converter::strideAndFrameSize()
> * \brief Retrieve the output stride and frame size for an input configutation
> @@ -118,6 +151,16 @@ Converter::~Converter()
> * \return A tuple indicating the stride and frame size or an empty tuple on error
> */
>
> +/**
> + * \fn Converter::validateOutput()
> + * \brief Validate and possibily adjust \a cfg to a valid converter output
> + * \param[inout] cfg The StreamConfiguration to validate and adjust
> + * \param[out] adjusted Set to true if \a cfg has been adjusted
> + * \param[in] align The desired alignment
> + * \return 0 if \a cfg is valid or has been adjusted, a negative error code
> + * otherwise if \a cfg cannot be adjusted
> + */
> +
> /**
> * \fn Converter::configure()
> * \brief Configure a set of output stream conversion from an input stream
> diff --git a/src/libcamera/converter/converter_v4l2_m2m.cpp b/src/libcamera/converter/converter_v4l2_m2m.cpp
> index bd7e5cce600d..6857472b29f2 100644
> --- a/src/libcamera/converter/converter_v4l2_m2m.cpp
> +++ b/src/libcamera/converter/converter_v4l2_m2m.cpp
> @@ -8,6 +8,7 @@
>
> #include "libcamera/internal/converter/converter_v4l2_m2m.h"
>
> +#include <algorithm>
> #include <limits.h>
>
> #include <libcamera/base/log.h>
> @@ -400,6 +401,127 @@ V4L2M2MConverter::strideAndFrameSize(const PixelFormat &pixelFormat,
> return std::make_tuple(format.planes[0].bpl, format.planes[0].size);
> }
>
> +/**
> + * \copydoc libcamera::Converter::adjustInputSize
> + */
> +Size V4L2M2MConverter::adjustInputSize(const PixelFormat &pixFmt,
> + const Size &size, Alignments align)
> +{
> + auto formats = m2m_->output()->formats();
> + V4L2PixelFormat v4l2PixFmt = m2m_->output()->toV4L2PixelFormat(pixFmt);
> +
> + auto it = formats.find(v4l2PixFmt);
> + if (it == formats.end()) {
> + LOG(Converter, Info)
> + << "Unsupported pixel format " << pixFmt;
> + return {};
> + }
> +
> + return adjustSizes(size, it->second, align);
> +}
> +
> +/**
> + * \copydoc libcamera::Converter::adjustOutputSize
> + */
> +Size V4L2M2MConverter::adjustOutputSize(const PixelFormat &pixFmt,
> + const Size &size, Alignments align)
> +{
> + auto formats = m2m_->capture()->formats();
> + V4L2PixelFormat v4l2PixFmt = m2m_->capture()->toV4L2PixelFormat(pixFmt);
> +
> + auto it = formats.find(v4l2PixFmt);
> + if (it == formats.end()) {
> + LOG(Converter, Info)
> + << "Unsupported pixel format " << pixFmt;
> + return {};
> + }
> +
> + return adjustSizes(size, it->second, align);
> +}
> +
> +Size V4L2M2MConverter::adjustSizes(const Size &cfgSize,
> + const std::vector<SizeRange> &ranges,
> + Alignments align)
> +{
> + Size size = cfgSize;
> +
> + if (ranges.size() == 1) {
> + /*
> + * The device supports either V4L2_FRMSIZE_TYPE_CONTINUOUS or
> + * V4L2_FRMSIZE_TYPE_STEPWISE.
> + */
> + const SizeRange &range = *ranges.begin();
> +
> + size.width = std::clamp(size.width, range.min.width,
> + range.max.width);
> + size.height = std::clamp(size.height, range.min.height,
> + range.max.height);
> +
> + /*
> + * Check if any alignment is needed. If the sizes are already
> + * aligned, or the device supports V4L2_FRMSIZE_TYPE_CONTINUOUS
> + * with hStep and vStep equal to 1, we're done here.
> + */
> + int widthR = size.width % range.hStep;
> + int heightR = size.height % range.vStep;
> +
> + /* Align up or down according to the caller request. */
> +
> + if (widthR != 0)
> + size.width = size.width - widthR
> + + ((align == Alignment::Up) ? range.hStep : 0);
> +
> + if (heightR != 0)
> + size.height = size.height - heightR
> + + ((align == Alignment::Up) ? range.vStep : 0);
> + } else {
> + /*
> + * The device supports V4L2_FRMSIZE_TYPE_DISCRETE, find the
> + * size closer to the requested output configuration.
> + *
> + * The size ranges vector is not ordered, so we sort it first.
> + * If we align up, start from the larger element.
> + */
> + std::vector<Size> sizes(ranges.size());
> + std::transform(ranges.begin(), ranges.end(), std::back_inserter(sizes),
> + [](const SizeRange &range) { return range.max; });
> + std::sort(sizes.begin(), sizes.end());
> +
> + if (align == Alignment::Up)
> + std::reverse(sizes.begin(), sizes.end());
> +
> + /*
> + * Return true if s2 is valid according to the desired
> + * alignment: smaller than s1 if we align down, larger than s1
> + * if we align up.
> + */
> + auto nextSizeValid = [](const Size &s1, const Size &s2, Alignments a) {
> + return a == Alignment::Down
> + ? (s1.width > s2.width && s1.height > s2.height)
> + : (s1.width < s2.width && s1.height < s2.height);
> + };
> +
> + Size newSize;
> + for (const Size &sz : sizes) {
> + if (!nextSizeValid(size, sz, align))
> + break;
> +
> + newSize = sz;
> + }
> +
> + if (newSize.isNull()) {
> + LOG(Converter, Error)
> + << "Cannot adjust " << cfgSize
> + << " to a supported converter size";
> + return {};
> + }
> +
> + size = newSize;
> + }
> +
> + return size;
> +}
> +
> /**
> * \copydoc libcamera::Converter::configure
> */
> @@ -507,6 +629,53 @@ void V4L2M2MConverter::stop()
> iter.second->stop();
> }
>
> +/**
> + * \copydoc libcamera::Converter::validateOutput
> + */
> +int V4L2M2MConverter::validateOutput(StreamConfiguration *cfg, bool *adjusted,
> + Alignments align)
> +{
> + V4L2VideoDevice *capture = m2m_->capture();
> + V4L2VideoDevice::Formats fmts = capture->formats();
> +
> + if (adjusted)
> + *adjusted = false;
> +
> + PixelFormat fmt = cfg->pixelFormat;
> + V4L2PixelFormat v4l2PixFmt = capture->toV4L2PixelFormat(fmt);
> +
> + auto it = fmts.find(v4l2PixFmt);
> + if (it == fmts.end()) {
> + it = fmts.begin();
> + v4l2PixFmt = it->first;
> + cfg->pixelFormat = v4l2PixFmt.toPixelFormat();
> +
> + if (adjusted)
> + *adjusted = true;
> +
> + LOG(Converter, Info)
> + << "Converter output pixel format adjusted to "
> + << cfg->pixelFormat;
> + }
> +
> + const Size cfgSize = cfg->size;
> + cfg->size = adjustSizes(cfgSize, it->second, align);
> +
> + if (cfg->size.isNull())
> + return -EINVAL;
> +
> + if (cfg->size.width != cfgSize.width ||
> + cfg->size.height != cfgSize.height) {
> + LOG(Converter, Info)
> + << "Converter size adjusted to "
> + << cfg->size;
> + if (adjusted)
> + *adjusted = true;
> + }
> +
> + return 0;
> +}
> +
> /**
> * \copydoc libcamera::Converter::queueBuffers
> */
> --
> 2.43.0
>
More information about the libcamera-devel
mailing list