[libcamera-devel] [PATCH 1/3] pipeline: raspberrypi: Allow Stream::returnBuffer() to handle internal buffers
Laurent Pinchart
laurent.pinchart at ideasonboard.com
Tue Feb 8 11:41:45 CET 2022
Hi Naush,
On Tue, Feb 08, 2022 at 08:25:05AM +0000, Naushir Patuck wrote:
> On Mon, 7 Feb 2022 at 23:36, Laurent Pinchart wrote:
> > On Mon, Feb 07, 2022 at 03:12:12PM +0000, Naushir Patuck wrote:
> > > If Stream::returnBuffer() gets passed an internally allocated buffer, it now
> > > simply re-queues it back to the device. With this change, the pipeline handler
> > > code can be simplified slightly as it does not need multiple code paths for
> > > internally allocated and non-internally allocated buffers.
> > >
> > > Signed-off-by: Naushir Patuck <naush at raspberrypi.com>
> > > ---
> > > .../pipeline/raspberrypi/raspberrypi.cpp | 53 +++++++++----------
> > > .../pipeline/raspberrypi/rpi_stream.cpp | 7 ++-
> > > 2 files changed, 29 insertions(+), 31 deletions(-)
> > >
> > > diff --git a/src/libcamera/pipeline/raspberrypi/raspberrypi.cpp b/src/libcamera/pipeline/raspberrypi/raspberrypi.cpp
> > > index 49af56edc1f9..0755de84c70c 100644
> > > --- a/src/libcamera/pipeline/raspberrypi/raspberrypi.cpp
> > > +++ b/src/libcamera/pipeline/raspberrypi/raspberrypi.cpp
> > > @@ -1877,34 +1877,29 @@ void RPiCameraData::clearIncompleteRequests()
> > >
> > > void RPiCameraData::handleStreamBuffer(FrameBuffer *buffer, RPi::Stream *stream)
> > > {
> > > - if (stream->isExternal()) {
> > > + /*
> > > + * It is possible to be here without a pending request, so check
> > > + * that we actually have one to action, otherwise we just return
> > > + * buffer back to the stream.
> > > + */
> > > + Request *request = requestQueue_.empty() ? nullptr : requestQueue_.front();
> > > + if (!dropFrameCount_ && request && request->findBuffer(stream) == buffer) {
> > > /*
> > > - * It is possible to be here without a pending request, so check
> > > - * that we actually have one to action, otherwise we just return
> > > - * buffer back to the stream.
> > > + * Check if this is an externally provided buffer, and if
> > > + * so, we must stop tracking it in the pipeline handler.
> > > */
> > > - Request *request = requestQueue_.empty() ? nullptr : requestQueue_.front();
> > > - if (!dropFrameCount_ && request && request->findBuffer(stream) == buffer) {
> > > - /*
> > > - * Check if this is an externally provided buffer, and if
> > > - * so, we must stop tracking it in the pipeline handler.
> > > - */
> > > - handleExternalBuffer(buffer, stream);
> > > - /*
> > > - * Tag the buffer as completed, returning it to the
> > > - * application.
> > > - */
> > > - pipe()->completeBuffer(request, buffer);
> > > - } else {
> > > - /*
> > > - * This buffer was not part of the Request, or there is no
> > > - * pending request, so we can recycle it.
> > > - */
> > > - stream->returnBuffer(buffer);
> > > - }
> > > + handleExternalBuffer(buffer, stream);
> > > + /*
> > > + * Tag the buffer as completed, returning it to the
> > > + * application.
> > > + */
> > > + pipe()->completeBuffer(request, buffer);
> > > } else {
> > > - /* Simply re-queue the buffer to the requested stream. */
> > > - stream->queueBuffer(buffer);
> > > + /*
> > > + * This buffer was not part of the Request, or there is no
> > > + * pending request, so we can recycle it.
> >
> > As buffer handling code isn't necessarily straightforward to follow, I'd
> > expand this to
> >
> > * This buffer was not part of the Request (which happens if an
> > * internal buffer was used for an external stream, or
> > * unconditionally for internal streams), or there is no pending
> > * request, so we can recycle it.
>
> Sure, I can change this. Or would it be possible to update before merging
> provided there are no further changes needed?
The updated text is already in my tree :-)
> > Reviewed-by: Laurent Pinchart <laurent.pinchart at ideasonboard.com>
> >
> > > + */
> > > + stream->returnBuffer(buffer);
> > > }
> > > }
> > >
> > > @@ -2104,7 +2099,7 @@ bool RPiCameraData::findMatchingBuffers(BayerFrame &bayerFrame, FrameBuffer *&em
> > > FrameBuffer *b = embeddedQueue_.front();
> > > if (!unicam_[Unicam::Embedded].isExternal() && b->metadata().timestamp < ts) {
> > > embeddedQueue_.pop();
> > > - unicam_[Unicam::Embedded].queueBuffer(b);
> > > + unicam_[Unicam::Embedded].returnBuffer(b);
> > > embeddedRequeueCount++;
> > > LOG(RPI, Warning) << "Dropping unmatched input frame in stream "
> > > << unicam_[Unicam::Embedded].name();
> > > @@ -2140,7 +2135,7 @@ bool RPiCameraData::findMatchingBuffers(BayerFrame &bayerFrame, FrameBuffer *&em
> > > * the front of the queue. This buffer is now orphaned, so requeue
> > > * it back to the device.
> > > */
> > > - unicam_[Unicam::Image].queueBuffer(bayerQueue_.front().buffer);
> > > + unicam_[Unicam::Image].returnBuffer(bayerQueue_.front().buffer);
> > > bayerQueue_.pop();
> > > bayerRequeueCount++;
> > > LOG(RPI, Warning) << "Dropping unmatched input frame in stream "
> > > @@ -2158,7 +2153,7 @@ bool RPiCameraData::findMatchingBuffers(BayerFrame &bayerFrame, FrameBuffer *&em
> > >
> > > LOG(RPI, Warning) << "Flushing bayer stream!";
> > > while (!bayerQueue_.empty()) {
> > > - unicam_[Unicam::Image].queueBuffer(bayerQueue_.front().buffer);
> > > + unicam_[Unicam::Image].returnBuffer(bayerQueue_.front().buffer);
> > > bayerQueue_.pop();
> > > }
> > > flushedBuffers = true;
> > > @@ -2175,7 +2170,7 @@ bool RPiCameraData::findMatchingBuffers(BayerFrame &bayerFrame, FrameBuffer *&em
> > >
> > > LOG(RPI, Warning) << "Flushing embedded data stream!";
> > > while (!embeddedQueue_.empty()) {
> > > - unicam_[Unicam::Embedded].queueBuffer(embeddedQueue_.front());
> > > + unicam_[Unicam::Embedded].returnBuffer(embeddedQueue_.front());
> > > embeddedQueue_.pop();
> > > }
> > > flushedBuffers = true;
> > > diff --git a/src/libcamera/pipeline/raspberrypi/rpi_stream.cpp b/src/libcamera/pipeline/raspberrypi/rpi_stream.cpp
> > > index a421ad09ba50..f446e1ce66c7 100644
> > > --- a/src/libcamera/pipeline/raspberrypi/rpi_stream.cpp
> > > +++ b/src/libcamera/pipeline/raspberrypi/rpi_stream.cpp
> > > @@ -163,8 +163,11 @@ int Stream::queueBuffer(FrameBuffer *buffer)
> > >
> > > void Stream::returnBuffer(FrameBuffer *buffer)
> > > {
> > > - /* This can only be called for external streams. */
> > > - ASSERT(external_);
> > > + if (!external_) {
> > > + /* For internal buffers, simply requeue back to the device. */
> > > + queueToDevice(buffer);
> > > + return;
> > > + }
> > >
> > > /* Push this buffer back into the queue to be used again. */
> > > availableBuffers_.push(buffer);
--
Regards,
Laurent Pinchart
More information about the libcamera-devel
mailing list