[libcamera-devel,v4,9/9] libcamera: pipeline: ipa: raspberrypi: Remove use of FrameBuffer cookie
diff mbox series

Message ID 20200720091311.805092-10-naush@raspberrypi.com
State Superseded
Headers show
Series
  • Zero-copy RAW stream work
Related show

Commit Message

Naushir Patuck July 20, 2020, 9:13 a.m. UTC 
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@raspberrypi.com>
Reviewed-by: Niklas Söderlund <niklas.soderlund@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(-)

Comments

Kieran Bingham July 21, 2020, 11:27 a.m. UTC | #1 
Hi Naush,

On 20/07/2020 10:13, 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@raspberrypi.com>
> Reviewed-by: Niklas Söderlund <niklas.soderlund@ragnatech.se>

I wonder if this means we should add an internal cookie or ability to
identify the IPA buffers in the buffer class itself.

But still, this looks like it works and fixes the issue.

Reviewed-by: Kieran Bingham <kieran.bingham@ideasonboard.com>

> ---
>  .../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 c28fe997..e4fc5ac7 100644
> --- a/src/libcamera/pipeline/raspberrypi/raspberrypi.cpp
> +++ b/src/libcamera/pipeline/raspberrypi/raspberrypi.cpp
> @@ -876,7 +876,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
> @@ -896,30 +897,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_);
> @@ -1097,7 +1092,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);
> @@ -1117,12 +1112,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;
>  		}
> @@ -1132,7 +1129,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]) {
> @@ -1172,7 +1169,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. */
> @@ -1183,12 +1180,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;
>  		}
> @@ -1198,7 +1197,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;
>  
>  	/*
> @@ -1208,7 +1207,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. */
> @@ -1427,13 +1426,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 6635a751..73eb04a1 100644
> --- a/src/libcamera/pipeline/raspberrypi/rpi_stream.cpp
> +++ b/src/libcamera/pipeline/raspberrypi/rpi_stream.cpp
> @@ -178,15 +178,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()
> @@ -199,7 +201,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 6222c867..45cf5237 100644
> --- a/src/libcamera/pipeline/raspberrypi/rpi_stream.h
> +++ b/src/libcamera/pipeline/raspberrypi/rpi_stream.h
> @@ -48,7 +48,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();
>
Naushir Patuck July 21, 2020, 12:38 p.m. UTC | #2 
HI Kieran,

On Tue, 21 Jul 2020 at 12:28, Kieran Bingham
<kieran.bingham@ideasonboard.com> wrote:
>
> Hi Naush,
>
> On 20/07/2020 10:13, 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@raspberrypi.com>
> > Reviewed-by: Niklas Söderlund <niklas.soderlund@ragnatech.se>
>
> I wonder if this means we should add an internal cookie or ability to
> identify the IPA buffers in the buffer class itself.
>
> But still, this looks like it works and fixes the issue.

I did think cookies were for internal use only :-)  However, given the
simplicity of using a buffer index, an internal-only cookie probably
not needed.

Regards,
Naush


>
> Reviewed-by: Kieran Bingham <kieran.bingham@ideasonboard.com>
>
> > ---
> >  .../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 c28fe997..e4fc5ac7 100644
> > --- a/src/libcamera/pipeline/raspberrypi/raspberrypi.cpp
> > +++ b/src/libcamera/pipeline/raspberrypi/raspberrypi.cpp
> > @@ -876,7 +876,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
> > @@ -896,30 +897,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_);
> > @@ -1097,7 +1092,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);
> > @@ -1117,12 +1112,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;
> >               }
> > @@ -1132,7 +1129,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]) {
> > @@ -1172,7 +1169,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. */
> > @@ -1183,12 +1180,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;
> >               }
> > @@ -1198,7 +1197,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;
> >
> >       /*
> > @@ -1208,7 +1207,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. */
> > @@ -1427,13 +1426,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 6635a751..73eb04a1 100644
> > --- a/src/libcamera/pipeline/raspberrypi/rpi_stream.cpp
> > +++ b/src/libcamera/pipeline/raspberrypi/rpi_stream.cpp
> > @@ -178,15 +178,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()
> > @@ -199,7 +201,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 6222c867..45cf5237 100644
> > --- a/src/libcamera/pipeline/raspberrypi/rpi_stream.h
> > +++ b/src/libcamera/pipeline/raspberrypi/rpi_stream.h
> > @@ -48,7 +48,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();
> >
>
> --
> Regards
> --
> Kieran
Laurent Pinchart July 22, 2020, 4:03 p.m. UTC | #3 
Hi Naush,

Thank you for the patch.

On Mon, Jul 20, 2020 at 10:13:11AM +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@raspberrypi.com>
> Reviewed-by: Niklas Söderlund <niklas.soderlund@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 c28fe997..e4fc5ac7 100644
> --- a/src/libcamera/pipeline/raspberrypi/raspberrypi.cpp
> +++ b/src/libcamera/pipeline/raspberrypi/raspberrypi.cpp
> @@ -876,7 +876,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
> @@ -896,30 +897,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_);
> @@ -1097,7 +1092,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);
> @@ -1117,12 +1112,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;
>  		}
> @@ -1132,7 +1129,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]) {
> @@ -1172,7 +1169,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. */
> @@ -1183,12 +1180,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;
>  		}
> @@ -1198,7 +1197,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;
>  
>  	/*
> @@ -1208,7 +1207,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. */
> @@ -1427,13 +1426,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 6635a751..73eb04a1 100644
> --- a/src/libcamera/pipeline/raspberrypi/rpi_stream.cpp
> +++ b/src/libcamera/pipeline/raspberrypi/rpi_stream.cpp
> @@ -178,15 +178,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);

