From patchwork Wed Mar 25 15:14:03 2026 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Stefan Klug X-Patchwork-Id: 26373 Return-Path: X-Original-To: parsemail@patchwork.libcamera.org Delivered-To: parsemail@patchwork.libcamera.org Received: from lancelot.ideasonboard.com (lancelot.ideasonboard.com [92.243.16.209]) by patchwork.libcamera.org (Postfix) with ESMTPS id A9D8AC330F for ; Wed, 25 Mar 2026 15:16:10 +0000 (UTC) Received: from lancelot.ideasonboard.com (localhost [IPv6:::1]) by lancelot.ideasonboard.com (Postfix) with ESMTP id 0A83D62CDA; Wed, 25 Mar 2026 16:16:10 +0100 (CET) Authentication-Results: lancelot.ideasonboard.com; dkim=pass (1024-bit key; unprotected) header.d=ideasonboard.com header.i=@ideasonboard.com header.b="Nx44KS3C"; dkim-atps=neutral Received: from perceval.ideasonboard.com (perceval.ideasonboard.com [213.167.242.64]) by lancelot.ideasonboard.com (Postfix) with ESMTPS id 9349A62CDA for ; Wed, 25 Mar 2026 16:16:08 +0100 (CET) Received: from ideasonboard.com (unknown [IPv6:2a00:6020:448c:6c00:b16a:5ed9:4ada:a95a]) by perceval.ideasonboard.com (Postfix) with UTF8SMTPSA id 7491C1BCF; Wed, 25 Mar 2026 16:14:50 +0100 (CET) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com; s=mail; t=1774451690; bh=SA7jM4U/YCt/5CDGIYRGgwJ47FMrJHgOfmAWoKdKPYk=; h=From:To:Cc:Subject:Date:In-Reply-To:References:From; b=Nx44KS3C9tKyCKc+gRJHdtdDudTqeEhxeKyxR9uMGycBaD3M3d7ezKMTK7YDU13xV s1taJ/84apRU0b8qCxH4nXnmZyK4DSlPOsOt5A5uqtX90ay9QR37l7KtyZw3N7DfZP D/cQ1FLKRWqzWtLI0vWv1eNG7GpSGZr4lo0t5q2Y= From: Stefan Klug To: libcamera-devel@lists.libcamera.org Cc: Stefan Klug Subject: [PATCH v2 31/32] pipeline: rkisp1: Use BufferQueue for buffer handling Date: Wed, 25 Mar 2026 16:14:03 +0100 Message-ID: <20260325151416.2114564-32-stefan.klug@ideasonboard.com> X-Mailer: git-send-email 2.51.0 In-Reply-To: <20260325151416.2114564-1-stefan.klug@ideasonboard.com> References: <20260325151416.2114564-1-stefan.klug@ideasonboard.com> MIME-Version: 1.0 X-BeenThere: libcamera-devel@lists.libcamera.org X-Mailman-Version: 2.1.29 Precedence: list List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Errors-To: libcamera-devel-bounces@lists.libcamera.org Sender: "libcamera-devel" Migrate the pipeline handler to use the BufferQueue helper. This simplifies the code and makes it easier to understand. Signed-off-by: Stefan Klug --- Changes in v2: - Added this patch --- src/libcamera/pipeline/rkisp1/rkisp1.cpp | 224 ++++++++--------------- 1 file changed, 79 insertions(+), 145 deletions(-) diff --git a/src/libcamera/pipeline/rkisp1/rkisp1.cpp b/src/libcamera/pipeline/rkisp1/rkisp1.cpp index 998ee3c813bc..e047f073d695 100644 --- a/src/libcamera/pipeline/rkisp1/rkisp1.cpp +++ b/src/libcamera/pipeline/rkisp1/rkisp1.cpp @@ -12,7 +12,6 @@ #include #include #include -#include #include #include @@ -35,6 +34,7 @@ #include #include +#include "libcamera/internal/buffer_queue.h" #include "libcamera/internal/camera.h" #include "libcamera/internal/camera_sensor.h" #include "libcamera/internal/camera_sensor_properties.h" @@ -89,6 +89,7 @@ public: */ unsigned int frame_; std::vector ipaBuffers_; + std::vector ipaFrameBuffers_; RkISP1MainPath *mainPath_; RkISP1SelfPath *selfPath_; @@ -156,11 +157,6 @@ struct RequestInfo { bool sequenceValid = false; }; -struct ParamBufferInfo { - FrameBuffer *buffer = nullptr; - size_t expectedSequence = 0; -}; - struct DewarpBufferInfo { FrameBuffer *inputBuffer; struct { @@ -247,6 +243,9 @@ private: std::unique_ptr param_; std::unique_ptr stat_; + std::unique_ptr paramQueue_; + std::unique_ptr statQueue_; + bool hasSelfPath_; bool isRaw_; @@ -256,28 +255,18 @@ private: std::unique_ptr dewarper_; /* Internal buffers used when dewarper is being used */ - std::vector> mainPathBuffers_; - std::queue availableMainPathBuffers_; + std::unique_ptr mainPathQueue_; bool running_ = false; - std::vector> paramBuffers_; - std::vector> statBuffers_; - std::queue availableParamBuffers_; - std::queue availableStatBuffers_; - std::deque queuedRequests_; std::map sensorFrameInfos_; std::deque queuedDewarpBuffers_; - SequenceSyncHelper paramsSyncHelper_; + SequenceSyncHelper imageSyncHelper_; - std::queue computingParamBuffers_; - std::queue queuedParamBuffers_; - - uint32_t nextParamsSequence_; uint32_t nextStatsToProcess_; Camera *activeCamera_; @@ -379,24 +368,19 @@ int RkISP1CameraData::loadTuningFile(const std::string &path) void RkISP1CameraData::paramsComputed(unsigned int frame, unsigned int bufferId, unsigned int bytesused) { PipelineHandlerRkISP1 *pipe = RkISP1CameraData::pipe(); - ParamBufferInfo &pInfo = pipe->computingParamBuffers_.front(); - pipe->computingParamBuffers_.pop(); - FrameBuffer *buffer = pInfo.buffer; + FrameBuffer *buffer = pipe->paramQueue_->front(BufferQueue::Preparing); + ASSERT(buffer->cookie() == bufferId); LOG(RkISP1Schedule, Debug) << "Queue params for " << frame << " " << buffer; buffer->_d()->metadata().planes()[0].bytesused = bytesused; - int ret = pipe->param_->queueBuffer(buffer); - if (ret < 0) { + + int ret = pipe->paramQueue_->preparedBuffer(); + if (ret < 0) LOG(RkISP1, Error) << "Failed to queue parameter buffer: " << strerror(-ret); - pipe->availableParamBuffers_.push(buffer); - return; - } - - pipe->queuedParamBuffers_.push({ buffer, frame }); } void RkISP1CameraData::setSensorControls(unsigned int frame, @@ -462,8 +446,11 @@ void RkISP1CameraData::metadataReady(unsigned int frame, const ControlList &meta * info.statsBuffer can be null, if ipa->processStats() was called * without a buffer to just fill the metadata. */ - if (info.statsBuffer) - pipe->availableStatBuffers_.push(info.statsBuffer); + if (info.statsBuffer) { + ASSERT(pipe->statQueue_->front(BufferQueue::Postprocessing) == info.statsBuffer); + pipe->statQueue_->postprocessedBuffer(); + } + info.statsBuffer = nullptr; pipe->tryCompleteRequests(); @@ -879,6 +866,15 @@ int PipelineHandlerRkISP1::configure(Camera *camera, CameraConfiguration *c) isRaw_ = info.colourEncoding == PixelFormatInfo::ColourEncodingRAW; data->usesDewarper_ = data->canUseDewarper_ && !isRaw_; + auto delegate = std::make_unique>(&mainPath_); + if (data->usesDewarper_) { + mainPathQueue_ = std::make_unique(std::move(delegate), BufferQueue::PostprocessStage, "MainPath"); + } else { + mainPathQueue_ = std::make_unique(std::move(delegate), 0, "MainPath"); + } + + mainPathQueue_->bufferReady.connect(this, &PipelineHandlerRkISP1::imageBufferReady); + Transform transform = config->combinedTransform(); bool transposeAfterIsp = false; if (data->usesDewarper_) { @@ -1088,9 +1084,9 @@ int PipelineHandlerRkISP1::exportFrameBuffers([[maybe_unused]] Camera *camera, S if (data->usesDewarper_) return dewarper_->exportBuffers(&data->mainPathStream_, count, buffers); else - return mainPath_.exportBuffers(count, buffers); + return data->mainPath_->exportBuffers(count, buffers); } else if (hasSelfPath_ && stream == &data->selfPathStream_) { - return selfPath_.exportBuffers(count, buffers); + return data->selfPath_->exportBuffers(count, buffers); } return -EINVAL; @@ -1102,39 +1098,35 @@ int PipelineHandlerRkISP1::allocateBuffers(Camera *camera) utils::ScopeExitActions actions; unsigned int ipaBufferId = 1; int ret; + /* Start at index 1 */ + data->ipaFrameBuffers_.resize(1); actions += [&]() { - paramBuffers_.clear(); - statBuffers_.clear(); - mainPathBuffers_.clear(); + paramQueue_->releaseBuffers(); + statQueue_->releaseBuffers(); + mainPathQueue_->releaseBuffers(); }; if (!isRaw_) { - ret = param_->allocateBuffers(kRkISP1InternalBufferCount, ¶mBuffers_); + ret = paramQueue_->allocateBuffers(kRkISP1InternalBufferCount); if (ret < 0) return ret; - ret = stat_->allocateBuffers(kRkISP1InternalBufferCount, &statBuffers_); + ret = statQueue_->allocateBuffers(kRkISP1InternalBufferCount); if (ret < 0) return ret; } /* If the dewarper is being used, allocate internal buffers for ISP. */ if (data->usesDewarper_) { - ret = mainPath_.exportBuffers(kRkISP1DewarpImageBufferCount, - &mainPathBuffers_); + ret = mainPathQueue_->allocateBuffers(kRkISP1DewarpImageBufferCount); if (ret < 0) return ret; - - for (std::unique_ptr &buffer : mainPathBuffers_) - availableMainPathBuffers_.push(buffer.get()); } else if (data->mainPath_->isEnabled()) { - ret = mainPath_.importBuffers(data->mainPathStream_.configuration().bufferCount); + ret = mainPathQueue_->importBuffers(data->mainPathStream_.configuration().bufferCount); if (ret < 0) return ret; - - actions += [&]() { mainPath_.releaseBuffers(); }; } if (hasSelfPath_ && data->selfPath_->isEnabled()) { @@ -1146,8 +1138,7 @@ int PipelineHandlerRkISP1::allocateBuffers(Camera *camera) actions += [&]() { selfPath_.releaseBuffers(); }; } - auto pushBuffers = [&](const std::vector> &buffers, - std::queue &queue) { + auto pushBuffers = [&](const std::vector> &buffers) { for (const std::unique_ptr &buffer : buffers) { Span planes = buffer->planes(); @@ -1155,12 +1146,12 @@ int PipelineHandlerRkISP1::allocateBuffers(Camera *camera) data->ipaBuffers_.emplace_back(buffer->cookie(), std::vector{ planes.begin(), planes.end() }); - queue.push(buffer.get()); + data->ipaFrameBuffers_.push_back(buffer.get()); } }; - pushBuffers(paramBuffers_, availableParamBuffers_); - pushBuffers(statBuffers_, availableStatBuffers_); + pushBuffers(paramQueue_->buffers()); + pushBuffers(statQueue_->buffers()); data->ipa_->mapBuffers(data->ipaBuffers_); @@ -1172,19 +1163,6 @@ int PipelineHandlerRkISP1::freeBuffers(Camera *camera) { RkISP1CameraData *data = cameraData(camera); - while (!availableStatBuffers_.empty()) - availableStatBuffers_.pop(); - - while (!availableParamBuffers_.empty()) - availableParamBuffers_.pop(); - - while (!availableMainPathBuffers_.empty()) - availableMainPathBuffers_.pop(); - - paramBuffers_.clear(); - statBuffers_.clear(); - mainPathBuffers_.clear(); - std::vector ids; for (IPABuffer &ipabuf : data->ipaBuffers_) ids.push_back(ipabuf.id); @@ -1192,14 +1170,9 @@ int PipelineHandlerRkISP1::freeBuffers(Camera *camera) data->ipa_->unmapBuffers(ids); data->ipaBuffers_.clear(); - if (param_->releaseBuffers()) - LOG(RkISP1, Error) << "Failed to release parameters buffers"; - - if (stat_->releaseBuffers()) - LOG(RkISP1, Error) << "Failed to release stat buffers"; - - if (mainPath_.releaseBuffers()) - LOG(RkISP1, Error) << "Failed to release main path buffers"; + paramQueue_->releaseBuffers(); + statQueue_->releaseBuffers(); + mainPathQueue_->releaseBuffers(); if (hasSelfPath_ && selfPath_.releaseBuffers()) LOG(RkISP1, Error) << "Failed to release self path buffers"; @@ -1223,16 +1196,14 @@ int PipelineHandlerRkISP1::start(Camera *camera, [[maybe_unused]] const ControlL if (!!controls) ctrls = *controls; - paramsSyncHelper_.reset(); imageSyncHelper_.reset(); - nextParamsSequence_ = 0; nextStatsToProcess_ = 0; data->frame_ = 0; uint32_t paramBufferId = 0; FrameBuffer *paramBuffer = nullptr; if (!isRaw_) { - paramBuffer = availableParamBuffers_.front(); + paramBuffer = paramQueue_->front(BufferQueue::Idle); paramBufferId = paramBuffer->cookie(); } @@ -1245,10 +1216,9 @@ int PipelineHandlerRkISP1::start(Camera *camera, [[maybe_unused]] const ControlL } if (paramBuffer) { - availableParamBuffers_.pop(); - computingParamBuffers_.push({ paramBuffer, nextParamsSequence_++ }); - paramsSyncHelper_.pushCorrection(0); - data->paramsComputed(0, paramBufferId, res.paramBufferBytesUsed); + uint32_t seq; + paramQueue_->prepareBuffer(&seq); + data->paramsComputed(seq, paramBufferId, res.paramBufferBytesUsed); } actions += [&]() { data->ipa_->stop(); }; @@ -1337,12 +1307,6 @@ void PipelineHandlerRkISP1::stopDevice(Camera *camera) LOG(RkISP1, Warning) << "Failed to stop parameters for " << camera->id(); - /* - * The param buffers are not returned in order, so the queue - * becomes useless. - */ - queuedParamBuffers_ = {}; - if (data->usesDewarper_) dewarper_->stop(); } @@ -1359,8 +1323,6 @@ void PipelineHandlerRkISP1::stopDevice(Camera *camera) sensorFrameInfos_.clear(); ASSERT(queuedDewarpBuffers_.empty()); - ASSERT(queuedParamBuffers_.empty()); - ASSERT(computingParamBuffers_.empty()); freeBuffers(camera); @@ -1374,29 +1336,30 @@ void PipelineHandlerRkISP1::queueInternalBuffers() RkISP1CameraData *data = cameraData(activeCamera_); - while (!availableStatBuffers_.empty()) { - FrameBuffer *buf = availableStatBuffers_.front(); - availableStatBuffers_.pop(); - data->pipe()->stat_->queueBuffer(buf); + while (!statQueue_->empty(BufferQueue::Idle)) { + int ret = statQueue_->queueBuffer(); + if (ret) + break; } /* * In case of the dewarper, there is a seperate buffer loop for the main * path */ - while (!availableMainPathBuffers_.empty()) { - FrameBuffer *buf = availableMainPathBuffers_.front(); - availableMainPathBuffers_.pop(); + if (!data->usesDewarper_) + return; - LOG(RkISP1Schedule, Debug) << "Queue mainPath " << buf; - data->mainPath_->queueBuffer(buf); + while (!mainPathQueue_->empty(BufferQueue::Idle)) { + LOG(RkISP1Schedule, Debug) << "Queue mainPath " << mainPathQueue_->front(BufferQueue::Idle); + int ret = mainPathQueue_->queueBuffer(); + if (ret) + break; } } void PipelineHandlerRkISP1::computeParamBuffers(uint32_t maxSequence) { RkISP1CameraData *data = cameraData(activeCamera_); - if (isRaw_) { /* * Call computeParams with an empty param buffer to trigger the @@ -1406,38 +1369,24 @@ void PipelineHandlerRkISP1::computeParamBuffers(uint32_t maxSequence) return; } - while (nextParamsSequence_ <= maxSequence) { - if (availableParamBuffers_.empty()) { + while (paramQueue_->nextSequence() <= maxSequence) { + if (paramQueue_->empty(BufferQueue::Idle)) { LOG(RkISP1Schedule, Warning) << "Ran out of parameter buffers"; return; } - int correction = paramsSyncHelper_.correction(); + int correction = paramQueue_->sequenceCorrection(); if (correction != 0) LOG(RkISP1Schedule, Warning) << "Correcting params sequence " << correction; uint32_t paramsSequence; - if (correction >= 0) { - nextParamsSequence_ += correction; - paramsSyncHelper_.pushCorrection(correction); - paramsSequence = nextParamsSequence_++; - } else { - /* - * Inject the same sequence multiple times, to correct - * for the offset. - */ - paramsSyncHelper_.pushCorrection(-1); - paramsSequence = nextParamsSequence_; - } - - FrameBuffer *buf = availableParamBuffers_.front(); - availableParamBuffers_.pop(); - computingParamBuffers_.push({ buf, paramsSequence }); + FrameBuffer *buffer = paramQueue_->front(BufferQueue::Idle); + paramQueue_->prepareBuffer(¶msSequence); LOG(RkISP1Schedule, Debug) << "Request params for " << paramsSequence; - data->ipa_->computeParams(paramsSequence, buf->cookie()); + data->ipa_->computeParams(paramsSequence, buffer->cookie()); } } @@ -1473,7 +1422,7 @@ int PipelineHandlerRkISP1::queueRequestDevice(Camera *camera, Request *request) FrameBuffer *mainPathBuffer = request->findBuffer(&data->mainPathStream_); FrameBuffer *selfPathBuffer = request->findBuffer(&data->selfPathStream_); if (mainPathBuffer) - data->mainPath_->queueBuffer(mainPathBuffer); + mainPathQueue_->queueBuffer(mainPathBuffer); if (data->selfPath_ && selfPathBuffer) data->selfPath_->queueBuffer(selfPathBuffer); @@ -1676,10 +1625,18 @@ bool PipelineHandlerRkISP1::match(DeviceEnumerator *enumerator) if (stat_->open() < 0) return false; + statQueue_ = std::make_unique(std::make_unique>(stat_.get()), + BufferQueue::PostprocessStage, + "Stat"); + param_ = V4L2VideoDevice::fromEntityName(media_.get(), "rkisp1_params"); if (param_->open() < 0) return false; + paramQueue_ = std::make_unique(std::make_unique>(param_.get()), + BufferQueue::PrepareStage, + "Params"); + /* Locate and open the ISP main and self paths. */ if (!mainPath_.init(media_)) return false; @@ -1688,7 +1645,6 @@ bool PipelineHandlerRkISP1::match(DeviceEnumerator *enumerator) return false; isp_->frameStart.connect(this, &PipelineHandlerRkISP1::frameStart); - mainPath_.bufferReady.connect(this, &PipelineHandlerRkISP1::imageBufferReady); if (hasSelfPath_) selfPath_.bufferReady.connect(this, &PipelineHandlerRkISP1::imageBufferReady); stat_->bufferReady.connect(this, &PipelineHandlerRkISP1::statBufferReady); @@ -1816,7 +1772,7 @@ void PipelineHandlerRkISP1::imageBufferReady(FrameBuffer *buffer) if (data->usesDewarper_) { LOG(RkISP1Schedule, Info) << "Image buffer ready, but no corresponding request"; - availableMainPathBuffers_.push(buffer); + mainPathQueue_->postprocessedBuffer(); return; } @@ -1938,7 +1894,7 @@ void PipelineHandlerRkISP1::dewarpBufferReady(FrameBuffer *buffer) outputMeta.sequence = dwInfo.inputMeta.sequence; - availableMainPathBuffers_.push(dwInfo.inputBuffer); + mainPathQueue_->postprocessedBuffer(); dwInfo.inputBuffer = nullptr; dwInfo.outputBuffer = nullptr; @@ -1957,25 +1913,10 @@ void PipelineHandlerRkISP1::paramBufferReady(FrameBuffer *buffer) { LOG(RkISP1Schedule, Debug) << "Param buffer ready " << buffer; - /* After stream off, the buffers are returned out of order, so - * we don't care about the rest. - */ - if (!running_) { - availableParamBuffers_.push(buffer); - return; - } - - ParamBufferInfo pInfo = queuedParamBuffers_.front(); - queuedParamBuffers_.pop(); - - ASSERT(pInfo.buffer == buffer); - size_t metaSequence = buffer->metadata().sequence; LOG(RkISP1Schedule, Debug) << "Params buffer ready " - << " Expected: " << pInfo.expectedSequence + << " Expected: " << paramQueue_->expectedSequence(buffer) << " got: " << metaSequence; - paramsSyncHelper_.gotFrame(pInfo.expectedSequence, metaSequence); - availableParamBuffers_.push(buffer); } void PipelineHandlerRkISP1::statBufferReady(FrameBuffer *buffer) @@ -1985,15 +1926,8 @@ void PipelineHandlerRkISP1::statBufferReady(FrameBuffer *buffer) size_t sequence = buffer->metadata().sequence; - if (buffer->metadata().status == FrameMetadata::FrameCancelled) { - LOG(RkISP1Schedule, Warning) << "Stats cancelled " << sequence; - /* - * We can't assume that the sequence of the stat buffer is valid, - * so there is nothing left to do. - */ - availableStatBuffers_.push(buffer); + if (buffer->metadata().status == FrameMetadata::FrameCancelled) return; - } LOG(RkISP1Schedule, Debug) << "Stats ready " << sequence; @@ -2004,7 +1938,7 @@ void PipelineHandlerRkISP1::statBufferReady(FrameBuffer *buffer) if (nextStatsToProcess_ > sequence) { LOG(RkISP1Schedule, Warning) << "Stats were too late. Ignored"; - availableStatBuffers_.push(buffer); + statQueue_->postprocessedBuffer(); return; }