[libcamera-devel] [PATCH 1/4] pipeline: raspberrypi: Move RPiStream and RPiDevice to its own namespace
Naushir Patuck
naush at raspberrypi.com
Thu Sep 24 10:30:28 CEST 2020
Hi Kieran and Jacopo,
Thank you for the review. Entirely right, the commit message is
completely misleading. Not sure where my mind was when I typed that
in :) I will fix up and submit a v2 patch set.
Regards,
Naush
On Wed, 23 Sep 2020 at 12:01, Kieran Bingham
<kieran.bingham at ideasonboard.com> wrote:
>
> Hi Naush,
>
> On 23/09/2020 08:44, Jacopo Mondi wrote:
> > Hi Naush,
> >
> > nit on subject: both classes are already in their own RPi namespace
> > You are just renaming them
> >
> > On Tue, Sep 22, 2020 at 10:50:15AM +0100, Naushir Patuck wrote:
> >> Rename RpiStream -> RPi::Stream and RPiDevice -> RPi::Device.
> >
> > Rename RPi::RPiStream -> RPi::Stream and RPi::RPiDevice -> RPi::Device
> >>
> >> There are no functional changes in this commit.
> >>
> >> Signed-off-by: Naushir Patuck <naush at raspberrypi.com>
> >
> > Reviewed-by: Jacopo Mondi <jacopo at jmondi.org>
>
> Agreed, the small clarification helps in the commit message, and perhaps
> the title as otherwise it looks like this patch does more than it
> actually does.
>
> With that updated:
>
> Reviewed-by: Kieran Bingham <kieran.bingham at ideasonboard.com>
>
>
>
> >
> > Thanks
> > j
> >
> >> ---
> >> .../pipeline/raspberrypi/raspberrypi.cpp | 42 +++++++++----------
> >> .../pipeline/raspberrypi/rpi_stream.cpp | 34 +++++++--------
> >> .../pipeline/raspberrypi/rpi_stream.h | 16 +++----
> >> 3 files changed, 46 insertions(+), 46 deletions(-)
> >>
> >> diff --git a/src/libcamera/pipeline/raspberrypi/raspberrypi.cpp b/src/libcamera/pipeline/raspberrypi/raspberrypi.cpp
> >> index 50f07182..5ed74d51 100644
> >> --- a/src/libcamera/pipeline/raspberrypi/raspberrypi.cpp
> >> +++ b/src/libcamera/pipeline/raspberrypi/raspberrypi.cpp
> >> @@ -150,16 +150,16 @@ public:
> >> void ispOutputDequeue(FrameBuffer *buffer);
> >>
> >> void clearIncompleteRequests();
> >> - void handleStreamBuffer(FrameBuffer *buffer, RPi::RPiStream *stream);
> >> - void handleExternalBuffer(FrameBuffer *buffer, RPi::RPiStream *stream);
> >> + void handleStreamBuffer(FrameBuffer *buffer, RPi::Stream *stream);
> >> + void handleExternalBuffer(FrameBuffer *buffer, RPi::Stream *stream);
> >> void handleState();
> >>
> >> CameraSensor *sensor_;
> >> /* Array of Unicam and ISP device streams and associated buffers/streams. */
> >> - RPi::RPiDevice<Unicam, 2> unicam_;
> >> - RPi::RPiDevice<Isp, 4> isp_;
> >> + RPi::Device<Unicam, 2> unicam_;
> >> + RPi::Device<Isp, 4> isp_;
> >> /* The vector below is just for convenience when iterating over all streams. */
> >> - std::vector<RPi::RPiStream *> streams_;
> >> + std::vector<RPi::Stream *> streams_;
> >> /* Stores the ids of the buffers mapped in the IPA. */
> >> std::unordered_set<unsigned int> ipaBuffers_;
> >>
> >> @@ -189,7 +189,7 @@ private:
> >> void tryRunPipeline();
> >> void tryFlushQueues();
> >> FrameBuffer *updateQueue(std::queue<FrameBuffer *> &q, uint64_t timestamp,
> >> - RPi::RPiStream *stream);
> >> + RPi::Stream *stream);
> >>
> >> unsigned int ispOutputCount_;
> >> };
> >> @@ -618,7 +618,7 @@ int PipelineHandlerRPi::configure(Camera *camera, CameraConfiguration *config)
> >> int PipelineHandlerRPi::exportFrameBuffers([[maybe_unused]] Camera *camera, Stream *stream,
> >> std::vector<std::unique_ptr<FrameBuffer>> *buffers)
> >> {
> >> - RPi::RPiStream *s = static_cast<RPi::RPiStream *>(stream);
> >> + RPi::Stream *s = static_cast<RPi::Stream *>(stream);
> >> unsigned int count = stream->configuration().bufferCount;
> >> int ret = s->dev()->exportBuffers(count, buffers);
> >>
> >> @@ -787,14 +787,14 @@ bool PipelineHandlerRPi::match(DeviceEnumerator *enumerator)
> >> return false;
> >>
> >> /* Locate and open the unicam video streams. */
> >> - data->unicam_[Unicam::Embedded] = RPi::RPiStream("Unicam Embedded", unicam_->getEntityByName("unicam-embedded"));
> >> - data->unicam_[Unicam::Image] = RPi::RPiStream("Unicam Image", unicam_->getEntityByName("unicam-image"));
> >> + data->unicam_[Unicam::Embedded] = RPi::Stream("Unicam Embedded", unicam_->getEntityByName("unicam-embedded"));
> >> + data->unicam_[Unicam::Image] = RPi::Stream("Unicam Image", unicam_->getEntityByName("unicam-image"));
> >>
> >> /* Tag the ISP input stream as an import stream. */
> >> - 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"));
> >> + data->isp_[Isp::Input] = RPi::Stream("ISP Input", isp_->getEntityByName("bcm2835-isp0-output0"), true);
> >> + data->isp_[Isp::Output0] = RPi::Stream("ISP Output0", isp_->getEntityByName("bcm2835-isp0-capture1"));
> >> + data->isp_[Isp::Output1] = RPi::Stream("ISP Output1", isp_->getEntityByName("bcm2835-isp0-capture2"));
> >> + data->isp_[Isp::Stats] = RPi::Stream("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_)
> >> @@ -912,7 +912,7 @@ int PipelineHandlerRPi::prepareBuffers(Camera *camera)
> >> */
> >> unsigned int maxBuffers = 0;
> >> for (const Stream *s : camera->streams())
> >> - if (static_cast<const RPi::RPiStream *>(s)->isExternal())
> >> + if (static_cast<const RPi::Stream *>(s)->isExternal())
> >> maxBuffers = std::max(maxBuffers, s->configuration().bufferCount);
> >>
> >> for (auto const stream : data->streams_) {
> >> @@ -1133,13 +1133,13 @@ done:
> >>
> >> void RPiCameraData::unicamBufferDequeue(FrameBuffer *buffer)
> >> {
> >> - RPi::RPiStream *stream = nullptr;
> >> + RPi::Stream *stream = nullptr;
> >> int index;
> >>
> >> if (state_ == State::Stopped)
> >> return;
> >>
> >> - for (RPi::RPiStream &s : unicam_) {
> >> + for (RPi::Stream &s : unicam_) {
> >> index = s.getBufferId(buffer);
> >> if (index != -1) {
> >> stream = &s;
> >> @@ -1201,13 +1201,13 @@ void RPiCameraData::ispInputDequeue(FrameBuffer *buffer)
> >>
> >> void RPiCameraData::ispOutputDequeue(FrameBuffer *buffer)
> >> {
> >> - RPi::RPiStream *stream = nullptr;
> >> + RPi::Stream *stream = nullptr;
> >> int index;
> >>
> >> if (state_ == State::Stopped)
> >> return;
> >>
> >> - for (RPi::RPiStream &s : isp_) {
> >> + for (RPi::Stream &s : isp_) {
> >> index = s.getBufferId(buffer);
> >> if (index != -1) {
> >> stream = &s;
> >> @@ -1297,7 +1297,7 @@ void RPiCameraData::clearIncompleteRequests()
> >> }
> >> }
> >>
> >> -void RPiCameraData::handleStreamBuffer(FrameBuffer *buffer, RPi::RPiStream *stream)
> >> +void RPiCameraData::handleStreamBuffer(FrameBuffer *buffer, RPi::Stream *stream)
> >> {
> >> if (stream->isExternal()) {
> >> /*
> >> @@ -1330,7 +1330,7 @@ void RPiCameraData::handleStreamBuffer(FrameBuffer *buffer, RPi::RPiStream *stre
> >> }
> >> }
> >>
> >> -void RPiCameraData::handleExternalBuffer(FrameBuffer *buffer, RPi::RPiStream *stream)
> >> +void RPiCameraData::handleExternalBuffer(FrameBuffer *buffer, RPi::Stream *stream)
> >> {
> >> unsigned int id = stream->getBufferId(buffer);
> >>
> >> @@ -1510,7 +1510,7 @@ void RPiCameraData::tryFlushQueues()
> >> }
> >>
> >> FrameBuffer *RPiCameraData::updateQueue(std::queue<FrameBuffer *> &q, uint64_t timestamp,
> >> - RPi::RPiStream *stream)
> >> + RPi::Stream *stream)
> >> {
> >> /*
> >> * If the unicam streams are external (both have be to the same), then we
> >> diff --git a/src/libcamera/pipeline/raspberrypi/rpi_stream.cpp b/src/libcamera/pipeline/raspberrypi/rpi_stream.cpp
> >> index 2dcf96ca..3ee859e9 100644
> >> --- a/src/libcamera/pipeline/raspberrypi/rpi_stream.cpp
> >> +++ b/src/libcamera/pipeline/raspberrypi/rpi_stream.cpp
> >> @@ -14,46 +14,46 @@ LOG_DEFINE_CATEGORY(RPISTREAM)
> >>
> >> namespace RPi {
> >>
> >> -V4L2VideoDevice *RPiStream::dev() const
> >> +V4L2VideoDevice *Stream::dev() const
> >> {
> >> return dev_.get();
> >> }
> >>
> >> -std::string RPiStream::name() const
> >> +std::string Stream::name() const
> >> {
> >> return name_;
> >> }
> >>
> >> -void RPiStream::reset()
> >> +void Stream::reset()
> >> {
> >> external_ = false;
> >> clearBuffers();
> >> }
> >>
> >> -void RPiStream::setExternal(bool external)
> >> +void Stream::setExternal(bool external)
> >> {
> >> /* Import streams cannot be external. */
> >> ASSERT(!external || !importOnly_);
> >> external_ = external;
> >> }
> >>
> >> -bool RPiStream::isExternal() const
> >> +bool Stream::isExternal() const
> >> {
> >> return external_;
> >> }
> >>
> >> -void RPiStream::setExportedBuffers(std::vector<std::unique_ptr<FrameBuffer>> *buffers)
> >> +void Stream::setExportedBuffers(std::vector<std::unique_ptr<FrameBuffer>> *buffers)
> >> {
> >> for (auto const &buffer : *buffers)
> >> bufferMap_.emplace(id_.get(), buffer.get());
> >> }
> >>
> >> -const BufferMap &RPiStream::getBuffers() const
> >> +const BufferMap &Stream::getBuffers() const
> >> {
> >> return bufferMap_;
> >> }
> >>
> >> -int RPiStream::getBufferId(FrameBuffer *buffer) const
> >> +int Stream::getBufferId(FrameBuffer *buffer) const
> >> {
> >> if (importOnly_)
> >> return -1;
> >> @@ -68,12 +68,12 @@ int RPiStream::getBufferId(FrameBuffer *buffer) const
> >> return it->first;
> >> }
> >>
> >> -void RPiStream::setExternalBuffer(FrameBuffer *buffer)
> >> +void Stream::setExternalBuffer(FrameBuffer *buffer)
> >> {
> >> bufferMap_.emplace(RPiBufferMask::EXTERNAL_BUFFER | id_.get(), buffer);
> >> }
> >>
> >> -void RPiStream::removeExternalBuffer(FrameBuffer *buffer)
> >> +void Stream::removeExternalBuffer(FrameBuffer *buffer)
> >> {
> >> int id = getBufferId(buffer);
> >>
> >> @@ -82,7 +82,7 @@ void RPiStream::removeExternalBuffer(FrameBuffer *buffer)
> >> bufferMap_.erase(id);
> >> }
> >>
> >> -int RPiStream::prepareBuffers(unsigned int count)
> >> +int Stream::prepareBuffers(unsigned int count)
> >> {
> >> int ret;
> >>
> >> @@ -108,7 +108,7 @@ int RPiStream::prepareBuffers(unsigned int count)
> >> return dev_->importBuffers(count);
> >> }
> >>
> >> -int RPiStream::queueBuffer(FrameBuffer *buffer)
> >> +int Stream::queueBuffer(FrameBuffer *buffer)
> >> {
> >> /*
> >> * A nullptr buffer implies an external stream, but no external
> >> @@ -147,7 +147,7 @@ int RPiStream::queueBuffer(FrameBuffer *buffer)
> >> return 0;
> >> }
> >>
> >> -void RPiStream::returnBuffer(FrameBuffer *buffer)
> >> +void Stream::returnBuffer(FrameBuffer *buffer)
> >> {
> >> /* This can only be called for external streams. */
> >> ASSERT(external_);
> >> @@ -186,7 +186,7 @@ void RPiStream::returnBuffer(FrameBuffer *buffer)
> >> }
> >> }
> >>
> >> -int RPiStream::queueAllBuffers()
> >> +int Stream::queueAllBuffers()
> >> {
> >> int ret;
> >>
> >> @@ -204,13 +204,13 @@ int RPiStream::queueAllBuffers()
> >> return 0;
> >> }
> >>
> >> -void RPiStream::releaseBuffers()
> >> +void Stream::releaseBuffers()
> >> {
> >> dev_->releaseBuffers();
> >> clearBuffers();
> >> }
> >>
> >> -void RPiStream::clearBuffers()
> >> +void Stream::clearBuffers()
> >> {
> >> availableBuffers_ = std::queue<FrameBuffer *>{};
> >> requestBuffers_ = std::queue<FrameBuffer *>{};
> >> @@ -219,7 +219,7 @@ void RPiStream::clearBuffers()
> >> id_.reset();
> >> }
> >>
> >> -int RPiStream::queueToDevice(FrameBuffer *buffer)
> >> +int Stream::queueToDevice(FrameBuffer *buffer)
> >> {
> >> LOG(RPISTREAM, Debug) << "Queuing buffer " << getBufferId(buffer)
> >> << " for " << name_;
> >> diff --git a/src/libcamera/pipeline/raspberrypi/rpi_stream.h b/src/libcamera/pipeline/raspberrypi/rpi_stream.h
> >> index a2c21bcd..cb097e1c 100644
> >> --- a/src/libcamera/pipeline/raspberrypi/rpi_stream.h
> >> +++ b/src/libcamera/pipeline/raspberrypi/rpi_stream.h
> >> @@ -27,15 +27,15 @@ using BufferMap = std::unordered_map<unsigned int, FrameBuffer *>;
> >> * Device stream abstraction for either an internal or external stream.
> >> * Used for both Unicam and the ISP.
> >> */
> >> -class RPiStream : public Stream
> >> +class Stream : public libcamera::Stream
> >> {
> >> public:
> >> - RPiStream()
> >> + Stream()
> >> : id_(RPiBufferMask::ID)
> >> {
> >> }
> >>
> >> - RPiStream(const char *name, MediaEntity *dev, bool importOnly = false)
> >> + Stream(const char *name, MediaEntity *dev, bool importOnly = false)
> >> : external_(false), importOnly_(importOnly), name_(name),
> >> dev_(std::make_unique<V4L2VideoDevice>(dev)), id_(RPiBufferMask::ID)
> >> {
> >> @@ -155,7 +155,7 @@ private:
> >> * streams indexed with an enum class.
> >> */
> >> template<typename E, std::size_t N>
> >> -class RPiDevice : public std::array<class RPiStream, N>
> >> +class Device : public std::array<class Stream, N>
> >> {
> >> private:
> >> constexpr auto index(E e) const noexcept
> >> @@ -163,13 +163,13 @@ private:
> >> return static_cast<std::underlying_type_t<E>>(e);
> >> }
> >> public:
> >> - RPiStream &operator[](E e)
> >> + Stream &operator[](E e)
> >> {
> >> - return std::array<class RPiStream, N>::operator[](index(e));
> >> + return std::array<class Stream, N>::operator[](index(e));
> >> }
> >> - const RPiStream &operator[](E e) const
> >> + const Stream &operator[](E e) const
> >> {
> >> - return std::array<class RPiStream, N>::operator[](index(e));
> >> + return std::array<class Stream, N>::operator[](index(e));
> >> }
> >> };
> >>
> >> --
> >> 2.25.1
> >>
> >> _______________________________________________
> >> libcamera-devel mailing list
> >> libcamera-devel at lists.libcamera.org
> >> https://lists.libcamera.org/listinfo/libcamera-devel
> > _______________________________________________
> > libcamera-devel mailing list
> > libcamera-devel at lists.libcamera.org
> > https://lists.libcamera.org/listinfo/libcamera-devel
> >
>
> --
> Regards
> --
> Kieran
More information about the libcamera-devel
mailing list