[libcamera-devel] [PATCH 1/9] libcamera: pipeline: raspberrypi: Move RPiStream into a separate header
Niklas Söderlund
niklas.soderlund at ragnatech.se
Mon Jul 13 13:26:37 CEST 2020
Hi Naushir,
Thanks for your work.
On 2020-07-13 09:47:20 +0100, Naushir Patuck wrote:
> Put RPiStream into the RPi namespace and add a new log category (RPISTREAM)
I would add that there is no functional change in this patch.
>
> Signed-off-by: Naushir Patuck <naush at raspberrypi.com>
> ---
> .../pipeline/raspberrypi/raspberrypi.cpp | 189 ++----------------
> .../pipeline/raspberrypi/rpi_stream.h | 182 +++++++++++++++++
I wonder if it would make sens to move some of the code to a
rpi_stream.cpp file?
> 2 files changed, 199 insertions(+), 172 deletions(-)
> create mode 100644 src/libcamera/pipeline/raspberrypi/rpi_stream.h
>
> diff --git a/src/libcamera/pipeline/raspberrypi/raspberrypi.cpp b/src/libcamera/pipeline/raspberrypi/raspberrypi.cpp
> index a08ad6a8..7bae72f9 100644
> --- a/src/libcamera/pipeline/raspberrypi/raspberrypi.cpp
> +++ b/src/libcamera/pipeline/raspberrypi/raspberrypi.cpp
> @@ -17,7 +17,6 @@
> #include <libcamera/ipa/raspberrypi.h>
> #include <libcamera/logging.h>
> #include <libcamera/request.h>
> -#include <libcamera/stream.h>
>
> #include <linux/videodev2.h>
>
> @@ -30,6 +29,7 @@
> #include "libcamera/internal/v4l2_controls.h"
> #include "libcamera/internal/v4l2_videodevice.h"
>
> +#include "rpi_stream.h"
> #include "staggered_ctrl.h"
> #include "vcsm.h"
>
> @@ -121,165 +121,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
> {
> @@ -318,7 +163,7 @@ 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_;
> @@ -326,7 +171,7 @@ public:
> RPiDevice<Unicam, 2> unicam_;
> 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_;
>
> @@ -782,7 +627,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);
>
> @@ -914,14 +759,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_)
> @@ -1070,7 +915,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_) {
> @@ -1271,12 +1116,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;
> @@ -1332,12 +1177,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;
> @@ -1418,7 +1263,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.h b/src/libcamera/pipeline/raspberrypi/rpi_stream.h
> new file mode 100644
> index 00000000..ed4f4987
> --- /dev/null
> +++ b/src/libcamera/pipeline/raspberrypi/rpi_stream.h
> @@ -0,0 +1,182 @@
> +/* 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/internal/v4l2_videodevice.h"
> +#include <libcamera/stream.h>
> +
> +namespace libcamera {
> +
> +LOG_DEFINE_CATEGORY(RPISTREAM)
> +
> +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
> + {
> + 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(RPISTREAM, 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.
> + */
> +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__ */
> --
> 2.25.1
>
> _______________________________________________
> libcamera-devel mailing list
> libcamera-devel at lists.libcamera.org
> https://lists.libcamera.org/listinfo/libcamera-devel
--
Regards,
Niklas Söderlund
More information about the libcamera-devel
mailing list