[libcamera-devel] [PATCH 12/13] pipeline: raspberrypi: Add stream flags to RPi::Stream
Jacopo Mondi
jacopo.mondi at ideasonboard.com
Wed Apr 26 18:24:13 CEST 2023
Hi Naush
On Wed, Apr 26, 2023 at 02:10:56PM +0100, Naushir Patuck via libcamera-devel wrote:
> Add a flags_ field to indicate stream state information in RPi::Stream.
> This replaces the existing external_ and importOnly_ boolean flags.
>
> Signed-off-by: Naushir Patuck <naush at raspberrypi.com>
> ---
> .../pipeline/rpi/common/pipeline_base.cpp | 8 ++--
> .../pipeline/rpi/common/rpi_stream.cpp | 40 ++++++++++--------
> .../pipeline/rpi/common/rpi_stream.h | 42 ++++++++++++-------
> src/libcamera/pipeline/rpi/vc4/vc4.cpp | 8 ++--
> 4 files changed, 60 insertions(+), 38 deletions(-)
>
> diff --git a/src/libcamera/pipeline/rpi/common/pipeline_base.cpp b/src/libcamera/pipeline/rpi/common/pipeline_base.cpp
> index 012766b38c32..4d38d511c383 100644
> --- a/src/libcamera/pipeline/rpi/common/pipeline_base.cpp
> +++ b/src/libcamera/pipeline/rpi/common/pipeline_base.cpp
> @@ -31,6 +31,8 @@ using namespace RPi;
>
> LOG_DEFINE_CATEGORY(RPI)
>
> +using Flag = RPi::Stream::StreamFlag;
> +
The only doubt I have is about "Flag" not being better expressed as
"StreamFlags" in the two 'using' directives in this patch.
The rest looks good
Reviewed-by: Jacopo Mondi <jacopo.mondi at ideasonboard.com>
Thanks
j
> namespace {
>
> constexpr unsigned int defaultRawBitDepth = 12;
> @@ -504,7 +506,7 @@ int PipelineHandlerBase::configure(Camera *camera, CameraConfiguration *config)
> /* Start by freeing all buffers and reset the stream states. */
> data->freeBuffers();
> for (auto const stream : data->streams_)
> - stream->setExternal(false);
> + stream->clearFlags(Flag::External);
>
> std::vector<CameraData::StreamParams> rawStreams, ispStreams;
> std::optional<BayerFormat::Packing> packing;
> @@ -752,7 +754,7 @@ int PipelineHandlerBase::queueRequestDevice(Camera *camera, Request *request)
>
> /* Push all buffers supplied in the Request to the respective streams. */
> for (auto stream : data->streams_) {
> - if (!stream->isExternal())
> + if (!(stream->getFlags() & Flag::External))
> continue;
>
> FrameBuffer *buffer = request->findBuffer(stream);
> @@ -931,7 +933,7 @@ int PipelineHandlerBase::queueAllBuffers(Camera *camera)
> int ret;
>
> for (auto const stream : data->streams_) {
> - if (!stream->isExternal()) {
> + if (!(stream->getFlags() & Flag::External)) {
> ret = stream->queueAllBuffers();
> if (ret < 0)
> return ret;
> diff --git a/src/libcamera/pipeline/rpi/common/rpi_stream.cpp b/src/libcamera/pipeline/rpi/common/rpi_stream.cpp
> index b7e4130f5e56..c158843cb0ed 100644
> --- a/src/libcamera/pipeline/rpi/common/rpi_stream.cpp
> +++ b/src/libcamera/pipeline/rpi/common/rpi_stream.cpp
> @@ -14,6 +14,24 @@ LOG_DEFINE_CATEGORY(RPISTREAM)
>
> namespace RPi {
>
> +void Stream::setFlags(StreamFlags flags)
> +{
> + flags_ |= flags;
> +
> + /* Import streams cannot be external. */
> + ASSERT(!(flags_ & StreamFlag::External) || !(flags_ & StreamFlag::ImportOnly));
> +}
> +
> +void Stream::clearFlags(StreamFlags flags)
> +{
> + flags_ &= ~flags;
> +}
> +
> +RPi::Stream::StreamFlags Stream::getFlags() const
> +{
> + return flags_;
> +}
> +
> V4L2VideoDevice *Stream::dev() const
> {
> return dev_.get();
> @@ -32,18 +50,6 @@ void Stream::resetBuffers()
> availableBuffers_.push(buffer.get());
> }
>
> -void Stream::setExternal(bool external)
> -{
> - /* Import streams cannot be external. */
> - ASSERT(!external || !importOnly_);
> - external_ = external;
> -}
> -
> -bool Stream::isExternal() const
> -{
> - return external_;
> -}
> -
> void Stream::setExportedBuffers(std::vector<std::unique_ptr<FrameBuffer>> *buffers)
> {
> for (auto const &buffer : *buffers)
> @@ -57,7 +63,7 @@ const BufferMap &Stream::getBuffers() const
>
> unsigned int Stream::getBufferId(FrameBuffer *buffer) const
> {
> - if (importOnly_)
> + if (flags_ & StreamFlag::ImportOnly)
> return 0;
>
> /* Find the buffer in the map, and return the buffer id. */
> @@ -88,7 +94,7 @@ int Stream::prepareBuffers(unsigned int count)
> {
> int ret;
>
> - if (!importOnly_) {
> + if (!(flags_ & StreamFlag::ImportOnly)) {
> if (count) {
> /* Export some frame buffers for internal use. */
> ret = dev_->exportBuffers(count, &internalBuffers_);
> @@ -113,7 +119,7 @@ int Stream::prepareBuffers(unsigned int count)
> * \todo Find a better heuristic, or, even better, an exact solution to
> * this issue.
> */
> - if (isExternal() || importOnly_)
> + if ((flags_ & StreamFlag::External) || (flags_ & StreamFlag::ImportOnly))
> count = count * 2;
>
> return dev_->importBuffers(count);
> @@ -160,7 +166,7 @@ int Stream::queueBuffer(FrameBuffer *buffer)
>
> void Stream::returnBuffer(FrameBuffer *buffer)
> {
> - if (!external_) {
> + if (!(flags_ & StreamFlag::External)) {
> /* For internal buffers, simply requeue back to the device. */
> queueToDevice(buffer);
> return;
> @@ -204,7 +210,7 @@ int Stream::queueAllBuffers()
> {
> int ret;
>
> - if (external_)
> + if (flags_ & StreamFlag::External)
> return 0;
>
> while (!availableBuffers_.empty()) {
> diff --git a/src/libcamera/pipeline/rpi/common/rpi_stream.h b/src/libcamera/pipeline/rpi/common/rpi_stream.h
> index b8c74de35863..6edd304bdfe2 100644
> --- a/src/libcamera/pipeline/rpi/common/rpi_stream.h
> +++ b/src/libcamera/pipeline/rpi/common/rpi_stream.h
> @@ -12,6 +12,7 @@
> #include <unordered_map>
> #include <vector>
>
> +#include <libcamera/base/flags.h>
> #include <libcamera/stream.h>
>
> #include "libcamera/internal/v4l2_videodevice.h"
> @@ -37,25 +38,41 @@ enum BufferMask {
> class Stream : public libcamera::Stream
> {
> public:
> + enum class StreamFlag {
> + None = 0,
> + /*
> + * Indicates that this stream only imports buffers, e.g. the ISP
> + * input stream.
> + */
> + ImportOnly = (1 << 0),
> + /*
> + * Indicates that this stream is active externally, i.e. the
> + * buffers might be provided by (and returned to) the application.
> + */
> + External = (1 << 1),
> + };
> +
> + using StreamFlags = Flags<StreamFlag>;
> +
> Stream()
> - : id_(BufferMask::MaskID)
> + : flags_(StreamFlag::None), id_(BufferMask::MaskID)
> {
> }
>
> - Stream(const char *name, MediaEntity *dev, bool importOnly = false)
> - : external_(false), importOnly_(importOnly), name_(name),
> + Stream(const char *name, MediaEntity *dev, StreamFlags flags = StreamFlag::None)
> + : flags_(flags), name_(name),
> dev_(std::make_unique<V4L2VideoDevice>(dev)), id_(BufferMask::MaskID)
> {
> }
>
> + void setFlags(StreamFlags flags);
> + void clearFlags(StreamFlags flags);
> + StreamFlags getFlags() const;
> +
> V4L2VideoDevice *dev() const;
> const std::string &name() const;
> - bool isImporter() const;
> void resetBuffers();
>
> - void setExternal(bool external);
> - bool isExternal() const;
> -
> void setExportedBuffers(std::vector<std::unique_ptr<FrameBuffer>> *buffers);
> const BufferMap &getBuffers() const;
> unsigned int getBufferId(FrameBuffer *buffer) const;
> @@ -112,14 +129,7 @@ private:
> void clearBuffers();
> int queueToDevice(FrameBuffer *buffer);
>
> - /*
> - * Indicates that this stream is active externally, i.e. the buffers
> - * might be provided by (and returned to) the application.
> - */
> - bool external_;
> -
> - /* Indicates that this stream only imports buffers, e.g. ISP input. */
> - bool importOnly_;
> + StreamFlags flags_;
>
> /* Stream name identifier. */
> std::string name_;
> @@ -182,4 +192,6 @@ public:
>
> } /* namespace RPi */
>
> +LIBCAMERA_FLAGS_ENABLE_OPERATORS(RPi::Stream::StreamFlag)
> +
> } /* namespace libcamera */
> diff --git a/src/libcamera/pipeline/rpi/vc4/vc4.cpp b/src/libcamera/pipeline/rpi/vc4/vc4.cpp
> index a085a06376a8..bd7bfb3a7663 100644
> --- a/src/libcamera/pipeline/rpi/vc4/vc4.cpp
> +++ b/src/libcamera/pipeline/rpi/vc4/vc4.cpp
> @@ -23,6 +23,8 @@ namespace libcamera {
>
> LOG_DECLARE_CATEGORY(RPI)
>
> +using Flag = RPi::Stream::StreamFlag;
> +
> namespace {
>
> enum class Unicam : unsigned int { Image, Embedded };
> @@ -320,7 +322,7 @@ int PipelineHandlerVc4::platformRegister(std::unique_ptr<RPi::CameraData> &camer
> }
>
> /* Tag the ISP input stream as an import stream. */
> - data->isp_[Isp::Input] = RPi::Stream("ISP Input", ispOutput0, true);
> + data->isp_[Isp::Input] = RPi::Stream("ISP Input", ispOutput0, Flag::ImportOnly);
> data->isp_[Isp::Output0] = RPi::Stream("ISP Output0", ispCapture1);
> data->isp_[Isp::Output1] = RPi::Stream("ISP Output1", ispCapture2);
> data->isp_[Isp::Stats] = RPi::Stream("ISP Stats", ispCapture3);
> @@ -502,7 +504,7 @@ int Vc4CameraData::platformConfigure(const V4L2SubdeviceFormat &sensorFormat,
> */
> if (!rawStreams.empty()) {
> rawStreams[0].cfg->setStream(&unicam_[Unicam::Image]);
> - unicam_[Unicam::Image].setExternal(true);
> + unicam_[Unicam::Image].setFlags(Flag::External);
> }
>
> ret = isp_[Isp::Input].dev()->setFormat(&unicamFormat);
> @@ -547,7 +549,7 @@ int Vc4CameraData::platformConfigure(const V4L2SubdeviceFormat &sensorFormat,
> << ColorSpace::toString(cfg->colorSpace);
>
> cfg->setStream(stream);
> - stream->setExternal(true);
> + stream->setFlags(Flag::External);
> }
>
> ispOutputTotal_ = outStreams.size();
> --
> 2.34.1
>
More information about the libcamera-devel
mailing list