[{"id":33192,"web_url":"https://patchwork.libcamera.org/comment/33192/","msgid":"<20250127065936.GE17899@pendragon.ideasonboard.com>","date":"2025-01-27T06:59:36","subject":"Re: [PATCH v3 1/6] libcamera: software_isp: Track frames and\n\trequests","submitter":{"id":2,"url":"https://patchwork.libcamera.org/api/people/2/","name":"Laurent Pinchart","email":"laurent.pinchart@ideasonboard.com"},"content":"Hi Milan,\n\nThank you for the patch.\n\nOn Mon, Jan 13, 2025 at 02:34:00PM +0100, Milan Zamazal wrote:\n> Hardware pipelines track requests and other information related to\n> particular frames. This hasn't been needed in software ISP so far. But\n> in order to be able to track metadata corresponding to a given frame,\n> frame-request tracking mechanism starts being useful. This patch\n> introduces the basic tracking structure, actual metadata handling is\n> added in the following patch.\n> \n> Signed-off-by: Milan Zamazal \n> ---\n> src/libcamera/pipeline/simple/simple.cpp | 81 ++++++++++++++++++++++--\n> 1 file changed, 76 insertions(+), 5 deletions(-)\n> \n> diff --git a/src/libcamera/pipeline/simple/simple.cpp b/src/libcamera/pipeline/simple/simple.cpp\n> index 8ac24e6e..123179b6 100644\n> --- a/src/libcamera/pipeline/simple/simple.cpp\n> +++ b/src/libcamera/pipeline/simple/simple.cpp\n> @@ -181,6 +181,64 @@ LOG_DEFINE_CATEGORY(SimplePipeline)\n> \n> class SimplePipelineHandler;\n> \n> +struct SimpleFrameInfo {\n> +\tuint32_t frame;\n> +\tRequest *request;\n> +};\n> +\n> +class SimpleFrames\n> +{\n> +public:\n> +\tvoid create(Request *request);\n> +\tvoid destroy(uint32_t frame);\n> +\tvoid clear();\n> +\n> +\tSimpleFrameInfo *find(uint32_t frame);\n> +\tSimpleFrameInfo *find(Request *request);\n> +\n> +private:\n> +\tstd::map frameInfo_;\n> +};\n> +\n> +void SimpleFrames::create(Request *request)\n> +{\n> +\tuint32_t frame = request->sequence();\n> +\tauto [it, inserted] = frameInfo_.try_emplace(request->sequence());\n> +\tASSERT(inserted);\n> +\tit->second.frame = frame;\n> +\tit->second.request = request;\n> +}\n> +\n> +void SimpleFrames::destroy(uint32_t frame)\n> +{\n> +\tframeInfo_.erase(frame);\n> +}\n> +\n> +void SimpleFrames::clear()\n> +{\n> +\tframeInfo_.clear();\n> +}\n> +\n> +SimpleFrameInfo *SimpleFrames::find(uint32_t frame)\n> +{\n> +\tauto info = frameInfo_.find(frame);\n> +\tif (info == frameInfo_.end())\n> +\t\treturn nullptr;\n> +\treturn &info->second;\n> +}\n> +\n> +SimpleFrameInfo *SimpleFrames::find(Request *request)\n> +{\n> +\tfor (auto &itInfo : frameInfo_) {\n> +\t\tSimpleFrameInfo *info = &itInfo.second;\n> +\n> +\t\tif (info->request == request)\n> +\t\t\treturn info;\n> +\t}\n> +\n> +\treturn nullptr;\n> +}\n> +\n> struct SimplePipelineInfo {\n> \tconst char *driver;\n> \t/*\n> @@ -293,11 +351,13 @@ public:\n> \n> \tstd::unique_ptr converter_;\n> \tstd::unique_ptr swIsp_;\n> +\tSimpleFrames frameInfo_;\n> \n> private:\n> \tvoid tryPipeline(unsigned int code, const Size &size);\n> \tstatic std::vector routedSourcePads(MediaPad *sink);\n> \n> +\tvoid completeRequest(Request *request);\n> \tvoid conversionInputDone(FrameBuffer *buffer);\n> \tvoid conversionOutputDone(FrameBuffer *buffer);\n> \n> @@ -799,7 +859,7 @@ void SimpleCameraData::imageBufferReady(FrameBuffer *buffer)\n> \t\t\t/* No conversion, just complete the request. */\n> \t\t\tRequest *request = buffer->request();\n> \t\t\tpipe->completeBuffer(request, buffer);\n> -\t\t\tpipe->completeRequest(request);\n> +\t\t\tcompleteRequest(request);\n> \t\t\treturn;\n> \t\t}\n> \n> @@ -817,7 +877,7 @@ void SimpleCameraData::imageBufferReady(FrameBuffer *buffer)\n> \t\tconst RequestOutputs &outputs = conversionQueue_.front();\n> \t\tfor (auto &[stream, buf] : outputs.outputs)\n> \t\t\tpipe->completeBuffer(outputs.request, buf);\n> -\t\tpipe->completeRequest(outputs.request);\n> +\t\tcompleteRequest(outputs.request);\n> \t\tconversionQueue_.pop();\n> \n> \t\treturn;\n> @@ -875,7 +935,7 @@ void SimpleCameraData::imageBufferReady(FrameBuffer *buffer)\n> \n> \t/* Otherwise simply complete the request. */\n> \tpipe->completeBuffer(request, buffer);\n> -\tpipe->completeRequest(request);\n> +\tcompleteRequest(request);\n> }\n> \n> void SimpleCameraData::clearIncompleteRequests()\n> @@ -886,6 +946,14 @@ void SimpleCameraData::clearIncompleteRequests()\n> \t}\n> }\n> \n> +void SimpleCameraData::completeRequest(Request *request)\n> +{\n> +\tSimpleFrameInfo *info = frameInfo_.find(request);\n> +\tif (info)\n\nIf there's no frame info something is really wrong. I don't think\ncompleting the request makes much sense in that case.\n\n> +\t\tframeInfo_.destroy(info->frame);\n> +\tpipe()->completeRequest(request);\n> +}\n> +\n> void SimpleCameraData::conversionInputDone(FrameBuffer *buffer)\n> {\n> \t/* Queue the input buffer back for capture. */\n> @@ -899,7 +967,7 @@ void SimpleCameraData::conversionOutputDone(FrameBuffer *buffer)\n> \t/* Complete the buffer and the request. */\n> \tRequest *request = buffer->request();\n> \tif (pipe->completeBuffer(request, buffer))\n> -\t\tpipe->completeRequest(request);\n> +\t\tcompleteRequest(request);\n> }\n> \n> void SimpleCameraData::ispStatsReady(uint32_t frame, uint32_t bufferId)\n> @@ -1412,6 +1480,7 @@ void SimplePipelineHandler::stopDevice(Camera *camera)\n> \n> \tvideo->bufferReady.disconnect(data, &SimpleCameraData::imageBufferReady);\n> \n> +\tdata->frameInfo_.clear();\n> \tdata->clearIncompleteRequests();\n> \tdata->conversionBuffers_.clear();\n> \n> @@ -1442,8 +1511,10 @@ int SimplePipelineHandler::queueRequestDevice(Camera *camera, Request *request)\n> \n> \tif (data->useConversion_) {\n> \t\tdata->conversionQueue_.push({ request, std::move(buffers) });\n> -\t\tif (data->swIsp_)\n> +\t\tif (data->swIsp_) {\n> +\t\t\tdata->frameInfo_.create(request);\n> \t\t\tdata->swIsp_->queueRequest(request->sequence(), request->controls());\n> +\t\t}\n> \t}\n> \n> \treturn 0;","headers":{"Return-Path":"","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 391FABEFBE\n\tfor ;\n\tMon, 27 Jan 2025 06:59:50 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id 5EFCB6855E;\n\tMon, 27 Jan 2025 07:59:49 +0100 (CET)","from perceval.ideasonboard.com (perceval.ideasonboard.com\n\t[213.167.242.64])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTPS id 2577A68557\n\tfor ;\n\tMon, 27 Jan 2025 07:59:48 +0100 (CET)","from pendragon.ideasonboard.com (81-175-209-231.bb.dnainternet.fi\n\t[81.175.209.231])\n\tby perceval.ideasonboard.com (Postfix) with ESMTPSA id 8FCE9496;\n\tMon, 27 Jan 2025 07:58:41 +0100 (CET)"],"Authentication-Results":"lancelot.ideasonboard.com; dkim=pass (1024-bit key;\n\tunprotected) header.d=ideasonboard.com header.i=@ideasonboard.com\n\theader.b=\"r2KFb+YR\"; dkim-atps=neutral","DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com;\n\ts=mail; t=1737961121;\n\tbh=ajIK1y5MVa0E40qhsG+B562atLAj+jeNbb6I2ordfvA=;\n\th=Date:From:To:Cc:Subject:References:In-Reply-To:From;\n\tb=r2KFb+YRoVVqAtShB6KGEKWZ6y20ZmXkyxxWe27vvRNbAwwWK/0lU1k37RqhKsrdR\n\tzGZ/Y9O/ite7CnOCRNSO4E+tekoTy3LsUJnAPKPotuYABHYQbXVcVUNXZHgXGmfMEN\n\tu3AxknMOl0S4NgDXDRKFlQyBivokNdXbHZEMs8BU=","Date":"Mon, 27 Jan 2025 08:59:36 +0200","From":"Laurent Pinchart ","To":"Milan Zamazal ","Cc":"libcamera-devel@lists.libcamera.org, Kieran Bingham\n\t, =?utf-8?q?Barnab=C3=A1s_P=C5=91cze?=\n\t","Subject":"Re: [PATCH v3 1/6] libcamera: software_isp: Track frames and\n\trequests","Message-ID":"<20250127065936.GE17899@pendragon.ideasonboard.com>","References":"<20250113133405.12167-1-mzamazal@redhat.com>\n\t<20250113133405.12167-2-mzamazal@redhat.com>","MIME-Version":"1.0","Content-Type":"text/plain; charset=utf-8","Content-Disposition":"inline","In-Reply-To":"<20250113133405.12167-2-mzamazal@redhat.com>","X-BeenThere":"libcamera-devel@lists.libcamera.org","X-Mailman-Version":"2.1.29","Precedence":"list","List-Id":"","List-Unsubscribe":",\n\t","List-Archive":"","List-Post":"","List-Help":"","List-Subscribe":",\n\t","Errors-To":"libcamera-devel-bounces@lists.libcamera.org","Sender":"\"libcamera-devel\" "}},{"id":33214,"web_url":"https://patchwork.libcamera.org/comment/33214/","msgid":"<173800293718.1773152.13505862234848026616@ping.linuxembedded.co.uk>","date":"2025-01-27T18:35:37","subject":"Re: [PATCH v3 1/6] libcamera: software_isp: Track frames and\n\trequests","submitter":{"id":4,"url":"https://patchwork.libcamera.org/api/people/4/","name":"Kieran Bingham","email":"kieran.bingham@ideasonboard.com"},"content":"Quoting Laurent Pinchart (2025-01-27 06:59:36)\n> Hi Milan,\n> \n> Thank you for the patch.\n> \n> On Mon, Jan 13, 2025 at 02:34:00PM +0100, Milan Zamazal wrote:\n> > Hardware pipelines track requests and other information related to\n> > particular frames. This hasn't been needed in software ISP so far. But\n> > in order to be able to track metadata corresponding to a given frame,\n> > frame-request tracking mechanism starts being useful. This patch\n> > introduces the basic tracking structure, actual metadata handling is\n> > added in the following patch.\n> > \n\nI'm also weary that Barnabás made some fixes to other IPA 'frame\nhandlers' recently.\n\nI think somehow this concept needs to be templated to make sure we don't\nhave different bugs and different implementation in each IPA...\n\nNot a fault of 'this' patch, but I'm really weary about seeing multiple\nimplementations of the same object.\n\n> > Signed-off-by: Milan Zamazal \n> > ---\n> > src/libcamera/pipeline/simple/simple.cpp | 81 ++++++++++++++++++++++--\n> > 1 file changed, 76 insertions(+), 5 deletions(-)\n> > \n> > diff --git a/src/libcamera/pipeline/simple/simple.cpp b/src/libcamera/pipeline/simple/simple.cpp\n> > index 8ac24e6e..123179b6 100644\n> > --- a/src/libcamera/pipeline/simple/simple.cpp\n> > +++ b/src/libcamera/pipeline/simple/simple.cpp\n> > @@ -181,6 +181,64 @@ LOG_DEFINE_CATEGORY(SimplePipeline)\n> > \n> > class SimplePipelineHandler;\n> > \n> > +struct SimpleFrameInfo {\n> > + uint32_t frame;\n> > + Request *request;\n> > +};\n> > +\n> > +class SimpleFrames\n> > +{\n> > +public:\n> > + void create(Request *request);\n> > + void destroy(uint32_t frame);\n> > + void clear();\n> > +\n> > + SimpleFrameInfo *find(uint32_t frame);\n> > + SimpleFrameInfo *find(Request *request);\n> > +\n> > +private:\n> > + std::map frameInfo_;\n> > +};\n> > +\n> > +void SimpleFrames::create(Request *request)\n> > +{\n> > + uint32_t frame = request->sequence();\n> > + auto [it, inserted] = frameInfo_.try_emplace(request->sequence());\n> > + ASSERT(inserted);\n> > + it->second.frame = frame;\n> > + it->second.request = request;\n> > +}\n> > +\n> > +void SimpleFrames::destroy(uint32_t frame)\n> > +{\n> > + frameInfo_.erase(frame);\n> > +}\n> > +\n> > +void SimpleFrames::clear()\n> > +{\n> > + frameInfo_.clear();\n> > +}\n> > +\n> > +SimpleFrameInfo *SimpleFrames::find(uint32_t frame)\n> > +{\n> > + auto info = frameInfo_.find(frame);\n> > + if (info == frameInfo_.end())\n> > + return nullptr;\n> > + return &info->second;\n> > +}\n> > +\n> > +SimpleFrameInfo *SimpleFrames::find(Request *request)\n> > +{\n> > + for (auto &itInfo : frameInfo_) {\n> > + SimpleFrameInfo *info = &itInfo.second;\n> > +\n> > + if (info->request == request)\n> > + return info;\n> > + }\n> > +\n> > + return nullptr;\n> > +}\n> > +\n> > struct SimplePipelineInfo {\n> > const char *driver;\n> > /*\n> > @@ -293,11 +351,13 @@ public:\n> > \n> > std::unique_ptr converter_;\n> > std::unique_ptr swIsp_;\n> > + SimpleFrames frameInfo_;\n> > \n> > private:\n> > void tryPipeline(unsigned int code, const Size &size);\n> > static std::vector routedSourcePads(MediaPad *sink);\n> > \n> > + void completeRequest(Request *request);\n> > void conversionInputDone(FrameBuffer *buffer);\n> > void conversionOutputDone(FrameBuffer *buffer);\n> > \n> > @@ -799,7 +859,7 @@ void SimpleCameraData::imageBufferReady(FrameBuffer *buffer)\n> > /* No conversion, just complete the request. */\n> > Request *request = buffer->request();\n> > pipe->completeBuffer(request, buffer);\n> > - pipe->completeRequest(request);\n> > + completeRequest(request);\n> > return;\n> > }\n> > \n> > @@ -817,7 +877,7 @@ void SimpleCameraData::imageBufferReady(FrameBuffer *buffer)\n> > const RequestOutputs &outputs = conversionQueue_.front();\n> > for (auto &[stream, buf] : outputs.outputs)\n> > pipe->completeBuffer(outputs.request, buf);\n> > - pipe->completeRequest(outputs.request);\n> > + completeRequest(outputs.request);\n> > conversionQueue_.pop();\n> > \n> > return;\n> > @@ -875,7 +935,7 @@ void SimpleCameraData::imageBufferReady(FrameBuffer *buffer)\n> > \n> > /* Otherwise simply complete the request. */\n> > pipe->completeBuffer(request, buffer);\n> > - pipe->completeRequest(request);\n> > + completeRequest(request);\n> > }\n> > \n> > void SimpleCameraData::clearIncompleteRequests()\n> > @@ -886,6 +946,14 @@ void SimpleCameraData::clearIncompleteRequests()\n> > }\n> > }\n> > \n> > +void SimpleCameraData::completeRequest(Request *request)\n> > +{\n> > + SimpleFrameInfo *info = frameInfo_.find(request);\n> > + if (info)\n> \n> If there's no frame info something is really wrong. I don't think\n> completing the request makes much sense in that case.\n> \n> > + frameInfo_.destroy(info->frame);\n> > + pipe()->completeRequest(request);\n> > +}\n> > +\n> > void SimpleCameraData::conversionInputDone(FrameBuffer *buffer)\n> > {\n> > /* Queue the input buffer back for capture. */\n> > @@ -899,7 +967,7 @@ void SimpleCameraData::conversionOutputDone(FrameBuffer *buffer)\n> > /* Complete the buffer and the request. */\n> > Request *request = buffer->request();\n> > if (pipe->completeBuffer(request, buffer))\n> > - pipe->completeRequest(request);\n> > + completeRequest(request);\n> > }\n> > \n> > void SimpleCameraData::ispStatsReady(uint32_t frame, uint32_t bufferId)\n> > @@ -1412,6 +1480,7 @@ void SimplePipelineHandler::stopDevice(Camera *camera)\n> > \n> > video->bufferReady.disconnect(data, &SimpleCameraData::imageBufferReady);\n> > \n> > + data->frameInfo_.clear();\n> > data->clearIncompleteRequests();\n> > data->conversionBuffers_.clear();\n> > \n> > @@ -1442,8 +1511,10 @@ int SimplePipelineHandler::queueRequestDevice(Camera *camera, Request *request)\n> > \n> > if (data->useConversion_) {\n> > data->conversionQueue_.push({ request, std::move(buffers) });\n> > - if (data->swIsp_)\n> > + if (data->swIsp_) {\n> > + data->frameInfo_.create(request);\n> > data->swIsp_->queueRequest(request->sequence(), request->controls());\n> > + }\n> > }\n> > \n> > return 0;\n> \n> -- \n> Regards,\n> \n> Laurent Pinchart","headers":{"Return-Path":"","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 520CABEFBE\n\tfor ;\n\tMon, 27 Jan 2025 18:35:42 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id 9764E6855D;\n\tMon, 27 Jan 2025 19:35:41 +0100 (CET)","from perceval.ideasonboard.com (perceval.ideasonboard.com\n\t[IPv6:2001:4b98:dc2:55:216:3eff:fef7:d647])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTPS id D50FA68549\n\tfor ;\n\tMon, 27 Jan 2025 19:35:39 +0100 (CET)","from pendragon.ideasonboard.com\n\t(cpc89244-aztw30-2-0-cust6594.18-1.cable.virginm.net [86.31.185.195])\n\tby perceval.ideasonboard.com (Postfix) with ESMTPSA id 22D8318D;\n\tMon, 27 Jan 2025 19:34:33 +0100 (CET)"],"Authentication-Results":"lancelot.ideasonboard.com; dkim=pass (1024-bit key;\n\tunprotected) header.d=ideasonboard.com header.i=@ideasonboard.com\n\theader.b=\"fFsKfAnt\"; dkim-atps=neutral","DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com;\n\ts=mail; t=1738002873;\n\tbh=kQ42cYMAYM5ohVRKEo23lbKkbvLyz5ZzSvb9dG8wYB0=;\n\th=In-Reply-To:References:Subject:From:Cc:To:Date:From;\n\tb=fFsKfAntLIY2wYo5o2klZ8lyxDGq8tXEpCOQ6anI9ykiwYV1MGDQ5OxnQ3bArtyIx\n\tjbLifjitcHMPdH4IUZzrKvJDySdUAcACMrBAH8yI2u7rKla/rQj9ptS4OBFhqHrVjk\n\tuDYVX8mSn/cmqp8AW6pmUPF3/uewdOi17omcQ5/s=","Content-Type":"text/plain; charset=\"utf-8\"","MIME-Version":"1.0","Content-Transfer-Encoding":"quoted-printable","In-Reply-To":"<20250127065936.GE17899@pendragon.ideasonboard.com>","References":"<20250113133405.12167-1-mzamazal@redhat.com>\n\t<20250113133405.12167-2-mzamazal@redhat.com>\n\t<20250127065936.GE17899@pendragon.ideasonboard.com>","Subject":"Re: [PATCH v3 1/6] libcamera: software_isp: Track frames and\n\trequests","From":"Kieran Bingham ","Cc":"libcamera-devel@lists.libcamera.org, =?utf-8?b?QmFybmFiw6FzIFDFkWN6?=\n\t=?utf-8?q?e?= ","To":"Laurent Pinchart ,\n\tMilan Zamazal ","Date":"Mon, 27 Jan 2025 18:35:37 +0000","Message-ID":"<173800293718.1773152.13505862234848026616@ping.linuxembedded.co.uk>","User-Agent":"alot/0.10","X-BeenThere":"libcamera-devel@lists.libcamera.org","X-Mailman-Version":"2.1.29","Precedence":"list","List-Id":"","List-Unsubscribe":",\n\t","List-Archive":"","List-Post":"","List-Help":"","List-Subscribe":",\n\t","Errors-To":"libcamera-devel-bounces@lists.libcamera.org","Sender":"\"libcamera-devel\" "}},{"id":33217,"web_url":"https://patchwork.libcamera.org/comment/33217/","msgid":"","date":"2025-01-27T20:17:35","subject":"Re: [PATCH v3 1/6] libcamera: software_isp: Track frames and\n\trequests","submitter":{"id":133,"url":"https://patchwork.libcamera.org/api/people/133/","name":"Pőcze Barnabás","email":"pobrn@protonmail.com"},"content":"2025. január 27., hétfő 19:35 keltezéssel, Kieran Bingham írta:\n\n> Quoting Laurent Pinchart (2025-01-27 06:59:36)\n> > Hi Milan,\n> >\n> > Thank you for the patch.\n> >\n> > On Mon, Jan 13, 2025 at 02:34:00PM +0100, Milan Zamazal wrote:\n> > > Hardware pipelines track requests and other information related to\n> > > particular frames. This hasn't been needed in software ISP so far. But\n> > > in order to be able to track metadata corresponding to a given frame,\n> > > frame-request tracking mechanism starts being useful. This patch\n> > > introduces the basic tracking structure, actual metadata handling is\n> > > added in the following patch.\n> > >\n> \n> I'm also weary that Barnabás made some fixes to other IPA 'frame\n> handlers' recently.\n\nThe changes in the proposed https://patchwork.libcamera.org/patch/22610/ patch\nhave already been incorporated into this patch set.\n\n\n> \n> I think somehow this concept needs to be templated to make sure we don't\n> have different bugs and different implementation in each IPA...\n> \n> Not a fault of 'this' patch, but I'm really weary about seeing multiple\n> implementations of the same object.\n\nI agree.\n\n\nRegards,\nBarnabás Pőcze\n\n\n> \n> > > Signed-off-by: Milan Zamazal \n> > > ---\n> > > src/libcamera/pipeline/simple/simple.cpp | 81 ++++++++++++++++++++++--\n> > > 1 file changed, 76 insertions(+), 5 deletions(-)\n> > >\n> > > diff --git a/src/libcamera/pipeline/simple/simple.cpp b/src/libcamera/pipeline/simple/simple.cpp\n> > > index 8ac24e6e..123179b6 100644\n> > > --- a/src/libcamera/pipeline/simple/simple.cpp\n> > > +++ b/src/libcamera/pipeline/simple/simple.cpp\n> > > @@ -181,6 +181,64 @@ LOG_DEFINE_CATEGORY(SimplePipeline)\n> > >\n> > > class SimplePipelineHandler;\n> > >\n> > > +struct SimpleFrameInfo {\n> > > + uint32_t frame;\n> > > + Request *request;\n> > > +};\n> > > +\n> > > +class SimpleFrames\n> > > +{\n> > > +public:\n> > > + void create(Request *request);\n> > > + void destroy(uint32_t frame);\n> > > + void clear();\n> > > +\n> > > + SimpleFrameInfo *find(uint32_t frame);\n> > > + SimpleFrameInfo *find(Request *request);\n> > > +\n> > > +private:\n> > > + std::map frameInfo_;\n> > > +};\n> > > +\n> > > +void SimpleFrames::create(Request *request)\n> > > +{\n> > > + uint32_t frame = request->sequence();\n> > > + auto [it, inserted] = frameInfo_.try_emplace(request->sequence());\n> > > + ASSERT(inserted);\n> > > + it->second.frame = frame;\n> > > + it->second.request = request;\n> > > +}\n> > > +\n> > > +void SimpleFrames::destroy(uint32_t frame)\n> > > +{\n> > > + frameInfo_.erase(frame);\n> > > +}\n> > > +\n> > > +void SimpleFrames::clear()\n> > > +{\n> > > + frameInfo_.clear();\n> > > +}\n> > > +\n> > > +SimpleFrameInfo *SimpleFrames::find(uint32_t frame)\n> > > +{\n> > > + auto info = frameInfo_.find(frame);\n> > > + if (info == frameInfo_.end())\n> > > + return nullptr;\n> > > + return &info->second;\n> > > +}\n> > > +\n> > > +SimpleFrameInfo *SimpleFrames::find(Request *request)\n> > > +{\n> > > + for (auto &itInfo : frameInfo_) {\n> > > + SimpleFrameInfo *info = &itInfo.second;\n> > > +\n> > > + if (info->request == request)\n> > > + return info;\n> > > + }\n> > > +\n> > > + return nullptr;\n> > > +}\n> > > +\n> > > struct SimplePipelineInfo {\n> > > const char *driver;\n> > > /*\n> > > @@ -293,11 +351,13 @@ public:\n> > >\n> > > std::unique_ptr converter_;\n> > > std::unique_ptr swIsp_;\n> > > + SimpleFrames frameInfo_;\n> > >\n> > > private:\n> > > void tryPipeline(unsigned int code, const Size &size);\n> > > static std::vector routedSourcePads(MediaPad *sink);\n> > >\n> > > + void completeRequest(Request *request);\n> > > void conversionInputDone(FrameBuffer *buffer);\n> > > void conversionOutputDone(FrameBuffer *buffer);\n> > >\n> > > @@ -799,7 +859,7 @@ void SimpleCameraData::imageBufferReady(FrameBuffer *buffer)\n> > > /* No conversion, just complete the request. */\n> > > Request *request = buffer->request();\n> > > pipe->completeBuffer(request, buffer);\n> > > - pipe->completeRequest(request);\n> > > + completeRequest(request);\n> > > return;\n> > > }\n> > >\n> > > @@ -817,7 +877,7 @@ void SimpleCameraData::imageBufferReady(FrameBuffer *buffer)\n> > > const RequestOutputs &outputs = conversionQueue_.front();\n> > > for (auto &[stream, buf] : outputs.outputs)\n> > > pipe->completeBuffer(outputs.request, buf);\n> > > - pipe->completeRequest(outputs.request);\n> > > + completeRequest(outputs.request);\n> > > conversionQueue_.pop();\n> > >\n> > > return;\n> > > @@ -875,7 +935,7 @@ void SimpleCameraData::imageBufferReady(FrameBuffer *buffer)\n> > >\n> > > /* Otherwise simply complete the request. */\n> > > pipe->completeBuffer(request, buffer);\n> > > - pipe->completeRequest(request);\n> > > + completeRequest(request);\n> > > }\n> > >\n> > > void SimpleCameraData::clearIncompleteRequests()\n> > > @@ -886,6 +946,14 @@ void SimpleCameraData::clearIncompleteRequests()\n> > > }\n> > > }\n> > >\n> > > +void SimpleCameraData::completeRequest(Request *request)\n> > > +{\n> > > + SimpleFrameInfo *info = frameInfo_.find(request);\n> > > + if (info)\n> >\n> > If there's no frame info something is really wrong. I don't think\n> > completing the request makes much sense in that case.\n> >\n> > > + frameInfo_.destroy(info->frame);\n> > > + pipe()->completeRequest(request);\n> > > +}\n> > > +\n> > > void SimpleCameraData::conversionInputDone(FrameBuffer *buffer)\n> > > {\n> > > /* Queue the input buffer back for capture. */\n> > > @@ -899,7 +967,7 @@ void SimpleCameraData::conversionOutputDone(FrameBuffer *buffer)\n> > > /* Complete the buffer and the request. */\n> > > Request *request = buffer->request();\n> > > if (pipe->completeBuffer(request, buffer))\n> > > - pipe->completeRequest(request);\n> > > + completeRequest(request);\n> > > }\n> > >\n> > > void SimpleCameraData::ispStatsReady(uint32_t frame, uint32_t bufferId)\n> > > @@ -1412,6 +1480,7 @@ void SimplePipelineHandler::stopDevice(Camera *camera)\n> > >\n> > > video->bufferReady.disconnect(data, &SimpleCameraData::imageBufferReady);\n> > >\n> > > + data->frameInfo_.clear();\n> > > data->clearIncompleteRequests();\n> > > data->conversionBuffers_.clear();\n> > >\n> > > @@ -1442,8 +1511,10 @@ int SimplePipelineHandler::queueRequestDevice(Camera *camera, Request *request)\n> > >\n> > > if (data->useConversion_) {\n> > > data->conversionQueue_.push({ request, std::move(buffers) });\n> > > - if (data->swIsp_)\n> > > + if (data->swIsp_) {\n> > > + data->frameInfo_.create(request);\n> > > data->swIsp_->queueRequest(request->sequence(), request->controls());\n> > > + }\n> > > }\n> > >\n> > > return 0;\n> >\n> > --\n> > Regards,\n> >\n> > Laurent Pinchart\n>","headers":{"Return-Path":"","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 7DC7FBDC71\n\tfor ;\n\tMon, 27 Jan 2025 20:17:42 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id 93BA86855D;\n\tMon, 27 Jan 2025 21:17:41 +0100 (CET)","from mail-10630.protonmail.ch (mail-10630.protonmail.ch\n\t[79.135.106.30])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTPS id 70CFE68549\n\tfor ;\n\tMon, 27 Jan 2025 21:17:39 +0100 (CET)"],"Authentication-Results":"lancelot.ideasonboard.com;\n\tdkim=fail reason=\"signature verification failed\" (2048-bit key;\n\tunprotected) header.d=protonmail.com header.i=@protonmail.com\n\theader.b=\"sZW/yxC4\"; dkim-atps=neutral","DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/relaxed; d=protonmail.com;\n\ts=protonmail3; t=1738009058; x=1738268258;\n\tbh=7MsZ5Er+Hrb+fNWOo+UOr/WlTECxX4Z+59kA1J18770=;\n\th=Date:To:From:Cc:Subject:Message-ID:In-Reply-To:References:\n\tFeedback-ID:From:To:Cc:Date:Subject:Reply-To:Feedback-ID:\n\tMessage-ID:BIMI-Selector:List-Unsubscribe:List-Unsubscribe-Post;\n\tb=sZW/yxC4ArhkG1T+C0e7jjNzoO+i0+HXWY/XOiFonXzcMNSLesN4Qwl7xh0xGQqh4\n\t+pf3lGP87Nn2OExs+yLkvWrj/hXKuhoTctN/gRZiO1vscFlN8+tqNjaqsGQA6G/ZC3\n\t1EjTQ6tslMVrcYtPxecWP19qFtcji0o0xnGycWNMbaalQ7eyKJwFECHSoZlz5070Jj\n\tnseu+DSwoWri5PT/Se4iKQ3nt2vko3OpSz0Is59sOhmy5Y/hQ9EU7mqYOKQIHnxvzT\n\tb5Oab4FHtUTccQDY/QFdnj+Ogh3YUI1KrJPrNpp6nCaTr0c6LPmuKS/xzx/Z72JdfB\n\tKMbE7lS+yjvbA==","Date":"Mon, 27 Jan 2025 20:17:35 +0000","To":"Kieran Bingham ","From":"=?utf-8?q?Barnab=C3=A1s_P=C5=91cze?= ","Cc":"Laurent Pinchart ,\n\tMilan Zamazal , libcamera-devel@lists.libcamera.org","Subject":"Re: [PATCH v3 1/6] libcamera: software_isp: Track frames and\n\trequests","Message-ID":"","In-Reply-To":"<173800293718.1773152.13505862234848026616@ping.linuxembedded.co.uk>","References":"<20250113133405.12167-1-mzamazal@redhat.com>\n\t<20250113133405.12167-2-mzamazal@redhat.com>\n\t<20250127065936.GE17899@pendragon.ideasonboard.com>\n\t<173800293718.1773152.13505862234848026616@ping.linuxembedded.co.uk>","Feedback-ID":"20568564:user:proton","X-Pm-Message-ID":"cc2b24a80c2ed74f47c3e4efb339de2153392d08","MIME-Version":"1.0","Content-Type":"text/plain; charset=utf-8","Content-Transfer-Encoding":"quoted-printable","X-BeenThere":"libcamera-devel@lists.libcamera.org","X-Mailman-Version":"2.1.29","Precedence":"list","List-Id":"","List-Unsubscribe":",\n\t","List-Archive":"","List-Post":"","List-Help":"","List-Subscribe":",\n\t","Errors-To":"libcamera-devel-bounces@lists.libcamera.org","Sender":"\"libcamera-devel\" "}}]