[libcamera-devel] [PATCH v4 1/9] libcamera: pipeline: raspberrypi: Move RPiStream into a separate file
Kieran Bingham
kieran.bingham at ideasonboard.com
Tue Jul 21 12:45:20 CEST 2020
Hi Naush,
On 20/07/2020 10:13, Naushir Patuck wrote:
> Put RPiStream into the RPi namespace and add a new log category (RPISTREAM).
> There are no functional changes in this commit.
>
> Signed-off-by: Naushir Patuck <naush at raspberrypi.com>
> Reviewed-by: Niklas Söderlund <niklas.soderlund at ragnatech.se>
Great, I love seeing this become more modular, I think that will help
navigating code and maintenance.
Reviewed-by: Kieran Bingham <kieran.bingham at ideasonboard.com>
> ---
> .../pipeline/raspberrypi/meson.build | 1 +
> .../pipeline/raspberrypi/raspberrypi.cpp | 193 ++----------------
> .../pipeline/raspberrypi/rpi_stream.cpp | 116 +++++++++++
> .../pipeline/raspberrypi/rpi_stream.h | 98 +++++++++
> 4 files changed, 234 insertions(+), 174 deletions(-)
> create mode 100644 src/libcamera/pipeline/raspberrypi/rpi_stream.cpp
> create mode 100644 src/libcamera/pipeline/raspberrypi/rpi_stream.h
>
> diff --git a/src/libcamera/pipeline/raspberrypi/meson.build b/src/libcamera/pipeline/raspberrypi/meson.build
> index ae0aed3b..7c5b6ff7 100644
> --- a/src/libcamera/pipeline/raspberrypi/meson.build
> +++ b/src/libcamera/pipeline/raspberrypi/meson.build
> @@ -3,5 +3,6 @@
> libcamera_sources += files([
> 'dma_heaps.cpp',
> 'raspberrypi.cpp',
> + 'rpi_stream.cpp',
> 'staggered_ctrl.cpp',
> ])
> diff --git a/src/libcamera/pipeline/raspberrypi/raspberrypi.cpp b/src/libcamera/pipeline/raspberrypi/raspberrypi.cpp
> index bf1c7714..6630ef57 100644
> --- a/src/libcamera/pipeline/raspberrypi/raspberrypi.cpp
> +++ b/src/libcamera/pipeline/raspberrypi/raspberrypi.cpp
> @@ -19,7 +19,6 @@
> #include <libcamera/logging.h>
> #include <libcamera/property_ids.h>
> #include <libcamera/request.h>
> -#include <libcamera/stream.h>
>
> #include <linux/videodev2.h>
>
> @@ -33,6 +32,7 @@
> #include "libcamera/internal/v4l2_videodevice.h"
>
> #include "dma_heaps.h"
> +#include "rpi_stream.h"
> #include "staggered_ctrl.h"
>
> namespace libcamera {
> @@ -123,165 +123,10 @@ V4L2DeviceFormat findBestMode(V4L2PixFmtMap &formatsMap, const Size &req)
> return bestMode;
> }
>
> -} /* namespace */
> -
> -/*
> - * Device stream abstraction for either an internal or external stream.
> - * Used for both Unicam and the ISP.
> - */
> -class RPiStream : public Stream
> -{
> -public:
> - RPiStream()
> - {
> - }
> -
> - RPiStream(const char *name, MediaEntity *dev, bool importOnly = false)
> - : external_(false), importOnly_(importOnly), name_(name),
> - dev_(std::make_unique<V4L2VideoDevice>(dev))
> - {
> - }
> -
> - V4L2VideoDevice *dev() const
> - {
> - return dev_.get();
> - }
> -
> - void setExternal(bool external)
> - {
> - external_ = external;
> - }
> -
> - bool isExternal() const
> - {
> - /*
> - * Import streams cannot be external.
> - *
> - * RAW capture is a special case where we simply copy the RAW
> - * buffer out of the request. All other buffer handling happens
> - * as if the stream is internal.
> - */
> - return external_ && !importOnly_;
> - }
> -
> - bool isImporter() const
> - {
> - return importOnly_;
> - }
> -
> - void reset()
> - {
> - external_ = false;
> - internalBuffers_.clear();
> - }
> -
> - std::string name() const
> - {
> - return name_;
> - }
> -
> - void setExternalBuffers(std::vector<std::unique_ptr<FrameBuffer>> *buffers)
> - {
> - externalBuffers_ = buffers;
> - }
> -
> - const std::vector<std::unique_ptr<FrameBuffer>> *getBuffers() const
> - {
> - return external_ ? externalBuffers_ : &internalBuffers_;
> - }
> -
> - void releaseBuffers()
> - {
> - dev_->releaseBuffers();
> - if (!external_ && !importOnly_)
> - internalBuffers_.clear();
> - }
> -
> - int importBuffers(unsigned int count)
> - {
> - return dev_->importBuffers(count);
> - }
> -
> - int allocateBuffers(unsigned int count)
> - {
> - return dev_->allocateBuffers(count, &internalBuffers_);
> - }
> -
> - int queueBuffers()
> - {
> - if (external_)
> - return 0;
> -
> - for (auto &b : internalBuffers_) {
> - int ret = dev_->queueBuffer(b.get());
> - if (ret) {
> - LOG(RPI, Error) << "Failed to queue buffers for "
> - << name_;
> - return ret;
> - }
> - }
> -
> - return 0;
> - }
> -
> - bool findFrameBuffer(FrameBuffer *buffer) const
> - {
> - auto start = external_ ? externalBuffers_->begin() : internalBuffers_.begin();
> - auto end = external_ ? externalBuffers_->end() : internalBuffers_.end();
> -
> - if (importOnly_)
> - return false;
> -
> - if (std::find_if(start, end,
> - [buffer](std::unique_ptr<FrameBuffer> const &ref) { return ref.get() == buffer; }) != end)
> - return true;
> -
> - return false;
> - }
> -
> -private:
> - /*
> - * Indicates that this stream is active externally, i.e. the buffers
> - * are provided by the application.
> - */
> - bool external_;
> - /* Indicates that this stream only imports buffers, e.g. ISP input. */
> - bool importOnly_;
> - /* Stream name identifier. */
> - std::string name_;
> - /* The actual device stream. */
> - std::unique_ptr<V4L2VideoDevice> dev_;
> - /* Internally allocated framebuffers associated with this device stream. */
> - std::vector<std::unique_ptr<FrameBuffer>> internalBuffers_;
> - /* Externally allocated framebuffers associated with this device stream. */
> - std::vector<std::unique_ptr<FrameBuffer>> *externalBuffers_;
> -};
> -
> -/*
> - * The following class is just a convenient (and typesafe) array of device
> - * streams indexed with an enum class.
> - */
> enum class Unicam : unsigned int { Image, Embedded };
> enum class Isp : unsigned int { Input, Output0, Output1, Stats };
>
> -template<typename E, std::size_t N>
> -class RPiDevice : public std::array<class RPiStream, N>
> -{
> -private:
> - constexpr auto index(E e) const noexcept
> - {
> - return static_cast<std::underlying_type_t<E>>(e);
> - }
> -public:
> - RPiStream &operator[](E e)
> - {
> - return std::array<class RPiStream, N>::operator[](index(e));
> - }
> - const RPiStream &operator[](E e) const
> - {
> - return std::array<class RPiStream, N>::operator[](index(e));
> - }
> -};
> +} /* namespace */
>
> class RPiCameraData : public CameraData
> {
> @@ -305,15 +150,15 @@ public:
> void ispOutputDequeue(FrameBuffer *buffer);
>
> void clearIncompleteRequests();
> - void handleStreamBuffer(FrameBuffer *buffer, const RPiStream *stream);
> + void handleStreamBuffer(FrameBuffer *buffer, const RPi::RPiStream *stream);
> void handleState();
>
> CameraSensor *sensor_;
> /* Array of Unicam and ISP device streams and associated buffers/streams. */
> - RPiDevice<Unicam, 2> unicam_;
> - RPiDevice<Isp, 4> isp_;
> + RPi::RPiDevice<Unicam, 2> unicam_;
> + RPi::RPiDevice<Isp, 4> isp_;
> /* The vector below is just for convenience when iterating over all streams. */
> - std::vector<RPiStream *> streams_;
> + std::vector<RPi::RPiStream *> streams_;
> /* Buffers passed to the IPA. */
> std::vector<IPABuffer> ipaBuffers_;
>
> @@ -762,7 +607,7 @@ int PipelineHandlerRPi::configure(Camera *camera, CameraConfiguration *config)
> int PipelineHandlerRPi::exportFrameBuffers(Camera *camera, Stream *stream,
> std::vector<std::unique_ptr<FrameBuffer>> *buffers)
> {
> - RPiStream *s = static_cast<RPiStream *>(stream);
> + RPi::RPiStream *s = static_cast<RPi::RPiStream *>(stream);
> unsigned int count = stream->configuration().bufferCount;
> int ret = s->dev()->exportBuffers(count, buffers);
>
> @@ -908,14 +753,14 @@ bool PipelineHandlerRPi::match(DeviceEnumerator *enumerator)
> std::unique_ptr<RPiCameraData> data = std::make_unique<RPiCameraData>(this);
>
> /* Locate and open the unicam video streams. */
> - data->unicam_[Unicam::Embedded] = RPiStream("Unicam Embedded", unicam_->getEntityByName("unicam-embedded"));
> - data->unicam_[Unicam::Image] = RPiStream("Unicam Image", unicam_->getEntityByName("unicam-image"));
> + data->unicam_[Unicam::Embedded] = RPi::RPiStream("Unicam Embedded", unicam_->getEntityByName("unicam-embedded"));
> + data->unicam_[Unicam::Image] = RPi::RPiStream("Unicam Image", unicam_->getEntityByName("unicam-image"));
>
> /* Tag the ISP input stream as an import stream. */
> - data->isp_[Isp::Input] = RPiStream("ISP Input", isp_->getEntityByName("bcm2835-isp0-output0"), true);
> - data->isp_[Isp::Output0] = RPiStream("ISP Output0", isp_->getEntityByName("bcm2835-isp0-capture1"));
> - data->isp_[Isp::Output1] = RPiStream("ISP Output1", isp_->getEntityByName("bcm2835-isp0-capture2"));
> - data->isp_[Isp::Stats] = RPiStream("ISP Stats", isp_->getEntityByName("bcm2835-isp0-capture3"));
> + data->isp_[Isp::Input] = RPi::RPiStream("ISP Input", isp_->getEntityByName("bcm2835-isp0-output0"), true);
> + data->isp_[Isp::Output0] = RPi::RPiStream("ISP Output0", isp_->getEntityByName("bcm2835-isp0-capture1"));
> + data->isp_[Isp::Output1] = RPi::RPiStream("ISP Output1", isp_->getEntityByName("bcm2835-isp0-capture2"));
> + data->isp_[Isp::Stats] = RPi::RPiStream("ISP Stats", isp_->getEntityByName("bcm2835-isp0-capture3"));
>
> /* This is just for convenience so that we can easily iterate over all streams. */
> for (auto &stream : data->unicam_)
> @@ -1005,7 +850,7 @@ int PipelineHandlerRPi::prepareBuffers(Camera *camera)
> */
> unsigned int maxBuffers = 0;
> for (const Stream *s : camera->streams())
> - if (static_cast<const RPiStream *>(s)->isExternal())
> + if (static_cast<const RPi::RPiStream *>(s)->isExternal())
> maxBuffers = std::max(maxBuffers, s->configuration().bufferCount);
>
> for (auto const stream : data->streams_) {
> @@ -1255,12 +1100,12 @@ done:
>
> void RPiCameraData::unicamBufferDequeue(FrameBuffer *buffer)
> {
> - const RPiStream *stream = nullptr;
> + const RPi::RPiStream *stream = nullptr;
>
> if (state_ == State::Stopped)
> return;
>
> - for (RPiStream const &s : unicam_) {
> + for (RPi::RPiStream const &s : unicam_) {
> if (s.findFrameBuffer(buffer)) {
> stream = &s;
> break;
> @@ -1316,12 +1161,12 @@ void RPiCameraData::ispInputDequeue(FrameBuffer *buffer)
>
> void RPiCameraData::ispOutputDequeue(FrameBuffer *buffer)
> {
> - const RPiStream *stream = nullptr;
> + const RPi::RPiStream *stream = nullptr;
>
> if (state_ == State::Stopped)
> return;
>
> - for (RPiStream const &s : isp_) {
> + for (RPi::RPiStream const &s : isp_) {
> if (s.findFrameBuffer(buffer)) {
> stream = &s;
> break;
> @@ -1402,7 +1247,7 @@ void RPiCameraData::clearIncompleteRequests()
> }
> }
>
> -void RPiCameraData::handleStreamBuffer(FrameBuffer *buffer, const RPiStream *stream)
> +void RPiCameraData::handleStreamBuffer(FrameBuffer *buffer, const RPi::RPiStream *stream)
> {
> if (stream->isExternal()) {
> if (!dropFrame_) {
> diff --git a/src/libcamera/pipeline/raspberrypi/rpi_stream.cpp b/src/libcamera/pipeline/raspberrypi/rpi_stream.cpp
> new file mode 100644
> index 00000000..2edb8b59
> --- /dev/null
> +++ b/src/libcamera/pipeline/raspberrypi/rpi_stream.cpp
> @@ -0,0 +1,116 @@
> +/* SPDX-License-Identifier: LGPL-2.1-or-later */
> +/*
> + * Copyright (C) 2020, Raspberry Pi (Trading) Ltd.
> + *
> + * rpi_stream.cpp - Raspberry Pi device stream abstraction class.
> + */
> +#include "rpi_stream.h"
> +
> +#include "libcamera/internal/log.h"
> +
> +namespace libcamera {
> +
> +LOG_DEFINE_CATEGORY(RPISTREAM)
> +
> +namespace RPi {
> +
> +V4L2VideoDevice *RPiStream::dev() const
> +{
> + return dev_.get();
> +}
> +
> +void RPiStream::setExternal(bool external)
> +{
> + external_ = external;
> +}
> +
> +bool RPiStream::isExternal() const
> +{
> + /*
> + * Import streams cannot be external.
> + *
> + * RAW capture is a special case where we simply copy the RAW
> + * buffer out of the request. All other buffer handling happens
> + * as if the stream is internal.
> + */
> + return external_ && !importOnly_;
> +}
> +
> +bool RPiStream::isImporter() const
> +{
> + return importOnly_;
> +}
> +
> +void RPiStream::reset()
> +{
> + external_ = false;
> + internalBuffers_.clear();
> +}
> +
> +std::string RPiStream::name() const
> +{
> + return name_;
> +}
> +
> +void RPiStream::setExternalBuffers(std::vector<std::unique_ptr<FrameBuffer>> *buffers)
> +{
> + externalBuffers_ = buffers;
> +}
> +
> +const std::vector<std::unique_ptr<FrameBuffer>> *RPiStream::getBuffers() const
> +{
> + return external_ ? externalBuffers_ : &internalBuffers_;
> +}
> +
> +void RPiStream::releaseBuffers()
> +{
> + dev_->releaseBuffers();
> + if (!external_ && !importOnly_)
> + internalBuffers_.clear();
> +}
> +
> +int RPiStream::importBuffers(unsigned int count)
> +{
> + return dev_->importBuffers(count);
> +}
> +
> +int RPiStream::allocateBuffers(unsigned int count)
> +{
> + return dev_->allocateBuffers(count, &internalBuffers_);
> +}
> +
> +int RPiStream::queueBuffers()
> +{
> + if (external_)
> + return 0;
> +
> + for (auto &b : internalBuffers_) {
> + int ret = dev_->queueBuffer(b.get());
> + if (ret) {
> + LOG(RPISTREAM, Error) << "Failed to queue buffers for "
> + << name_;
> + return ret;
> + }
> + }
> +
> + return 0;
> +}
> +
> +bool RPiStream::findFrameBuffer(FrameBuffer *buffer) const
> +{
> + auto start = external_ ? externalBuffers_->begin() : internalBuffers_.begin();
> + auto end = external_ ? externalBuffers_->end() : internalBuffers_.end();
> +
> + if (importOnly_)
> + return false;
> +
> + if (std::find_if(start, end,
> + [buffer](std::unique_ptr<FrameBuffer> const &ref) { return ref.get() == buffer; }) != end)
> + return true;
> +
> + return false;
> +}
> +
> +} /* namespace RPi */
> +
> +} /* namespace libcamera */
> diff --git a/src/libcamera/pipeline/raspberrypi/rpi_stream.h b/src/libcamera/pipeline/raspberrypi/rpi_stream.h
> new file mode 100644
> index 00000000..3957e342
> --- /dev/null
> +++ b/src/libcamera/pipeline/raspberrypi/rpi_stream.h
> @@ -0,0 +1,98 @@
> +/* SPDX-License-Identifier: LGPL-2.1-or-later */
> +/*
> + * Copyright (C) 2020, Raspberry Pi (Trading) Ltd.
> + *
> + * rpi_stream.h - Raspberry Pi device stream abstraction class.
> + */
> +#ifndef __LIBCAMERA_PIPELINE_RPI_STREAM_H__
> +#define __LIBCAMERA_PIPELINE_RPI_STREAM_H__
> +
> +#include <queue>
> +#include <string>
> +#include <vector>
> +
> +#include <libcamera/stream.h>
> +
> +#include "libcamera/internal/v4l2_videodevice.h"
> +
> +namespace libcamera {
> +
> +namespace RPi {
> +
> +/*
> + * Device stream abstraction for either an internal or external stream.
> + * Used for both Unicam and the ISP.
> + */
> +class RPiStream : public Stream
> +{
> +public:
> + RPiStream()
> + {
> + }
> +
> + RPiStream(const char *name, MediaEntity *dev, bool importOnly = false)
> + : external_(false), importOnly_(importOnly), name_(name),
> + dev_(std::make_unique<V4L2VideoDevice>(dev))
> + {
> + }
> +
> + V4L2VideoDevice *dev() const;
> + void setExternal(bool external);
> + bool isExternal() const;
> + bool isImporter() const;
> + void reset();
> + std::string name() const;
> + void setExternalBuffers(std::vector<std::unique_ptr<FrameBuffer>> *buffers);
> + const std::vector<std::unique_ptr<FrameBuffer>> *getBuffers() const;
> + void releaseBuffers();
> + int importBuffers(unsigned int count);
> + int allocateBuffers(unsigned int count);
> + int queueBuffers();
> + bool findFrameBuffer(FrameBuffer *buffer) const;
> +
> +private:
> + /*
> + * Indicates that this stream is active externally, i.e. the buffers
> + * are provided by the application.
> + */
> + bool external_;
> + /* Indicates that this stream only imports buffers, e.g. ISP input. */
> + bool importOnly_;
> + /* Stream name identifier. */
> + std::string name_;
> + /* The actual device stream. */
> + std::unique_ptr<V4L2VideoDevice> dev_;
> + /* Internally allocated framebuffers associated with this device stream. */
> + std::vector<std::unique_ptr<FrameBuffer>> internalBuffers_;
> + /* Externally allocated framebuffers associated with this device stream. */
> + std::vector<std::unique_ptr<FrameBuffer>> *externalBuffers_;
> +};
> +
> +/*
> + * The following class is just a convenient (and typesafe) array of device
> + * streams indexed with an enum class.
> + */
> +template<typename E, std::size_t N>
> +class RPiDevice : public std::array<class RPiStream, N>
> +{
> +private:
> + constexpr auto index(E e) const noexcept
> + {
> + return static_cast<std::underlying_type_t<E>>(e);
> + }
> +public:
> + RPiStream &operator[](E e)
> + {
> + return std::array<class RPiStream, N>::operator[](index(e));
> + }
> + const RPiStream &operator[](E e) const
> + {
> + return std::array<class RPiStream, N>::operator[](index(e));
> + }
> +};
> +
> +} /* namespace RPi */
> +
> +} /* namespace libcamera */
> +
> +#endif /* __LIBCAMERA_PIPELINE_RPI_STREAM_H__ */
>
--
Regards
--
Kieran
More information about the libcamera-devel
mailing list