[{"id":13792,"web_url":"https://patchwork.libcamera.org/comment/13792/","msgid":"<20201118171724.7tx4y5m7hbybmwtx@uno.localdomain>","date":"2020-11-18T17:17:24","subject":"Re: [libcamera-devel] [PATCH 11/11] libcamera: ipu3: Share\n\tparameter and statistic buffers with IPA","submitter":{"id":3,"url":"https://patchwork.libcamera.org/api/people/3/","name":"Jacopo Mondi","email":"jacopo@jmondi.org"},"content":"Hi Niklas,\n\nOn Thu, Nov 05, 2020 at 01:15:46AM +0100, Niklas Söderlund wrote:\n> Use the IPU3Frames helper to share parameter and statistic buffers with\n> the IPA. Track which parameter and statistic buffer is used for witch\n\nparameters and statistics ?\ns/witch/which/\n\n> request and make sure the parameter buffers is filled in by the IPA\n> before it's needed and that the statistic buffer is consumed and meta\n> data generated before completing the request.\n>\n> With this change the IPU3 pipeline is prepared to fully operate with an\n> IPA component.\n>\n> Signed-off-by: Niklas Söderlund \n> ---\n> src/libcamera/pipeline/ipu3/ipu3.cpp | 106 +++++++++++++++++++++++++--\n> 1 file changed, 100 insertions(+), 6 deletions(-)\n>\n> diff --git a/src/libcamera/pipeline/ipu3/ipu3.cpp b/src/libcamera/pipeline/ipu3/ipu3.cpp\n> index d161c2e68c0db0f2..58f8feaba4e87c54 100644\n> --- a/src/libcamera/pipeline/ipu3/ipu3.cpp\n> +++ b/src/libcamera/pipeline/ipu3/ipu3.cpp\n> @@ -28,6 +28,7 @@\n> #include \"libcamera/internal/v4l2_controls.h\"\n>\n> #include \"cio2.h\"\n> +#include \"frames.h\"\n> #include \"imgu.h\"\n>\n> namespace libcamera {\n> @@ -59,6 +60,8 @@ public:\n>\n> \tvoid imguOutputBufferReady(FrameBuffer *buffer);\n> \tvoid cio2BufferReady(FrameBuffer *buffer);\n> +\tvoid paramBufferReady(FrameBuffer *buffer);\n> +\tvoid statBufferReady(FrameBuffer *buffer);\n>\n> \tCIO2Device cio2_;\n> \tImgUDevice *imgu_;\n> @@ -68,6 +71,7 @@ public:\n> \tStream rawStream_;\n>\n> \tDelayedControls *delayedCtrls_;\n> +\tstd::unique_ptr frameInfo_;\n>\n> private:\n> \tvoid actOnIpa(unsigned int id, const IPAOperationData &op);\n> @@ -582,6 +586,8 @@ int PipelineHandlerIPU3::allocateBuffers(Camera *camera)\n> \tif (ret < 0)\n> \t\treturn ret;\n>\n> +\tdata->frameInfo_->mapBuffers(imgu->paramBuffers_, imgu->statBuffers_);\n> +\n> \treturn 0;\n> }\n>\n> @@ -589,6 +595,8 @@ int PipelineHandlerIPU3::freeBuffers(Camera *camera)\n> {\n> \tIPU3CameraData *data = cameraData(camera);\n>\n> +\tdata->frameInfo_->unmapBuffers();\n> +\n> \tdata->imgu_->freeBuffers();\n>\n> \treturn 0;\n> @@ -684,6 +692,10 @@ int PipelineHandlerIPU3::queueRequestDevice(Camera *camera, Request *request)\n> \tIPU3CameraData *data = cameraData(camera);\n> \tint error = 0;\n>\n> +\tIPU3Frames::Info *info = data->frameInfo_->create(request);\n> +\tif (!info)\n> +\t\treturn -ENOENT;\n> +\n> \t/*\n> \t * Queue a buffer on the CIO2, using the raw stream buffer provided in\n> \t * the request, if any, or a CIO2 internal buffer otherwise.\n> @@ -693,7 +705,10 @@ int PipelineHandlerIPU3::queueRequestDevice(Camera *camera, Request *request)\n> \tif (!rawBuffer)\n> \t\treturn -ENOMEM;\n>\n> +\tinfo->rawBuffer = rawBuffer;\n> +\n> \t/* Queue all buffers from the request aimed for the ImgU. */\n> +\tbool onlyRaw = true;\n> \tfor (auto it : request->buffers()) {\n> \t\tconst Stream *stream = it.first;\n> \t\tFrameBuffer *buffer = it.second;\n> @@ -708,6 +723,23 @@ int PipelineHandlerIPU3::queueRequestDevice(Camera *camera, Request *request)\n>\n> \t\tif (ret < 0)\n> \t\t\terror = ret;\n> +\n> +\t\tonlyRaw = false;\n> +\t}\n> +\n> +\t/* If request only contains a raw buffer do not involve IPA. */\n> +\tif (onlyRaw) {\n> +\t\tinfo->paramDequeued = true;\n> +\t\tinfo->metadataProcessed = true;\n> +\t} else {\n> +\t\tIPAOperationData op;\n> +\t\top.operation = IPU3_IPA_EVENT_FILL_PARAMS;\n> +\t\top.data = { info->id, info->paramBuffer->cookie() };\n> +\t\top.controls = { request->controls() };\n> +\t\tdata->ipa_->processEvent(op);\n> +\n> +\t\tdata->imgu_->param_->queueBuffer(info->paramBuffer);\n> +\t\tdata->imgu_->stat_->queueBuffer(info->statBuffer);\n> \t}\n>\n> \treturn error;\n> @@ -845,6 +877,10 @@ int PipelineHandlerIPU3::registerCameras()\n> \t\t\t\t\t&IPU3CameraData::imguOutputBufferReady);\n> \t\tdata->imgu_->viewfinder_->bufferReady.connect(data.get(),\n> \t\t\t\t\t&IPU3CameraData::imguOutputBufferReady);\n> +\t\tdata->imgu_->param_->bufferReady.connect(data.get(),\n> +\t\t\t\t\t&IPU3CameraData::paramBufferReady);\n> +\t\tdata->imgu_->stat_->bufferReady.connect(data.get(),\n> +\t\t\t\t\t&IPU3CameraData::statBufferReady);\n>\n> \t\t/* Create and register the Camera instance. */\n> \t\tstd::string cameraId = cio2->sensor()->id();\n> @@ -874,10 +910,12 @@ int IPU3CameraData::loadIPA()\n>\n> \tipa_->init(IPASettings{});\n>\n> +\tframeInfo_ = std::make_unique(pipe_, ipa_.get());\n\nnit: this a a collection of frame info, I would use a plural name like\nframesInfo_\n\n> +\n> \treturn 0;\n> }\n>\n> -void IPU3CameraData::actOnIpa([[maybe_unused]] unsigned int id,\n> +void IPU3CameraData::actOnIpa(unsigned int id,\n> \t\t\t const IPAOperationData &action)\n> {\n> \tswitch (action.operation) {\n> @@ -886,6 +924,24 @@ void IPU3CameraData::actOnIpa([[maybe_unused]] unsigned int id,\n> \t\tdelayedCtrls_->push(controls);\n> \t\tbreak;\n> \t}\n> +\tcase IPU3_IPA_ACTION_PARAM_FILLED: {\n> +\t\tIPU3Frames::Info *info = frameInfo_->find(id);\n> +\t\tif (!info)\n> +\t\t\tbreak;\n> +\n> +\t\tinfo->paramFilled = true;\n> +\t\tbreak;\n> +\t}\n> +\tcase IPU3_IPA_ACTION_METADATA_READY: {\n> +\t\tIPU3Frames::Info *info = frameInfo_->find(id);\n> +\t\tif (!info)\n> +\t\t\tbreak;\n> +\n> +\t\tinfo->request->metadata() = action.controls[0];\n> +\t\tinfo->metadataProcessed = true;\n> +\t\tframeInfo_->tryComplete(info);\n> +\t\tbreak;\n> +\t}\n> \tdefault:\n> \t\tLOG(IPU3, Error) << \"Unknown action \" << action.operation;\n> \t\tbreak;\n> @@ -906,13 +962,15 @@ void IPU3CameraData::imguOutputBufferReady(FrameBuffer *buffer)\n> {\n> \tRequest *request = buffer->request();\n>\n> -\tif (!pipe_->completeBuffer(request, buffer))\n> -\t/* Request not completed yet, return here. */\n\nI would not drop the early return, if the request has pending buffers\nthe below frameInfo_->tryComplete() will fail too.\n\n> +\tpipe_->completeBuffer(request, buffer);\n> +\n> +\tIPU3Frames::Info *info = frameInfo_->find(buffer);\n> +\tif (!info)\n> \t\treturn;\n>\n> -\t/* Mark the request as complete. */\n> \trequest->metadata().set(controls::draft::PipelineDepth, 3);\n\nIf we complete in a different call to frameInfo_->tryCompelte() this\nmetadata won't be set I guess.\n\n> -\tpipe_->completeRequest(request);\n> +\n> +\tframeInfo_->tryComplete(info);\n> }\n>\n> /**\n> @@ -928,6 +986,10 @@ void IPU3CameraData::cio2BufferReady(FrameBuffer *buffer)\n> \tif (buffer->metadata().status == FrameMetadata::FrameCancelled)\n> \t\treturn;\n>\n> +\tIPU3Frames::Info *info = frameInfo_->find(buffer);\n> +\tif (!info)\n> +\t\treturn;\n> +\n> \tRequest *request = buffer->request();\n>\n> \t/*\n> @@ -938,14 +1000,46 @@ void IPU3CameraData::cio2BufferReady(FrameBuffer *buffer)\n> \t\tbool isComplete = pipe_->completeBuffer(request, buffer);\n> \t\tif (isComplete) {\n\nThis might be a good occasion to drop 'isComplete' and replace it with\n if (pipe_->completeBuffer(request, buffer)\n\n> \t\t\trequest->metadata().set(controls::draft::PipelineDepth, 2);\n> -\t\t\tpipe_->completeRequest(request);\n> +\t\t\tframeInfo_->tryComplete(info);\n> \t\t\treturn;\n> \t\t}\n> \t}\n>\n> +\tif (!info->paramFilled)\n> +\t\tLOG(IPU3, Info)\n> +\t\t\t<< \"Parameters not ready on time for id \" << info->id;\n> +\n\nOh, now I see what paramFilled is for, but if we queue a onlyRaw\nrequest, we never trigger the IPA's FILL_PARAMS operation and will\nnever receive a PARAMS_FILLED back, so this will always be false ?\n\nRecording if the request was onlyRaw might help ? In this case you can\ncheck both here and in the frame helper\n if (!onlyRaw && !info->paramFilled)\n\ndropping paramDequeued completely ?\n\n\n> \timgu_->input_->queueBuffer(buffer);\n> }\n>\n> +void IPU3CameraData::paramBufferReady(FrameBuffer *buffer)\n> +{\n> +\tif (buffer->metadata().status == FrameMetadata::FrameCancelled)\n> +\t\treturn;\n> +\n> +\tIPU3Frames::Info *info = frameInfo_->find(buffer);\n> +\tif (!info)\n> +\t\treturn;\n> +\n> +\tinfo->paramDequeued = true;\n> +\tframeInfo_->tryComplete(info);\n> +}\n> +\n> +void IPU3CameraData::statBufferReady(FrameBuffer *buffer)\n> +{\n> +\tif (buffer->metadata().status == FrameMetadata::FrameCancelled)\n> +\t\treturn;\n> +\n> +\tIPU3Frames::Info *info = frameInfo_->find(buffer);\n> +\tif (!info)\n> +\t\treturn;\n> +\n> +\tIPAOperationData op;\n> +\top.operation = IPU3_IPA_EVENT_PARSE_STAT;\n> +\top.data = { info->id, info->statBuffer->cookie() };\n> +\tipa_->processEvent(op);\n> +}\n> +\n> REGISTER_PIPELINE_HANDLER(PipelineHandlerIPU3)\n>\n> } /* namespace libcamera */\n> --\n> 2.29.2\n>\n> _______________________________________________\n> libcamera-devel mailing list\n> libcamera-devel@lists.libcamera.org\n> https://lists.libcamera.org/listinfo/libcamera-devel","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 06CF1BE176\n\tfor ;\n\tWed, 18 Nov 2020 17:17:23 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id 951A77E9B0;\n\tWed, 18 Nov 2020 18:17:22 +0100 (CET)","from relay6-d.mail.gandi.net (relay6-d.mail.gandi.net\n\t[217.70.183.198])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTPS id CD347632AF\n\tfor ;\n\tWed, 18 Nov 2020 18:17:21 +0100 (CET)","from uno.localdomain (93-34-118-233.ip49.fastwebnet.it\n\t[93.34.118.233]) (Authenticated sender: jacopo@jmondi.org)\n\tby relay6-d.mail.gandi.net (Postfix) with ESMTPSA id 4D8CDC001A;\n\tWed, 18 Nov 2020 17:17:20 +0000 (UTC)"],"X-Originating-IP":"93.34.118.233","Date":"Wed, 18 Nov 2020 18:17:24 +0100","From":"Jacopo Mondi ","To":"Niklas =?utf-8?q?S=C3=B6derlund?= ","Message-ID":"<20201118171724.7tx4y5m7hbybmwtx@uno.localdomain>","References":"<20201105001546.1690179-1-niklas.soderlund@ragnatech.se>\n\t<20201105001546.1690179-12-niklas.soderlund@ragnatech.se>","MIME-Version":"1.0","Content-Disposition":"inline","In-Reply-To":"<20201105001546.1690179-12-niklas.soderlund@ragnatech.se>","Subject":"Re: [libcamera-devel] [PATCH 11/11] libcamera: ipu3: Share\n\tparameter and statistic buffers with IPA","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","Cc":"libcamera-devel@lists.libcamera.org","Content-Type":"text/plain; charset=\"utf-8\"","Content-Transfer-Encoding":"base64","Errors-To":"libcamera-devel-bounces@lists.libcamera.org","Sender":"\"libcamera-devel\" "}},{"id":14395,"web_url":"https://patchwork.libcamera.org/comment/14395/","msgid":"","date":"2020-12-29T14:53:09","subject":"Re: [libcamera-devel] [PATCH 11/11] libcamera: ipu3: Share\n\tparameter and statistic buffers with IPA","submitter":{"id":5,"url":"https://patchwork.libcamera.org/api/people/5/","name":"Niklas Söderlund","email":"niklas.soderlund@ragnatech.se"},"content":"Hi Jacopo,\n\nThanks for your comments.\n\nOn 2020-11-18 18:17:24 +0100, Jacopo Mondi wrote:\n> Hi Niklas,\n> \n> On Thu, Nov 05, 2020 at 01:15:46AM +0100, Niklas Söderlund wrote:\n> > Use the IPU3Frames helper to share parameter and statistic buffers with\n> > the IPA. Track which parameter and statistic buffer is used for witch\n> \n> parameters and statistics ?\n> s/witch/which/\n> \n> > request and make sure the parameter buffers is filled in by the IPA\n> > before it's needed and that the statistic buffer is consumed and meta\n> > data generated before completing the request.\n> >\n> > With this change the IPU3 pipeline is prepared to fully operate with an\n> > IPA component.\n> >\n> > Signed-off-by: Niklas Söderlund \n> > ---\n> > src/libcamera/pipeline/ipu3/ipu3.cpp | 106 +++++++++++++++++++++++++--\n> > 1 file changed, 100 insertions(+), 6 deletions(-)\n> >\n> > diff --git a/src/libcamera/pipeline/ipu3/ipu3.cpp b/src/libcamera/pipeline/ipu3/ipu3.cpp\n> > index d161c2e68c0db0f2..58f8feaba4e87c54 100644\n> > --- a/src/libcamera/pipeline/ipu3/ipu3.cpp\n> > +++ b/src/libcamera/pipeline/ipu3/ipu3.cpp\n> > @@ -28,6 +28,7 @@\n> > #include \"libcamera/internal/v4l2_controls.h\"\n> >\n> > #include \"cio2.h\"\n> > +#include \"frames.h\"\n> > #include \"imgu.h\"\n> >\n> > namespace libcamera {\n> > @@ -59,6 +60,8 @@ public:\n> >\n> > \tvoid imguOutputBufferReady(FrameBuffer *buffer);\n> > \tvoid cio2BufferReady(FrameBuffer *buffer);\n> > +\tvoid paramBufferReady(FrameBuffer *buffer);\n> > +\tvoid statBufferReady(FrameBuffer *buffer);\n> >\n> > \tCIO2Device cio2_;\n> > \tImgUDevice *imgu_;\n> > @@ -68,6 +71,7 @@ public:\n> > \tStream rawStream_;\n> >\n> > \tDelayedControls *delayedCtrls_;\n> > +\tstd::unique_ptr frameInfo_;\n> >\n> > private:\n> > \tvoid actOnIpa(unsigned int id, const IPAOperationData &op);\n> > @@ -582,6 +586,8 @@ int PipelineHandlerIPU3::allocateBuffers(Camera *camera)\n> > \tif (ret < 0)\n> > \t\treturn ret;\n> >\n> > +\tdata->frameInfo_->mapBuffers(imgu->paramBuffers_, imgu->statBuffers_);\n> > +\n> > \treturn 0;\n> > }\n> >\n> > @@ -589,6 +595,8 @@ int PipelineHandlerIPU3::freeBuffers(Camera *camera)\n> > {\n> > \tIPU3CameraData *data = cameraData(camera);\n> >\n> > +\tdata->frameInfo_->unmapBuffers();\n> > +\n> > \tdata->imgu_->freeBuffers();\n> >\n> > \treturn 0;\n> > @@ -684,6 +692,10 @@ int PipelineHandlerIPU3::queueRequestDevice(Camera *camera, Request *request)\n> > \tIPU3CameraData *data = cameraData(camera);\n> > \tint error = 0;\n> >\n> > +\tIPU3Frames::Info *info = data->frameInfo_->create(request);\n> > +\tif (!info)\n> > +\t\treturn -ENOENT;\n> > +\n> > \t/*\n> > \t * Queue a buffer on the CIO2, using the raw stream buffer provided in\n> > \t * the request, if any, or a CIO2 internal buffer otherwise.\n> > @@ -693,7 +705,10 @@ int PipelineHandlerIPU3::queueRequestDevice(Camera *camera, Request *request)\n> > \tif (!rawBuffer)\n> > \t\treturn -ENOMEM;\n> >\n> > +\tinfo->rawBuffer = rawBuffer;\n> > +\n> > \t/* Queue all buffers from the request aimed for the ImgU. */\n> > +\tbool onlyRaw = true;\n> > \tfor (auto it : request->buffers()) {\n> > \t\tconst Stream *stream = it.first;\n> > \t\tFrameBuffer *buffer = it.second;\n> > @@ -708,6 +723,23 @@ int PipelineHandlerIPU3::queueRequestDevice(Camera *camera, Request *request)\n> >\n> > \t\tif (ret < 0)\n> > \t\t\terror = ret;\n> > +\n> > +\t\tonlyRaw = false;\n> > +\t}\n> > +\n> > +\t/* If request only contains a raw buffer do not involve IPA. */\n> > +\tif (onlyRaw) {\n> > +\t\tinfo->paramDequeued = true;\n> > +\t\tinfo->metadataProcessed = true;\n> > +\t} else {\n> > +\t\tIPAOperationData op;\n> > +\t\top.operation = IPU3_IPA_EVENT_FILL_PARAMS;\n> > +\t\top.data = { info->id, info->paramBuffer->cookie() };\n> > +\t\top.controls = { request->controls() };\n> > +\t\tdata->ipa_->processEvent(op);\n> > +\n> > +\t\tdata->imgu_->param_->queueBuffer(info->paramBuffer);\n> > +\t\tdata->imgu_->stat_->queueBuffer(info->statBuffer);\n> > \t}\n> >\n> > \treturn error;\n> > @@ -845,6 +877,10 @@ int PipelineHandlerIPU3::registerCameras()\n> > \t\t\t\t\t&IPU3CameraData::imguOutputBufferReady);\n> > \t\tdata->imgu_->viewfinder_->bufferReady.connect(data.get(),\n> > \t\t\t\t\t&IPU3CameraData::imguOutputBufferReady);\n> > +\t\tdata->imgu_->param_->bufferReady.connect(data.get(),\n> > +\t\t\t\t\t&IPU3CameraData::paramBufferReady);\n> > +\t\tdata->imgu_->stat_->bufferReady.connect(data.get(),\n> > +\t\t\t\t\t&IPU3CameraData::statBufferReady);\n> >\n> > \t\t/* Create and register the Camera instance. */\n> > \t\tstd::string cameraId = cio2->sensor()->id();\n> > @@ -874,10 +910,12 @@ int IPU3CameraData::loadIPA()\n> >\n> > \tipa_->init(IPASettings{});\n> >\n> > +\tframeInfo_ = std::make_unique(pipe_, ipa_.get());\n> \n> nit: this a a collection of frame info, I would use a plural name like\n> framesInfo_\n> \n> > +\n> > \treturn 0;\n> > }\n> >\n> > -void IPU3CameraData::actOnIpa([[maybe_unused]] unsigned int id,\n> > +void IPU3CameraData::actOnIpa(unsigned int id,\n> > \t\t\t const IPAOperationData &action)\n> > {\n> > \tswitch (action.operation) {\n> > @@ -886,6 +924,24 @@ void IPU3CameraData::actOnIpa([[maybe_unused]] unsigned int id,\n> > \t\tdelayedCtrls_->push(controls);\n> > \t\tbreak;\n> > \t}\n> > +\tcase IPU3_IPA_ACTION_PARAM_FILLED: {\n> > +\t\tIPU3Frames::Info *info = frameInfo_->find(id);\n> > +\t\tif (!info)\n> > +\t\t\tbreak;\n> > +\n> > +\t\tinfo->paramFilled = true;\n> > +\t\tbreak;\n> > +\t}\n> > +\tcase IPU3_IPA_ACTION_METADATA_READY: {\n> > +\t\tIPU3Frames::Info *info = frameInfo_->find(id);\n> > +\t\tif (!info)\n> > +\t\t\tbreak;\n> > +\n> > +\t\tinfo->request->metadata() = action.controls[0];\n> > +\t\tinfo->metadataProcessed = true;\n> > +\t\tframeInfo_->tryComplete(info);\n> > +\t\tbreak;\n> > +\t}\n> > \tdefault:\n> > \t\tLOG(IPU3, Error) << \"Unknown action \" << action.operation;\n> > \t\tbreak;\n> > @@ -906,13 +962,15 @@ void IPU3CameraData::imguOutputBufferReady(FrameBuffer *buffer)\n> > {\n> > \tRequest *request = buffer->request();\n> >\n> > -\tif (!pipe_->completeBuffer(request, buffer))\n> > -\t/* Request not completed yet, return here. */\n> \n> I would not drop the early return, if the request has pending buffers\n> the below frameInfo_->tryComplete() will fail too.\n\nIf we do that the pipeline depth won't be set.\n\n> \n> > +\tpipe_->completeBuffer(request, buffer);\n> > +\n> > +\tIPU3Frames::Info *info = frameInfo_->find(buffer);\n> > +\tif (!info)\n> > \t\treturn;\n> >\n> > -\t/* Mark the request as complete. */\n> > \trequest->metadata().set(controls::draft::PipelineDepth, 3);\n> \n> If we complete in a different call to frameInfo_->tryCompelte() this\n> metadata won't be set I guess.\n\nCorrect, it's set to 2 when the CIO2 buffer completes and is then \nupdated to 3 when the IMGU buffer completes. This way we deal correctly \nwith both Requests that contains only RAW buffers and once that are \nprocessed by the IMGU.\n\n> \n> > -\tpipe_->completeRequest(request);\n> > +\n> > +\tframeInfo_->tryComplete(info);\n> > }\n> >\n> > /**\n> > @@ -928,6 +986,10 @@ void IPU3CameraData::cio2BufferReady(FrameBuffer *buffer)\n> > \tif (buffer->metadata().status == FrameMetadata::FrameCancelled)\n> > \t\treturn;\n> >\n> > +\tIPU3Frames::Info *info = frameInfo_->find(buffer);\n> > +\tif (!info)\n> > +\t\treturn;\n> > +\n> > \tRequest *request = buffer->request();\n> >\n> > \t/*\n> > @@ -938,14 +1000,46 @@ void IPU3CameraData::cio2BufferReady(FrameBuffer *buffer)\n> > \t\tbool isComplete = pipe_->completeBuffer(request, buffer);\n> > \t\tif (isComplete) {\n> \n> This might be a good occasion to drop 'isComplete' and replace it with\n> if (pipe_->completeBuffer(request, buffer)\n\nGood point.\n\n> \n> > \t\t\trequest->metadata().set(controls::draft::PipelineDepth, 2);\n> > -\t\t\tpipe_->completeRequest(request);\n> > +\t\t\tframeInfo_->tryComplete(info);\n> > \t\t\treturn;\n> > \t\t}\n> > \t}\n> >\n> > +\tif (!info->paramFilled)\n> > +\t\tLOG(IPU3, Info)\n> > +\t\t\t<< \"Parameters not ready on time for id \" << info->id;\n> > +\n> \n> Oh, now I see what paramFilled is for, but if we queue a onlyRaw\n> request, we never trigger the IPA's FILL_PARAMS operation and will\n> never receive a PARAMS_FILLED back, so this will always be false ?\n\nCorrect. But in the onlyRaw case isComplete will be true and we will \nnever reach this check.\n\n> \n> Recording if the request was onlyRaw might help ? In this case you can\n> check both here and in the frame helper\n> if (!onlyRaw && !info->paramFilled)\n> dropping paramDequeued completely ?\n\nWe could turn paramFilled flag into a onlyRaw flag but I don't see how \nthat is much different.\n\n> \n> \n> \n> > \timgu_->input_->queueBuffer(buffer);\n> > }\n> >\n> > +void IPU3CameraData::paramBufferReady(FrameBuffer *buffer)\n> > +{\n> > +\tif (buffer->metadata().status == FrameMetadata::FrameCancelled)\n> > +\t\treturn;\n> > +\n> > +\tIPU3Frames::Info *info = frameInfo_->find(buffer);\n> > +\tif (!info)\n> > +\t\treturn;\n> > +\n> > +\tinfo->paramDequeued = true;\n> > +\tframeInfo_->tryComplete(info);\n> > +}\n> > +\n> > +void IPU3CameraData::statBufferReady(FrameBuffer *buffer)\n> > +{\n> > +\tif (buffer->metadata().status == FrameMetadata::FrameCancelled)\n> > +\t\treturn;\n> > +\n> > +\tIPU3Frames::Info *info = frameInfo_->find(buffer);\n> > +\tif (!info)\n> > +\t\treturn;\n> > +\n> > +\tIPAOperationData op;\n> > +\top.operation = IPU3_IPA_EVENT_PARSE_STAT;\n> > +\top.data = { info->id, info->statBuffer->cookie() };\n> > +\tipa_->processEvent(op);\n> > +}\n> > +\n> > REGISTER_PIPELINE_HANDLER(PipelineHandlerIPU3)\n> >\n> > } /* namespace libcamera */\n> > --\n> > 2.29.2\n> >\n> > _______________________________________________\n> > libcamera-devel mailing list\n> > libcamera-devel@lists.libcamera.org\n> > https://lists.libcamera.org/listinfo/libcamera-devel","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 36E48C0F1A\n\tfor ;\n\tTue, 29 Dec 2020 14:53:15 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id A71B4615B2;\n\tTue, 29 Dec 2020 15:53:14 +0100 (CET)","from mail-lf1-x12f.google.com (mail-lf1-x12f.google.com\n\t[IPv6:2a00:1450:4864:20::12f])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTPS id C4FB76031F\n\tfor ;\n\tTue, 29 Dec 2020 15:53:12 +0100 (CET)","by mail-lf1-x12f.google.com with SMTP id o13so31345624lfr.3\n\tfor ;\n\tTue, 29 Dec 2020 06:53:12 -0800 (PST)","from localhost (h-209-203.A463.priv.bahnhof.se. [155.4.209.203])\n\tby smtp.gmail.com with ESMTPSA id\n\th23sm6868461ljh.115.2020.12.29.06.53.10\n\t(version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256);\n\tTue, 29 Dec 2020 06:53:10 -0800 (PST)"],"Authentication-Results":"lancelot.ideasonboard.com;\n\tdkim=fail reason=\"signature verification failed\" (2048-bit key;\n\tunprotected) header.d=ragnatech-se.20150623.gappssmtp.com\n\theader.i=@ragnatech-se.20150623.gappssmtp.com\n\theader.b=\"yTAGZL+U\"; dkim-atps=neutral","DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/relaxed;\n\td=ragnatech-se.20150623.gappssmtp.com; s=20150623;\n\th=date:from:to:cc:subject:message-id:references:mime-version\n\t:content-disposition:content-transfer-encoding:in-reply-to;\n\tbh=+ZYvocL7B8XPoSyoMEP3rkrcZcZx0JiKcG/RRPSOz7Y=;\n\tb=yTAGZL+UI26x/5mkJQoaaZS910uwOsTHdi+dj7mKGlAOtDMxwQoUGi6Y8rVxu6AwCe\n\t8kvL7DX9Ny1J/wbbDMxHxD3kVvqCyELr83K2oesdWbcAWJxHg0H9Y+oSBG+/2tDF54LX\n\tizPh2I20fLxB3qhn7opC2uxQ+GBigbJV1vn9+LAOX8+BUuZsRbR7MCLTJRxOZiKJGv2Z\n\tp33ovgPGbfPmpcoxlLxZBeD6P8JcJEeNGvNlll20k6ts9fVIOd07Vltun/4SvqyfkNLC\n\tXvyZOAWvT2Gy0gfXrCBnaVI7FplIWoTfAxPZdoTnwiSEir2xfNtjzcKqlYAN38HwzmMP\n\tEbcA==","X-Google-DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/relaxed;\n\td=1e100.net; s=20161025;\n\th=x-gm-message-state:date:from:to:cc:subject:message-id:references\n\t:mime-version:content-disposition:content-transfer-encoding\n\t:in-reply-to;\n\tbh=+ZYvocL7B8XPoSyoMEP3rkrcZcZx0JiKcG/RRPSOz7Y=;\n\tb=QWOyVF2jgCnnw0xNzime/avJGA69ZJ8v4bcUhQIBo3whTZoTN2FXrKBcTuFPSDQtpK\n\tfbuOcYuKipXkJ+RAziuc/uiRY4wxegG6HsLbSpUhKtpiqRhvYMkMw3heBkxzymbJj4vZ\n\tMO3RnFGQbD9K6LqgOdRtr3+T+x3K7RLkY6EUPDAEEwD/biPwQ+RRaBVvRqzQzL/GMOuO\n\tkRZpBT88J4gYElznHOAU+nk3nPZkgTCgdj5JacnT00EA3zZVfOjGENMrtSmPq2FUHv8M\n\tgdEVGiaaIlibD9j3O6Ou1wdtqxIvH6k6a7yyqvYgxAPW+RoUE4n/KgNuIFz9XFl2qwS3\n\tHn3A==","X-Gm-Message-State":"AOAM533fdS8LRzIasBE7IKFQ4RuCsKcSgyJkMaLT7dDwnmzcYgKetuTA\n\tIsZ8eCgx2Z+gg5yUu62vCcerlw==","X-Google-Smtp-Source":"ABdhPJxnM7ONZVJ/beuzIjxwOj46bgevNj0+AhwNvYF1EOBrzSE/Mj0mAKDKc8wH8CKZcmZ6+38P2A==","X-Received":"by 2002:a19:7109:: with SMTP id\n\tm9mr17956000lfc.351.1609253591554; \n\tTue, 29 Dec 2020 06:53:11 -0800 (PST)","Date":"Tue, 29 Dec 2020 15:53:09 +0100","From":"Niklas =?iso-8859-1?q?S=F6derlund?= ","To":"Jacopo Mondi ","Message-ID":"","References":"<20201105001546.1690179-1-niklas.soderlund@ragnatech.se>\n\t<20201105001546.1690179-12-niklas.soderlund@ragnatech.se>\n\t<20201118171724.7tx4y5m7hbybmwtx@uno.localdomain>","MIME-Version":"1.0","Content-Disposition":"inline","In-Reply-To":"<20201118171724.7tx4y5m7hbybmwtx@uno.localdomain>","Subject":"Re: [libcamera-devel] [PATCH 11/11] libcamera: ipu3: Share\n\tparameter and statistic buffers with IPA","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","Cc":"libcamera-devel@lists.libcamera.org","Content-Type":"text/plain; charset=\"iso-8859-1\"","Content-Transfer-Encoding":"quoted-printable","Errors-To":"libcamera-devel-bounces@lists.libcamera.org","Sender":"\"libcamera-devel\" "}}]