[PATCH v3 09/17] libcamera: converter: Add functions to adjust config
Jacopo Mondi
jacopo.mondi at ideasonboard.com
Thu Dec 12 18:28:16 CET 2024
Hi Paul
On Thu, Dec 12, 2024 at 08:43:15PM +0900, Paul Elder wrote:
> 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 confused. Here (and everywhere else it's used), since the type is
> Alignments (Flags<Alignment>), doesn't that mean that
> Alignment::Down | Alignment::Up is a valid input? Wouldn't that cause
> unexpected behavior?
>
> Although I suppose if Alignment::Down | Alignment::Up was actually
> passed then it wouldn't be equal Alignment::Down nor Alignment::Up so
> nothing would happen? Except maybe the ternary operators ig, those would
> just activate the false clause.
>
> Am I missing something here?
>
nah, you're absolutely right, Alignments should be a Flag<>, it's fine
as an enum!
I'll send fixups
>
> Paul
>
>
> > +
> > 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
> > + */
> > +
> > +/**
> > + * \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