[libcamera-devel] [PATCH 1/9] libcamera: pipeline: raspberrypi: Move RPiStream into a separate header
Naushir Patuck
naush at raspberrypi.com
Mon Jul 13 13:32:25 CEST 2020
Hi Niklas,
On Mon, 13 Jul 2020 at 12:26, Niklas Söderlund
<niklas.soderlund at ragnatech.se> wrote:
>
> 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?
Indeed you are correct. Please see patch 9 :)
Regards,
Naush
>
> > 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