[libcamera-devel] [PATCH v3 09/10] libcamera: pipeline: ipa: raspberrypi: Remove use of FrameBuffer cookie
Niklas Söderlund
niklas.soderlund at ragnatech.se
Sat Jul 18 17:56:48 CEST 2020
Hi Naushir,
Thanks for your work.
On 2020-07-17 09:54:09 +0100, Naushir Patuck wrote:
> The FrameBuffer cookie may be set by the application, so this cannot
> be set by the pipeline handler as well. Revert to using a simple index
> into the buffer list to identify buffers passing to and from the IPA.
>
> Signed-off-by: Naushir Patuck <naush at raspberrypi.com>
With the understanding that this do not yet support external buffers for
the bayer stream.
Reviewed-by: Niklas Söderlund <niklas.soderlund at ragnatech.se>
> ---
> .../pipeline/raspberrypi/raspberrypi.cpp | 60 ++++++++++---------
> .../pipeline/raspberrypi/rpi_stream.cpp | 14 +++--
> .../pipeline/raspberrypi/rpi_stream.h | 2 +-
> 3 files changed, 40 insertions(+), 36 deletions(-)
>
> diff --git a/src/libcamera/pipeline/raspberrypi/raspberrypi.cpp b/src/libcamera/pipeline/raspberrypi/raspberrypi.cpp
> index ce56ad1a..e400c10c 100644
> --- a/src/libcamera/pipeline/raspberrypi/raspberrypi.cpp
> +++ b/src/libcamera/pipeline/raspberrypi/raspberrypi.cpp
> @@ -888,7 +888,8 @@ int PipelineHandlerRPi::queueAllBuffers(Camera *camera)
> int PipelineHandlerRPi::prepareBuffers(Camera *camera)
> {
> RPiCameraData *data = cameraData(camera);
> - int count, ret;
> + unsigned int index;
> + int ret;
>
> /*
> * Decide how many internal buffers to allocate. For now, simply look
> @@ -908,30 +909,24 @@ int PipelineHandlerRPi::prepareBuffers(Camera *camera)
> }
>
> /*
> - * Add cookies to the ISP Input buffers so that we can link them with
> - * the IPA and RPI_IPA_EVENT_SIGNAL_ISP_PREPARE event.
> - */
> - count = 0;
> - for (auto const &b : data->unicam_[Unicam::Image].getBuffers()) {
> - b->setCookie(count++);
> - }
> -
> - /*
> - * Add cookies to the stats and embedded data buffers and link them with
> - * the IPA.
> + * Link the FrameBuffers with the index of their position in the vector
> + * stored in the RPi stream object.
> + *
> + * This will allow us to identify buffers passed between the pipeline
> + * handler and the IPA.
> */
> - count = 0;
> + index = 0;
> for (auto const &b : data->isp_[Isp::Stats].getBuffers()) {
> - b->setCookie(count++);
> - data->ipaBuffers_.push_back({ .id = RPiIpaMask::STATS | b->cookie(),
> + data->ipaBuffers_.push_back({ .id = RPiIpaMask::STATS | index,
> .planes = b->planes() });
> + index++;
> }
>
> - count = 0;
> + index = 0;
> for (auto const &b : data->unicam_[Unicam::Embedded].getBuffers()) {
> - b->setCookie(count++);
> - data->ipaBuffers_.push_back({ .id = RPiIpaMask::EMBEDDED_DATA | b->cookie(),
> + data->ipaBuffers_.push_back({ .id = RPiIpaMask::EMBEDDED_DATA | index,
> .planes = b->planes() });
> + index++;
> }
>
> data->ipa_->mapBuffers(data->ipaBuffers_);
> @@ -1120,7 +1115,7 @@ void RPiCameraData::queueFrameAction(unsigned int frame, const IPAOperationData
> unsigned int bufferId = action.data[0];
> FrameBuffer *buffer = unicam_[Unicam::Image].getBuffers().at(bufferId);
>
> - LOG(RPI, Debug) << "Input re-queue to ISP, buffer id " << buffer->cookie()
> + LOG(RPI, Debug) << "Input re-queue to ISP, buffer id " << bufferId
> << ", timestamp: " << buffer->metadata().timestamp;
>
> isp_[Isp::Input].queueBuffer(buffer);
> @@ -1140,12 +1135,14 @@ done:
> void RPiCameraData::unicamBufferDequeue(FrameBuffer *buffer)
> {
> RPi::RPiStream *stream = nullptr;
> + int index;
>
> if (state_ == State::Stopped)
> return;
>
> for (RPi::RPiStream &s : unicam_) {
> - if (s.findFrameBuffer(buffer)) {
> + index = s.getBufferIndex(buffer);
> + if (index != -1) {
> stream = &s;
> break;
> }
> @@ -1155,7 +1152,7 @@ void RPiCameraData::unicamBufferDequeue(FrameBuffer *buffer)
> ASSERT(stream);
>
> LOG(RPI, Debug) << "Stream " << stream->name() << " buffer dequeue"
> - << ", buffer id " << buffer->cookie()
> + << ", buffer id " << index
> << ", timestamp: " << buffer->metadata().timestamp;
>
> if (stream == &unicam_[Unicam::Image]) {
> @@ -1195,7 +1192,7 @@ void RPiCameraData::ispInputDequeue(FrameBuffer *buffer)
> return;
>
> LOG(RPI, Debug) << "Stream ISP Input buffer complete"
> - << ", buffer id " << buffer->cookie()
> + << ", buffer id " << unicam_[Unicam::Image].getBufferIndex(buffer)
> << ", timestamp: " << buffer->metadata().timestamp;
>
> /* The ISP input buffer gets re-queued into Unicam. */
> @@ -1206,12 +1203,14 @@ void RPiCameraData::ispInputDequeue(FrameBuffer *buffer)
> void RPiCameraData::ispOutputDequeue(FrameBuffer *buffer)
> {
> RPi::RPiStream *stream = nullptr;
> + int index;
>
> if (state_ == State::Stopped)
> return;
>
> for (RPi::RPiStream &s : isp_) {
> - if (s.findFrameBuffer(buffer)) {
> + index = s.getBufferIndex(buffer);
> + if (index != -1) {
> stream = &s;
> break;
> }
> @@ -1221,7 +1220,7 @@ void RPiCameraData::ispOutputDequeue(FrameBuffer *buffer)
> ASSERT(stream);
>
> LOG(RPI, Debug) << "Stream " << stream->name() << " buffer complete"
> - << ", buffer id " << buffer->cookie()
> + << ", buffer id " << index
> << ", timestamp: " << buffer->metadata().timestamp;
>
> /*
> @@ -1231,7 +1230,7 @@ void RPiCameraData::ispOutputDequeue(FrameBuffer *buffer)
> if (stream == &isp_[Isp::Stats]) {
> IPAOperationData op;
> op.operation = RPI_IPA_EVENT_SIGNAL_STAT_READY;
> - op.data = { RPiIpaMask::STATS | buffer->cookie() };
> + op.data = { RPiIpaMask::STATS | static_cast<unsigned int>(index) };
> ipa_->processEvent(op);
> } else {
> /* Any other ISP output can be handed back to the application now. */
> @@ -1450,13 +1449,16 @@ void RPiCameraData::tryRunPipeline()
> /* Set our state to say the pipeline is active. */
> state_ = State::Busy;
>
> + unsigned int bayerIndex = unicam_[Unicam::Image].getBufferIndex(bayerBuffer);
> + unsigned int embeddedIndex = unicam_[Unicam::Embedded].getBufferIndex(embeddedBuffer);
> +
> LOG(RPI, Debug) << "Signalling RPI_IPA_EVENT_SIGNAL_ISP_PREPARE:"
> - << " Bayer buffer id: " << bayerBuffer->cookie()
> - << " Embedded buffer id: " << embeddedBuffer->cookie();
> + << " Bayer buffer id: " << bayerIndex
> + << " Embedded buffer id: " << embeddedIndex;
>
> op.operation = RPI_IPA_EVENT_SIGNAL_ISP_PREPARE;
> - op.data = { RPiIpaMask::EMBEDDED_DATA | embeddedBuffer->cookie(),
> - RPiIpaMask::BAYER_DATA | bayerBuffer->cookie() };
> + op.data = { RPiIpaMask::EMBEDDED_DATA | embeddedIndex,
> + RPiIpaMask::BAYER_DATA | bayerIndex };
> ipa_->processEvent(op);
> }
>
> diff --git a/src/libcamera/pipeline/raspberrypi/rpi_stream.cpp b/src/libcamera/pipeline/raspberrypi/rpi_stream.cpp
> index 4818117e..61226e94 100644
> --- a/src/libcamera/pipeline/raspberrypi/rpi_stream.cpp
> +++ b/src/libcamera/pipeline/raspberrypi/rpi_stream.cpp
> @@ -179,15 +179,17 @@ void RPiStream::returnBuffer(FrameBuffer *buffer)
> }
> }
>
> -bool RPiStream::findFrameBuffer(FrameBuffer *buffer) const
> +int RPiStream::getBufferIndex(FrameBuffer *buffer) const
> {
> if (importOnly_)
> - return false;
> + return -1;
>
> - if (std::find(bufferList_.begin(), bufferList_.end(), buffer) != bufferList_.end())
> - return true;
> + /* Find the buffer in the list, and return the index position. */
> + auto it = std::find(bufferList_.begin(), bufferList_.end(), buffer);
> + if (it != bufferList_.end())
> + return std::distance(bufferList_.begin(), it);
>
> - return false;
> + return -1;
> }
>
> void RPiStream::clearBuffers()
> @@ -200,7 +202,7 @@ void RPiStream::clearBuffers()
>
> int RPiStream::queueToDevice(FrameBuffer *buffer)
> {
> - LOG(RPISTREAM, Debug) << "Queuing buffer " << buffer->cookie()
> + LOG(RPISTREAM, Debug) << "Queuing buffer " << getBufferIndex(buffer)
> << " for " << name_;
>
> int ret = dev_->queueBuffer(buffer);
> diff --git a/src/libcamera/pipeline/raspberrypi/rpi_stream.h b/src/libcamera/pipeline/raspberrypi/rpi_stream.h
> index 6c8dfa83..a6fd5c8e 100644
> --- a/src/libcamera/pipeline/raspberrypi/rpi_stream.h
> +++ b/src/libcamera/pipeline/raspberrypi/rpi_stream.h
> @@ -47,7 +47,7 @@ public:
> int queueAllBuffers();
> int queueBuffer(FrameBuffer *buffer);
> void returnBuffer(FrameBuffer *buffer);
> - bool findFrameBuffer(FrameBuffer *buffer) const;
> + int getBufferIndex(FrameBuffer *buffer) const;
>
> private:
> void clearBuffers();
> --
> 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