I also have trouble understanding this patch (likely due to the trouble
understanding the previous ones though). bufferList_ is documented as
"All framebuffers associated with this device stream" and is filled when
exporting buffers or at start() time. I don't understand how this can
work, as applications can provide new frame buffers at any time. What am
I missing ?

>  
> -	return false;
> +	return -1;
>  }
>  
>  void RPiStream::clearBuffers()
> @@ -199,7 +201,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 6222c867..45cf5237 100644
> --- a/src/libcamera/pipeline/raspberrypi/rpi_stream.h
> +++ b/src/libcamera/pipeline/raspberrypi/rpi_stream.h
> @@ -48,7 +48,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();
Naushir Patuck July 22, 2020, 4:09 p.m. UTC | #4 
Hi Laurent,

On Wed, 22 Jul 2020 at 17:03, Laurent Pinchart
<laurent.pinchart@ideasonboard.com> wrote:
>
> Hi Naush,
>
> Thank you for the patch.
>
> On Mon, Jul 20, 2020 at 10:13:11AM +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@raspberrypi.com>
> > Reviewed-by: Niklas Söderlund <niklas.soderlund@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 c28fe997..e4fc5ac7 100644
> > --- a/src/libcamera/pipeline/raspberrypi/raspberrypi.cpp
> > +++ b/src/libcamera/pipeline/raspberrypi/raspberrypi.cpp
> > @@ -876,7 +876,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
> > @@ -896,30 +897,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_);
> > @@ -1097,7 +1092,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);
> > @@ -1117,12 +1112,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;
> >               }
> > @@ -1132,7 +1129,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]) {
> > @@ -1172,7 +1169,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. */
> > @@ -1183,12 +1180,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;
> >               }
> > @@ -1198,7 +1197,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;
> >
> >       /*
> > @@ -1208,7 +1207,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. */
> > @@ -1427,13 +1426,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 6635a751..73eb04a1 100644
> > --- a/src/libcamera/pipeline/raspberrypi/rpi_stream.cpp
> > +++ b/src/libcamera/pipeline/raspberrypi/rpi_stream.cpp
> > @@ -178,15 +178,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);
>
> I also have trouble understanding this patch (likely due to the trouble
> understanding the previous ones though). bufferList_ is documented as
> "All framebuffers associated with this device stream" and is filled when
> exporting buffers or at start() time. I don't understand how this can
> work, as applications can provide new frame buffers at any time. What am
> I missing ?

Currently, an application can only provide our pipeline with buffers
it allocates using exportFrameBuffers().  I was not aware that it
could allocate them outside of that until Niklas mentioned it.  This
code does *not* work for those cases.  Luckily the apps don't do this
either so all is good for now :-)

I do have a change that is in progress for the cases where new
framebuffers (from any external allocation) can be used and tracked
correctly.  I wanted this to go through before introducing that set of
changes.

Regards,
Naush
>
> >
> > -     return false;
> > +     return -1;
> >  }
> >
> >  void RPiStream::clearBuffers()
> > @@ -199,7 +201,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 6222c867..45cf5237 100644
> > --- a/src/libcamera/pipeline/raspberrypi/rpi_stream.h
> > +++ b/src/libcamera/pipeline/raspberrypi/rpi_stream.h
> > @@ -48,7 +48,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();
>
> --
> Regards,
>
> Laurent Pinchart

Patch
diff mbox series 

diff --git a/src/libcamera/pipeline/raspberrypi/raspberrypi.cpp b/src/libcamera/pipeline/raspberrypi/raspberrypi.cpp
index c28fe997..e4fc5ac7 100644
--- a/src/libcamera/pipeline/raspberrypi/raspberrypi.cpp
+++ b/src/libcamera/pipeline/raspberrypi/raspberrypi.cpp
@@ -876,7 +876,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
@@ -896,30 +897,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_);
@@ -1097,7 +1092,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);
@@ -1117,12 +1112,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;
 		}
@@ -1132,7 +1129,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]) {
@@ -1172,7 +1169,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. */
@@ -1183,12 +1180,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;
 		}
@@ -1198,7 +1197,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;
 
 	/*
@@ -1208,7 +1207,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. */
@@ -1427,13 +1426,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 6635a751..73eb04a1 100644
--- a/src/libcamera/pipeline/raspberrypi/rpi_stream.cpp
+++ b/src/libcamera/pipeline/raspberrypi/rpi_stream.cpp
@@ -178,15 +178,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()
@@ -199,7 +201,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 6222c867..45cf5237 100644
--- a/src/libcamera/pipeline/raspberrypi/rpi_stream.h
+++ b/src/libcamera/pipeline/raspberrypi/rpi_stream.h
@@ -48,7 +48,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();