[libcamera-devel] [PATCH 1/4] pipeline: raspberrypi: Move RPiStream and RPiDevice to its own namespace

Kieran Bingham kieran.bingham at ideasonboard.com
Wed Sep 23 13:01:38 CEST 2020


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