[{"id":3409,"web_url":"https://patchwork.libcamera.org/comment/3409/","msgid":"<20200111005524.GL4859@pendragon.ideasonboard.com>","date":"2020-01-11T00:55:24","subject":"Re: [libcamera-devel] [PATCH v3 22/33] libcamera: pipeline: rkisp1:\n\tSwitch to FrameBuffer interface for stat and param","submitter":{"id":2,"url":"https://patchwork.libcamera.org/api/people/2/","name":"Laurent Pinchart","email":"laurent.pinchart@ideasonboard.com"},"content":"Hi Niklas,\n\nThank you for the patch.\n\nOn Fri, Jan 10, 2020 at 08:37:57PM +0100, Niklas Söderlund wrote:\n> The V4L2VideoDevice class can now operate using a FrameBuffer interface,\n> switch the RkISP1 statistics and parameters buffer to use it. We can not\n> convert the application-facing buffers yet.\n> \n> Signed-off-by: Niklas Söderlund \n> Reviewed-by: Laurent Pinchart \n> ---\n> * Changes since v2\n> - Rename {param,stat}BuffersMemory to {param,stat}Buffers_\n> - Rename {param,stat}Buffers_ to available{Param,Stat}Buffers_\n> - s/err:/error:/\n> - Beutification of small code blocks\n> ---\n> src/libcamera/pipeline/rkisp1/rkisp1.cpp | 132 ++++++++++++-----------\n> 1 file changed, 72 insertions(+), 60 deletions(-)\n> \n> diff --git a/src/libcamera/pipeline/rkisp1/rkisp1.cpp b/src/libcamera/pipeline/rkisp1/rkisp1.cpp\n> index 607ff85539a22324..dbc5df801f30e76b 100644\n> --- a/src/libcamera/pipeline/rkisp1/rkisp1.cpp\n> +++ b/src/libcamera/pipeline/rkisp1/rkisp1.cpp\n> @@ -32,9 +32,6 @@\n> #include \"v4l2_subdevice.h\"\n> #include \"v4l2_videodevice.h\"\n> \n> -#define RKISP1_PARAM_BASE 0x100\n> -#define RKISP1_STAT_BASE 0x200\n> -\n> namespace libcamera {\n> \n> LOG_DEFINE_CATEGORY(RkISP1)\n> @@ -52,8 +49,8 @@ struct RkISP1FrameInfo {\n> \tunsigned int frame;\n> \tRequest *request;\n> \n> -\tBuffer *paramBuffer;\n> -\tBuffer *statBuffer;\n> +\tFrameBuffer *paramBuffer;\n> +\tFrameBuffer *statBuffer;\n> \tBuffer *videoBuffer;\n> \n> \tbool paramFilled;\n> @@ -71,6 +68,7 @@ public:\n> \n> \tRkISP1FrameInfo *find(unsigned int frame);\n> \tRkISP1FrameInfo *find(Buffer *buffer);\n> +\tRkISP1FrameInfo *find(FrameBuffer *buffer);\n> \tRkISP1FrameInfo *find(Request *request);\n> \n> private:\n> @@ -203,8 +201,8 @@ private:\n> \tint createCamera(MediaEntity *sensor);\n> \tvoid tryCompleteRequest(Request *request);\n> \tvoid bufferReady(Buffer *buffer);\n> -\tvoid paramReady(Buffer *buffer);\n> -\tvoid statReady(Buffer *buffer);\n> +\tvoid paramReady(FrameBuffer *buffer);\n> +\tvoid statReady(FrameBuffer *buffer);\n> \n> \tMediaDevice *media_;\n> \tV4L2Subdevice *dphy_;\n> @@ -213,11 +211,10 @@ private:\n> \tV4L2VideoDevice *param_;\n> \tV4L2VideoDevice *stat_;\n> \n> -\tBufferPool paramPool_;\n> -\tBufferPool statPool_;\n> -\n> -\tstd::queue paramBuffers_;\n> -\tstd::queue statBuffers_;\n> +\tstd::vector> paramBuffers_;\n> +\tstd::vector> statBuffers_;\n> +\tstd::queue availableParamBuffers_;\n> +\tstd::queue availableStatBuffers_;\n> \n> \tCamera *activeCamera_;\n> };\n> @@ -229,17 +226,17 @@ RkISP1Frames::RkISP1Frames(PipelineHandler *pipe)\n> \n> RkISP1FrameInfo *RkISP1Frames::create(unsigned int frame, Request *request, Stream *stream)\n> {\n> -\tif (pipe_->paramBuffers_.empty()) {\n> +\tif (pipe_->availableParamBuffers_.empty()) {\n> \t\tLOG(RkISP1, Error) << \"Parameters buffer underrun\";\n> \t\treturn nullptr;\n> \t}\n> -\tBuffer *paramBuffer = pipe_->paramBuffers_.front();\n> +\tFrameBuffer *paramBuffer = pipe_->availableParamBuffers_.front();\n> \n> -\tif (pipe_->statBuffers_.empty()) {\n> +\tif (pipe_->availableStatBuffers_.empty()) {\n> \t\tLOG(RkISP1, Error) << \"Statisitc buffer underrun\";\n> \t\treturn nullptr;\n> \t}\n> -\tBuffer *statBuffer = pipe_->statBuffers_.front();\n> +\tFrameBuffer *statBuffer = pipe_->availableStatBuffers_.front();\n> \n> \tBuffer *videoBuffer = request->findBuffer(stream);\n> \tif (!videoBuffer) {\n> @@ -248,8 +245,8 @@ RkISP1FrameInfo *RkISP1Frames::create(unsigned int frame, Request *request, Stre\n> \t\treturn nullptr;\n> \t}\n> \n> -\tpipe_->paramBuffers_.pop();\n> -\tpipe_->statBuffers_.pop();\n> +\tpipe_->availableParamBuffers_.pop();\n> +\tpipe_->availableStatBuffers_.pop();\n> \n> \tRkISP1FrameInfo *info = new RkISP1FrameInfo;\n> \n> @@ -273,8 +270,8 @@ int RkISP1Frames::destroy(unsigned int frame)\n> \tif (!info)\n> \t\treturn -ENOENT;\n> \n> -\tpipe_->paramBuffers_.push(info->paramBuffer);\n> -\tpipe_->statBuffers_.push(info->statBuffer);\n> +\tpipe_->availableParamBuffers_.push(info->paramBuffer);\n> +\tpipe_->availableStatBuffers_.push(info->statBuffer);\n> \n> \tframeInfo_.erase(info->frame);\n> \n> @@ -299,9 +296,21 @@ RkISP1FrameInfo *RkISP1Frames::find(Buffer *buffer)\n> \tfor (auto &itInfo : frameInfo_) {\n> \t\tRkISP1FrameInfo *info = itInfo.second;\n> \n> +\t\tif (info->videoBuffer == buffer)\n> +\t\t\treturn info;\n> +\t}\n> +\n> +\tLOG(RkISP1, Error) << \"Can't locate info from buffer\";\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->videoBuffer == buffer)\n> +\t\t info->statBuffer == buffer)\n> \t\t\treturn info;\n> \t}\n> \n> @@ -661,6 +670,7 @@ int PipelineHandlerRkISP1::allocateBuffers(Camera *camera,\n> {\n> \tRkISP1CameraData *data = cameraData(camera);\n> \tStream *stream = *streams.begin();\n> +\tunsigned int count = 1;\n> \tint ret;\n> \n> \tif (stream->memoryType() == InternalMemory)\n> @@ -671,35 +681,41 @@ int PipelineHandlerRkISP1::allocateBuffers(Camera *camera,\n> \tif (ret)\n> \t\treturn ret;\n> \n> -\tparamPool_.createBuffers(stream->configuration().bufferCount);\n> -\tret = param_->exportBuffers(¶mPool_);\n> -\tif (ret) {\n> -\t\tvideo_->releaseBuffers();\n> -\t\treturn ret;\n> -\t}\n> +\tunsigned int maxBuffers = 0;\n> +\tfor (const Stream *s : camera->streams())\n> +\t\tmaxBuffers = std::max(maxBuffers, s->configuration().bufferCount);\n> \n> -\tstatPool_.createBuffers(stream->configuration().bufferCount);\n> -\tret = stat_->exportBuffers(&statPool_);\n> -\tif (ret) {\n> -\t\tparam_->releaseBuffers();\n> -\t\tvideo_->releaseBuffers();\n> -\t\treturn ret;\n> -\t}\n> +\tret = param_->exportBuffers(maxBuffers, ¶mBuffers_);\n> +\tif (ret < 0)\n> +\t\tgoto error;\n> +\n> +\tret = stat_->exportBuffers(maxBuffers, &statBuffers_);\n> +\tif (ret < 0)\n> +\t\tgoto error;\n> \n> -\tfor (unsigned int i = 0; i < stream->configuration().bufferCount; i++) {\n> -\t\tdata->ipaBuffers_.push_back({ .id = RKISP1_PARAM_BASE | i,\n> -\t\t\t\t\t .planes = paramPool_.buffers()[i].planes() });\n> -\t\tparamBuffers_.push(new Buffer(i));\n> +\tfor (std::unique_ptr &buffer : paramBuffers_) {\n> +\t\tbuffer->setCookie(count++);\n> +\t\tdata->ipaBuffers_.push_back({ .id = buffer->cookie(),\n> +\t\t\t\t\t .planes = buffer->planes() });\n> +\t\tavailableParamBuffers_.push(buffer.get());\n> \t}\n> \n> -\tfor (unsigned int i = 0; i < stream->configuration().bufferCount; i++) {\n> -\t\tdata->ipaBuffers_.push_back({ .id = RKISP1_STAT_BASE | i,\n> -\t\t\t\t\t .planes = statPool_.buffers()[i].planes() });\n> -\t\tstatBuffers_.push(new Buffer(i));\n> +\tfor (std::unique_ptr &buffer : statBuffers_) {\n> +\t\tbuffer->setCookie(count++);\n> +\t\tdata->ipaBuffers_.push_back({ .id = buffer->cookie(),\n> +\t\t\t\t\t .planes = buffer->planes() });\n> +\t\tavailableStatBuffers_.push(buffer.get());\n> \t}\n> \n> \tdata->ipa_->mapBuffers(data->ipaBuffers_);\n> \n> +\treturn 0;\n> +\n> +error:\n> +\tparamBuffers_.clear();\n> +\tstatBuffers_.clear();\n> +\tvideo_->releaseBuffers();\n> +\n> \treturn ret;\n> }\n> \n> @@ -708,15 +724,14 @@ int PipelineHandlerRkISP1::freeBuffers(Camera *camera,\n> {\n> \tRkISP1CameraData *data = cameraData(camera);\n> \n> -\twhile (!statBuffers_.empty()) {\n> -\t\tdelete statBuffers_.front();\n> -\t\tstatBuffers_.pop();\n> -\t}\n> +\twhile (!availableStatBuffers_.empty())\n> +\t\tavailableStatBuffers_.pop();\n> \n> -\twhile (!paramBuffers_.empty()) {\n> -\t\tdelete paramBuffers_.front();\n> -\t\tparamBuffers_.pop();\n> -\t}\n> +\twhile (!availableParamBuffers_.empty())\n> +\t\tavailableParamBuffers_.pop();\n\nCan't you replace this with\n\n\tavailableStatBuffers_.clear();\n\tavailableParamBuffers_.clear();\n\n?\n\nPlease keep my Reviewed-by.\n\n> +\n> +\tparamBuffers_.clear();\n> +\tstatBuffers_.clear();\n> \n> \tstd::vector ids;\n> \tfor (IPABuffer &ipabuf : data->ipaBuffers_)\n> @@ -823,7 +838,7 @@ int PipelineHandlerRkISP1::queueRequestDevice(Camera *camera,\n> \n> \tIPAOperationData op;\n> \top.operation = RKISP1_IPA_EVENT_QUEUE_REQUEST;\n> -\top.data = { data->frame_, RKISP1_PARAM_BASE | info->paramBuffer->index() };\n> +\top.data = { data->frame_, info->paramBuffer->cookie() };\n> \top.controls = { request->controls() };\n> \tdata->ipa_->processEvent(op);\n> \n> @@ -938,8 +953,8 @@ bool PipelineHandlerRkISP1::match(DeviceEnumerator *enumerator)\n> \t\treturn false;\n> \n> \tvideo_->bufferReady.connect(this, &PipelineHandlerRkISP1::bufferReady);\n> -\tstat_->bufferReady.connect(this, &PipelineHandlerRkISP1::statReady);\n> -\tparam_->bufferReady.connect(this, &PipelineHandlerRkISP1::paramReady);\n> +\tstat_->frameBufferReady.connect(this, &PipelineHandlerRkISP1::statReady);\n> +\tparam_->frameBufferReady.connect(this, &PipelineHandlerRkISP1::paramReady);\n> \n> \t/* Configure default links. */\n> \tif (initLinks() < 0) {\n> @@ -1001,7 +1016,7 @@ void PipelineHandlerRkISP1::bufferReady(Buffer *buffer)\n> \ttryCompleteRequest(request);\n> }\n> \n> -void PipelineHandlerRkISP1::paramReady(Buffer *buffer)\n> +void PipelineHandlerRkISP1::paramReady(FrameBuffer *buffer)\n> {\n> \tASSERT(activeCamera_);\n> \tRkISP1CameraData *data = cameraData(activeCamera_);\n> @@ -1012,7 +1027,7 @@ void PipelineHandlerRkISP1::paramReady(Buffer *buffer)\n> \ttryCompleteRequest(info->request);\n> }\n> \n> -void PipelineHandlerRkISP1::statReady(Buffer *buffer)\n> +void PipelineHandlerRkISP1::statReady(FrameBuffer *buffer)\n> {\n> \tASSERT(activeCamera_);\n> \tRkISP1CameraData *data = cameraData(activeCamera_);\n> @@ -1021,12 +1036,9 @@ void PipelineHandlerRkISP1::statReady(Buffer *buffer)\n> \tif (!info)\n> \t\treturn;\n> \n> -\tunsigned int frame = info->frame;\n> -\tunsigned int statid = RKISP1_STAT_BASE | info->statBuffer->index();\n> -\n> \tIPAOperationData op;\n> \top.operation = RKISP1_IPA_EVENT_SIGNAL_STAT_BUFFER;\n> -\top.data = { frame, statid };\n> +\top.data = { info->frame, info->statBuffer->cookie() };\n> \tdata->ipa_->processEvent(op);\n> }\n>","headers":{"Return-Path":"","Received":["from perceval.ideasonboard.com (perceval.ideasonboard.com\n\t[213.167.242.64])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTPS id 42373606AC\n\tfor ;\n\tSat, 11 Jan 2020 01:55:40 +0100 (CET)","from pendragon.ideasonboard.com (81-175-216-236.bb.dnainternet.fi\n\t[81.175.216.236])\n\tby perceval.ideasonboard.com (Postfix) with ESMTPSA id 4B8C652F;\n\tSat, 11 Jan 2020 01:55:39 +0100 (CET)"],"DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com;\n\ts=mail; t=1578704139;\n\tbh=dfTW9mTWu7y4YbOhBoPX8/pftcEtp3V6pR2sZkfZ6iw=;\n\th=Date:From:To:Cc:Subject:References:In-Reply-To:From;\n\tb=NUFHqcc7GWzU9O+fET/DQGE6BwjEG4KcvAImUaAztZR06otCCmgki4+IsZOH5rQV1\n\txyR0ZGrBxvIND5Jk40x6vE+FrrqdEqyqDt3SwNdJeeXJeA6296aVPXW8j+UL5hC+o6\n\tdlruA9yjIFQ+XbjszVvcvOGDJnXPQ4mt+xOOFcGQ=","Date":"Sat, 11 Jan 2020 02:55:24 +0200","From":"Laurent Pinchart ","To":"Niklas =?utf-8?q?S=C3=B6derlund?= ","Cc":"libcamera-devel@lists.libcamera.org","Message-ID":"<20200111005524.GL4859@pendragon.ideasonboard.com>","References":"<20200110193808.2266294-1-niklas.soderlund@ragnatech.se>\n\t<20200110193808.2266294-23-niklas.soderlund@ragnatech.se>","MIME-Version":"1.0","Content-Type":"text/plain; charset=utf-8","Content-Disposition":"inline","Content-Transfer-Encoding":"8bit","In-Reply-To":"<20200110193808.2266294-23-niklas.soderlund@ragnatech.se>","User-Agent":"Mutt/1.10.1 (2018-07-13)","Subject":"Re: [libcamera-devel] [PATCH v3 22/33] libcamera: pipeline: rkisp1:\n\tSwitch to FrameBuffer interface for stat and param","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","X-List-Received-Date":"Sat, 11 Jan 2020 00:55:40 -0000"}},{"id":3422,"web_url":"https://patchwork.libcamera.org/comment/3422/","msgid":"<20200111233134.GB697401@oden.dyn.berto.se>","date":"2020-01-11T23:31:34","subject":"Re: [libcamera-devel] [PATCH v3 22/33] libcamera: pipeline: rkisp1:\n\tSwitch to FrameBuffer interface for stat and param","submitter":{"id":5,"url":"https://patchwork.libcamera.org/api/people/5/","name":"Niklas Söderlund","email":"niklas.soderlund@ragnatech.se"},"content":"Hi Laurent,\n\nThanks for your feedback.\n\nOn 2020-01-11 02:55:24 +0200, Laurent Pinchart wrote:\n> Hi Niklas,\n> \n> Thank you for the patch.\n> \n> On Fri, Jan 10, 2020 at 08:37:57PM +0100, Niklas Söderlund wrote:\n> > The V4L2VideoDevice class can now operate using a FrameBuffer interface,\n> > switch the RkISP1 statistics and parameters buffer to use it. We can not\n> > convert the application-facing buffers yet.\n> > \n> > Signed-off-by: Niklas Söderlund \n> > Reviewed-by: Laurent Pinchart \n> > ---\n> > * Changes since v2\n> > - Rename {param,stat}BuffersMemory to {param,stat}Buffers_\n> > - Rename {param,stat}Buffers_ to available{Param,Stat}Buffers_\n> > - s/err:/error:/\n> > - Beutification of small code blocks\n> > ---\n> > src/libcamera/pipeline/rkisp1/rkisp1.cpp | 132 ++++++++++++-----------\n> > 1 file changed, 72 insertions(+), 60 deletions(-)\n> > \n> > diff --git a/src/libcamera/pipeline/rkisp1/rkisp1.cpp b/src/libcamera/pipeline/rkisp1/rkisp1.cpp\n> > index 607ff85539a22324..dbc5df801f30e76b 100644\n> > --- a/src/libcamera/pipeline/rkisp1/rkisp1.cpp\n> > +++ b/src/libcamera/pipeline/rkisp1/rkisp1.cpp\n> > @@ -32,9 +32,6 @@\n> > #include \"v4l2_subdevice.h\"\n> > #include \"v4l2_videodevice.h\"\n> > \n> > -#define RKISP1_PARAM_BASE 0x100\n> > -#define RKISP1_STAT_BASE 0x200\n> > -\n> > namespace libcamera {\n> > \n> > LOG_DEFINE_CATEGORY(RkISP1)\n> > @@ -52,8 +49,8 @@ struct RkISP1FrameInfo {\n> > \tunsigned int frame;\n> > \tRequest *request;\n> > \n> > -\tBuffer *paramBuffer;\n> > -\tBuffer *statBuffer;\n> > +\tFrameBuffer *paramBuffer;\n> > +\tFrameBuffer *statBuffer;\n> > \tBuffer *videoBuffer;\n> > \n> > \tbool paramFilled;\n> > @@ -71,6 +68,7 @@ public:\n> > \n> > \tRkISP1FrameInfo *find(unsigned int frame);\n> > \tRkISP1FrameInfo *find(Buffer *buffer);\n> > +\tRkISP1FrameInfo *find(FrameBuffer *buffer);\n> > \tRkISP1FrameInfo *find(Request *request);\n> > \n> > private:\n> > @@ -203,8 +201,8 @@ private:\n> > \tint createCamera(MediaEntity *sensor);\n> > \tvoid tryCompleteRequest(Request *request);\n> > \tvoid bufferReady(Buffer *buffer);\n> > -\tvoid paramReady(Buffer *buffer);\n> > -\tvoid statReady(Buffer *buffer);\n> > +\tvoid paramReady(FrameBuffer *buffer);\n> > +\tvoid statReady(FrameBuffer *buffer);\n> > \n> > \tMediaDevice *media_;\n> > \tV4L2Subdevice *dphy_;\n> > @@ -213,11 +211,10 @@ private:\n> > \tV4L2VideoDevice *param_;\n> > \tV4L2VideoDevice *stat_;\n> > \n> > -\tBufferPool paramPool_;\n> > -\tBufferPool statPool_;\n> > -\n> > -\tstd::queue paramBuffers_;\n> > -\tstd::queue statBuffers_;\n> > +\tstd::vector> paramBuffers_;\n> > +\tstd::vector> statBuffers_;\n> > +\tstd::queue availableParamBuffers_;\n> > +\tstd::queue availableStatBuffers_;\n> > \n> > \tCamera *activeCamera_;\n> > };\n> > @@ -229,17 +226,17 @@ RkISP1Frames::RkISP1Frames(PipelineHandler *pipe)\n> > \n> > RkISP1FrameInfo *RkISP1Frames::create(unsigned int frame, Request *request, Stream *stream)\n> > {\n> > -\tif (pipe_->paramBuffers_.empty()) {\n> > +\tif (pipe_->availableParamBuffers_.empty()) {\n> > \t\tLOG(RkISP1, Error) << \"Parameters buffer underrun\";\n> > \t\treturn nullptr;\n> > \t}\n> > -\tBuffer *paramBuffer = pipe_->paramBuffers_.front();\n> > +\tFrameBuffer *paramBuffer = pipe_->availableParamBuffers_.front();\n> > \n> > -\tif (pipe_->statBuffers_.empty()) {\n> > +\tif (pipe_->availableStatBuffers_.empty()) {\n> > \t\tLOG(RkISP1, Error) << \"Statisitc buffer underrun\";\n> > \t\treturn nullptr;\n> > \t}\n> > -\tBuffer *statBuffer = pipe_->statBuffers_.front();\n> > +\tFrameBuffer *statBuffer = pipe_->availableStatBuffers_.front();\n> > \n> > \tBuffer *videoBuffer = request->findBuffer(stream);\n> > \tif (!videoBuffer) {\n> > @@ -248,8 +245,8 @@ RkISP1FrameInfo *RkISP1Frames::create(unsigned int frame, Request *request, Stre\n> > \t\treturn nullptr;\n> > \t}\n> > \n> > -\tpipe_->paramBuffers_.pop();\n> > -\tpipe_->statBuffers_.pop();\n> > +\tpipe_->availableParamBuffers_.pop();\n> > +\tpipe_->availableStatBuffers_.pop();\n> > \n> > \tRkISP1FrameInfo *info = new RkISP1FrameInfo;\n> > \n> > @@ -273,8 +270,8 @@ int RkISP1Frames::destroy(unsigned int frame)\n> > \tif (!info)\n> > \t\treturn -ENOENT;\n> > \n> > -\tpipe_->paramBuffers_.push(info->paramBuffer);\n> > -\tpipe_->statBuffers_.push(info->statBuffer);\n> > +\tpipe_->availableParamBuffers_.push(info->paramBuffer);\n> > +\tpipe_->availableStatBuffers_.push(info->statBuffer);\n> > \n> > \tframeInfo_.erase(info->frame);\n> > \n> > @@ -299,9 +296,21 @@ RkISP1FrameInfo *RkISP1Frames::find(Buffer *buffer)\n> > \tfor (auto &itInfo : frameInfo_) {\n> > \t\tRkISP1FrameInfo *info = itInfo.second;\n> > \n> > +\t\tif (info->videoBuffer == buffer)\n> > +\t\t\treturn info;\n> > +\t}\n> > +\n> > +\tLOG(RkISP1, Error) << \"Can't locate info from buffer\";\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->videoBuffer == buffer)\n> > +\t\t info->statBuffer == buffer)\n> > \t\t\treturn info;\n> > \t}\n> > \n> > @@ -661,6 +670,7 @@ int PipelineHandlerRkISP1::allocateBuffers(Camera *camera,\n> > {\n> > \tRkISP1CameraData *data = cameraData(camera);\n> > \tStream *stream = *streams.begin();\n> > +\tunsigned int count = 1;\n> > \tint ret;\n> > \n> > \tif (stream->memoryType() == InternalMemory)\n> > @@ -671,35 +681,41 @@ int PipelineHandlerRkISP1::allocateBuffers(Camera *camera,\n> > \tif (ret)\n> > \t\treturn ret;\n> > \n> > -\tparamPool_.createBuffers(stream->configuration().bufferCount);\n> > -\tret = param_->exportBuffers(¶mPool_);\n> > -\tif (ret) {\n> > -\t\tvideo_->releaseBuffers();\n> > -\t\treturn ret;\n> > -\t}\n> > +\tunsigned int maxBuffers = 0;\n> > +\tfor (const Stream *s : camera->streams())\n> > +\t\tmaxBuffers = std::max(maxBuffers, s->configuration().bufferCount);\n> > \n> > -\tstatPool_.createBuffers(stream->configuration().bufferCount);\n> > -\tret = stat_->exportBuffers(&statPool_);\n> > -\tif (ret) {\n> > -\t\tparam_->releaseBuffers();\n> > -\t\tvideo_->releaseBuffers();\n> > -\t\treturn ret;\n> > -\t}\n> > +\tret = param_->exportBuffers(maxBuffers, ¶mBuffers_);\n> > +\tif (ret < 0)\n> > +\t\tgoto error;\n> > +\n> > +\tret = stat_->exportBuffers(maxBuffers, &statBuffers_);\n> > +\tif (ret < 0)\n> > +\t\tgoto error;\n> > \n> > -\tfor (unsigned int i = 0; i < stream->configuration().bufferCount; i++) {\n> > -\t\tdata->ipaBuffers_.push_back({ .id = RKISP1_PARAM_BASE | i,\n> > -\t\t\t\t\t .planes = paramPool_.buffers()[i].planes() });\n> > -\t\tparamBuffers_.push(new Buffer(i));\n> > +\tfor (std::unique_ptr &buffer : paramBuffers_) {\n> > +\t\tbuffer->setCookie(count++);\n> > +\t\tdata->ipaBuffers_.push_back({ .id = buffer->cookie(),\n> > +\t\t\t\t\t .planes = buffer->planes() });\n> > +\t\tavailableParamBuffers_.push(buffer.get());\n> > \t}\n> > \n> > -\tfor (unsigned int i = 0; i < stream->configuration().bufferCount; i++) {\n> > -\t\tdata->ipaBuffers_.push_back({ .id = RKISP1_STAT_BASE | i,\n> > -\t\t\t\t\t .planes = statPool_.buffers()[i].planes() });\n> > -\t\tstatBuffers_.push(new Buffer(i));\n> > +\tfor (std::unique_ptr &buffer : statBuffers_) {\n> > +\t\tbuffer->setCookie(count++);\n> > +\t\tdata->ipaBuffers_.push_back({ .id = buffer->cookie(),\n> > +\t\t\t\t\t .planes = buffer->planes() });\n> > +\t\tavailableStatBuffers_.push(buffer.get());\n> > \t}\n> > \n> > \tdata->ipa_->mapBuffers(data->ipaBuffers_);\n> > \n> > +\treturn 0;\n> > +\n> > +error:\n> > +\tparamBuffers_.clear();\n> > +\tstatBuffers_.clear();\n> > +\tvideo_->releaseBuffers();\n> > +\n> > \treturn ret;\n> > }\n> > \n> > @@ -708,15 +724,14 @@ int PipelineHandlerRkISP1::freeBuffers(Camera *camera,\n> > {\n> > \tRkISP1CameraData *data = cameraData(camera);\n> > \n> > -\twhile (!statBuffers_.empty()) {\n> > -\t\tdelete statBuffers_.front();\n> > -\t\tstatBuffers_.pop();\n> > -\t}\n> > +\twhile (!availableStatBuffers_.empty())\n> > +\t\tavailableStatBuffers_.pop();\n> > \n> > -\twhile (!paramBuffers_.empty()) {\n> > -\t\tdelete paramBuffers_.front();\n> > -\t\tparamBuffers_.pop();\n> > -\t}\n> > +\twhile (!availableParamBuffers_.empty())\n> > +\t\tavailableParamBuffers_.pop();\n> \n> Can't you replace this with\n> \n> \tavailableStatBuffers_.clear();\n> \tavailableParamBuffers_.clear();\n> \n> ?\n\nWish I could, but std::queue does not implement clear() :-(\n\n> \n> Please keep my Reviewed-by.\n> \n> > +\n> > +\tparamBuffers_.clear();\n> > +\tstatBuffers_.clear();\n> > \n> > \tstd::vector ids;\n> > \tfor (IPABuffer &ipabuf : data->ipaBuffers_)\n> > @@ -823,7 +838,7 @@ int PipelineHandlerRkISP1::queueRequestDevice(Camera *camera,\n> > \n> > \tIPAOperationData op;\n> > \top.operation = RKISP1_IPA_EVENT_QUEUE_REQUEST;\n> > -\top.data = { data->frame_, RKISP1_PARAM_BASE | info->paramBuffer->index() };\n> > +\top.data = { data->frame_, info->paramBuffer->cookie() };\n> > \top.controls = { request->controls() };\n> > \tdata->ipa_->processEvent(op);\n> > \n> > @@ -938,8 +953,8 @@ bool PipelineHandlerRkISP1::match(DeviceEnumerator *enumerator)\n> > \t\treturn false;\n> > \n> > \tvideo_->bufferReady.connect(this, &PipelineHandlerRkISP1::bufferReady);\n> > -\tstat_->bufferReady.connect(this, &PipelineHandlerRkISP1::statReady);\n> > -\tparam_->bufferReady.connect(this, &PipelineHandlerRkISP1::paramReady);\n> > +\tstat_->frameBufferReady.connect(this, &PipelineHandlerRkISP1::statReady);\n> > +\tparam_->frameBufferReady.connect(this, &PipelineHandlerRkISP1::paramReady);\n> > \n> > \t/* Configure default links. */\n> > \tif (initLinks() < 0) {\n> > @@ -1001,7 +1016,7 @@ void PipelineHandlerRkISP1::bufferReady(Buffer *buffer)\n> > \ttryCompleteRequest(request);\n> > }\n> > \n> > -void PipelineHandlerRkISP1::paramReady(Buffer *buffer)\n> > +void PipelineHandlerRkISP1::paramReady(FrameBuffer *buffer)\n> > {\n> > \tASSERT(activeCamera_);\n> > \tRkISP1CameraData *data = cameraData(activeCamera_);\n> > @@ -1012,7 +1027,7 @@ void PipelineHandlerRkISP1::paramReady(Buffer *buffer)\n> > \ttryCompleteRequest(info->request);\n> > }\n> > \n> > -void PipelineHandlerRkISP1::statReady(Buffer *buffer)\n> > +void PipelineHandlerRkISP1::statReady(FrameBuffer *buffer)\n> > {\n> > \tASSERT(activeCamera_);\n> > \tRkISP1CameraData *data = cameraData(activeCamera_);\n> > @@ -1021,12 +1036,9 @@ void PipelineHandlerRkISP1::statReady(Buffer *buffer)\n> > \tif (!info)\n> > \t\treturn;\n> > \n> > -\tunsigned int frame = info->frame;\n> > -\tunsigned int statid = RKISP1_STAT_BASE | info->statBuffer->index();\n> > -\n> > \tIPAOperationData op;\n> > \top.operation = RKISP1_IPA_EVENT_SIGNAL_STAT_BUFFER;\n> > -\top.data = { frame, statid };\n> > +\top.data = { info->frame, info->statBuffer->cookie() };\n> > \tdata->ipa_->processEvent(op);\n> > }\n> > \n> \n> -- \n> Regards,\n> \n> Laurent Pinchart","headers":{"Return-Path":"","Received":["from mail-lj1-x242.google.com (mail-lj1-x242.google.com\n\t[IPv6:2a00:1450:4864:20::242])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTPS id 173E96045B\n\tfor ;\n\tSun, 12 Jan 2020 00:31:37 +0100 (CET)","by mail-lj1-x242.google.com with SMTP id y6so6062599lji.0\n\tfor ;\n\tSat, 11 Jan 2020 15:31:37 -0800 (PST)","from localhost (h-93-159.A463.priv.bahnhof.se. [46.59.93.159])\n\tby smtp.gmail.com with ESMTPSA id\n\ti5sm3280282ljj.29.2020.01.11.15.31.35\n\t(version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256);\n\tSat, 11 Jan 2020 15:31:35 -0800 (PST)"],"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=73c6XMty4SC177OsPHZUZVeIr98y7aNDFH34SjMMil4=;\n\tb=mE2HGA/V/ihncmKvGHlDwNZFdQ1RT1TPjVHqNZf59Eu3biblh3jMm7iCemG4TK/k2C\n\tAz6xKtOPPTKo4sxmLhs5gWByiVbIuuee2upfKAi7K/p6npGY4rYYfyKBleeD9Ase71mx\n\tTuWqN+l5Oc+XONAwFNcQCAJ7+DIuLv9U/KT1yCZCVosRAnJ67qqMGRDjnEuRSccF92nC\n\tVIU0aLz99L7SMjP6qDPz/j5UjIE6T+M3i0KxChQolIITgZLPfp9HrNpASfcY/wu+9kKo\n\tdIbT79E1nbjIH4wJN5UtKJXIlM2JwWGbpR1C9g3wusr/SJ3P/pB/Bg0zpqS1vjk2xEea\n\tSfeA==","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=73c6XMty4SC177OsPHZUZVeIr98y7aNDFH34SjMMil4=;\n\tb=iZQHftcDwF81z8fIqAkic597tMa342eM23VzpWMkpmhwZpgvQj20cB90sHF63Y+fGT\n\trOsHJeKJKiwJVXO9iIoWL1zfBDE9F9yymE80t62pyn7H47BzBhAwtpkyXJ/7F/cknpdr\n\tHEZAU2uJ0Ma4EctVfxklga5i8S6KEQi21gGeiNsorxjZBXhEuIpijw17DhinTcV2WyHQ\n\tQNYyb4cOoJSzmsOSClh3P1QtStjVF2UCCwNoxceTmpfO+CAqjRbcz5Qiz8zYAMjLhu0H\n\t8Sz39d1bwfTNox3DEdRWGTJJhLp7WEiEiK1NHBb12vU5DmohMh8YTxoEZLB+o0rF4QsK\n\tng5A==","X-Gm-Message-State":"APjAAAW1TWmMRzZzwzx3mLd6ppmh34L//X+vRa/1FU7Dy9rvYYRYeYb6\n\toDe7RWTCG0Wlj1XfBIuLSrU9TWjyXmA=","X-Google-Smtp-Source":"APXvYqwFAn4nG3meNRKcq4RcGaWIG+Iy5z6GE0WEDIlvACor1ITc88eb7ZcL+Or5MLMp1C/4fxcv+A==","X-Received":"by 2002:a2e:9a51:: with SMTP id\n\tk17mr6628560ljj.206.1578785496173; \n\tSat, 11 Jan 2020 15:31:36 -0800 (PST)","Date":"Sun, 12 Jan 2020 00:31:34 +0100","From":"Niklas =?iso-8859-1?q?S=F6derlund?= ","To":"Laurent Pinchart ","Cc":"libcamera-devel@lists.libcamera.org","Message-ID":"<20200111233134.GB697401@oden.dyn.berto.se>","References":"<20200110193808.2266294-1-niklas.soderlund@ragnatech.se>\n\t<20200110193808.2266294-23-niklas.soderlund@ragnatech.se>\n\t<20200111005524.GL4859@pendragon.ideasonboard.com>","MIME-Version":"1.0","Content-Type":"text/plain; charset=iso-8859-1","Content-Disposition":"inline","Content-Transfer-Encoding":"8bit","In-Reply-To":"<20200111005524.GL4859@pendragon.ideasonboard.com>","Subject":"Re: [libcamera-devel] [PATCH v3 22/33] libcamera: pipeline: rkisp1:\n\tSwitch to FrameBuffer interface for stat and param","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","X-List-Received-Date":"Sat, 11 Jan 2020 23:31:37 -0000"}}]