{"id":19516,"url":"https://patchwork.libcamera.org/api/1.1/patches/19516/?format=json","web_url":"https://patchwork.libcamera.org/patch/19516/","project":{"id":1,"url":"https://patchwork.libcamera.org/api/1.1/projects/1/?format=json","name":"libcamera","link_name":"libcamera","list_id":"libcamera_core","list_email":"libcamera-devel@lists.libcamera.org","web_url":"","scm_url":"","webscm_url":""},"msgid":"<20240220164317.998477-6-dan.scally@ideasonboard.com>","date":"2024-02-20T16:43:15","name":"[v2,5/7] libcamera: rkisp1: Remove RkISP1FrameInfo","commit_ref":null,"pull_url":null,"state":"rejected","archived":true,"hash":"0d092f95734076b91d3ceeee1418ed5e3005d5b7","submitter":{"id":156,"url":"https://patchwork.libcamera.org/api/1.1/people/156/?format=json","name":"Dan Scally","email":"dan.scally@ideasonboard.com"},"delegate":null,"mbox":"https://patchwork.libcamera.org/patch/19516/mbox/","series":[{"id":4172,"url":"https://patchwork.libcamera.org/api/1.1/series/4172/?format=json","web_url":"https://patchwork.libcamera.org/project/libcamera/list/?series=4172","date":"2024-02-20T16:43:10","name":"Remove RkISP1FrameInfo and IPU3Frames classes","version":2,"mbox":"https://patchwork.libcamera.org/series/4172/mbox/"}],"comments":"https://patchwork.libcamera.org/api/patches/19516/comments/","check":"pending","checks":"https://patchwork.libcamera.org/api/patches/19516/checks/","tags":{},"headers":{"Return-Path":"<libcamera-devel-bounces@lists.libcamera.org>","X-Original-To":"parsemail@patchwork.libcamera.org","Delivered-To":"parsemail@patchwork.libcamera.org","Received":["from lancelot.ideasonboard.com (lancelot.ideasonboard.com\n\t[92.243.16.209])\n\tby patchwork.libcamera.org (Postfix) with ESMTPS id 49F5CC3261\n\tfor <parsemail@patchwork.libcamera.org>;\n\tTue, 20 Feb 2024 16:43:38 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id F130B62816;\n\tTue, 20 Feb 2024 17:43:37 +0100 (CET)","from perceval.ideasonboard.com (perceval.ideasonboard.com\n\t[213.167.242.64])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTPS id 153C062805\n\tfor <libcamera-devel@lists.libcamera.org>;\n\tTue, 20 Feb 2024 17:43:25 +0100 (CET)","from mail.ideasonboard.com\n\t(cpc141996-chfd3-2-0-cust928.12-3.cable.virginm.net [86.13.91.161])\n\tby perceval.ideasonboard.com (Postfix) with ESMTPSA id 5E19714B0;\n\tTue, 20 Feb 2024 17:43:18 +0100 (CET)"],"Authentication-Results":"lancelot.ideasonboard.com;\n\tdkim=fail reason=\"signature verification failed\" (1024-bit key;\n\tunprotected) header.d=ideasonboard.com header.i=@ideasonboard.com\n\theader.b=\"LcByrR4I\"; dkim-atps=neutral","DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com;\n\ts=mail; t=1708447398;\n\tbh=SusT3ITljRDCsmlVRh/ICXW4OxPR3B3+GORNLGYAD4w=;\n\th=From:To:Cc:Subject:Date:In-Reply-To:References:From;\n\tb=LcByrR4ItRnyy8mugEt/t/GncQogvZku/K2GrlrSqOVHhpMd/0RaGdvKOF2ZSmMsz\n\tnOGTVuxPVj814ldhbN4Ka0xZPwua/8uQRAyWBZAV21vrPG9Y/DwjB6YR/v+o8GrRHN\n\tw6yTDIkJl3fTIgHkAVOfYch423I6bx8Hi7VweJ8g=","From":"Daniel Scally <dan.scally@ideasonboard.com>","To":"libcamera-devel@lists.libcamera.org","Subject":"[PATCH v2 5/7] libcamera: rkisp1: Remove RkISP1FrameInfo","Date":"Tue, 20 Feb 2024 16:43:15 +0000","Message-Id":"<20240220164317.998477-6-dan.scally@ideasonboard.com>","X-Mailer":"git-send-email 2.34.1","In-Reply-To":"<20240220164317.998477-1-dan.scally@ideasonboard.com>","References":"<20240220164317.998477-1-dan.scally@ideasonboard.com>","MIME-Version":"1.0","Content-Transfer-Encoding":"8bit","X-BeenThere":"libcamera-devel@lists.libcamera.org","X-Mailman-Version":"2.1.29","Precedence":"list","List-Id":"<libcamera-devel.lists.libcamera.org>","List-Unsubscribe":"<https://lists.libcamera.org/options/libcamera-devel>,\n\t<mailto:libcamera-devel-request@lists.libcamera.org?subject=unsubscribe>","List-Archive":"<https://lists.libcamera.org/pipermail/libcamera-devel/>","List-Post":"<mailto:libcamera-devel@lists.libcamera.org>","List-Help":"<mailto:libcamera-devel-request@lists.libcamera.org?subject=help>","List-Subscribe":"<https://lists.libcamera.org/listinfo/libcamera-devel>,\n\t<mailto:libcamera-devel-request@lists.libcamera.org?subject=subscribe>","Errors-To":"libcamera-devel-bounces@lists.libcamera.org","Sender":"\"libcamera-devel\" <libcamera-devel-bounces@lists.libcamera.org>"},"content":"The RkISP1FrameInfo class existed to hold varying bits of info to\nhelp guarantee that a Request is not completed until all of the\nimage buffers are returned to userspace and the metadata has been\nfilled by the IPA module. Now that we're no longer using it for\nthat function it can be removed.\n\nRemove the class and refactor its code out to the rest of the\nPipeline Handler.\n\nSigned-off-by: Daniel Scally <dan.scally@ideasonboard.com>\n---\nChanges in v2:\n\n\t- None\n\n src/libcamera/pipeline/rkisp1/rkisp1.cpp | 287 +++++------------------\n 1 file changed, 64 insertions(+), 223 deletions(-)","diff":"diff --git a/src/libcamera/pipeline/rkisp1/rkisp1.cpp b/src/libcamera/pipeline/rkisp1/rkisp1.cpp\nindex 43e9d98b..ec4665a9 100644\n--- a/src/libcamera/pipeline/rkisp1/rkisp1.cpp\n+++ b/src/libcamera/pipeline/rkisp1/rkisp1.cpp\n@@ -54,45 +54,13 @@ using InternalStream = Request::Private::InternalStream;\n class PipelineHandlerRkISP1;\n class RkISP1CameraData;\n \n-struct RkISP1FrameInfo {\n-\tunsigned int frame;\n-\tRequest *request;\n-\n-\tFrameBuffer *paramBuffer;\n-\tFrameBuffer *statBuffer;\n-\tFrameBuffer *mainPathBuffer;\n-\tFrameBuffer *selfPathBuffer;\n-\n-\tbool paramDequeued;\n-\tbool metadataProcessed;\n-};\n-\n-class RkISP1Frames\n-{\n-public:\n-\tRkISP1Frames(PipelineHandler *pipe);\n-\n-\tRkISP1FrameInfo *create(const RkISP1CameraData *data, Request *request,\n-\t\t\t\tbool isRaw);\n-\tint destroy(unsigned int frame);\n-\tvoid clear();\n-\n-\tRkISP1FrameInfo *find(unsigned int frame);\n-\tRkISP1FrameInfo *find(FrameBuffer *buffer);\n-\tRkISP1FrameInfo *find(Request *request);\n-\n-private:\n-\tPipelineHandlerRkISP1 *pipe_;\n-\tstd::map<unsigned int, RkISP1FrameInfo *> frameInfo_;\n-};\n-\n class RkISP1CameraData : public Camera::Private\n {\n public:\n \tRkISP1CameraData(PipelineHandler *pipe, RkISP1MainPath *mainPath,\n \t\t\t RkISP1SelfPath *selfPath)\n-\t\t: Camera::Private(pipe), frameInfo_(pipe),\n-\t\t  mainPath_(mainPath), selfPath_(selfPath)\n+\t\t: Camera::Private(pipe), mainPath_(mainPath),\n+\t\t  selfPath_(selfPath)\n \t{\n \t}\n \n@@ -104,7 +72,6 @@ public:\n \tstd::unique_ptr<CameraSensor> sensor_;\n \tstd::unique_ptr<DelayedControls> delayedCtrls_;\n \tstd::vector<IPABuffer> ipaBuffers_;\n-\tRkISP1Frames frameInfo_;\n \n \tRkISP1MainPath *mainPath_;\n \tRkISP1SelfPath *selfPath_;\n@@ -172,7 +139,6 @@ private:\n \t}\n \n \tfriend RkISP1CameraData;\n-\tfriend RkISP1Frames;\n \n \tint initLinks(Camera *camera, const CameraSensor *sensor,\n \t\t      const RkISP1CameraConfiguration &config);\n@@ -208,131 +174,6 @@ private:\n \tconst MediaPad *ispSink_;\n };\n \n-RkISP1Frames::RkISP1Frames(PipelineHandler *pipe)\n-\t: pipe_(static_cast<PipelineHandlerRkISP1 *>(pipe))\n-{\n-}\n-\n-RkISP1FrameInfo *RkISP1Frames::create(const RkISP1CameraData *data, Request *request,\n-\t\t\t\t      bool isRaw)\n-{\n-\tunsigned int frame = request->sequence();\n-\n-\tFrameBuffer *paramBuffer = nullptr;\n-\tFrameBuffer *statBuffer = nullptr;\n-\n-\tif (!isRaw) {\n-\t\tif (pipe_->availableParamBuffers_.empty()) {\n-\t\t\tLOG(RkISP1, Error) << \"Parameters buffer underrun\";\n-\t\t\treturn nullptr;\n-\t\t}\n-\n-\t\tif (pipe_->availableStatBuffers_.empty()) {\n-\t\t\tLOG(RkISP1, Error) << \"Statistic buffer underrun\";\n-\t\t\treturn nullptr;\n-\t\t}\n-\n-\t\tparamBuffer = pipe_->availableParamBuffers_.front();\n-\t\tpipe_->availableParamBuffers_.pop();\n-\t\tparamBuffer->_d()->setRequest(request);\n-\n-\t\tstatBuffer = pipe_->availableStatBuffers_.front();\n-\t\tpipe_->availableStatBuffers_.pop();\n-\t\tstatBuffer->_d()->setRequest(request);\n-\t}\n-\n-\tFrameBuffer *mainPathBuffer = request->findBuffer(&data->mainPathStream_);\n-\tFrameBuffer *selfPathBuffer = request->findBuffer(&data->selfPathStream_);\n-\n-\tRkISP1FrameInfo *info = new RkISP1FrameInfo;\n-\n-\tinfo->frame = frame;\n-\tinfo->request = request;\n-\tinfo->paramBuffer = paramBuffer;\n-\tinfo->mainPathBuffer = mainPathBuffer;\n-\tinfo->selfPathBuffer = selfPathBuffer;\n-\tinfo->statBuffer = statBuffer;\n-\tinfo->paramDequeued = false;\n-\tinfo->metadataProcessed = false;\n-\n-\tframeInfo_[frame] = info;\n-\n-\treturn info;\n-}\n-\n-int RkISP1Frames::destroy(unsigned int frame)\n-{\n-\tRkISP1FrameInfo *info = find(frame);\n-\tif (!info)\n-\t\treturn -ENOENT;\n-\n-\tpipe_->availableParamBuffers_.push(info->paramBuffer);\n-\tpipe_->availableStatBuffers_.push(info->statBuffer);\n-\n-\tframeInfo_.erase(info->frame);\n-\n-\tdelete info;\n-\n-\treturn 0;\n-}\n-\n-void RkISP1Frames::clear()\n-{\n-\tfor (const auto &entry : frameInfo_) {\n-\t\tRkISP1FrameInfo *info = entry.second;\n-\n-\t\tpipe_->availableParamBuffers_.push(info->paramBuffer);\n-\t\tpipe_->availableStatBuffers_.push(info->statBuffer);\n-\n-\t\tdelete info;\n-\t}\n-\n-\tframeInfo_.clear();\n-}\n-\n-RkISP1FrameInfo *RkISP1Frames::find(unsigned int frame)\n-{\n-\tauto itInfo = frameInfo_.find(frame);\n-\n-\tif (itInfo != frameInfo_.end())\n-\t\treturn itInfo->second;\n-\n-\tLOG(RkISP1, Fatal) << \"Can't locate info from frame\";\n-\n-\treturn nullptr;\n-}\n-\n-RkISP1FrameInfo *RkISP1Frames::find(FrameBuffer *buffer)\n-{\n-\tfor (auto &itInfo : frameInfo_) {\n-\t\tRkISP1FrameInfo *info = itInfo.second;\n-\n-\t\tif (info->paramBuffer == buffer ||\n-\t\t    info->statBuffer == buffer ||\n-\t\t    info->mainPathBuffer == buffer ||\n-\t\t    info->selfPathBuffer == buffer)\n-\t\t\treturn info;\n-\t}\n-\n-\tLOG(RkISP1, Fatal) << \"Can't locate info from buffer\";\n-\n-\treturn nullptr;\n-}\n-\n-RkISP1FrameInfo *RkISP1Frames::find(Request *request)\n-{\n-\tfor (auto &itInfo : frameInfo_) {\n-\t\tRkISP1FrameInfo *info = itInfo.second;\n-\n-\t\tif (info->request == request)\n-\t\t\treturn info;\n-\t}\n-\n-\tLOG(RkISP1, Fatal) << \"Can't locate info from request\";\n-\n-\treturn nullptr;\n-}\n-\n PipelineHandlerRkISP1 *RkISP1CameraData::pipe()\n {\n \treturn static_cast<PipelineHandlerRkISP1 *>(Camera::Private::pipe());\n@@ -386,20 +227,37 @@ int RkISP1CameraData::loadIPA(unsigned int hwRevision)\n void RkISP1CameraData::paramFilled(unsigned int frame)\n {\n \tPipelineHandlerRkISP1 *pipe = RkISP1CameraData::pipe();\n-\tRkISP1FrameInfo *info = frameInfo_.find(frame);\n-\tif (!info)\n+\n+\tRequest *request = queuedRequests_[frame];\n+\tASSERT(request);\n+\n+\tFrameBuffer *paramBuffer = request->_d()->findInternalBuffer(InternalStream::Parameters);\n+\tASSERT(paramBuffer);\n+\n+\tFrameBuffer *mainPathBuffer = request->findBuffer(&mainPathStream_);\n+\tFrameBuffer *selfPathBuffer = request->findBuffer(&selfPathStream_);\n+\n+\tif (pipe->availableStatBuffers_.empty()) {\n+\t\tLOG(RkISP1, Fatal) << \"Statistics buffer underrun\";\n \t\treturn;\n+\t}\n+\n+\tFrameBuffer *statBuffer = pipe->availableStatBuffers_.front();\n+\tpipe->availableStatBuffers_.pop();\n+\trequest->_d()->addInternalBuffer(InternalStream::Statistics, statBuffer);\n \n-\tinfo->paramBuffer->_d()->metadata().planes()[0].bytesused =\n+\tparamBuffer->_d()->metadata().planes()[0].bytesused =\n \t\tsizeof(struct rkisp1_params_cfg);\n-\tpipe->param_->queueBuffer(info->paramBuffer);\n-\tpipe->stat_->queueBuffer(info->statBuffer);\n+\tpipe->param_->queueBuffer(paramBuffer);\n+\tpipe->stat_->queueBuffer(statBuffer);\n+\n+\tif (mainPathBuffer)\n+\t\tmainPath_->queueBuffer(mainPathBuffer);\n+\n+\tif (selfPath_ && selfPathBuffer)\n+\t\tselfPath_->queueBuffer(selfPathBuffer);\n \n-\tif (info->mainPathBuffer)\n-\t\tmainPath_->queueBuffer(info->mainPathBuffer);\n \n-\tif (selfPath_ && info->selfPathBuffer)\n-\t\tselfPath_->queueBuffer(info->selfPathBuffer);\n }\n \n void RkISP1CameraData::setSensorControls([[maybe_unused]] unsigned int frame,\n@@ -410,15 +268,16 @@ void RkISP1CameraData::setSensorControls([[maybe_unused]] unsigned int frame,\n \n void RkISP1CameraData::metadataReady(unsigned int frame, const ControlList &metadata)\n {\n-\tRkISP1FrameInfo *info = frameInfo_.find(frame);\n-\tif (!info)\n-\t\treturn;\n+\tRequest *request = queuedRequests_[frame];\n+\tASSERT(request);\n \n-\tinfo->request->metadata().merge(metadata);\n-\tinfo->metadataProcessed = true;\n+\tFrameBuffer *statBuffer = request->_d()->findInternalBuffer(InternalStream::Statistics);\n+\tASSERT(statBuffer);\n \n-\tpipe()->completeBuffer(info->request, info->statBuffer);\n-\tpipe()->tryCompleteRequest(info->request);\n+\trequest->metadata().merge(metadata);\n+\tpipe()->availableStatBuffers_.push(statBuffer);\n+\tpipe()->completeBuffer(request, statBuffer);\n+\tpipe()->tryCompleteRequest(request);\n }\n \n /* -----------------------------------------------------------------------------\n@@ -1016,7 +875,6 @@ void PipelineHandlerRkISP1::stopDevice(Camera *camera)\n \t}\n \n \tASSERT(data->queuedRequests_.empty());\n-\tdata->frameInfo_.clear();\n \n \tfreeBuffers(camera);\n \n@@ -1026,24 +884,34 @@ void PipelineHandlerRkISP1::stopDevice(Camera *camera)\n int PipelineHandlerRkISP1::queueRequestDevice(Camera *camera, Request *request)\n {\n \tRkISP1CameraData *data = cameraData(camera);\n+\tFrameBuffer *paramBuffer = nullptr;\n \n-\tRkISP1FrameInfo *info = data->frameInfo_.create(data, request, isRaw_);\n-\tif (!info)\n-\t\treturn -ENOENT;\n+\tif (!isRaw_) {\n+\t\tif (availableParamBuffers_.empty()) {\n+\t\t\tLOG(RkISP1, Error) << \"Parameters buffer underrun\";\n+\t\t\treturn -ENOENT;\n+\t\t}\n+\n+\t\tparamBuffer = availableParamBuffers_.front();\n+\t\tavailableParamBuffers_.pop();\n+\t\tparamBuffer->_d()->setRequest(request);\n+\t}\n \n-\trequest->_d()->addInternalBuffer(InternalStream::Statistics, info->statBuffer);\n-\trequest->_d()->addInternalBuffer(InternalStream::Parameters, info->paramBuffer);\n+\trequest->_d()->addInternalBuffer(InternalStream::Parameters, paramBuffer);\n \n \tdata->ipa_->queueRequest(request->sequence(), request->controls());\n \tif (isRaw_) {\n-\t\tif (info->mainPathBuffer)\n-\t\t\tdata->mainPath_->queueBuffer(info->mainPathBuffer);\n+\t\tFrameBuffer *mainPathBuffer = request->findBuffer(&data->mainPathStream_);\n+\t\tFrameBuffer *selfPathBuffer = request->findBuffer(&data->selfPathStream_);\n+\n+\t\tif (mainPathBuffer)\n+\t\t\tdata->mainPath_->queueBuffer(mainPathBuffer);\n \n-\t\tif (data->selfPath_ && info->selfPathBuffer)\n-\t\t\tdata->selfPath_->queueBuffer(info->selfPathBuffer);\n+\t\tif (data->selfPath_ && selfPathBuffer)\n+\t\t\tdata->selfPath_->queueBuffer(selfPathBuffer);\n \t} else {\n \t\tdata->ipa_->fillParamsBuffer(request->sequence(),\n-\t\t\t\t\t     info->paramBuffer->cookie());\n+\t\t\t\t\t     paramBuffer->cookie());\n \t}\n \n \treturn 0;\n@@ -1238,20 +1106,9 @@ bool PipelineHandlerRkISP1::match(DeviceEnumerator *enumerator)\n \n void PipelineHandlerRkISP1::tryCompleteRequest(Request *request)\n {\n-\tRkISP1CameraData *data = cameraData(activeCamera_);\n-\n-\tRkISP1FrameInfo *info = data->frameInfo_.find(request);\n-\tif (!info)\n-\t\treturn;\n-\n \tif (request->hasPendingBuffers())\n \t\treturn;\n \n-\tif (!isRaw_ && !info->paramDequeued)\n-\t\treturn;\n-\n-\tdata->frameInfo_.destroy(info->frame);\n-\n \tcompleteRequest(request);\n }\n \n@@ -1259,11 +1116,6 @@ void PipelineHandlerRkISP1::bufferReady(FrameBuffer *buffer)\n {\n \tASSERT(activeCamera_);\n \tRkISP1CameraData *data = cameraData(activeCamera_);\n-\n-\tRkISP1FrameInfo *info = data->frameInfo_.find(buffer);\n-\tif (!info)\n-\t\treturn;\n-\n \tconst FrameMetadata &metadata = buffer->metadata();\n \tRequest *request = buffer->request();\n \n@@ -1283,9 +1135,6 @@ void PipelineHandlerRkISP1::bufferReady(FrameBuffer *buffer)\n \t\t\tdata->ipa_->processStatsBuffer(request->sequence(),\n \t\t\t\t\t\t       0, ctrls);\n \t\t}\n-\t} else {\n-\t\tif (isRaw_)\n-\t\t\tinfo->metadataProcessed = true;\n \t}\n \n \tcompleteBuffer(request, buffer);\n@@ -1295,15 +1144,11 @@ void PipelineHandlerRkISP1::bufferReady(FrameBuffer *buffer)\n void PipelineHandlerRkISP1::paramReady(FrameBuffer *buffer)\n {\n \tASSERT(activeCamera_);\n-\tRkISP1CameraData *data = cameraData(activeCamera_);\n-\n-\tRkISP1FrameInfo *info = data->frameInfo_.find(buffer);\n-\tif (!info)\n-\t\treturn;\n+\tRequest *request = buffer->request();\n \n-\tinfo->paramDequeued = true;\n-\tcompleteBuffer(info->request, buffer);\n-\ttryCompleteRequest(info->request);\n+\tcompleteBuffer(buffer->request(), buffer);\n+\ttryCompleteRequest(request);\n+\tavailableParamBuffers_.push(buffer);\n }\n \n void PipelineHandlerRkISP1::statReady(FrameBuffer *buffer)\n@@ -1312,18 +1157,14 @@ void PipelineHandlerRkISP1::statReady(FrameBuffer *buffer)\n \tRkISP1CameraData *data = cameraData(activeCamera_);\n \tRequest *request = buffer->request();\n \n-\tRkISP1FrameInfo *info = data->frameInfo_.find(buffer);\n-\tif (!info)\n-\t\treturn;\n-\n \tif (buffer->metadata().status == FrameMetadata::FrameCancelled) {\n-\t\tinfo->metadataProcessed = true;\n-\t\tcompleteBuffer(info->request, buffer);\n-\t\ttryCompleteRequest(info->request);\n+\t\tcompleteBuffer(request, buffer);\n+\t\ttryCompleteRequest(request);\n+\t\tavailableStatBuffers_.push(buffer);\n \t\treturn;\n \t}\n \n-\tdata->ipa_->processStatsBuffer(request->sequence(), info->statBuffer->cookie(),\n+\tdata->ipa_->processStatsBuffer(request->sequence(), buffer->cookie(),\n \t\t\t\t       data->delayedCtrls_->get(buffer->metadata().sequence));\n }\n \n","prefixes":["v2","5/7"]}