[PATCH v3 3/6] pipeline: ipa: rpi: Split RPiCameraData::dropFrameCount_
Naushir Patuck
naush at raspberrypi.com
Fri Jun 6 13:45:35 CEST 2025
Hi Barnabás,
On Fri, 6 Jun 2025 at 12:40, Barnabás Pőcze <barnabas.pocze at ideasonboard.com>
wrote:
> Hi
>
>
> 2025. 06. 06. 12:55 keltezéssel, Naushir Patuck írta:
> > Split the pipeline handler drop frame tracking into startup frames and
> > invalid frames, as reported by the IPA.
> >
> > Remove the drop buffer handling logic in the pipeline handler. Now all
> > image buffers are returned out with the appropriate FrameStatus set
> > for startup or invalid frames.
> >
> > Signed-off-by: Naushir Patuck <naush at raspberrypi.com>
> > Reviewed-by: David Plowman <david.plowman at raspberrypi.com>
> > Reviewed-by: Jacopo Mondi <jacopo.mondi at ideasonboard.com>
> > ---
> > .../pipeline/rpi/common/pipeline_base.cpp | 98 ++++++++-----------
> > .../pipeline/rpi/common/pipeline_base.h | 5 +-
> > 2 files changed, 42 insertions(+), 61 deletions(-)
> >
> > diff --git a/src/libcamera/pipeline/rpi/common/pipeline_base.cpp
> b/src/libcamera/pipeline/rpi/common/pipeline_base.cpp
> > index 5956485953a2..3f0b7abdc59a 100644
> > --- a/src/libcamera/pipeline/rpi/common/pipeline_base.cpp
> > +++ b/src/libcamera/pipeline/rpi/common/pipeline_base.cpp
> > @@ -659,9 +659,9 @@ int PipelineHandlerBase::start(Camera *camera, const
> ControlList *controls)
> > if (!result.controls.empty())
> > data->setSensorControls(result.controls);
> >
> > - /* Configure the number of dropped frames required on startup. */
> > - data->dropFrameCount_ = data->config_.disableStartupFrameDrops ?
> > - 0 : result.startupFrameCount +
> result.invalidFrameCount;
> > + /* Configure the number of startup and invalid frames reported by
> the IPA. */
> > + data->startupFrameCount_ = result.startupFrameCount;
> > + data->invalidFrameCount_ = result.invalidFrameCount;
> >
> > for (auto const stream : data->streams_)
> > stream->resetBuffers();
> > @@ -678,7 +678,6 @@ int PipelineHandlerBase::start(Camera *camera, const
> ControlList *controls)
> > data->buffersAllocated_ = true;
> > }
> >
> > - /* We need to set the dropFrameCount_ before queueing buffers. */
> > ret = queueAllBuffers(camera);
> > if (ret) {
> > LOG(RPI, Error) << "Failed to queue buffers";
> > @@ -894,28 +893,12 @@ int PipelineHandlerBase::queueAllBuffers(Camera
> *camera)
> > int ret;
> >
> > for (auto const stream : data->streams_) {
> > - if (!(stream->getFlags() & StreamFlag::External)) {
> > - ret = stream->queueAllBuffers();
> > - if (ret < 0)
> > - return ret;
> > - } else {
> > - /*
> > - * For external streams, we must queue up a set of
> internal
> > - * buffers to handle the number of drop frames
> requested by
> > - * the IPA. This is done by passing nullptr in
> queueBuffer().
> > - *
> > - * The below queueBuffer() call will do nothing if
> there
> > - * are not enough internal buffers allocated, but
> this will
> > - * be handled by queuing the request for buffers
> in the
> > - * RPiStream object.
> > - */
> > - unsigned int i;
> > - for (i = 0; i < data->dropFrameCount_; i++) {
> > - ret = stream->queueBuffer(nullptr);
> > - if (ret)
> > - return ret;
> > - }
> > - }
> > + if (stream->getFlags() & StreamFlag::External)
> > + continue;
> > +
> > + ret = stream->queueAllBuffers();
> > + if (ret < 0)
> > + return ret;
> > }
> >
> > return 0;
> > @@ -1412,7 +1395,15 @@ void CameraData::handleStreamBuffer(FrameBuffer
> *buffer, RPi::Stream *stream)
> > * buffer back to the stream.
> > */
> > Request *request = requestQueue_.empty() ? nullptr :
> requestQueue_.front();
> > - if (!dropFrameCount_ && request && request->findBuffer(stream) ==
> buffer) {
> > + if (request && request->findBuffer(stream) == buffer) {
> > + FrameMetadata &md = buffer->_d()->metadata();
> > +
> > + /* Mark the non-converged and invalid frames in the
> metadata. */
> > + if (invalidFrameCount_)
> > + md.status = FrameMetadata::Status::FrameError;
> > + else if (startupFrameCount_)
> > + md.status = FrameMetadata::Status::FrameStartup;
> > +
> > /*
> > * Tag the buffer as completed, returning it to the
> > * application.
> > @@ -1458,42 +1449,31 @@ void CameraData::handleState()
> >
> > void CameraData::checkRequestCompleted()
> > {
> > - bool requestCompleted = false;
> > - /*
> > - * If we are dropping this frame, do not touch the request, simply
> > - * change the state to IDLE when ready.
> > - */
> > - if (!dropFrameCount_) {
>
> There are two `request->metadata().clear()` calls in
> `{PiSP,Vc4}CameraData::tryRunPipeline()`:
>
> /*
> * Clear the request metadata and fill it with some initial non-IPA
> * related controls. We clear it first because the request metadata
> * may have been populated if we have dropped the previous frame.
> */
> request->metadata().clear();
>
>
> Are those still needed?
>
I believe this can be removed now. I can post a patch on top once this is
merged.
.
Regards,
Naush
>
>
> Regards,
> Barnabás Pőcze
>
> > - Request *request = requestQueue_.front();
> > - if (request->hasPendingBuffers())
> > - return;
> > + Request *request = requestQueue_.front();
> > + if (request->hasPendingBuffers())
> > + return;
> >
> > - /* Must wait for metadata to be filled in before
> completing. */
> > - if (state_ != State::IpaComplete)
> > - return;
> > + /* Must wait for metadata to be filled in before completing. */
> > + if (state_ != State::IpaComplete)
> > + return;
> >
> > - LOG(RPI, Debug) << "Completing request sequence: "
> > - << request->sequence();
> > + LOG(RPI, Debug) << "Completing request sequence: "
> > + << request->sequence();
> >
> > - pipe()->completeRequest(request);
> > - requestQueue_.pop();
> > - requestCompleted = true;
> > - }
> > + pipe()->completeRequest(request);
> > + requestQueue_.pop();
> >
> > - /*
> > - * Make sure we have three outputs completed in the case of a
> dropped
> > - * frame.
> > - */
> > - if (state_ == State::IpaComplete &&
> > - ((ispOutputCount_ == ispOutputTotal_ && dropFrameCount_) ||
> > - requestCompleted)) {
> > - LOG(RPI, Debug) << "Going into Idle state";
> > - state_ = State::Idle;
> > - if (dropFrameCount_) {
> > - dropFrameCount_--;
> > - LOG(RPI, Debug) << "Dropping frame at the request
> of the IPA ("
> > - << dropFrameCount_ << " left)";
> > - }
> > + LOG(RPI, Debug) << "Going into Idle state";
> > + state_ = State::Idle;
> > +
> > + if (invalidFrameCount_) {
> > + invalidFrameCount_--;
> > + LOG(RPI, Debug) << "Decrementing invalid frames to "
> > + << invalidFrameCount_;
> > + } else if (startupFrameCount_) {
> > + startupFrameCount_--;
> > + LOG(RPI, Debug) << "Decrementing startup frames to "
> > + << startupFrameCount_;
> > }
> > }
> >
> > diff --git a/src/libcamera/pipeline/rpi/common/pipeline_base.h
> b/src/libcamera/pipeline/rpi/common/pipeline_base.h
> > index aae0c2f35888..6023f9f9d6b3 100644
> > --- a/src/libcamera/pipeline/rpi/common/pipeline_base.h
> > +++ b/src/libcamera/pipeline/rpi/common/pipeline_base.h
> > @@ -48,7 +48,7 @@ class CameraData : public Camera::Private
> > public:
> > CameraData(PipelineHandler *pipe)
> > : Camera::Private(pipe), state_(State::Stopped),
> > - dropFrameCount_(0), buffersAllocated_(false),
> > + startupFrameCount_(0), invalidFrameCount_(0),
> buffersAllocated_(false),
> > ispOutputCount_(0), ispOutputTotal_(0)
> > {
> > }
> > @@ -151,7 +151,8 @@ public:
> > /* Mapping of CropParams keyed by the output stream order in
> CameraConfiguration */
> > std::map<unsigned int, CropParams> cropParams_;
> >
> > - unsigned int dropFrameCount_;
> > + unsigned int startupFrameCount_;
> > + unsigned int invalidFrameCount_;
> >
> > /*
> > * If set, this stores the value that represets a gain of one for
>
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <https://lists.libcamera.org/pipermail/libcamera-devel/attachments/20250606/e06773a7/attachment.htm>
More information about the libcamera-devel
mailing list