| Message ID | 20221010131744.513261-3-xavier.roumegue@oss.nxp.com |
|---|---|
| State | Accepted |
| Headers | show |
| Series |
|
| Related | show |
Hi Xavier On Mon, Oct 10, 2022 at 03:17:44PM +0200, Xavier Roumegue (OSS) wrote: > From: Xavier Roumegue <xavier.roumegue@oss.nxp.com> > > Move the simple converter implementation to a generic V4L2 M2M class > derived from the converter interface. This latter could be used by > other pipeline implementations and as base class for customized V4L2 M2M > converters. > > Signed-off-by: Xavier Roumegue <xavier.roumegue@oss.nxp.com> > --- > .../internal/converter/converter_v4l2_m2m.h | 18 +-- > .../libcamera/internal/converter/meson.build | 5 + > include/libcamera/internal/meson.build | 2 + > .../converter_v4l2_m2m.cpp} | 149 +++++++++++------- > src/libcamera/converter/meson.build | 5 + > src/libcamera/meson.build | 1 + > src/libcamera/pipeline/simple/meson.build | 1 - > src/libcamera/pipeline/simple/simple.cpp | 6 +- > 8 files changed, 119 insertions(+), 68 deletions(-) > rename src/libcamera/pipeline/simple/converter.h => include/libcamera/internal/converter/converter_v4l2_m2m.h (83%) > create mode 100644 include/libcamera/internal/converter/meson.build > rename src/libcamera/{pipeline/simple/converter.cpp => converter/converter_v4l2_m2m.cpp} (69%) > create mode 100644 src/libcamera/converter/meson.build > > diff --git a/src/libcamera/pipeline/simple/converter.h b/include/libcamera/internal/converter/converter_v4l2_m2m.h > similarity index 83% > rename from src/libcamera/pipeline/simple/converter.h > rename to include/libcamera/internal/converter/converter_v4l2_m2m.h > index f0ebe2e0..ef31eeba 100644 > --- a/src/libcamera/pipeline/simple/converter.h > +++ b/include/libcamera/internal/converter/converter_v4l2_m2m.h > @@ -1,8 +1,9 @@ > /* SPDX-License-Identifier: LGPL-2.1-or-later */ > /* > * Copyright (C) 2020, Laurent Pinchart > + * Copyright 2022 NXP > * > - * converter.h - Format converter for simple pipeline handler > + * converter_v4l2_m2m.h - V4l2 M2M Format converter interface > */ > > #pragma once > @@ -19,6 +20,8 @@ > #include <libcamera/base/log.h> > #include <libcamera/base/signal.h> > > +#include "libcamera/internal/converter.h" > + > namespace libcamera { > > class FrameBuffer; > @@ -28,11 +31,12 @@ class SizeRange; > struct StreamConfiguration; > class V4L2M2MDevice; > > -class SimpleConverter > +class V4L2M2MConverter : public Converter > { > public: > - SimpleConverter(MediaDevice *media); > + V4L2M2MConverter(MediaDevice *media); > > + int loadConfiguration([[maybe_unused]] const std::string &filename) { return 0; } How do you envision this function to be called ? If it is only your derived class that calls this, should this be part of the abstract base class ? > bool isValid() const { return m2m_ != nullptr; } > > std::vector<PixelFormat> formats(PixelFormat input); > @@ -52,14 +56,11 @@ public: > int queueBuffers(FrameBuffer *input, > const std::map<unsigned int, FrameBuffer *> &outputs); > > - Signal<FrameBuffer *> inputBufferReady; > - Signal<FrameBuffer *> outputBufferReady; > - > private: > class Stream : protected Loggable > { > public: > - Stream(SimpleConverter *converter, unsigned int index); > + Stream(V4L2M2MConverter *converter, unsigned int index); > > bool isValid() const { return m2m_ != nullptr; } > > @@ -80,7 +81,7 @@ private: > void captureBufferReady(FrameBuffer *buffer); > void outputBufferReady(FrameBuffer *buffer); > > - SimpleConverter *converter_; > + V4L2M2MConverter *converter_; > unsigned int index_; > std::unique_ptr<V4L2M2MDevice> m2m_; > > @@ -88,7 +89,6 @@ private: > unsigned int outputBufferCount_; > }; > > - std::string deviceNode_; > std::unique_ptr<V4L2M2MDevice> m2m_; > > std::vector<Stream> streams_; > diff --git a/include/libcamera/internal/converter/meson.build b/include/libcamera/internal/converter/meson.build > new file mode 100644 > index 00000000..891e79e7 > --- /dev/null > +++ b/include/libcamera/internal/converter/meson.build > @@ -0,0 +1,5 @@ > +# SPDX-License-Identifier: CC0-1.0 > + > +libcamera_internal_headers += files([ > + 'converter_v4l2_m2m.h', > +]) > diff --git a/include/libcamera/internal/meson.build b/include/libcamera/internal/meson.build > index 8f50d755..b9db5a8c 100644 > --- a/include/libcamera/internal/meson.build > +++ b/include/libcamera/internal/meson.build > @@ -45,3 +45,5 @@ libcamera_internal_headers = files([ > 'v4l2_videodevice.h', > 'yaml_parser.h', > ]) > + > +subdir('converter') > diff --git a/src/libcamera/pipeline/simple/converter.cpp b/src/libcamera/converter/converter_v4l2_m2m.cpp > similarity index 69% > rename from src/libcamera/pipeline/simple/converter.cpp > rename to src/libcamera/converter/converter_v4l2_m2m.cpp > index acaaa64c..5af054be 100644 > --- a/src/libcamera/pipeline/simple/converter.cpp > +++ b/src/libcamera/converter/converter_v4l2_m2m.cpp > @@ -1,12 +1,11 @@ > /* SPDX-License-Identifier: LGPL-2.1-or-later */ > /* > * Copyright (C) 2020, Laurent Pinchart > + * Copyright 2022 NXP > * > - * converter.cpp - Format converter for simple pipeline handler > + * converter_v4l2_m2m.cpp - V4L2 M2M Format converter > */ > > -#include "converter.h" > - > #include <algorithm> > #include <limits.h> > > @@ -20,19 +19,32 @@ > > #include "libcamera/internal/media_device.h" > #include "libcamera/internal/v4l2_videodevice.h" > +#include "libcamera/internal/converter/converter_v4l2_m2m.h" Shouldn't this be included first ? > + > +/** > + * \file internal/converter/converter_v4l2_m2m.h > + * \brief V4L2 M2M based converter > + */ > > namespace libcamera { > > -LOG_DECLARE_CATEGORY(SimplePipeline) > +LOG_DECLARE_CATEGORY(Converter) > + > +/** > + * \class V4L2M2MConverter > + * \brief V4L2 M2M based converter > + * > + * The V4L2 M2M converter implements the converter interface based on V4L2 M2M device. > + */ > > /* ----------------------------------------------------------------------------- > - * SimpleConverter::Stream > + * V4L2M2MConverter::Stream > */ > > -SimpleConverter::Stream::Stream(SimpleConverter *converter, unsigned int index) > +V4L2M2MConverter::Stream::Stream(V4L2M2MConverter *converter, unsigned int index) > : converter_(converter), index_(index) > { > - m2m_ = std::make_unique<V4L2M2MDevice>(converter->deviceNode_); > + m2m_ = std::make_unique<V4L2M2MDevice>(converter->deviceNode()); isValid() simply checks for m2m_ != nullptr. The m2m video dev might fail at open() time. I would use a valid_ flag, to be set to true at the end of the constructor > > m2m_->output()->bufferReady.connect(this, &Stream::outputBufferReady); > m2m_->capture()->bufferReady.connect(this, &Stream::captureBufferReady); > @@ -42,8 +54,8 @@ SimpleConverter::Stream::Stream(SimpleConverter *converter, unsigned int index) > m2m_.reset(); > } > > -int SimpleConverter::Stream::configure(const StreamConfiguration &inputCfg, > - const StreamConfiguration &outputCfg) > +int V4L2M2MConverter::Stream::configure(const StreamConfiguration &inputCfg, > + const StreamConfiguration &outputCfg) > { > V4L2PixelFormat videoFormat = > m2m_->output()->toV4L2PixelFormat(inputCfg.pixelFormat); > @@ -56,14 +68,14 @@ int SimpleConverter::Stream::configure(const StreamConfiguration &inputCfg, > > int ret = m2m_->output()->setFormat(&format); > if (ret < 0) { > - LOG(SimplePipeline, Error) > + LOG(Converter, Error) > << "Failed to set input format: " << strerror(-ret); > return ret; > } > > if (format.fourcc != videoFormat || format.size != inputCfg.size || > format.planes[0].bpl != inputCfg.stride) { > - LOG(SimplePipeline, Error) > + LOG(Converter, Error) > << "Input format not supported (requested " > << inputCfg.size << "-" << videoFormat > << ", got " << format << ")"; > @@ -78,13 +90,13 @@ int SimpleConverter::Stream::configure(const StreamConfiguration &inputCfg, > > ret = m2m_->capture()->setFormat(&format); > if (ret < 0) { > - LOG(SimplePipeline, Error) > + LOG(Converter, Error) > << "Failed to set output format: " << strerror(-ret); > return ret; > } > > if (format.fourcc != videoFormat || format.size != outputCfg.size) { > - LOG(SimplePipeline, Error) > + LOG(Converter, Error) > << "Output format not supported"; > return -EINVAL; > } > @@ -95,13 +107,13 @@ int SimpleConverter::Stream::configure(const StreamConfiguration &inputCfg, > return 0; > } > > -int SimpleConverter::Stream::exportBuffers(unsigned int count, > - std::vector<std::unique_ptr<FrameBuffer>> *buffers) > +int V4L2M2MConverter::Stream::exportBuffers(unsigned int count, > + std::vector<std::unique_ptr<FrameBuffer>> *buffers) > { > return m2m_->capture()->exportBuffers(count, buffers); > } > > -int SimpleConverter::Stream::start() > +int V4L2M2MConverter::Stream::start() > { > int ret = m2m_->output()->importBuffers(inputBufferCount_); > if (ret < 0) > @@ -128,7 +140,7 @@ int SimpleConverter::Stream::start() > return 0; > } > > -void SimpleConverter::Stream::stop() > +void V4L2M2MConverter::Stream::stop() > { > m2m_->capture()->streamOff(); > m2m_->output()->streamOff(); > @@ -136,8 +148,7 @@ void SimpleConverter::Stream::stop() > m2m_->output()->releaseBuffers(); > } > > -int SimpleConverter::Stream::queueBuffers(FrameBuffer *input, > - FrameBuffer *output) > +int V4L2M2MConverter::Stream::queueBuffers(FrameBuffer *input, FrameBuffer *output) > { > int ret = m2m_->output()->queueBuffer(input); > if (ret < 0) > @@ -150,12 +161,12 @@ int SimpleConverter::Stream::queueBuffers(FrameBuffer *input, > return 0; > } > > -std::string SimpleConverter::Stream::logPrefix() const > +std::string V4L2M2MConverter::Stream::logPrefix() const > { > return "stream" + std::to_string(index_); > } > > -void SimpleConverter::Stream::outputBufferReady(FrameBuffer *buffer) > +void V4L2M2MConverter::Stream::outputBufferReady(FrameBuffer *buffer) > { > auto it = converter_->queue_.find(buffer); > if (it == converter_->queue_.end()) > @@ -167,32 +178,26 @@ void SimpleConverter::Stream::outputBufferReady(FrameBuffer *buffer) > } > } > > -void SimpleConverter::Stream::captureBufferReady(FrameBuffer *buffer) > +void V4L2M2MConverter::Stream::captureBufferReady(FrameBuffer *buffer) > { > converter_->outputBufferReady.emit(buffer); > } > > /* ----------------------------------------------------------------------------- > - * SimpleConverter > + * V4L2M2MConverter > */ > > -SimpleConverter::SimpleConverter(MediaDevice *media) > +/** > + * \brief Construct a V4L2M2MConverter instance > + * \param[in] media The media device implementing the converter > + */ > +V4L2M2MConverter::V4L2M2MConverter(MediaDevice *media) > + : Converter(media) > { > - /* > - * Locate the video node. There's no need to validate the pipeline > - * further, the caller guarantees that this is a V4L2 mem2mem device. > - */ > - const std::vector<MediaEntity *> &entities = media->entities(); > - auto it = std::find_if(entities.begin(), entities.end(), > - [](MediaEntity *entity) { > - return entity->function() == MEDIA_ENT_F_IO_V4L; > - }); > - if (it == entities.end()) > + if (deviceNode().empty()) > return; > > - deviceNode_ = (*it)->deviceNode(); > - > - m2m_ = std::make_unique<V4L2M2MDevice>(deviceNode_); > + m2m_ = std::make_unique<V4L2M2MDevice>(deviceNode()); > int ret = m2m_->open(); > if (ret < 0) { > m2m_.reset(); > @@ -200,7 +205,18 @@ SimpleConverter::SimpleConverter(MediaDevice *media) > } > } > > -std::vector<PixelFormat> SimpleConverter::formats(PixelFormat input) > +/** > + * \copydoc libcamera::Converter::loadConfiguration > + */ > + > +/** > + * \copydoc libcamera::Converter::isValid > + */ > + > +/** > + * \copydoc libcamera::Converter::formats > + */ > +std::vector<PixelFormat> V4L2M2MConverter::formats(PixelFormat input) > { > if (!m2m_) > return {}; > @@ -215,13 +231,13 @@ std::vector<PixelFormat> SimpleConverter::formats(PixelFormat input) > > int ret = m2m_->output()->setFormat(&v4l2Format); > if (ret < 0) { > - LOG(SimplePipeline, Error) > + LOG(Converter, Error) > << "Failed to set format: " << strerror(-ret); > return {}; > } > > if (v4l2Format.fourcc != m2m_->output()->toV4L2PixelFormat(input)) { > - LOG(SimplePipeline, Debug) > + LOG(Converter, Debug) > << "Input format " << input << " not supported."; > return {}; > } > @@ -237,7 +253,10 @@ std::vector<PixelFormat> SimpleConverter::formats(PixelFormat input) > return pixelFormats; > } > > -SizeRange SimpleConverter::sizes(const Size &input) > +/** > + * \copydoc libcamera::Converter::sizes > + */ > +SizeRange V4L2M2MConverter::sizes(const Size &input) > { > if (!m2m_) > return {}; > @@ -252,7 +271,7 @@ SizeRange SimpleConverter::sizes(const Size &input) > > int ret = m2m_->output()->setFormat(&format); > if (ret < 0) { > - LOG(SimplePipeline, Error) > + LOG(Converter, Error) > << "Failed to set format: " << strerror(-ret); > return {}; > } > @@ -262,7 +281,7 @@ SizeRange SimpleConverter::sizes(const Size &input) > format.size = { 1, 1 }; > ret = m2m_->capture()->setFormat(&format); > if (ret < 0) { > - LOG(SimplePipeline, Error) > + LOG(Converter, Error) > << "Failed to set format: " << strerror(-ret); > return {}; > } > @@ -272,7 +291,7 @@ SizeRange SimpleConverter::sizes(const Size &input) > format.size = { UINT_MAX, UINT_MAX }; > ret = m2m_->capture()->setFormat(&format); > if (ret < 0) { > - LOG(SimplePipeline, Error) > + LOG(Converter, Error) > << "Failed to set format: " << strerror(-ret); > return {}; > } > @@ -282,9 +301,12 @@ SizeRange SimpleConverter::sizes(const Size &input) > return sizes; > } > > +/** > + * \copydoc libcamera::Converter::strideAndFrameSize > + */ > std::tuple<unsigned int, unsigned int> > -SimpleConverter::strideAndFrameSize(const PixelFormat &pixelFormat, > - const Size &size) > +V4L2M2MConverter::strideAndFrameSize(const PixelFormat &pixelFormat, > + const Size &size) > { > V4L2DeviceFormat format; > format.fourcc = m2m_->capture()->toV4L2PixelFormat(pixelFormat); > @@ -297,8 +319,11 @@ SimpleConverter::strideAndFrameSize(const PixelFormat &pixelFormat, > return std::make_tuple(format.planes[0].bpl, format.planes[0].size); > } > > -int SimpleConverter::configure(const StreamConfiguration &inputCfg, > - const std::vector<std::reference_wrapper<StreamConfiguration>> &outputCfgs) > +/** > + * \copydoc libcamera::Converter::configure > + */ > +int V4L2M2MConverter::configure(const StreamConfiguration &inputCfg, > + const std::vector<std::reference_wrapper<StreamConfiguration>> &outputCfgs) > { > int ret = 0; > > @@ -309,7 +334,7 @@ int SimpleConverter::configure(const StreamConfiguration &inputCfg, > Stream &stream = streams_.emplace_back(this, i); > > if (!stream.isValid()) { > - LOG(SimplePipeline, Error) > + LOG(Converter, Error) > << "Failed to create stream " << i; > ret = -EINVAL; > break; > @@ -328,8 +353,11 @@ int SimpleConverter::configure(const StreamConfiguration &inputCfg, > return 0; > } > > -int SimpleConverter::exportBuffers(unsigned int output, unsigned int count, > - std::vector<std::unique_ptr<FrameBuffer>> *buffers) > +/** > + * \copydoc libcamera::Converter::exportBuffers > + */ > +int V4L2M2MConverter::exportBuffers(unsigned int output, unsigned int count, > + std::vector<std::unique_ptr<FrameBuffer>> *buffers) > { > if (output >= streams_.size()) > return -EINVAL; > @@ -337,7 +365,10 @@ int SimpleConverter::exportBuffers(unsigned int output, unsigned int count, > return streams_[output].exportBuffers(count, buffers); > } > > -int SimpleConverter::start() > +/** > + * \copydoc libcamera::Converter::start > + */ > +int V4L2M2MConverter::start() > { > int ret; > > @@ -352,14 +383,20 @@ int SimpleConverter::start() > return 0; > } > > -void SimpleConverter::stop() > +/** > + * \copydoc libcamera::Converter::stop > + */ > +void V4L2M2MConverter::stop() > { > for (Stream &stream : utils::reverse(streams_)) > stream.stop(); > } > > -int SimpleConverter::queueBuffers(FrameBuffer *input, > - const std::map<unsigned int, FrameBuffer *> &outputs) > +/** > + * \copydoc libcamera::Converter::queueBuffers > + */ > +int V4L2M2MConverter::queueBuffers(FrameBuffer *input, > + const std::map<unsigned int, FrameBuffer *> &outputs) > { > unsigned int mask = 0; > int ret; > @@ -402,4 +439,6 @@ int SimpleConverter::queueBuffers(FrameBuffer *input, > return 0; > } > > +REGISTER_CONVERTER("v4l2_m2m", V4L2M2MConverter, "pxp") > + > } /* namespace libcamera */ > diff --git a/src/libcamera/converter/meson.build b/src/libcamera/converter/meson.build > new file mode 100644 > index 00000000..2aa72fe4 > --- /dev/null > +++ b/src/libcamera/converter/meson.build > @@ -0,0 +1,5 @@ > +# SPDX-License-Identifier: CC0-1.0 > + > +libcamera_sources += files([ > + 'converter_v4l2_m2m.cpp' > +]) > diff --git a/src/libcamera/meson.build b/src/libcamera/meson.build > index 3a9fc31f..d2f75741 100644 > --- a/src/libcamera/meson.build > +++ b/src/libcamera/meson.build > @@ -62,6 +62,7 @@ libatomic = cc.find_library('atomic', required : false) > libthreads = dependency('threads') > > subdir('base') > +subdir('converter') > subdir('ipa') > subdir('pipeline') > subdir('proxy') > diff --git a/src/libcamera/pipeline/simple/meson.build b/src/libcamera/pipeline/simple/meson.build > index 9c99b32f..42b0896d 100644 > --- a/src/libcamera/pipeline/simple/meson.build > +++ b/src/libcamera/pipeline/simple/meson.build > @@ -1,6 +1,5 @@ > # SPDX-License-Identifier: CC0-1.0 > > libcamera_sources += files([ > - 'converter.cpp', > 'simple.cpp', > ]) > diff --git a/src/libcamera/pipeline/simple/simple.cpp b/src/libcamera/pipeline/simple/simple.cpp > index 37b3e7ac..5cc6bc6a 100644 > --- a/src/libcamera/pipeline/simple/simple.cpp > +++ b/src/libcamera/pipeline/simple/simple.cpp > @@ -30,13 +30,13 @@ > > #include "libcamera/internal/camera.h" > #include "libcamera/internal/camera_sensor.h" > +#include "libcamera/internal/converter.h" > #include "libcamera/internal/device_enumerator.h" > #include "libcamera/internal/media_device.h" > #include "libcamera/internal/pipeline_handler.h" > #include "libcamera/internal/v4l2_subdevice.h" > #include "libcamera/internal/v4l2_videodevice.h" > > -#include "converter.h" > > namespace libcamera { > > @@ -266,7 +266,7 @@ public: > std::vector<Configuration> configs_; > std::map<PixelFormat, std::vector<const Configuration *>> formats_; > > - std::unique_ptr<SimpleConverter> converter_; > + std::unique_ptr<Converter> converter_; > std::vector<std::unique_ptr<FrameBuffer>> converterBuffers_; > bool useConverter_; > std::queue<std::map<unsigned int, FrameBuffer *>> converterQueue_; > @@ -492,7 +492,7 @@ int SimpleCameraData::init() > /* Open the converter, if any. */ > MediaDevice *converter = pipe->converter(); > if (converter) { > - converter_ = std::make_unique<SimpleConverter>(converter); > + converter_ = ConverterFactoryBase::create(converter); > if (!converter_->isValid()) { > LOG(SimplePipeline, Warning) > << "Failed to create converter, disabling format conversion"; All minors: Reviewed-by: Jacopo Mondi <jacopo@jmondi.org> Thanks j > -- > 2.37.3 >
Hi Jacopo, On 11/9/22 08:54, Jacopo Mondi wrote: > Hi Xavier > > On Mon, Oct 10, 2022 at 03:17:44PM +0200, Xavier Roumegue (OSS) wrote: >> From: Xavier Roumegue <xavier.roumegue@oss.nxp.com> >> >> Move the simple converter implementation to a generic V4L2 M2M class >> derived from the converter interface. This latter could be used by >> other pipeline implementations and as base class for customized V4L2 M2M >> converters. >> >> Signed-off-by: Xavier Roumegue <xavier.roumegue@oss.nxp.com> >> --- >> .../internal/converter/converter_v4l2_m2m.h | 18 +-- >> .../libcamera/internal/converter/meson.build | 5 + >> include/libcamera/internal/meson.build | 2 + >> .../converter_v4l2_m2m.cpp} | 149 +++++++++++------- >> src/libcamera/converter/meson.build | 5 + >> src/libcamera/meson.build | 1 + >> src/libcamera/pipeline/simple/meson.build | 1 - >> src/libcamera/pipeline/simple/simple.cpp | 6 +- >> 8 files changed, 119 insertions(+), 68 deletions(-) >> rename src/libcamera/pipeline/simple/converter.h => include/libcamera/internal/converter/converter_v4l2_m2m.h (83%) >> create mode 100644 include/libcamera/internal/converter/meson.build >> rename src/libcamera/{pipeline/simple/converter.cpp => converter/converter_v4l2_m2m.cpp} (69%) >> create mode 100644 src/libcamera/converter/meson.build >> >> diff --git a/src/libcamera/pipeline/simple/converter.h b/include/libcamera/internal/converter/converter_v4l2_m2m.h >> similarity index 83% >> rename from src/libcamera/pipeline/simple/converter.h >> rename to include/libcamera/internal/converter/converter_v4l2_m2m.h >> index f0ebe2e0..ef31eeba 100644 >> --- a/src/libcamera/pipeline/simple/converter.h >> +++ b/include/libcamera/internal/converter/converter_v4l2_m2m.h >> @@ -1,8 +1,9 @@ >> /* SPDX-License-Identifier: LGPL-2.1-or-later */ >> /* >> * Copyright (C) 2020, Laurent Pinchart >> + * Copyright 2022 NXP >> * >> - * converter.h - Format converter for simple pipeline handler >> + * converter_v4l2_m2m.h - V4l2 M2M Format converter interface >> */ >> >> #pragma once >> @@ -19,6 +20,8 @@ >> #include <libcamera/base/log.h> >> #include <libcamera/base/signal.h> >> >> +#include "libcamera/internal/converter.h" >> + >> namespace libcamera { >> >> class FrameBuffer; >> @@ -28,11 +31,12 @@ class SizeRange; >> struct StreamConfiguration; >> class V4L2M2MDevice; >> >> -class SimpleConverter >> +class V4L2M2MConverter : public Converter >> { >> public: >> - SimpleConverter(MediaDevice *media); >> + V4L2M2MConverter(MediaDevice *media); >> >> + int loadConfiguration([[maybe_unused]] const std::string &filename) { return 0; } > > How do you envision this function to be called ? Should be called from the pipeline handlers as the configuration file depends on the pipeline handler. > If it is only your derived class that calls this, should this be part > of the abstract base class ? > >> bool isValid() const { return m2m_ != nullptr; } >> >> std::vector<PixelFormat> formats(PixelFormat input); >> @@ -52,14 +56,11 @@ public: >> int queueBuffers(FrameBuffer *input, >> const std::map<unsigned int, FrameBuffer *> &outputs); >> >> - Signal<FrameBuffer *> inputBufferReady; >> - Signal<FrameBuffer *> outputBufferReady; >> - >> private: >> class Stream : protected Loggable >> { >> public: >> - Stream(SimpleConverter *converter, unsigned int index); >> + Stream(V4L2M2MConverter *converter, unsigned int index); >> >> bool isValid() const { return m2m_ != nullptr; } >> >> @@ -80,7 +81,7 @@ private: >> void captureBufferReady(FrameBuffer *buffer); >> void outputBufferReady(FrameBuffer *buffer); >> >> - SimpleConverter *converter_; >> + V4L2M2MConverter *converter_; >> unsigned int index_; >> std::unique_ptr<V4L2M2MDevice> m2m_; >> >> @@ -88,7 +89,6 @@ private: >> unsigned int outputBufferCount_; >> }; >> >> - std::string deviceNode_; >> std::unique_ptr<V4L2M2MDevice> m2m_; >> >> std::vector<Stream> streams_; >> diff --git a/include/libcamera/internal/converter/meson.build b/include/libcamera/internal/converter/meson.build >> new file mode 100644 >> index 00000000..891e79e7 >> --- /dev/null >> +++ b/include/libcamera/internal/converter/meson.build >> @@ -0,0 +1,5 @@ >> +# SPDX-License-Identifier: CC0-1.0 >> + >> +libcamera_internal_headers += files([ >> + 'converter_v4l2_m2m.h', >> +]) >> diff --git a/include/libcamera/internal/meson.build b/include/libcamera/internal/meson.build >> index 8f50d755..b9db5a8c 100644 >> --- a/include/libcamera/internal/meson.build >> +++ b/include/libcamera/internal/meson.build >> @@ -45,3 +45,5 @@ libcamera_internal_headers = files([ >> 'v4l2_videodevice.h', >> 'yaml_parser.h', >> ]) >> + >> +subdir('converter') >> diff --git a/src/libcamera/pipeline/simple/converter.cpp b/src/libcamera/converter/converter_v4l2_m2m.cpp >> similarity index 69% >> rename from src/libcamera/pipeline/simple/converter.cpp >> rename to src/libcamera/converter/converter_v4l2_m2m.cpp >> index acaaa64c..5af054be 100644 >> --- a/src/libcamera/pipeline/simple/converter.cpp >> +++ b/src/libcamera/converter/converter_v4l2_m2m.cpp >> @@ -1,12 +1,11 @@ >> /* SPDX-License-Identifier: LGPL-2.1-or-later */ >> /* >> * Copyright (C) 2020, Laurent Pinchart >> + * Copyright 2022 NXP >> * >> - * converter.cpp - Format converter for simple pipeline handler >> + * converter_v4l2_m2m.cpp - V4L2 M2M Format converter >> */ >> >> -#include "converter.h" >> - >> #include <algorithm> >> #include <limits.h> >> >> @@ -20,19 +19,32 @@ >> >> #include "libcamera/internal/media_device.h" >> #include "libcamera/internal/v4l2_videodevice.h" >> +#include "libcamera/internal/converter/converter_v4l2_m2m.h" > > Shouldn't this be included first ? > >> + >> +/** >> + * \file internal/converter/converter_v4l2_m2m.h >> + * \brief V4L2 M2M based converter >> + */ >> >> namespace libcamera { >> >> -LOG_DECLARE_CATEGORY(SimplePipeline) >> +LOG_DECLARE_CATEGORY(Converter) >> + >> +/** >> + * \class V4L2M2MConverter >> + * \brief V4L2 M2M based converter >> + * >> + * The V4L2 M2M converter implements the converter interface based on V4L2 M2M device. >> + */ >> >> /* ----------------------------------------------------------------------------- >> - * SimpleConverter::Stream >> + * V4L2M2MConverter::Stream >> */ >> >> -SimpleConverter::Stream::Stream(SimpleConverter *converter, unsigned int index) >> +V4L2M2MConverter::Stream::Stream(V4L2M2MConverter *converter, unsigned int index) >> : converter_(converter), index_(index) >> { >> - m2m_ = std::make_unique<V4L2M2MDevice>(converter->deviceNode_); >> + m2m_ = std::make_unique<V4L2M2MDevice>(converter->deviceNode()); > > isValid() simply checks for m2m_ != nullptr. > The m2m video dev might fail at open() time. > I would use a valid_ flag, to be set to true at the end of the > constructor > >> >> m2m_->output()->bufferReady.connect(this, &Stream::outputBufferReady); >> m2m_->capture()->bufferReady.connect(this, &Stream::captureBufferReady); >> @@ -42,8 +54,8 @@ SimpleConverter::Stream::Stream(SimpleConverter *converter, unsigned int index) >> m2m_.reset(); >> } >> >> -int SimpleConverter::Stream::configure(const StreamConfiguration &inputCfg, >> - const StreamConfiguration &outputCfg) >> +int V4L2M2MConverter::Stream::configure(const StreamConfiguration &inputCfg, >> + const StreamConfiguration &outputCfg) >> { >> V4L2PixelFormat videoFormat = >> m2m_->output()->toV4L2PixelFormat(inputCfg.pixelFormat); >> @@ -56,14 +68,14 @@ int SimpleConverter::Stream::configure(const StreamConfiguration &inputCfg, >> >> int ret = m2m_->output()->setFormat(&format); >> if (ret < 0) { >> - LOG(SimplePipeline, Error) >> + LOG(Converter, Error) >> << "Failed to set input format: " << strerror(-ret); >> return ret; >> } >> >> if (format.fourcc != videoFormat || format.size != inputCfg.size || >> format.planes[0].bpl != inputCfg.stride) { >> - LOG(SimplePipeline, Error) >> + LOG(Converter, Error) >> << "Input format not supported (requested " >> << inputCfg.size << "-" << videoFormat >> << ", got " << format << ")"; >> @@ -78,13 +90,13 @@ int SimpleConverter::Stream::configure(const StreamConfiguration &inputCfg, >> >> ret = m2m_->capture()->setFormat(&format); >> if (ret < 0) { >> - LOG(SimplePipeline, Error) >> + LOG(Converter, Error) >> << "Failed to set output format: " << strerror(-ret); >> return ret; >> } >> >> if (format.fourcc != videoFormat || format.size != outputCfg.size) { >> - LOG(SimplePipeline, Error) >> + LOG(Converter, Error) >> << "Output format not supported"; >> return -EINVAL; >> } >> @@ -95,13 +107,13 @@ int SimpleConverter::Stream::configure(const StreamConfiguration &inputCfg, >> return 0; >> } >> >> -int SimpleConverter::Stream::exportBuffers(unsigned int count, >> - std::vector<std::unique_ptr<FrameBuffer>> *buffers) >> +int V4L2M2MConverter::Stream::exportBuffers(unsigned int count, >> + std::vector<std::unique_ptr<FrameBuffer>> *buffers) >> { >> return m2m_->capture()->exportBuffers(count, buffers); >> } >> >> -int SimpleConverter::Stream::start() >> +int V4L2M2MConverter::Stream::start() >> { >> int ret = m2m_->output()->importBuffers(inputBufferCount_); >> if (ret < 0) >> @@ -128,7 +140,7 @@ int SimpleConverter::Stream::start() >> return 0; >> } >> >> -void SimpleConverter::Stream::stop() >> +void V4L2M2MConverter::Stream::stop() >> { >> m2m_->capture()->streamOff(); >> m2m_->output()->streamOff(); >> @@ -136,8 +148,7 @@ void SimpleConverter::Stream::stop() >> m2m_->output()->releaseBuffers(); >> } >> >> -int SimpleConverter::Stream::queueBuffers(FrameBuffer *input, >> - FrameBuffer *output) >> +int V4L2M2MConverter::Stream::queueBuffers(FrameBuffer *input, FrameBuffer *output) >> { >> int ret = m2m_->output()->queueBuffer(input); >> if (ret < 0) >> @@ -150,12 +161,12 @@ int SimpleConverter::Stream::queueBuffers(FrameBuffer *input, >> return 0; >> } >> >> -std::string SimpleConverter::Stream::logPrefix() const >> +std::string V4L2M2MConverter::Stream::logPrefix() const >> { >> return "stream" + std::to_string(index_); >> } >> >> -void SimpleConverter::Stream::outputBufferReady(FrameBuffer *buffer) >> +void V4L2M2MConverter::Stream::outputBufferReady(FrameBuffer *buffer) >> { >> auto it = converter_->queue_.find(buffer); >> if (it == converter_->queue_.end()) >> @@ -167,32 +178,26 @@ void SimpleConverter::Stream::outputBufferReady(FrameBuffer *buffer) >> } >> } >> >> -void SimpleConverter::Stream::captureBufferReady(FrameBuffer *buffer) >> +void V4L2M2MConverter::Stream::captureBufferReady(FrameBuffer *buffer) >> { >> converter_->outputBufferReady.emit(buffer); >> } >> >> /* ----------------------------------------------------------------------------- >> - * SimpleConverter >> + * V4L2M2MConverter >> */ >> >> -SimpleConverter::SimpleConverter(MediaDevice *media) >> +/** >> + * \brief Construct a V4L2M2MConverter instance >> + * \param[in] media The media device implementing the converter >> + */ >> +V4L2M2MConverter::V4L2M2MConverter(MediaDevice *media) >> + : Converter(media) >> { >> - /* >> - * Locate the video node. There's no need to validate the pipeline >> - * further, the caller guarantees that this is a V4L2 mem2mem device. >> - */ >> - const std::vector<MediaEntity *> &entities = media->entities(); >> - auto it = std::find_if(entities.begin(), entities.end(), >> - [](MediaEntity *entity) { >> - return entity->function() == MEDIA_ENT_F_IO_V4L; >> - }); >> - if (it == entities.end()) >> + if (deviceNode().empty()) >> return; >> >> - deviceNode_ = (*it)->deviceNode(); >> - >> - m2m_ = std::make_unique<V4L2M2MDevice>(deviceNode_); >> + m2m_ = std::make_unique<V4L2M2MDevice>(deviceNode()); >> int ret = m2m_->open(); >> if (ret < 0) { >> m2m_.reset(); >> @@ -200,7 +205,18 @@ SimpleConverter::SimpleConverter(MediaDevice *media) >> } >> } >> >> -std::vector<PixelFormat> SimpleConverter::formats(PixelFormat input) >> +/** >> + * \copydoc libcamera::Converter::loadConfiguration >> + */ >> + >> +/** >> + * \copydoc libcamera::Converter::isValid >> + */ >> + >> +/** >> + * \copydoc libcamera::Converter::formats >> + */ >> +std::vector<PixelFormat> V4L2M2MConverter::formats(PixelFormat input) >> { >> if (!m2m_) >> return {}; >> @@ -215,13 +231,13 @@ std::vector<PixelFormat> SimpleConverter::formats(PixelFormat input) >> >> int ret = m2m_->output()->setFormat(&v4l2Format); >> if (ret < 0) { >> - LOG(SimplePipeline, Error) >> + LOG(Converter, Error) >> << "Failed to set format: " << strerror(-ret); >> return {}; >> } >> >> if (v4l2Format.fourcc != m2m_->output()->toV4L2PixelFormat(input)) { >> - LOG(SimplePipeline, Debug) >> + LOG(Converter, Debug) >> << "Input format " << input << " not supported."; >> return {}; >> } >> @@ -237,7 +253,10 @@ std::vector<PixelFormat> SimpleConverter::formats(PixelFormat input) >> return pixelFormats; >> } >> >> -SizeRange SimpleConverter::sizes(const Size &input) >> +/** >> + * \copydoc libcamera::Converter::sizes >> + */ >> +SizeRange V4L2M2MConverter::sizes(const Size &input) >> { >> if (!m2m_) >> return {}; >> @@ -252,7 +271,7 @@ SizeRange SimpleConverter::sizes(const Size &input) >> >> int ret = m2m_->output()->setFormat(&format); >> if (ret < 0) { >> - LOG(SimplePipeline, Error) >> + LOG(Converter, Error) >> << "Failed to set format: " << strerror(-ret); >> return {}; >> } >> @@ -262,7 +281,7 @@ SizeRange SimpleConverter::sizes(const Size &input) >> format.size = { 1, 1 }; >> ret = m2m_->capture()->setFormat(&format); >> if (ret < 0) { >> - LOG(SimplePipeline, Error) >> + LOG(Converter, Error) >> << "Failed to set format: " << strerror(-ret); >> return {}; >> } >> @@ -272,7 +291,7 @@ SizeRange SimpleConverter::sizes(const Size &input) >> format.size = { UINT_MAX, UINT_MAX }; >> ret = m2m_->capture()->setFormat(&format); >> if (ret < 0) { >> - LOG(SimplePipeline, Error) >> + LOG(Converter, Error) >> << "Failed to set format: " << strerror(-ret); >> return {}; >> } >> @@ -282,9 +301,12 @@ SizeRange SimpleConverter::sizes(const Size &input) >> return sizes; >> } >> >> +/** >> + * \copydoc libcamera::Converter::strideAndFrameSize >> + */ >> std::tuple<unsigned int, unsigned int> >> -SimpleConverter::strideAndFrameSize(const PixelFormat &pixelFormat, >> - const Size &size) >> +V4L2M2MConverter::strideAndFrameSize(const PixelFormat &pixelFormat, >> + const Size &size) >> { >> V4L2DeviceFormat format; >> format.fourcc = m2m_->capture()->toV4L2PixelFormat(pixelFormat); >> @@ -297,8 +319,11 @@ SimpleConverter::strideAndFrameSize(const PixelFormat &pixelFormat, >> return std::make_tuple(format.planes[0].bpl, format.planes[0].size); >> } >> >> -int SimpleConverter::configure(const StreamConfiguration &inputCfg, >> - const std::vector<std::reference_wrapper<StreamConfiguration>> &outputCfgs) >> +/** >> + * \copydoc libcamera::Converter::configure >> + */ >> +int V4L2M2MConverter::configure(const StreamConfiguration &inputCfg, >> + const std::vector<std::reference_wrapper<StreamConfiguration>> &outputCfgs) >> { >> int ret = 0; >> >> @@ -309,7 +334,7 @@ int SimpleConverter::configure(const StreamConfiguration &inputCfg, >> Stream &stream = streams_.emplace_back(this, i); >> >> if (!stream.isValid()) { >> - LOG(SimplePipeline, Error) >> + LOG(Converter, Error) >> << "Failed to create stream " << i; >> ret = -EINVAL; >> break; >> @@ -328,8 +353,11 @@ int SimpleConverter::configure(const StreamConfiguration &inputCfg, >> return 0; >> } >> >> -int SimpleConverter::exportBuffers(unsigned int output, unsigned int count, >> - std::vector<std::unique_ptr<FrameBuffer>> *buffers) >> +/** >> + * \copydoc libcamera::Converter::exportBuffers >> + */ >> +int V4L2M2MConverter::exportBuffers(unsigned int output, unsigned int count, >> + std::vector<std::unique_ptr<FrameBuffer>> *buffers) >> { >> if (output >= streams_.size()) >> return -EINVAL; >> @@ -337,7 +365,10 @@ int SimpleConverter::exportBuffers(unsigned int output, unsigned int count, >> return streams_[output].exportBuffers(count, buffers); >> } >> >> -int SimpleConverter::start() >> +/** >> + * \copydoc libcamera::Converter::start >> + */ >> +int V4L2M2MConverter::start() >> { >> int ret; >> >> @@ -352,14 +383,20 @@ int SimpleConverter::start() >> return 0; >> } >> >> -void SimpleConverter::stop() >> +/** >> + * \copydoc libcamera::Converter::stop >> + */ >> +void V4L2M2MConverter::stop() >> { >> for (Stream &stream : utils::reverse(streams_)) >> stream.stop(); >> } >> >> -int SimpleConverter::queueBuffers(FrameBuffer *input, >> - const std::map<unsigned int, FrameBuffer *> &outputs) >> +/** >> + * \copydoc libcamera::Converter::queueBuffers >> + */ >> +int V4L2M2MConverter::queueBuffers(FrameBuffer *input, >> + const std::map<unsigned int, FrameBuffer *> &outputs) >> { >> unsigned int mask = 0; >> int ret; >> @@ -402,4 +439,6 @@ int SimpleConverter::queueBuffers(FrameBuffer *input, >> return 0; >> } >> >> +REGISTER_CONVERTER("v4l2_m2m", V4L2M2MConverter, "pxp") >> + >> } /* namespace libcamera */ >> diff --git a/src/libcamera/converter/meson.build b/src/libcamera/converter/meson.build >> new file mode 100644 >> index 00000000..2aa72fe4 >> --- /dev/null >> +++ b/src/libcamera/converter/meson.build >> @@ -0,0 +1,5 @@ >> +# SPDX-License-Identifier: CC0-1.0 >> + >> +libcamera_sources += files([ >> + 'converter_v4l2_m2m.cpp' >> +]) >> diff --git a/src/libcamera/meson.build b/src/libcamera/meson.build >> index 3a9fc31f..d2f75741 100644 >> --- a/src/libcamera/meson.build >> +++ b/src/libcamera/meson.build >> @@ -62,6 +62,7 @@ libatomic = cc.find_library('atomic', required : false) >> libthreads = dependency('threads') >> >> subdir('base') >> +subdir('converter') >> subdir('ipa') >> subdir('pipeline') >> subdir('proxy') >> diff --git a/src/libcamera/pipeline/simple/meson.build b/src/libcamera/pipeline/simple/meson.build >> index 9c99b32f..42b0896d 100644 >> --- a/src/libcamera/pipeline/simple/meson.build >> +++ b/src/libcamera/pipeline/simple/meson.build >> @@ -1,6 +1,5 @@ >> # SPDX-License-Identifier: CC0-1.0 >> >> libcamera_sources += files([ >> - 'converter.cpp', >> 'simple.cpp', >> ]) >> diff --git a/src/libcamera/pipeline/simple/simple.cpp b/src/libcamera/pipeline/simple/simple.cpp >> index 37b3e7ac..5cc6bc6a 100644 >> --- a/src/libcamera/pipeline/simple/simple.cpp >> +++ b/src/libcamera/pipeline/simple/simple.cpp >> @@ -30,13 +30,13 @@ >> >> #include "libcamera/internal/camera.h" >> #include "libcamera/internal/camera_sensor.h" >> +#include "libcamera/internal/converter.h" >> #include "libcamera/internal/device_enumerator.h" >> #include "libcamera/internal/media_device.h" >> #include "libcamera/internal/pipeline_handler.h" >> #include "libcamera/internal/v4l2_subdevice.h" >> #include "libcamera/internal/v4l2_videodevice.h" >> >> -#include "converter.h" >> >> namespace libcamera { >> >> @@ -266,7 +266,7 @@ public: >> std::vector<Configuration> configs_; >> std::map<PixelFormat, std::vector<const Configuration *>> formats_; >> >> - std::unique_ptr<SimpleConverter> converter_; >> + std::unique_ptr<Converter> converter_; >> std::vector<std::unique_ptr<FrameBuffer>> converterBuffers_; >> bool useConverter_; >> std::queue<std::map<unsigned int, FrameBuffer *>> converterQueue_; >> @@ -492,7 +492,7 @@ int SimpleCameraData::init() >> /* Open the converter, if any. */ >> MediaDevice *converter = pipe->converter(); >> if (converter) { >> - converter_ = std::make_unique<SimpleConverter>(converter); >> + converter_ = ConverterFactoryBase::create(converter); >> if (!converter_->isValid()) { >> LOG(SimplePipeline, Warning) >> << "Failed to create converter, disabling format conversion"; > > All minors: > Reviewed-by: Jacopo Mondi <jacopo@jmondi.org> > > Thanks > j > >> -- >> 2.37.3 >>
diff --git a/src/libcamera/pipeline/simple/converter.h b/include/libcamera/internal/converter/converter_v4l2_m2m.h similarity index 83% rename from src/libcamera/pipeline/simple/converter.h rename to include/libcamera/internal/converter/converter_v4l2_m2m.h index f0ebe2e0..ef31eeba 100644 --- a/src/libcamera/pipeline/simple/converter.h +++ b/include/libcamera/internal/converter/converter_v4l2_m2m.h @@ -1,8 +1,9 @@ /* SPDX-License-Identifier: LGPL-2.1-or-later */ /* * Copyright (C) 2020, Laurent Pinchart + * Copyright 2022 NXP * - * converter.h - Format converter for simple pipeline handler + * converter_v4l2_m2m.h - V4l2 M2M Format converter interface */ #pragma once @@ -19,6 +20,8 @@ #include <libcamera/base/log.h> #include <libcamera/base/signal.h> +#include "libcamera/internal/converter.h" + namespace libcamera { class FrameBuffer; @@ -28,11 +31,12 @@ class SizeRange; struct StreamConfiguration; class V4L2M2MDevice; -class SimpleConverter +class V4L2M2MConverter : public Converter { public: - SimpleConverter(MediaDevice *media); + V4L2M2MConverter(MediaDevice *media); + int loadConfiguration([[maybe_unused]] const std::string &filename) { return 0; } bool isValid() const { return m2m_ != nullptr; } std::vector<PixelFormat> formats(PixelFormat input); @@ -52,14 +56,11 @@ public: int queueBuffers(FrameBuffer *input, const std::map<unsigned int, FrameBuffer *> &outputs); - Signal<FrameBuffer *> inputBufferReady; - Signal<FrameBuffer *> outputBufferReady; - private: class Stream : protected Loggable { public: - Stream(SimpleConverter *converter, unsigned int index); + Stream(V4L2M2MConverter *converter, unsigned int index); bool isValid() const { return m2m_ != nullptr; } @@ -80,7 +81,7 @@ private: void captureBufferReady(FrameBuffer *buffer); void outputBufferReady(FrameBuffer *buffer); - SimpleConverter *converter_; + V4L2M2MConverter *converter_; unsigned int index_; std::unique_ptr<V4L2M2MDevice> m2m_; @@ -88,7 +89,6 @@ private: unsigned int outputBufferCount_; }; - std::string deviceNode_; std::unique_ptr<V4L2M2MDevice> m2m_; std::vector<Stream> streams_; diff --git a/include/libcamera/internal/converter/meson.build b/include/libcamera/internal/converter/meson.build new file mode 100644 index 00000000..891e79e7 --- /dev/null +++ b/include/libcamera/internal/converter/meson.build @@ -0,0 +1,5 @@ +# SPDX-License-Identifier: CC0-1.0 + +libcamera_internal_headers += files([ + 'converter_v4l2_m2m.h', +]) diff --git a/include/libcamera/internal/meson.build b/include/libcamera/internal/meson.build index 8f50d755..b9db5a8c 100644 --- a/include/libcamera/internal/meson.build +++ b/include/libcamera/internal/meson.build @@ -45,3 +45,5 @@ libcamera_internal_headers = files([ 'v4l2_videodevice.h', 'yaml_parser.h', ]) + +subdir('converter') diff --git a/src/libcamera/pipeline/simple/converter.cpp b/src/libcamera/converter/converter_v4l2_m2m.cpp similarity index 69% rename from src/libcamera/pipeline/simple/converter.cpp rename to src/libcamera/converter/converter_v4l2_m2m.cpp index acaaa64c..5af054be 100644 --- a/src/libcamera/pipeline/simple/converter.cpp +++ b/src/libcamera/converter/converter_v4l2_m2m.cpp @@ -1,12 +1,11 @@ /* SPDX-License-Identifier: LGPL-2.1-or-later */ /* * Copyright (C) 2020, Laurent Pinchart + * Copyright 2022 NXP * - * converter.cpp - Format converter for simple pipeline handler + * converter_v4l2_m2m.cpp - V4L2 M2M Format converter */ -#include "converter.h" - #include <algorithm> #include <limits.h> @@ -20,19 +19,32 @@ #include "libcamera/internal/media_device.h" #include "libcamera/internal/v4l2_videodevice.h" +#include "libcamera/internal/converter/converter_v4l2_m2m.h" + +/** + * \file internal/converter/converter_v4l2_m2m.h + * \brief V4L2 M2M based converter + */ namespace libcamera { -LOG_DECLARE_CATEGORY(SimplePipeline) +LOG_DECLARE_CATEGORY(Converter) + +/** + * \class V4L2M2MConverter + * \brief V4L2 M2M based converter + * + * The V4L2 M2M converter implements the converter interface based on V4L2 M2M device. + */ /* ----------------------------------------------------------------------------- - * SimpleConverter::Stream + * V4L2M2MConverter::Stream */ -SimpleConverter::Stream::Stream(SimpleConverter *converter, unsigned int index) +V4L2M2MConverter::Stream::Stream(V4L2M2MConverter *converter, unsigned int index) : converter_(converter), index_(index) { - m2m_ = std::make_unique<V4L2M2MDevice>(converter->deviceNode_); + m2m_ = std::make_unique<V4L2M2MDevice>(converter->deviceNode()); m2m_->output()->bufferReady.connect(this, &Stream::outputBufferReady); m2m_->capture()->bufferReady.connect(this, &Stream::captureBufferReady); @@ -42,8 +54,8 @@ SimpleConverter::Stream::Stream(SimpleConverter *converter, unsigned int index) m2m_.reset(); } -int SimpleConverter::Stream::configure(const StreamConfiguration &inputCfg, - const StreamConfiguration &outputCfg) +int V4L2M2MConverter::Stream::configure(const StreamConfiguration &inputCfg, + const StreamConfiguration &outputCfg) { V4L2PixelFormat videoFormat = m2m_->output()->toV4L2PixelFormat(inputCfg.pixelFormat); @@ -56,14 +68,14 @@ int SimpleConverter::Stream::configure(const StreamConfiguration &inputCfg, int ret = m2m_->output()->setFormat(&format); if (ret < 0) { - LOG(SimplePipeline, Error) + LOG(Converter, Error) << "Failed to set input format: " << strerror(-ret); return ret; } if (format.fourcc != videoFormat || format.size != inputCfg.size || format.planes[0].bpl != inputCfg.stride) { - LOG(SimplePipeline, Error) + LOG(Converter, Error) << "Input format not supported (requested " << inputCfg.size << "-" << videoFormat << ", got " << format << ")"; @@ -78,13 +90,13 @@ int SimpleConverter::Stream::configure(const StreamConfiguration &inputCfg, ret = m2m_->capture()->setFormat(&format); if (ret < 0) { - LOG(SimplePipeline, Error) + LOG(Converter, Error) << "Failed to set output format: " << strerror(-ret); return ret; } if (format.fourcc != videoFormat || format.size != outputCfg.size) { - LOG(SimplePipeline, Error) + LOG(Converter, Error) << "Output format not supported"; return -EINVAL; } @@ -95,13 +107,13 @@ int SimpleConverter::Stream::configure(const StreamConfiguration &inputCfg, return 0; } -int SimpleConverter::Stream::exportBuffers(unsigned int count, - std::vector<std::unique_ptr<FrameBuffer>> *buffers) +int V4L2M2MConverter::Stream::exportBuffers(unsigned int count, + std::vector<std::unique_ptr<FrameBuffer>> *buffers) { return m2m_->capture()->exportBuffers(count, buffers); } -int SimpleConverter::Stream::start() +int V4L2M2MConverter::Stream::start() { int ret = m2m_->output()->importBuffers(inputBufferCount_); if (ret < 0) @@ -128,7 +140,7 @@ int SimpleConverter::Stream::start() return 0; } -void SimpleConverter::Stream::stop() +void V4L2M2MConverter::Stream::stop() { m2m_->capture()->streamOff(); m2m_->output()->streamOff(); @@ -136,8 +148,7 @@ void SimpleConverter::Stream::stop() m2m_->output()->releaseBuffers(); } -int SimpleConverter::Stream::queueBuffers(FrameBuffer *input, - FrameBuffer *output) +int V4L2M2MConverter::Stream::queueBuffers(FrameBuffer *input, FrameBuffer *output) { int ret = m2m_->output()->queueBuffer(input); if (ret < 0) @@ -150,12 +161,12 @@ int SimpleConverter::Stream::queueBuffers(FrameBuffer *input, return 0; } -std::string SimpleConverter::Stream::logPrefix() const +std::string V4L2M2MConverter::Stream::logPrefix() const { return "stream" + std::to_string(index_); } -void SimpleConverter::Stream::outputBufferReady(FrameBuffer *buffer) +void V4L2M2MConverter::Stream::outputBufferReady(FrameBuffer *buffer) { auto it = converter_->queue_.find(buffer); if (it == converter_->queue_.end()) @@ -167,32 +178,26 @@ void SimpleConverter::Stream::outputBufferReady(FrameBuffer *buffer) } } -void SimpleConverter::Stream::captureBufferReady(FrameBuffer *buffer) +void V4L2M2MConverter::Stream::captureBufferReady(FrameBuffer *buffer) { converter_->outputBufferReady.emit(buffer); } /* ----------------------------------------------------------------------------- - * SimpleConverter + * V4L2M2MConverter */ -SimpleConverter::SimpleConverter(MediaDevice *media) +/** + * \brief Construct a V4L2M2MConverter instance + * \param[in] media The media device implementing the converter + */ +V4L2M2MConverter::V4L2M2MConverter(MediaDevice *media) + : Converter(media) { - /* - * Locate the video node. There's no need to validate the pipeline - * further, the caller guarantees that this is a V4L2 mem2mem device. - */ - const std::vector<MediaEntity *> &entities = media->entities(); - auto it = std::find_if(entities.begin(), entities.end(), - [](MediaEntity *entity) { - return entity->function() == MEDIA_ENT_F_IO_V4L; - }); - if (it == entities.end()) + if (deviceNode().empty()) return; - deviceNode_ = (*it)->deviceNode(); - - m2m_ = std::make_unique<V4L2M2MDevice>(deviceNode_); + m2m_ = std::make_unique<V4L2M2MDevice>(deviceNode()); int ret = m2m_->open(); if (ret < 0) { m2m_.reset(); @@ -200,7 +205,18 @@ SimpleConverter::SimpleConverter(MediaDevice *media) } } -std::vector<PixelFormat> SimpleConverter::formats(PixelFormat input) +/** + * \copydoc libcamera::Converter::loadConfiguration + */ + +/** + * \copydoc libcamera::Converter::isValid + */ + +/** + * \copydoc libcamera::Converter::formats + */ +std::vector<PixelFormat> V4L2M2MConverter::formats(PixelFormat input) { if (!m2m_) return {}; @@ -215,13 +231,13 @@ std::vector<PixelFormat> SimpleConverter::formats(PixelFormat input) int ret = m2m_->output()->setFormat(&v4l2Format); if (ret < 0) { - LOG(SimplePipeline, Error) + LOG(Converter, Error) << "Failed to set format: " << strerror(-ret); return {}; } if (v4l2Format.fourcc != m2m_->output()->toV4L2PixelFormat(input)) { - LOG(SimplePipeline, Debug) + LOG(Converter, Debug) << "Input format " << input << " not supported."; return {}; } @@ -237,7 +253,10 @@ std::vector<PixelFormat> SimpleConverter::formats(PixelFormat input) return pixelFormats; } -SizeRange SimpleConverter::sizes(const Size &input) +/** + * \copydoc libcamera::Converter::sizes + */ +SizeRange V4L2M2MConverter::sizes(const Size &input) { if (!m2m_) return {}; @@ -252,7 +271,7 @@ SizeRange SimpleConverter::sizes(const Size &input) int ret = m2m_->output()->setFormat(&format); if (ret < 0) { - LOG(SimplePipeline, Error) + LOG(Converter, Error) << "Failed to set format: " << strerror(-ret); return {}; } @@ -262,7 +281,7 @@ SizeRange SimpleConverter::sizes(const Size &input) format.size = { 1, 1 }; ret = m2m_->capture()->setFormat(&format); if (ret < 0) { - LOG(SimplePipeline, Error) + LOG(Converter, Error) << "Failed to set format: " << strerror(-ret); return {}; } @@ -272,7 +291,7 @@ SizeRange SimpleConverter::sizes(const Size &input) format.size = { UINT_MAX, UINT_MAX }; ret = m2m_->capture()->setFormat(&format); if (ret < 0) { - LOG(SimplePipeline, Error) + LOG(Converter, Error) << "Failed to set format: " << strerror(-ret); return {}; } @@ -282,9 +301,12 @@ SizeRange SimpleConverter::sizes(const Size &input) return sizes; } +/** + * \copydoc libcamera::Converter::strideAndFrameSize + */ std::tuple<unsigned int, unsigned int> -SimpleConverter::strideAndFrameSize(const PixelFormat &pixelFormat, - const Size &size) +V4L2M2MConverter::strideAndFrameSize(const PixelFormat &pixelFormat, + const Size &size) { V4L2DeviceFormat format; format.fourcc = m2m_->capture()->toV4L2PixelFormat(pixelFormat); @@ -297,8 +319,11 @@ SimpleConverter::strideAndFrameSize(const PixelFormat &pixelFormat, return std::make_tuple(format.planes[0].bpl, format.planes[0].size); } -int SimpleConverter::configure(const StreamConfiguration &inputCfg, - const std::vector<std::reference_wrapper<StreamConfiguration>> &outputCfgs) +/** + * \copydoc libcamera::Converter::configure + */ +int V4L2M2MConverter::configure(const StreamConfiguration &inputCfg, + const std::vector<std::reference_wrapper<StreamConfiguration>> &outputCfgs) { int ret = 0; @@ -309,7 +334,7 @@ int SimpleConverter::configure(const StreamConfiguration &inputCfg, Stream &stream = streams_.emplace_back(this, i); if (!stream.isValid()) { - LOG(SimplePipeline, Error) + LOG(Converter, Error) << "Failed to create stream " << i; ret = -EINVAL; break; @@ -328,8 +353,11 @@ int SimpleConverter::configure(const StreamConfiguration &inputCfg, return 0; } -int SimpleConverter::exportBuffers(unsigned int output, unsigned int count, - std::vector<std::unique_ptr<FrameBuffer>> *buffers) +/** + * \copydoc libcamera::Converter::exportBuffers + */ +int V4L2M2MConverter::exportBuffers(unsigned int output, unsigned int count, + std::vector<std::unique_ptr<FrameBuffer>> *buffers) { if (output >= streams_.size()) return -EINVAL; @@ -337,7 +365,10 @@ int SimpleConverter::exportBuffers(unsigned int output, unsigned int count, return streams_[output].exportBuffers(count, buffers); } -int SimpleConverter::start() +/** + * \copydoc libcamera::Converter::start + */ +int V4L2M2MConverter::start() { int ret; @@ -352,14 +383,20 @@ int SimpleConverter::start() return 0; } -void SimpleConverter::stop() +/** + * \copydoc libcamera::Converter::stop + */ +void V4L2M2MConverter::stop() { for (Stream &stream : utils::reverse(streams_)) stream.stop(); } -int SimpleConverter::queueBuffers(FrameBuffer *input, - const std::map<unsigned int, FrameBuffer *> &outputs) +/** + * \copydoc libcamera::Converter::queueBuffers + */ +int V4L2M2MConverter::queueBuffers(FrameBuffer *input, + const std::map<unsigned int, FrameBuffer *> &outputs) { unsigned int mask = 0; int ret; @@ -402,4 +439,6 @@ int SimpleConverter::queueBuffers(FrameBuffer *input, return 0; } +REGISTER_CONVERTER("v4l2_m2m", V4L2M2MConverter, "pxp") + } /* namespace libcamera */ diff --git a/src/libcamera/converter/meson.build b/src/libcamera/converter/meson.build new file mode 100644 index 00000000..2aa72fe4 --- /dev/null +++ b/src/libcamera/converter/meson.build @@ -0,0 +1,5 @@ +# SPDX-License-Identifier: CC0-1.0 + +libcamera_sources += files([ + 'converter_v4l2_m2m.cpp' +]) diff --git a/src/libcamera/meson.build b/src/libcamera/meson.build index 3a9fc31f..d2f75741 100644 --- a/src/libcamera/meson.build +++ b/src/libcamera/meson.build @@ -62,6 +62,7 @@ libatomic = cc.find_library('atomic', required : false) libthreads = dependency('threads') subdir('base') +subdir('converter') subdir('ipa') subdir('pipeline') subdir('proxy') diff --git a/src/libcamera/pipeline/simple/meson.build b/src/libcamera/pipeline/simple/meson.build index 9c99b32f..42b0896d 100644 --- a/src/libcamera/pipeline/simple/meson.build +++ b/src/libcamera/pipeline/simple/meson.build @@ -1,6 +1,5 @@ # SPDX-License-Identifier: CC0-1.0 libcamera_sources += files([ - 'converter.cpp', 'simple.cpp', ]) diff --git a/src/libcamera/pipeline/simple/simple.cpp b/src/libcamera/pipeline/simple/simple.cpp index 37b3e7ac..5cc6bc6a 100644 --- a/src/libcamera/pipeline/simple/simple.cpp +++ b/src/libcamera/pipeline/simple/simple.cpp @@ -30,13 +30,13 @@ #include "libcamera/internal/camera.h" #include "libcamera/internal/camera_sensor.h" +#include "libcamera/internal/converter.h" #include "libcamera/internal/device_enumerator.h" #include "libcamera/internal/media_device.h" #include "libcamera/internal/pipeline_handler.h" #include "libcamera/internal/v4l2_subdevice.h" #include "libcamera/internal/v4l2_videodevice.h" -#include "converter.h" namespace libcamera { @@ -266,7 +266,7 @@ public: std::vector<Configuration> configs_; std::map<PixelFormat, std::vector<const Configuration *>> formats_; - std::unique_ptr<SimpleConverter> converter_; + std::unique_ptr<Converter> converter_; std::vector<std::unique_ptr<FrameBuffer>> converterBuffers_; bool useConverter_; std::queue<std::map<unsigned int, FrameBuffer *>> converterQueue_; @@ -492,7 +492,7 @@ int SimpleCameraData::init() /* Open the converter, if any. */ MediaDevice *converter = pipe->converter(); if (converter) { - converter_ = std::make_unique<SimpleConverter>(converter); + converter_ = ConverterFactoryBase::create(converter); if (!converter_->isValid()) { LOG(SimplePipeline, Warning) << "Failed to create converter, disabling format conversion";