[PATCH v3 09/17] libcamera: converter: Add functions to adjust config
Paul Elder
paul.elder at ideasonboard.com
Fri Dec 13 09:37:20 CET 2024
On Thu, Dec 12, 2024 at 06:28:16PM +0100, Jacopo Mondi wrote:
> 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
Fixup looks good! With that,
Reviewed-by: Paul Elder <paul.elder at ideasonboard.com>
> >
> >
> > > +
> > > 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