[{"id":24152,"web_url":"https://patchwork.libcamera.org/comment/24152/","msgid":"<9f328363-ce48-21e0-57b3-ef71046fa592@ideasonboard.com>","date":"2022-07-26T17:36:18","subject":"Re: [libcamera-devel] [PATCH v3 3/9] ipu3: Use imgu0 as default","submitter":{"id":86,"url":"https://patchwork.libcamera.org/api/people/86/","name":"Umang Jain","email":"umang.jain@ideasonboard.com"},"content":"Hi,\n\nOn 6/29/22 16:00, Harvey Yang via libcamera-devel wrote:\n> From: Harvey Yang\n>\n> With only one camera being started, we can always use imgu0 to process\n> frames (for video/preview). In the following patches, we'll use imgu1\n> for still capture if needed.\n>\n> Signed-off-by: Harvey Yang\n> ---\n> src/libcamera/pipeline/ipu3/ipu3.cpp | 86 ++++++++++++++++------------\n> 1 file changed, 48 insertions(+), 38 deletions(-)\n>\n> diff --git a/src/libcamera/pipeline/ipu3/ipu3.cpp b/src/libcamera/pipeline/ipu3/ipu3.cpp\n> index c943ee6a..e219f704 100644\n> --- a/src/libcamera/pipeline/ipu3/ipu3.cpp\n> +++ b/src/libcamera/pipeline/ipu3/ipu3.cpp\n> @@ -64,7 +64,8 @@ public:\n> \tvoid frameStart(uint32_t sequence);\n> \n> \tCIO2Device cio2_;\n> -\tImgUDevice *imgu_;\n> +\tImgUDevice *imgu0_;\n> +\tImgUDevice *imgu1_;\n\nYou might also be interested to at \nPipelineHandlerIPU3::registerCameras() which allows registering two \ncameras for IPU3, assigning the 2 exposed IMGUs to each camera.\n\n|/** * \\todo Dynamically assign ImgU and output devices to each * stream \nand camera; as of now, limit support to two cameras * only, and assign \nimgu0 to the first one and imgu1 to the * second. */ data->imgu_ = \nnumCameras ? &imgu1_ : &imgu0_; |||\n\nThis should be addressed as well, I think.\n\nRest bits of the patch, looks on the right track to me.\n\n> \n> \tStream outStream_;\n> \tStream vfStream_;\n> @@ -406,7 +407,7 @@CameraConfiguration::Status IPU3CameraConfiguration::validate()\n> \n> \t/* Only compute the ImgU configuration if a YUV stream has been requested. */\n> \tif (yuvCount) {\n> -\t\tpipeConfig_ = data_->imgu_->calculatePipeConfig(&pipe);\n> +\t\tpipeConfig_ = data_->imgu0_->calculatePipeConfig(&pipe);\n> \t\tif (pipeConfig_.isNull()) {\n> \t\t\tLOG(IPU3, Error) << \"Failed to calculate pipe configuration: \"\n> \t\t\t\t\t << \"unsupported resolutions.\";\n> @@ -518,7 +519,6 @@ intPipelineHandlerIPU3::configure(Camera *camera, CameraConfiguration *c)\n> \tStream *outStream = &data->outStream_;\n> \tStream *vfStream = &data->vfStream_;\n> \tCIO2Device *cio2 = &data->cio2_;\n> -\tImgUDevice *imgu = data->imgu_;\n> \tV4L2DeviceFormat outputFormat;\n> \tint ret;\n> \n> @@ -560,7 +560,7 @@ intPipelineHandlerIPU3::configure(Camera *camera, CameraConfiguration *c)\n> \t * stream which is for raw capture, in which case no buffers will\n> \t * ever be queued to the ImgU.\n> \t */\n> -\tret = data->imgu_->enableLinks(true);\n> +\tret = imgu0_.enableLinks(true);\n> \tif (ret)\n> \t\treturn ret;\n> \n> @@ -610,7 +610,7 @@ intPipelineHandlerIPU3::configure(Camera *camera, CameraConfiguration *c)\n> \tif (imguConfig.isNull())\n> \t\treturn 0;\n> \n> -\tret = imgu->configure(imguConfig, &cio2Format);\n> +\tret = imgu0_.configure(imguConfig, &cio2Format);\n> \tif (ret)\n> \t\treturn ret;\n> \n> @@ -624,12 +624,12 @@ intPipelineHandlerIPU3::configure(Camera *camera, CameraConfiguration *c)\n> \n> \t\tif (stream == outStream) {\n> \t\t\tmainCfg = &cfg;\n> -\t\t\tret = imgu->configureOutput(cfg, &outputFormat);\n> +\t\t\tret = imgu0_.configureOutput(cfg, &outputFormat);\n> \t\t\tif (ret)\n> \t\t\t\treturn ret;\n> \t\t} else if (stream == vfStream) {\n> \t\t\tvfCfg = &cfg;\n> -\t\t\tret = imgu->configureViewfinder(cfg, &outputFormat);\n> +\t\t\tret = imgu0_.configureViewfinder(cfg, &outputFormat);\n> \t\t\tif (ret)\n> \t\t\t\treturn ret;\n> \t\t}\n> @@ -641,13 +641,13 @@ intPipelineHandlerIPU3::configure(Camera *camera, CameraConfiguration *c)\n> \t * be at least one active stream in the configuration request).\n> \t */\n> \tif (!vfCfg) {\n> -\t\tret = imgu->configureViewfinder(*mainCfg, &outputFormat);\n> +\t\tret = imgu0_.configureViewfinder(*mainCfg, &outputFormat);\n> \t\tif (ret)\n> \t\t\treturn ret;\n> \t}\n> \n> \t/* Apply the \"pipe_mode\" control to the ImgU subdevice. */\n> -\tControlList ctrls(imgu->imgu_->controls());\n> +\tControlList ctrls(imgu0_.imgu_->controls());\n> \t/*\n> \t * Set the ImgU pipe mode to 'Video' unconditionally to have statistics\n> \t * generated.\n> @@ -657,7 +657,7 @@ intPipelineHandlerIPU3::configure(Camera *camera, CameraConfiguration *c)\n> \t */\n> \tctrls.set(V4L2_CID_IPU3_PIPE_MODE,\n> \t\t static_cast(IPU3PipeModeVideo));\n> -\tret = imgu->imgu_->setControls(&ctrls);\n> +\tret = imgu0_.imgu_->setControls(&ctrls);\n> \tif (ret) {\n> \t\tLOG(IPU3, Error) << \"Unable to set pipe_mode control\";\n> \t\treturn ret;\n> @@ -691,9 +691,9 @@ intPipelineHandlerIPU3::exportFrameBuffers(Camera *camera, Stream *stream,\n> \tunsigned int count = stream->configuration().bufferCount;\n> \n> \tif (stream == &data->outStream_)\n> -\t\treturn data->imgu_->output_->exportBuffers(count, buffers);\n> +\t\treturn imgu0_.output_->exportBuffers(count, buffers);\n> \telse if (stream == &data->vfStream_)\n> -\t\treturn data->imgu_->viewfinder_->exportBuffers(count, buffers);\n> +\t\treturn imgu0_.viewfinder_->exportBuffers(count, buffers);\n> \telse if (stream == &data->rawStream_)\n> \t\treturn data->cio2_.exportBuffers(count, buffers);\n> \n> @@ -711,7 +711,6 @@ intPipelineHandlerIPU3::exportFrameBuffers(Camera *camera, Stream *stream,\n> intPipelineHandlerIPU3::allocateBuffers(Camera *camera)\n> {\n> \tIPU3CameraData *data = cameraData(camera);\n> -\tImgUDevice *imgu = data->imgu_;\n> \tunsigned int bufferCount;\n> \tint ret;\n> \n> @@ -721,26 +720,26 @@ intPipelineHandlerIPU3::allocateBuffers(Camera *camera)\n> \t\tdata->rawStream_.configuration().bufferCount,\n> \t});\n> \n> -\tret = imgu->allocateBuffers(bufferCount);\n> +\tret = imgu0_.allocateBuffers(bufferCount);\n> \tif (ret < 0)\n> \t\treturn ret;\n> \n> \t/* Map buffers to the IPA. */\n> \tunsigned int ipaBufferId = 1;\n> \n> -\tfor (conststd::unique_ptr &buffer : imgu->paramBuffers_) {\n> +\tfor (conststd::unique_ptr &buffer : imgu0_.paramBuffers_) {\n> \t\tbuffer->setCookie(ipaBufferId++);\n> \t\tipaBuffers_.emplace_back(buffer->cookie(), buffer->planes());\n> \t}\n> \n> -\tfor (conststd::unique_ptr &buffer : imgu->statBuffers_) {\n> +\tfor (conststd::unique_ptr &buffer : imgu0_.statBuffers_) {\n> \t\tbuffer->setCookie(ipaBufferId++);\n> \t\tipaBuffers_.emplace_back(buffer->cookie(), buffer->planes());\n> \t}\n> \n> \tdata->ipa_->mapBuffers(ipaBuffers_);\n> \n> -\tdata->frameInfos_.init(imgu->paramBuffers_, imgu->statBuffers_);\n> +\tdata->frameInfos_.init(imgu0_.paramBuffers_, imgu0_.statBuffers_);\n> \tdata->frameInfos_.bufferAvailable.connect(\n> \t\tdata, &IPU3CameraData::queuePendingRequests);\n> \n> @@ -760,7 +759,7 @@ intPipelineHandlerIPU3::freeBuffers(Camera *camera)\n> \tdata->ipa_->unmapBuffers(ids);\n> \tipaBuffers_.clear();\n> \n> -\tdata->imgu_->freeBuffers();\n> +\timgu0_.freeBuffers();\n> \n> \treturn 0;\n> }\n> @@ -777,9 +776,18 @@ intPipelineHandlerIPU3::start(Camera *camera, [[maybe_unused]] const ControlLis\n> \n> \tIPU3CameraData *data = cameraData(camera);\n> \tCIO2Device *cio2 = &data->cio2_;\n> -\tImgUDevice *imgu = data->imgu_;\n> \tint ret;\n> \n> +\timgu0_.input_->bufferReady.connect(&data->cio2_,\n> +\t\t\t\t\t &CIO2Device::tryReturnBuffer);\n> +\timgu0_.output_->bufferReady.connect(data,\n> +\t\t\t\t\t &IPU3CameraData::imguOutputBufferReady);\n> +\timgu0_.viewfinder_->bufferReady.connect(data,\n> +\t\t\t\t\t\t&IPU3CameraData::imguOutputBufferReady);\n> +\timgu0_.param_->bufferReady.connect(data,\n> +\t\t\t\t\t &IPU3CameraData::paramBufferReady);\n> +\timgu0_.stat_->bufferReady.connect(data,\n> +\t\t\t\t\t &IPU3CameraData::statBufferReady);\n> \t/* Disable test pattern mode on the sensor, if any. */\n> \tret = cio2->sensor()->setTestPatternMode(\n> \t\tcontrols::draft::TestPatternModeEnum::TestPatternModeOff);\n> @@ -807,19 +815,24 @@ intPipelineHandlerIPU3::start(Camera *camera, [[maybe_unused]] const ControlLis\n> \tif (ret)\n> \t\tgoto error;\n> \n> -\tret = imgu->start();\n> +\tret = imgu0_.start();\n> \tif (ret)\n> \t\tgoto error;\n> \n> \treturn 0;\n> \n> error:\n> -\timgu->stop();\n> +\timgu0_.stop();\n> \tcio2->stop();\n> \tdata->ipa_->stop();\n> \tfreeBuffers(camera);\n> \tLOG(IPU3, Error) << \"Failed to start camera \" << camera->id();\n> \n> +\timgu0_.input_->bufferReady.disconnect();\n> +\timgu0_.output_->bufferReady.disconnect();\n> +\timgu0_.viewfinder_->bufferReady.disconnect();\n> +\timgu0_.param_->bufferReady.disconnect();\n> +\timgu0_.stat_->bufferReady.disconnect();\n> \tinUseCamera_ = nullptr;\n> \n> \treturn ret;\n> @@ -834,13 +847,19 @@ voidPipelineHandlerIPU3::stopDevice(Camera *camera)\n> \n> \tdata->ipa_->stop();\n> \n> -\tret |= data->imgu_->stop();\n> +\tret |= imgu0_.stop();\n> \tret |= data->cio2_.stop();\n> \tif (ret)\n> \t\tLOG(IPU3, Warning) << \"Failed to stop camera \" << camera->id();\n> \n> \tfreeBuffers(camera);\n> \n> +\tdata->imgu0_->input_->bufferReady.disconnect();\n> +\tdata->imgu0_->output_->bufferReady.disconnect();\n> +\tdata->imgu0_->viewfinder_->bufferReady.disconnect();\n> +\tdata->imgu0_->param_->bufferReady.disconnect();\n> +\tdata->imgu0_->stat_->bufferReady.disconnect();\n> +\n> \tinUseCamera_ = nullptr;\n> }\n> \n> @@ -1184,7 +1203,8 @@ intPipelineHandlerIPU3::registerCameras()\n> \t\t * only, and assign imgu0 to the first one and imgu1 to the\n> \t\t * second.\n> \t\t */\n> -\t\tdata->imgu_ = numCameras ? &imgu1_ : &imgu0_;\n> +\t\tdata->imgu0_ = &imgu0_;\n> +\t\tdata->imgu1_ = &imgu1_;\n> \n> \t\t/*\n> \t\t * Connect video devices' 'bufferReady' signals to their\n> @@ -1198,16 +1218,6 @@ intPipelineHandlerIPU3::registerCameras()\n> \t\t\t\t\t&IPU3CameraData::cio2BufferReady);\n> \t\tdata->cio2_.bufferAvailable.connect(\n> \t\t\tdata.get(), &IPU3CameraData::queuePendingRequests);\n> -\t\tdata->imgu_->input_->bufferReady.connect(&data->cio2_,\n> -\t\t\t\t\t&CIO2Device::tryReturnBuffer);\n> -\t\tdata->imgu_->output_->bufferReady.connect(data.get(),\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\tconststd::string &cameraId = cio2->sensor()->id();\n> @@ -1300,14 +1310,14 @@ voidIPU3CameraData::paramsBufferReady(unsigned int id)\n> \t\tFrameBuffer *outbuffer = it.second;\n> \n> \t\tif (stream == &outStream_)\n> -\t\t\timgu_->output_->queueBuffer(outbuffer);\n> +\t\t\timgu0_->output_->queueBuffer(outbuffer);\n> \t\telse if (stream == &vfStream_)\n> -\t\t\timgu_->viewfinder_->queueBuffer(outbuffer);\n> +\t\t\timgu0_->viewfinder_->queueBuffer(outbuffer);\n> \t}\n> \n> -\timgu_->param_->queueBuffer(info->paramBuffer);\n> -\timgu_->stat_->queueBuffer(info->statBuffer);\n> -\timgu_->input_->queueBuffer(info->rawBuffer);\n> +\timgu0_->param_->queueBuffer(info->paramBuffer);\n> +\timgu0_->stat_->queueBuffer(info->statBuffer);\n> +\timgu0_->input_->queueBuffer(info->rawBuffer);\n> }\n> \n> voidIPU3CameraData::metadataReady(unsigned int id, const ControlList &metadata)","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 0AB83C3275\n\tfor ;\n\tTue, 26 Jul 2022 17:36:28 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id 54EB16330E;\n\tTue, 26 Jul 2022 19:36:27 +0200 (CEST)","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 6EBA060487\n\tfor ;\n\tTue, 26 Jul 2022 19:36:25 +0200 (CEST)","from [IPV6:2401:4900:1f3e:f7a:bc8f:12ed:b45f:c35d] (unknown\n\t[IPv6:2401:4900:1f3e:f7a:bc8f:12ed:b45f:c35d])\n\tby perceval.ideasonboard.com (Postfix) with ESMTPSA id C9092735;\n\tTue, 26 Jul 2022 19:36:23 +0200 (CEST)"],"DKIM-Signature":["v=1; a=rsa-sha256; c=relaxed/simple; d=libcamera.org;\n\ts=mail; t=1658856987;\n\tbh=M2nr2qyl1CZAdU7iAZJILErnlOq18OXLOtcU4XsxCRo=;\n\th=Date:To:References:In-Reply-To:Subject:List-Id:List-Unsubscribe:\n\tList-Archive:List-Post:List-Help:List-Subscribe:From:Reply-To:\n\tFrom;\n\tb=b1KPjQzS0twCPc6LG2wDZ7NimKCZmUfEwBf2XuO9ypr6I2skBGEzlDZ/h8mCz2Ov0\n\tk5YNqtj1G/1823uvjqd9ZgiF8JnIRpgPBXWGX4h9UQY5m2lmHHWqH3p89q8n2mCdkL\n\t/VAYS5/+Dg3UCK8AK4mVtTVN2uStYfwkAZE6qMRX5zn6xBTrzb02LsYH2a0haXmzjE\n\t4NiLfeSplMIVVwtzNxSYHj4OVWvrrr2RSYR46Ajs9pxPlMp1/rjVrLHZALVMcAzh4b\n\tfOXg+9lI5tt/Wn3gh+5hujV5NxLnfDhAa0FvY2/2SvGrBj0vBSWCF9T2pNP7y6Vscc\n\tgIM2JXOJptxYw==","v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com;\n\ts=mail; t=1658856984;\n\tbh=M2nr2qyl1CZAdU7iAZJILErnlOq18OXLOtcU4XsxCRo=;\n\th=Date:Subject:To:References:From:In-Reply-To:From;\n\tb=uPPVjby+U6zx9HSLvab/T9Y4CIyAYDH6Lkh/O3JdgFbcWH/nfV3+NFEVwgBm5hltC\n\tDuBvKuCIJ+uryG8QOXyM3hvBS81sIs09Ltydf19YWOX2rK6v71PLQ1DTDuQsrLjTQz\n\t7YMMAC1sVeAgkhPDld+ogx5N43VIfkhqfCJoU0KI="],"Authentication-Results":"lancelot.ideasonboard.com; dkim=pass (1024-bit key; \n\tunprotected) header.d=ideasonboard.com\n\theader.i=@ideasonboard.com\n\theader.b=\"uPPVjby+\"; dkim-atps=neutral","Content-Type":"multipart/alternative;\n\tboundary=\"------------WUpZCxNQahAgfOOEjnvMFzBM\"","Message-ID":"<9f328363-ce48-21e0-57b3-ef71046fa592@ideasonboard.com>","Date":"Tue, 26 Jul 2022 23:06:18 +0530","MIME-Version":"1.0","User-Agent":"Mozilla/5.0 (X11; Linux x86_64; rv:91.0) Gecko/20100101\n\tThunderbird/91.4.1","Content-Language":"en-US","To":"Harvey Yang ,\n\tlibcamera-devel@lists.libcamera.org","References":"<20220629103018.4025635-1-chenghaoyang@google.com>\n\t<20220629103018.4025635-4-chenghaoyang@google.com>","In-Reply-To":"<20220629103018.4025635-4-chenghaoyang@google.com>","Subject":"Re: [libcamera-devel] [PATCH v3 3/9] ipu3: Use imgu0 as default","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","From":"Umang Jain via libcamera-devel ","Reply-To":"Umang Jain ","Errors-To":"libcamera-devel-bounces@lists.libcamera.org","Sender":"\"libcamera-devel\" "}},{"id":24154,"web_url":"https://patchwork.libcamera.org/comment/24154/","msgid":"<43720763-0e04-e7cb-e48f-1e6f28f01271@ideasonboard.com>","date":"2022-07-26T17:40:04","subject":"Re: [libcamera-devel] [PATCH v3 3/9] ipu3: Use imgu0 as default","submitter":{"id":86,"url":"https://patchwork.libcamera.org/api/people/86/","name":"Umang Jain","email":"umang.jain@ideasonboard.com"},"content":"Hi,\n\nOn 7/26/22 23:06, Umang Jain via libcamera-devel wrote:\n> Hi,\n>\n> On 6/29/22 16:00, Harvey Yang via libcamera-devel wrote:\n>> From: Harvey Yang\n>>\n>> With only one camera being started, we can always use imgu0 to process\n>> frames (for video/preview). In the following patches, we'll use imgu1\n>> for still capture if needed.\n>>\n>> Signed-off-by: Harvey Yang\n>> ---\n>>   src/libcamera/pipeline/ipu3/ipu3.cpp | 86 ++++++++++++++++------------\n>>   1 file changed, 48 insertions(+), 38 deletions(-)\n>>\n>> diff --git a/src/libcamera/pipeline/ipu3/ipu3.cpp \n>> b/src/libcamera/pipeline/ipu3/ipu3.cpp\n>> index c943ee6a..e219f704 100644\n>> --- a/src/libcamera/pipeline/ipu3/ipu3.cpp\n>> +++ b/src/libcamera/pipeline/ipu3/ipu3.cpp\n>> @@ -64,7 +64,8 @@ public:\n>>       void frameStart(uint32_t sequence);\n>>         CIO2Device cio2_;\n>> -    ImgUDevice *imgu_;\n>> +    ImgUDevice *imgu0_;\n>> +    ImgUDevice *imgu1_;\n>\n> You might also be interested to at \n> PipelineHandlerIPU3::registerCameras() which allows registering two \n> cameras for IPU3, assigning the 2 exposed IMGUs to each camera.\n>\n> |/** * \\todo Dynamically assign ImgU and output devices to each * \n> stream and camera; as of now, limit support to two cameras * only, and \n> assign imgu0 to the first one and imgu1 to the * second. */ \n> data->imgu_ = numCameras ? &imgu1_ : &imgu0_; |||\n>\n> This should be addressed as well, I think.\n\n\nAh, this is already addressed. Too bad I was looking at master branch \nthinking this series as been applied on top locally :S\n\nSorry for the noise!\n\n>\n> Rest bits of the patch, looks on the right track to me.\n>\n>>         Stream outStream_;\n>>       Stream vfStream_;\n>> @@ -406,7 +407,7 @@CameraConfiguration::Status \n>> IPU3CameraConfiguration::validate()\n>>         /* Only compute the ImgU configuration if a YUV stream has \n>> been requested. */\n>>       if (yuvCount) {\n>> -        pipeConfig_ = data_->imgu_->calculatePipeConfig(&pipe);\n>> +        pipeConfig_ = data_->imgu0_->calculatePipeConfig(&pipe);\n>>           if (pipeConfig_.isNull()) {\n>>               LOG(IPU3, Error) << \"Failed to calculate pipe \n>> configuration: \"\n>>                        << \"unsupported resolutions.\";\n>> @@ -518,7 +519,6 @@ intPipelineHandlerIPU3::configure(Camera *camera, \n>> CameraConfiguration *c)\n>>       Stream *outStream = &data->outStream_;\n>>       Stream *vfStream = &data->vfStream_;\n>>       CIO2Device *cio2 = &data->cio2_;\n>> -    ImgUDevice *imgu = data->imgu_;\n>>       V4L2DeviceFormat outputFormat;\n>>       int ret;\n>>   @@ -560,7 +560,7 @@ intPipelineHandlerIPU3::configure(Camera \n>> *camera, CameraConfiguration *c)\n>>        * stream which is for raw capture, in which case no buffers will\n>>        * ever be queued to the ImgU.\n>>        */\n>> -    ret = data->imgu_->enableLinks(true);\n>> +    ret = imgu0_.enableLinks(true);\n>>       if (ret)\n>>           return ret;\n>>   @@ -610,7 +610,7 @@ intPipelineHandlerIPU3::configure(Camera \n>> *camera, CameraConfiguration *c)\n>>       if (imguConfig.isNull())\n>>           return 0;\n>>   -    ret = imgu->configure(imguConfig, &cio2Format);\n>> +    ret = imgu0_.configure(imguConfig, &cio2Format);\n>>       if (ret)\n>>           return ret;\n>>   @@ -624,12 +624,12 @@ intPipelineHandlerIPU3::configure(Camera  \n>> *camera, CameraConfiguration *c)\n>>             if (stream == outStream) {\n>>               mainCfg = &cfg;\n>> -            ret = imgu->configureOutput(cfg, &outputFormat);\n>> +            ret = imgu0_.configureOutput(cfg, &outputFormat);\n>>               if (ret)\n>>                   return ret;\n>>           } else if (stream == vfStream) {\n>>               vfCfg = &cfg;\n>> -            ret = imgu->configureViewfinder(cfg, &outputFormat);\n>> +            ret = imgu0_.configureViewfinder(cfg, &outputFormat);\n>>               if (ret)\n>>                   return ret;\n>>           }\n>> @@ -641,13 +641,13 @@ intPipelineHandlerIPU3::configure(Camera \n>> *camera, CameraConfiguration *c)\n>>        * be at least one active stream in the configuration request).\n>>        */\n>>       if (!vfCfg) {\n>> -        ret = imgu->configureViewfinder(*mainCfg, &outputFormat);\n>> +        ret = imgu0_.configureViewfinder(*mainCfg, &outputFormat);\n>>           if (ret)\n>>               return ret;\n>>       }\n>>         /* Apply the \"pipe_mode\" control to the ImgU subdevice. */\n>> -    ControlList ctrls(imgu->imgu_->controls());\n>> +    ControlList ctrls(imgu0_.imgu_->controls());\n>>       /*\n>>        * Set the ImgU pipe mode to 'Video' unconditionally to have \n>> statistics\n>>        * generated.\n>> @@ -657,7 +657,7 @@ intPipelineHandlerIPU3::configure(Camera *camera, \n>> CameraConfiguration *c)\n>>        */\n>>       ctrls.set(V4L2_CID_IPU3_PIPE_MODE,\n>>             static_cast(IPU3PipeModeVideo));\n>> -    ret = imgu->imgu_->setControls(&ctrls);\n>> +    ret = imgu0_.imgu_->setControls(&ctrls);\n>>       if (ret) {\n>>           LOG(IPU3, Error) << \"Unable to set pipe_mode control\";\n>>           return ret;\n>> @@ -691,9 +691,9 @@ \n>> intPipelineHandlerIPU3::exportFrameBuffers(Camera  *camera, Stream \n>> *stream,\n>>       unsigned int count = stream->configuration().bufferCount;\n>>         if (stream == &data->outStream_)\n>> -        return data->imgu_->output_->exportBuffers(count, buffers);\n>> +        return imgu0_.output_->exportBuffers(count, buffers);\n>>       else if (stream == &data->vfStream_)\n>> -        return data->imgu_->viewfinder_->exportBuffers(count, buffers);\n>> +        return imgu0_.viewfinder_->exportBuffers(count, buffers);\n>>       else if (stream == &data->rawStream_)\n>>           return data->cio2_.exportBuffers(count, buffers);\n>>   @@ -711,7 +711,6 @@ \n>> intPipelineHandlerIPU3::exportFrameBuffers(Camera  *camera, Stream \n>> *stream,\n>>   intPipelineHandlerIPU3::allocateBuffers(Camera  *camera)\n>>   {\n>>       IPU3CameraData *data = cameraData(camera);\n>> -    ImgUDevice *imgu = data->imgu_;\n>>       unsigned int bufferCount;\n>>       int ret;\n>>   @@ -721,26 +720,26 @@ \n>> intPipelineHandlerIPU3::allocateBuffers(Camera  *camera)\n>>           data->rawStream_.configuration().bufferCount,\n>>       });\n>>   -    ret = imgu->allocateBuffers(bufferCount);\n>> +    ret = imgu0_.allocateBuffers(bufferCount);\n>>       if (ret < 0)\n>>           return ret;\n>>         /* Map buffers to the IPA. */\n>>       unsigned int ipaBufferId = 1;\n>>   -    for (conststd::unique_ptr &buffer : \n>> imgu->paramBuffers_) {\n>> +    for (conststd::unique_ptr &buffer : \n>> imgu0_.paramBuffers_) {\n>>           buffer->setCookie(ipaBufferId++);\n>>           ipaBuffers_.emplace_back(buffer->cookie(), buffer->planes());\n>>       }\n>>   -    for (conststd::unique_ptr &buffer : \n>> imgu->statBuffers_) {\n>> +    for (conststd::unique_ptr &buffer : \n>> imgu0_.statBuffers_) {\n>>           buffer->setCookie(ipaBufferId++);\n>>           ipaBuffers_.emplace_back(buffer->cookie(), buffer->planes());\n>>       }\n>>         data->ipa_->mapBuffers(ipaBuffers_);\n>>   -    data->frameInfos_.init(imgu->paramBuffers_, imgu->statBuffers_);\n>> +    data->frameInfos_.init(imgu0_.paramBuffers_, imgu0_.statBuffers_);\n>>       data->frameInfos_.bufferAvailable.connect(\n>>           data, &IPU3CameraData::queuePendingRequests);\n>>   @@ -760,7 +759,7 @@ intPipelineHandlerIPU3::freeBuffers(Camera  \n>> *camera)\n>>       data->ipa_->unmapBuffers(ids);\n>>       ipaBuffers_.clear();\n>>   -    data->imgu_->freeBuffers();\n>> +    imgu0_.freeBuffers();\n>>         return 0;\n>>   }\n>> @@ -777,9 +776,18 @@ intPipelineHandlerIPU3::start(Camera *camera, \n>> [[maybe_unused]] const ControlLis\n>>         IPU3CameraData *data = cameraData(camera);\n>>       CIO2Device *cio2 = &data->cio2_;\n>> -    ImgUDevice *imgu = data->imgu_;\n>>       int ret;\n>>   + imgu0_.input_->bufferReady.connect(&data->cio2_,\n>> +                       &CIO2Device::tryReturnBuffer);\n>> +    imgu0_.output_->bufferReady.connect(data,\n>> + &IPU3CameraData::imguOutputBufferReady);\n>> +    imgu0_.viewfinder_->bufferReady.connect(data,\n>> + &IPU3CameraData::imguOutputBufferReady);\n>> +    imgu0_.param_->bufferReady.connect(data,\n>> +                       &IPU3CameraData::paramBufferReady);\n>> +    imgu0_.stat_->bufferReady.connect(data,\n>> +                      &IPU3CameraData::statBufferReady);\n>>       /* Disable test pattern mode on the sensor, if any. */\n>>       ret = cio2->sensor()->setTestPatternMode(\n>> controls::draft::TestPatternModeEnum::TestPatternModeOff);\n>> @@ -807,19 +815,24 @@ intPipelineHandlerIPU3::start(Camera *camera, \n>> [[maybe_unused]] const ControlLis\n>>       if (ret)\n>>           goto error;\n>>   -    ret = imgu->start();\n>> +    ret = imgu0_.start();\n>>       if (ret)\n>>           goto error;\n>>         return 0;\n>>     error:\n>> -    imgu->stop();\n>> +    imgu0_.stop();\n>>       cio2->stop();\n>>       data->ipa_->stop();\n>>       freeBuffers(camera);\n>>       LOG(IPU3, Error) << \"Failed to start camera \" << camera->id();\n>>   +    imgu0_.input_->bufferReady.disconnect();\n>> +    imgu0_.output_->bufferReady.disconnect();\n>> +    imgu0_.viewfinder_->bufferReady.disconnect();\n>> +    imgu0_.param_->bufferReady.disconnect();\n>> +    imgu0_.stat_->bufferReady.disconnect();\n>>       inUseCamera_ = nullptr;\n>>         return ret;\n>> @@ -834,13 +847,19 @@ voidPipelineHandlerIPU3::stopDevice(Camera  \n>> *camera)\n>>         data->ipa_->stop();\n>>   -    ret |= data->imgu_->stop();\n>> +    ret |= imgu0_.stop();\n>>       ret |= data->cio2_.stop();\n>>       if (ret)\n>>           LOG(IPU3, Warning) << \"Failed to stop camera \" << \n>> camera->id();\n>>         freeBuffers(camera);\n>>   +    data->imgu0_->input_->bufferReady.disconnect();\n>> +    data->imgu0_->output_->bufferReady.disconnect();\n>> + data->imgu0_->viewfinder_->bufferReady.disconnect();\n>> +    data->imgu0_->param_->bufferReady.disconnect();\n>> +    data->imgu0_->stat_->bufferReady.disconnect();\n>> +\n>>       inUseCamera_ = nullptr;\n>>   }\n>>   @@ -1184,7 +1203,8 @@ intPipelineHandlerIPU3::registerCameras()\n>>            * only, and assign imgu0 to the first one and imgu1 to the\n>>            * second.\n>>            */\n>> -        data->imgu_ = numCameras ? &imgu1_ : &imgu0_;\n>> +        data->imgu0_ = &imgu0_;\n>> +        data->imgu1_ = &imgu1_;\n>>             /*\n>>            * Connect video devices' 'bufferReady' signals to their\n>> @@ -1198,16 +1218,6 @@ intPipelineHandlerIPU3::registerCameras()\n>>                       &IPU3CameraData::cio2BufferReady);\n>>           data->cio2_.bufferAvailable.connect(\n>>               data.get(), &IPU3CameraData::queuePendingRequests);\n>> - data->imgu_->input_->bufferReady.connect(&data->cio2_,\n>> -                    &CIO2Device::tryReturnBuffer);\n>> - data->imgu_->output_->bufferReady.connect(data.get(),\n>> - &IPU3CameraData::imguOutputBufferReady);\n>> - data->imgu_->viewfinder_->bufferReady.connect(data.get(),\n>> - &IPU3CameraData::imguOutputBufferReady);\n>> - data->imgu_->param_->bufferReady.connect(data.get(),\n>> -                    &IPU3CameraData::paramBufferReady);\n>> - data->imgu_->stat_->bufferReady.connect(data.get(),\n>> -                    &IPU3CameraData::statBufferReady);\n>>             /* Create and register the Camera instance. */\n>>           conststd::string  &cameraId = cio2->sensor()->id();\n>> @@ -1300,14 +1310,14 @@ \n>> voidIPU3CameraData::paramsBufferReady(unsigned  int id)\n>>           FrameBuffer *outbuffer = it.second;\n>>             if (stream == &outStream_)\n>> -            imgu_->output_->queueBuffer(outbuffer);\n>> +            imgu0_->output_->queueBuffer(outbuffer);\n>>           else if (stream == &vfStream_)\n>> -            imgu_->viewfinder_->queueBuffer(outbuffer);\n>> +            imgu0_->viewfinder_->queueBuffer(outbuffer);\n>>       }\n>>   -    imgu_->param_->queueBuffer(info->paramBuffer);\n>> -    imgu_->stat_->queueBuffer(info->statBuffer);\n>> -    imgu_->input_->queueBuffer(info->rawBuffer);\n>> +    imgu0_->param_->queueBuffer(info->paramBuffer);\n>> +    imgu0_->stat_->queueBuffer(info->statBuffer);\n>> +    imgu0_->input_->queueBuffer(info->rawBuffer);\n>>   }\n>>     voidIPU3CameraData::metadataReady(unsigned  int id, const \n>> ControlList &metadata)","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 2A9C1BE173\n\tfor ;\n\tTue, 26 Jul 2022 17:40:13 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id 7899C63312;\n\tTue, 26 Jul 2022 19:40:12 +0200 (CEST)","from perceval.ideasonboard.com (perceval.ideasonboard.com\n\t[213.167.242.64])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTPS id F10B760487\n\tfor ;\n\tTue, 26 Jul 2022 19:40:10 +0200 (CEST)","from [IPV6:2401:4900:1f3e:f7a:bc8f:12ed:b45f:c35d] (unknown\n\t[IPv6:2401:4900:1f3e:f7a:bc8f:12ed:b45f:c35d])\n\tby perceval.ideasonboard.com (Postfix) with ESMTPSA id AEDB5735;\n\tTue, 26 Jul 2022 19:40:09 +0200 (CEST)"],"DKIM-Signature":["v=1; a=rsa-sha256; c=relaxed/simple; d=libcamera.org;\n\ts=mail; t=1658857212;\n\tbh=F5DMbkCpJB95xw/yU7wJSK9AvPZLYjpKYOMMc+MVRiw=;\n\th=Date:To:References:In-Reply-To:Subject:List-Id:List-Unsubscribe:\n\tList-Archive:List-Post:List-Help:List-Subscribe:From:Reply-To:\n\tFrom;\n\tb=x5T6qjM8jPo/L3en6rdS8IRGspOjDvj4YwFDggxXhZKInWU6HMpzd42JSX7uMmgbG\n\tgqqmLjVoCgxQ3dfT88BRq/cQGLxJUzNkMSFqqMxb7QZAMu1IAivUiwHoQiY77MMQH0\n\tMk7O40DAFdLdTRaJumtgf0EsB3VpylOmEFnVL5/eb5PZKr/UgH354B5Oqqz94+tcVp\n\tN1L5wY959uq1FSmtE30Dtotfxt2g9H8YiQ24rHxcWXi1t5QY/su23EwHgTfQWIpIbN\n\tIhzolqlK+PBu+fBs7sQnAjcxcN5KifsqileXXFjsgSNkimq8Y1pAeiUhRirwjyzNi6\n\t3Vhv+e8wpVp/w==","v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com;\n\ts=mail; t=1658857210;\n\tbh=F5DMbkCpJB95xw/yU7wJSK9AvPZLYjpKYOMMc+MVRiw=;\n\th=Date:Subject:To:References:From:In-Reply-To:From;\n\tb=AMS1HQ30s8sGVRN6OO23MzBDxQLDBY6jx8jAzNWQcZJqZD7NyCmp5G40HO/GxoYHv\n\tRQ/KkvAJjoXtoyrzx7W52KB44+0nYAim/m3bs9EACrAS8xWMQJBeigWJFUZLfHudr8\n\tl3bWnQa4tZJNYESxOLljyUNK2wGjMQ/GEq4i29Zk="],"Authentication-Results":"lancelot.ideasonboard.com; dkim=pass (1024-bit key; \n\tunprotected) header.d=ideasonboard.com\n\theader.i=@ideasonboard.com\n\theader.b=\"AMS1HQ30\"; dkim-atps=neutral","Message-ID":"<43720763-0e04-e7cb-e48f-1e6f28f01271@ideasonboard.com>","Date":"Tue, 26 Jul 2022 23:10:04 +0530","MIME-Version":"1.0","User-Agent":"Mozilla/5.0 (X11; Linux x86_64; rv:91.0) Gecko/20100101\n\tThunderbird/91.4.1","Content-Language":"en-US","To":"Harvey Yang ,\n\tlibcamera-devel@lists.libcamera.org","References":"<20220629103018.4025635-1-chenghaoyang@google.com>\n\t<20220629103018.4025635-4-chenghaoyang@google.com>\n\t<9f328363-ce48-21e0-57b3-ef71046fa592@ideasonboard.com>","In-Reply-To":"<9f328363-ce48-21e0-57b3-ef71046fa592@ideasonboard.com>","Content-Type":"text/plain; charset=UTF-8; format=flowed","Content-Transfer-Encoding":"8bit","Subject":"Re: [libcamera-devel] [PATCH v3 3/9] ipu3: Use imgu0 as default","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","From":"Umang Jain via libcamera-devel ","Reply-To":"Umang Jain ","Errors-To":"libcamera-devel-bounces@lists.libcamera.org","Sender":"\"libcamera-devel\" "}},{"id":24177,"web_url":"https://patchwork.libcamera.org/comment/24177/","msgid":"<348b6804-ec21-3084-e33c-f07cd54bddbc@ideasonboard.com>","date":"2022-07-27T07:38:42","subject":"Re: [libcamera-devel] [PATCH v3 3/9] ipu3: Use imgu0 as default","submitter":{"id":86,"url":"https://patchwork.libcamera.org/api/people/86/","name":"Umang Jain","email":"umang.jain@ideasonboard.com"},"content":"Hi Harvey,\n\nThank you for the patch,\n\nOn 6/29/22 16:00, Harvey Yang via libcamera-devel wrote:\n> From: Harvey Yang \n>\n> With only one camera being started, we can always use imgu0 to process\n> frames (for video/preview). In the following patches, we'll use imgu1\n> for still capture if needed.\n>\n> Signed-off-by: Harvey Yang \n> ---\n> src/libcamera/pipeline/ipu3/ipu3.cpp | 86 ++++++++++++++++------------\n> 1 file changed, 48 insertions(+), 38 deletions(-)\n>\n> diff --git a/src/libcamera/pipeline/ipu3/ipu3.cpp b/src/libcamera/pipeline/ipu3/ipu3.cpp\n> index c943ee6a..e219f704 100644\n> --- a/src/libcamera/pipeline/ipu3/ipu3.cpp\n> +++ b/src/libcamera/pipeline/ipu3/ipu3.cpp\n> @@ -64,7 +64,8 @@ public:\n> \tvoid frameStart(uint32_t sequence);\n> \n> \tCIO2Device cio2_;\n> -\tImgUDevice *imgu_;\n> +\tImgUDevice *imgu0_;\n> +\tImgUDevice *imgu1_;\n> \n> \tStream outStream_;\n> \tStream vfStream_;\n> @@ -406,7 +407,7 @@ CameraConfiguration::Status IPU3CameraConfiguration::validate()\n> \n> \t/* Only compute the ImgU configuration if a YUV stream has been requested. */\n> \tif (yuvCount) {\n> -\t\tpipeConfig_ = data_->imgu_->calculatePipeConfig(&pipe);\n> +\t\tpipeConfig_ = data_->imgu0_->calculatePipeConfig(&pipe);\n> \t\tif (pipeConfig_.isNull()) {\n> \t\t\tLOG(IPU3, Error) << \"Failed to calculate pipe configuration: \"\n> \t\t\t\t\t << \"unsupported resolutions.\";\n> @@ -518,7 +519,6 @@ int PipelineHandlerIPU3::configure(Camera *camera, CameraConfiguration *c)\n> \tStream *outStream = &data->outStream_;\n> \tStream *vfStream = &data->vfStream_;\n> \tCIO2Device *cio2 = &data->cio2_;\n> -\tImgUDevice *imgu = data->imgu_;\n> \tV4L2DeviceFormat outputFormat;\n> \tint ret;\n> \n> @@ -560,7 +560,7 @@ int PipelineHandlerIPU3::configure(Camera *camera, CameraConfiguration *c)\n\n\nWe need to update the ::configure() comment about multiple camera \nstreaming (listed as FIXME) , since we will only stream one camera at a \ngiven time now.\n\n> \t * stream which is for raw capture, in which case no buffers will\n> \t * ever be queued to the ImgU.\n> \t */\n> -\tret = data->imgu_->enableLinks(true);\n> +\tret = imgu0_.enableLinks(true);\n> \tif (ret)\n> \t\treturn ret;\n> \n> @@ -610,7 +610,7 @@ int PipelineHandlerIPU3::configure(Camera *camera, CameraConfiguration *c)\n> \tif (imguConfig.isNull())\n> \t\treturn 0;\n> \n> -\tret = imgu->configure(imguConfig, &cio2Format);\n> +\tret = imgu0_.configure(imguConfig, &cio2Format);\n> \tif (ret)\n> \t\treturn ret;\n> \n> @@ -624,12 +624,12 @@ int PipelineHandlerIPU3::configure(Camera *camera, CameraConfiguration *c)\n> \n> \t\tif (stream == outStream) {\n> \t\t\tmainCfg = &cfg;\n> -\t\t\tret = imgu->configureOutput(cfg, &outputFormat);\n> +\t\t\tret = imgu0_.configureOutput(cfg, &outputFormat);\n> \t\t\tif (ret)\n> \t\t\t\treturn ret;\n> \t\t} else if (stream == vfStream) {\n> \t\t\tvfCfg = &cfg;\n> -\t\t\tret = imgu->configureViewfinder(cfg, &outputFormat);\n> +\t\t\tret = imgu0_.configureViewfinder(cfg, &outputFormat);\n> \t\t\tif (ret)\n> \t\t\t\treturn ret;\n> \t\t}\n> @@ -641,13 +641,13 @@ int PipelineHandlerIPU3::configure(Camera *camera, CameraConfiguration *c)\n> \t * be at least one active stream in the configuration request).\n> \t */\n> \tif (!vfCfg) {\n> -\t\tret = imgu->configureViewfinder(*mainCfg, &outputFormat);\n> +\t\tret = imgu0_.configureViewfinder(*mainCfg, &outputFormat);\n> \t\tif (ret)\n> \t\t\treturn ret;\n> \t}\n> \n> \t/* Apply the \"pipe_mode\" control to the ImgU subdevice. */\n> -\tControlList ctrls(imgu->imgu_->controls());\n> +\tControlList ctrls(imgu0_.imgu_->controls());\n> \t/*\n> \t * Set the ImgU pipe mode to 'Video' unconditionally to have statistics\n> \t * generated.\n> @@ -657,7 +657,7 @@ int PipelineHandlerIPU3::configure(Camera *camera, CameraConfiguration *c)\n> \t */\n> \tctrls.set(V4L2_CID_IPU3_PIPE_MODE,\n> \t\t static_cast(IPU3PipeModeVideo));\n> -\tret = imgu->imgu_->setControls(&ctrls);\n> +\tret = imgu0_.imgu_->setControls(&ctrls);\n> \tif (ret) {\n> \t\tLOG(IPU3, Error) << \"Unable to set pipe_mode control\";\n> \t\treturn ret;\n> @@ -691,9 +691,9 @@ int PipelineHandlerIPU3::exportFrameBuffers(Camera *camera, Stream *stream,\n> \tunsigned int count = stream->configuration().bufferCount;\n> \n> \tif (stream == &data->outStream_)\n> -\t\treturn data->imgu_->output_->exportBuffers(count, buffers);\n> +\t\treturn imgu0_.output_->exportBuffers(count, buffers);\n> \telse if (stream == &data->vfStream_)\n> -\t\treturn data->imgu_->viewfinder_->exportBuffers(count, buffers);\n> +\t\treturn imgu0_.viewfinder_->exportBuffers(count, buffers);\n> \telse if (stream == &data->rawStream_)\n> \t\treturn data->cio2_.exportBuffers(count, buffers);\n> \n> @@ -711,7 +711,6 @@ int PipelineHandlerIPU3::exportFrameBuffers(Camera *camera, Stream *stream,\n> int PipelineHandlerIPU3::allocateBuffers(Camera *camera)\n> {\n> \tIPU3CameraData *data = cameraData(camera);\n> -\tImgUDevice *imgu = data->imgu_;\n> \tunsigned int bufferCount;\n> \tint ret;\n> \n> @@ -721,26 +720,26 @@ int PipelineHandlerIPU3::allocateBuffers(Camera *camera)\n> \t\tdata->rawStream_.configuration().bufferCount,\n> \t});\n> \n> -\tret = imgu->allocateBuffers(bufferCount);\n> +\tret = imgu0_.allocateBuffers(bufferCount);\n> \tif (ret < 0)\n> \t\treturn ret;\n> \n> \t/* Map buffers to the IPA. */\n> \tunsigned int ipaBufferId = 1;\n> \n> -\tfor (const std::unique_ptr &buffer : imgu->paramBuffers_) {\n> +\tfor (const std::unique_ptr &buffer : imgu0_.paramBuffers_) {\n> \t\tbuffer->setCookie(ipaBufferId++);\n> \t\tipaBuffers_.emplace_back(buffer->cookie(), buffer->planes());\n> \t}\n> \n> -\tfor (const std::unique_ptr &buffer : imgu->statBuffers_) {\n> +\tfor (const std::unique_ptr &buffer : imgu0_.statBuffers_) {\n> \t\tbuffer->setCookie(ipaBufferId++);\n> \t\tipaBuffers_.emplace_back(buffer->cookie(), buffer->planes());\n> \t}\n> \n> \tdata->ipa_->mapBuffers(ipaBuffers_);\n> \n> -\tdata->frameInfos_.init(imgu->paramBuffers_, imgu->statBuffers_);\n> +\tdata->frameInfos_.init(imgu0_.paramBuffers_, imgu0_.statBuffers_);\n> \tdata->frameInfos_.bufferAvailable.connect(\n> \t\tdata, &IPU3CameraData::queuePendingRequests);\n> \n> @@ -760,7 +759,7 @@ int PipelineHandlerIPU3::freeBuffers(Camera *camera)\n> \tdata->ipa_->unmapBuffers(ids);\n> \tipaBuffers_.clear();\n> \n> -\tdata->imgu_->freeBuffers();\n> +\timgu0_.freeBuffers();\n> \n> \treturn 0;\n> }\n> @@ -777,9 +776,18 @@ int PipelineHandlerIPU3::start(Camera *camera, [[maybe_unused]] const ControlLis\n> \n> \tIPU3CameraData *data = cameraData(camera);\n> \tCIO2Device *cio2 = &data->cio2_;\n> -\tImgUDevice *imgu = data->imgu_;\n> \tint ret;\n> \n> +\timgu0_.input_->bufferReady.connect(&data->cio2_,\n> +\t\t\t\t\t &CIO2Device::tryReturnBuffer);\n> +\timgu0_.output_->bufferReady.connect(data,\n> +\t\t\t\t\t &IPU3CameraData::imguOutputBufferReady);\n> +\timgu0_.viewfinder_->bufferReady.connect(data,\n> +\t\t\t\t\t\t&IPU3CameraData::imguOutputBufferReady);\n> +\timgu0_.param_->bufferReady.connect(data,\n> +\t\t\t\t\t &IPU3CameraData::paramBufferReady);\n> +\timgu0_.stat_->bufferReady.connect(data,\n> +\t\t\t\t\t &IPU3CameraData::statBufferReady);\n> \t/* Disable test pattern mode on the sensor, if any. */\n> \tret = cio2->sensor()->setTestPatternMode(\n> \t\tcontrols::draft::TestPatternModeEnum::TestPatternModeOff);\n> @@ -807,19 +815,24 @@ int PipelineHandlerIPU3::start(Camera *camera, [[maybe_unused]] const ControlLis\n> \tif (ret)\n> \t\tgoto error;\n> \n> -\tret = imgu->start();\n> +\tret = imgu0_.start();\n> \tif (ret)\n> \t\tgoto error;\n> \n> \treturn 0;\n> \n> error:\n> -\timgu->stop();\n> +\timgu0_.stop();\n> \tcio2->stop();\n> \tdata->ipa_->stop();\n> \tfreeBuffers(camera);\n> \tLOG(IPU3, Error) << \"Failed to start camera \" << camera->id();\n> \n> +\timgu0_.input_->bufferReady.disconnect();\n> +\timgu0_.output_->bufferReady.disconnect();\n> +\timgu0_.viewfinder_->bufferReady.disconnect();\n> +\timgu0_.param_->bufferReady.disconnect();\n> +\timgu0_.stat_->bufferReady.disconnect();\n> \tinUseCamera_ = nullptr;\n> \n> \treturn ret;\n> @@ -834,13 +847,19 @@ void PipelineHandlerIPU3::stopDevice(Camera *camera)\n> \n> \tdata->ipa_->stop();\n> \n> -\tret |= data->imgu_->stop();\n> +\tret |= imgu0_.stop();\n> \tret |= data->cio2_.stop();\n> \tif (ret)\n> \t\tLOG(IPU3, Warning) << \"Failed to stop camera \" << camera->id();\n> \n> \tfreeBuffers(camera);\n> \n> +\tdata->imgu0_->input_->bufferReady.disconnect();\n> +\tdata->imgu0_->output_->bufferReady.disconnect();\n> +\tdata->imgu0_->viewfinder_->bufferReady.disconnect();\n> +\tdata->imgu0_->param_->bufferReady.disconnect();\n> +\tdata->imgu0_->stat_->bufferReady.disconnect();\n> +\n\n\nI think we should better abstract the signal connections/disconnections \nto IMGU somehow, if we are going to use them in ::start().\n\nProbably the abstractions can come through IPU3CameraData ?\n\n> \tinUseCamera_ = nullptr;\n> }\n> \n> @@ -1184,7 +1203,8 @@ int PipelineHandlerIPU3::registerCameras()\n> \t\t * only, and assign imgu0 to the first one and imgu1 to the\n> \t\t * second.\n> \t\t */\n> -\t\tdata->imgu_ = numCameras ? &imgu1_ : &imgu0_;\n> +\t\tdata->imgu0_ = &imgu0_;\n> +\t\tdata->imgu1_ = &imgu1_;\n> \n> \t\t/*\n> \t\t * Connect video devices' 'bufferReady' signals to their\n> @@ -1198,16 +1218,6 @@ int PipelineHandlerIPU3::registerCameras()\n> \t\t\t\t\t&IPU3CameraData::cio2BufferReady);\n> \t\tdata->cio2_.bufferAvailable.connect(\n> \t\t\tdata.get(), &IPU3CameraData::queuePendingRequests);\n\n\nHow about we move the cio2_ signals connection to \nPIpelineHandlerIPU3::Start() as well? There are 2 signals w.r.t cio2_ : \nbufferReady and bufferAvailable.\n\n\n> -\t\tdata->imgu_->input_->bufferReady.connect(&data->cio2_,\n> -\t\t\t\t\t&CIO2Device::tryReturnBuffer);\n> -\t\tdata->imgu_->output_->bufferReady.connect(data.get(),\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\tconst std::string &cameraId = cio2->sensor()->id();\n> @@ -1300,14 +1310,14 @@ void IPU3CameraData::paramsBufferReady(unsigned int id)\n> \t\tFrameBuffer *outbuffer = it.second;\n> \n> \t\tif (stream == &outStream_)\n> -\t\t\timgu_->output_->queueBuffer(outbuffer);\n> +\t\t\timgu0_->output_->queueBuffer(outbuffer);\n> \t\telse if (stream == &vfStream_)\n> -\t\t\timgu_->viewfinder_->queueBuffer(outbuffer);\n> +\t\t\timgu0_->viewfinder_->queueBuffer(outbuffer);\n> \t}\n> \n> -\timgu_->param_->queueBuffer(info->paramBuffer);\n> -\timgu_->stat_->queueBuffer(info->statBuffer);\n> -\timgu_->input_->queueBuffer(info->rawBuffer);\n> +\timgu0_->param_->queueBuffer(info->paramBuffer);\n> +\timgu0_->stat_->queueBuffer(info->statBuffer);\n> +\timgu0_->input_->queueBuffer(info->rawBuffer);\n> }\n> \n> void IPU3CameraData::metadataReady(unsigned int id, const ControlList &metadata)","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 6B155BE173\n\tfor ;\n\tWed, 27 Jul 2022 07:38:51 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id 68E4D63312;\n\tWed, 27 Jul 2022 09:38:50 +0200 (CEST)","from perceval.ideasonboard.com (perceval.ideasonboard.com\n\t[213.167.242.64])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTPS id 8C6EF603EC\n\tfor ;\n\tWed, 27 Jul 2022 09:38:48 +0200 (CEST)","from [IPV6:2401:4900:1f3e:f7a:bc8f:12ed:b45f:c35d] (unknown\n\t[IPv6:2401:4900:1f3e:f7a:bc8f:12ed:b45f:c35d])\n\tby perceval.ideasonboard.com (Postfix) with ESMTPSA id 8299B835;\n\tWed, 27 Jul 2022 09:38:47 +0200 (CEST)"],"DKIM-Signature":["v=1; a=rsa-sha256; c=relaxed/simple; d=libcamera.org;\n\ts=mail; t=1658907530;\n\tbh=68iVJ2etH/rw7JI6n5wzVNsH7r4Xsvr5zGqpZBspIK4=;\n\th=Date:To:References:In-Reply-To:Subject:List-Id:List-Unsubscribe:\n\tList-Archive:List-Post:List-Help:List-Subscribe:From:Reply-To:\n\tFrom;\n\tb=2DznAaVen43ti6JlfmdCkxlheYwipEMNKvtDUDwLUGYPbTcHJZGZO/utUcq1+zgzs\n\tD9jVj5wcM175ULkbR/7NUA82lRjfM0PZvsAdbt/u62YAeLUnCmqKw+wbrG2JhO3SxY\n\tGaCw90CENrHP/NZQS1WRbs0cZg6wsvhT2cYOQ+eaZh0+xwKBZLSu5QA8hQUfSz9+nr\n\t4Jnyfu9Ei+7CnIDK2DuT3JEeY9JRkS3QxuvdbMqt0eIUCrh6BZIUfzZOpXdTH/RzZb\n\tqZPBElswDqqBG5tcXEzHHxF912xl0OnMptxI9aISohkZOjHOmYW/ZvElrNxVJSwQHz\n\tJCIo/R8isplyQ==","v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com;\n\ts=mail; t=1658907528;\n\tbh=68iVJ2etH/rw7JI6n5wzVNsH7r4Xsvr5zGqpZBspIK4=;\n\th=Date:Subject:To:References:From:In-Reply-To:From;\n\tb=dNaKArNGT3x+Iy407o+Yum8U6QBL7LOVFkaWMAe6IPbjfESyNHj+j91yIgwzA7VZR\n\tp5UroFhnDqsYcfmGNIoolNFFE5CkO4nul0gDc0Gslv4kI5B16IVbAkAzkAxSkPgXxR\n\tKou0hxaiCd+QgzxoZjPxuqd3ccJP2Fk4PnKhauJw="],"Authentication-Results":"lancelot.ideasonboard.com; dkim=pass (1024-bit key; \n\tunprotected) header.d=ideasonboard.com\n\theader.i=@ideasonboard.com\n\theader.b=\"dNaKArNG\"; dkim-atps=neutral","Message-ID":"<348b6804-ec21-3084-e33c-f07cd54bddbc@ideasonboard.com>","Date":"Wed, 27 Jul 2022 13:08:42 +0530","MIME-Version":"1.0","User-Agent":"Mozilla/5.0 (X11; Linux x86_64; rv:91.0) Gecko/20100101\n\tThunderbird/91.4.1","Content-Language":"en-US","To":"Harvey Yang ,\n\tlibcamera-devel@lists.libcamera.org","References":"<20220629103018.4025635-1-chenghaoyang@google.com>\n\t<20220629103018.4025635-4-chenghaoyang@google.com>","In-Reply-To":"<20220629103018.4025635-4-chenghaoyang@google.com>","Content-Type":"text/plain; charset=UTF-8; format=flowed","Content-Transfer-Encoding":"7bit","Subject":"Re: [libcamera-devel] [PATCH v3 3/9] ipu3: Use imgu0 as default","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","From":"Umang Jain via libcamera-devel ","Reply-To":"Umang Jain ","Errors-To":"libcamera-devel-bounces@lists.libcamera.org","Sender":"\"libcamera-devel\" "}},{"id":24296,"web_url":"https://patchwork.libcamera.org/comment/24296/","msgid":"","date":"2022-08-02T10:30:28","subject":"Re: [libcamera-devel] [PATCH v3 3/9] ipu3: Use imgu0 as default","submitter":{"id":117,"url":"https://patchwork.libcamera.org/api/people/117/","name":"Cheng-Hao Yang","email":"chenghaoyang@chromium.org"},"content":"Hi Umang,\n\nThanks for your review!\nI guess there are only the three comments in your third message that\nneed updates, right?\nPlease check if I miss anything.\n\n\nOn Wed, Jul 27, 2022 at 3:38 PM Umang Jain \nwrote:\n\n> Hi Harvey,\n>\n> Thank you for the patch,\n>\n> On 6/29/22 16:00, Harvey Yang via libcamera-devel wrote:\n> > From: Harvey Yang \n> >\n> > With only one camera being started, we can always use imgu0 to process\n> > frames (for video/preview). In the following patches, we'll use imgu1\n> > for still capture if needed.\n> >\n> > Signed-off-by: Harvey Yang \n> > ---\n> > src/libcamera/pipeline/ipu3/ipu3.cpp | 86 ++++++++++++++++------------\n> > 1 file changed, 48 insertions(+), 38 deletions(-)\n> >\n> > diff --git a/src/libcamera/pipeline/ipu3/ipu3.cpp\n> b/src/libcamera/pipeline/ipu3/ipu3.cpp\n> > index c943ee6a..e219f704 100644\n> > --- a/src/libcamera/pipeline/ipu3/ipu3.cpp\n> > +++ b/src/libcamera/pipeline/ipu3/ipu3.cpp\n> > @@ -64,7 +64,8 @@ public:\n> > void frameStart(uint32_t sequence);\n> >\n> > CIO2Device cio2_;\n> > - ImgUDevice *imgu_;\n> > + ImgUDevice *imgu0_;\n> > + ImgUDevice *imgu1_;\n> >\n> > Stream outStream_;\n> > Stream vfStream_;\n> > @@ -406,7 +407,7 @@ CameraConfiguration::Status\n> IPU3CameraConfiguration::validate()\n> >\n> > /* Only compute the ImgU configuration if a YUV stream has been\n> requested. */\n> > if (yuvCount) {\n> > - pipeConfig_ = data_->imgu_->calculatePipeConfig(&pipe);\n> > + pipeConfig_ = data_->imgu0_->calculatePipeConfig(&pipe);\n> > if (pipeConfig_.isNull()) {\n> > LOG(IPU3, Error) << \"Failed to calculate pipe\n> configuration: \"\n> > << \"unsupported resolutions.\";\n> > @@ -518,7 +519,6 @@ int PipelineHandlerIPU3::configure(Camera *camera,\n> CameraConfiguration *c)\n> > Stream *outStream = &data->outStream_;\n> > Stream *vfStream = &data->vfStream_;\n> > CIO2Device *cio2 = &data->cio2_;\n> > - ImgUDevice *imgu = data->imgu_;\n> > V4L2DeviceFormat outputFormat;\n> > int ret;\n> >\n> > @@ -560,7 +560,7 @@ int PipelineHandlerIPU3::configure(Camera *camera,\n> CameraConfiguration *c)\n>\n>\n> We need to update the ::configure() comment about multiple camera\n> streaming (listed as FIXME) , since we will only stream one camera at a\n> given time now.\n>\n>\nUpdated the comment, not sure if I'm doing it right. Please check :)\n\n\n> > * stream which is for raw capture, in which case no buffers will\n> > * ever be queued to the ImgU.\n> > */\n> > - ret = data->imgu_->enableLinks(true);\n> > + ret = imgu0_.enableLinks(true);\n> > if (ret)\n> > return ret;\n> >\n> > @@ -610,7 +610,7 @@ int PipelineHandlerIPU3::configure(Camera *camera,\n> CameraConfiguration *c)\n> > if (imguConfig.isNull())\n> > return 0;\n> >\n> > - ret = imgu->configure(imguConfig, &cio2Format);\n> > + ret = imgu0_.configure(imguConfig, &cio2Format);\n> > if (ret)\n> > return ret;\n> >\n> > @@ -624,12 +624,12 @@ int PipelineHandlerIPU3::configure(Camera *camera,\n> CameraConfiguration *c)\n> >\n> > if (stream == outStream) {\n> > mainCfg = &cfg;\n> > - ret = imgu->configureOutput(cfg, &outputFormat);\n> > + ret = imgu0_.configureOutput(cfg, &outputFormat);\n> > if (ret)\n> > return ret;\n> > } else if (stream == vfStream) {\n> > vfCfg = &cfg;\n> > - ret = imgu->configureViewfinder(cfg,\n> &outputFormat);\n> > + ret = imgu0_.configureViewfinder(cfg,\n> &outputFormat);\n> > if (ret)\n> > return ret;\n> > }\n> > @@ -641,13 +641,13 @@ int PipelineHandlerIPU3::configure(Camera *camera,\n> CameraConfiguration *c)\n> > * be at least one active stream in the configuration request).\n> > */\n> > if (!vfCfg) {\n> > - ret = imgu->configureViewfinder(*mainCfg, &outputFormat);\n> > + ret = imgu0_.configureViewfinder(*mainCfg, &outputFormat);\n> > if (ret)\n> > return ret;\n> > }\n> >\n> > /* Apply the \"pipe_mode\" control to the ImgU subdevice. */\n> > - ControlList ctrls(imgu->imgu_->controls());\n> > + ControlList ctrls(imgu0_.imgu_->controls());\n> > /*\n> > * Set the ImgU pipe mode to 'Video' unconditionally to have\n> statistics\n> > * generated.\n> > @@ -657,7 +657,7 @@ int PipelineHandlerIPU3::configure(Camera *camera,\n> CameraConfiguration *c)\n> > */\n> > ctrls.set(V4L2_CID_IPU3_PIPE_MODE,\n> > static_cast(IPU3PipeModeVideo));\n> > - ret = imgu->imgu_->setControls(&ctrls);\n> > + ret = imgu0_.imgu_->setControls(&ctrls);\n> > if (ret) {\n> > LOG(IPU3, Error) << \"Unable to set pipe_mode control\";\n> > return ret;\n> > @@ -691,9 +691,9 @@ int PipelineHandlerIPU3::exportFrameBuffers(Camera\n> *camera, Stream *stream,\n> > unsigned int count = stream->configuration().bufferCount;\n> >\n> > if (stream == &data->outStream_)\n> > - return data->imgu_->output_->exportBuffers(count, buffers);\n> > + return imgu0_.output_->exportBuffers(count, buffers);\n> > else if (stream == &data->vfStream_)\n> > - return data->imgu_->viewfinder_->exportBuffers(count,\n> buffers);\n> > + return imgu0_.viewfinder_->exportBuffers(count, buffers);\n> > else if (stream == &data->rawStream_)\n> > return data->cio2_.exportBuffers(count, buffers);\n> >\n> > @@ -711,7 +711,6 @@ int PipelineHandlerIPU3::exportFrameBuffers(Camera\n> *camera, Stream *stream,\n> > int PipelineHandlerIPU3::allocateBuffers(Camera *camera)\n> > {\n> > IPU3CameraData *data = cameraData(camera);\n> > - ImgUDevice *imgu = data->imgu_;\n> > unsigned int bufferCount;\n> > int ret;\n> >\n> > @@ -721,26 +720,26 @@ int PipelineHandlerIPU3::allocateBuffers(Camera\n> *camera)\n> > data->rawStream_.configuration().bufferCount,\n> > });\n> >\n> > - ret = imgu->allocateBuffers(bufferCount);\n> > + ret = imgu0_.allocateBuffers(bufferCount);\n> > if (ret < 0)\n> > return ret;\n> >\n> > /* Map buffers to the IPA. */\n> > unsigned int ipaBufferId = 1;\n> >\n> > - for (const std::unique_ptr &buffer :\n> imgu->paramBuffers_) {\n> > + for (const std::unique_ptr &buffer :\n> imgu0_.paramBuffers_) {\n> > buffer->setCookie(ipaBufferId++);\n> > ipaBuffers_.emplace_back(buffer->cookie(),\n> buffer->planes());\n> > }\n> >\n> > - for (const std::unique_ptr &buffer :\n> imgu->statBuffers_) {\n> > + for (const std::unique_ptr &buffer :\n> imgu0_.statBuffers_) {\n> > buffer->setCookie(ipaBufferId++);\n> > ipaBuffers_.emplace_back(buffer->cookie(),\n> buffer->planes());\n> > }\n> >\n> > data->ipa_->mapBuffers(ipaBuffers_);\n> >\n> > - data->frameInfos_.init(imgu->paramBuffers_, imgu->statBuffers_);\n> > + data->frameInfos_.init(imgu0_.paramBuffers_, imgu0_.statBuffers_);\n> > data->frameInfos_.bufferAvailable.connect(\n> > data, &IPU3CameraData::queuePendingRequests);\n> >\n> > @@ -760,7 +759,7 @@ int PipelineHandlerIPU3::freeBuffers(Camera *camera)\n> > data->ipa_->unmapBuffers(ids);\n> > ipaBuffers_.clear();\n> >\n> > - data->imgu_->freeBuffers();\n> > + imgu0_.freeBuffers();\n> >\n> > return 0;\n> > }\n> > @@ -777,9 +776,18 @@ int PipelineHandlerIPU3::start(Camera *camera,\n> [[maybe_unused]] const ControlLis\n> >\n> > IPU3CameraData *data = cameraData(camera);\n> > CIO2Device *cio2 = &data->cio2_;\n> > - ImgUDevice *imgu = data->imgu_;\n> > int ret;\n> >\n> > + imgu0_.input_->bufferReady.connect(&data->cio2_,\n> > + &CIO2Device::tryReturnBuffer);\n> > + imgu0_.output_->bufferReady.connect(data,\n> > +\n> &IPU3CameraData::imguOutputBufferReady);\n> > + imgu0_.viewfinder_->bufferReady.connect(data,\n> > +\n> &IPU3CameraData::imguOutputBufferReady);\n> > + imgu0_.param_->bufferReady.connect(data,\n> > +\n> &IPU3CameraData::paramBufferReady);\n> > + imgu0_.stat_->bufferReady.connect(data,\n> > +\n> &IPU3CameraData::statBufferReady);\n> > /* Disable test pattern mode on the sensor, if any. */\n> > ret = cio2->sensor()->setTestPatternMode(\n> > controls::draft::TestPatternModeEnum::TestPatternModeOff);\n> > @@ -807,19 +815,24 @@ int PipelineHandlerIPU3::start(Camera *camera,\n> [[maybe_unused]] const ControlLis\n> > if (ret)\n> > goto error;\n> >\n> > - ret = imgu->start();\n> > + ret = imgu0_.start();\n> > if (ret)\n> > goto error;\n> >\n> > return 0;\n> >\n> > error:\n> > - imgu->stop();\n> > + imgu0_.stop();\n> > cio2->stop();\n> > data->ipa_->stop();\n> > freeBuffers(camera);\n> > LOG(IPU3, Error) << \"Failed to start camera \" << camera->id();\n> >\n> > + imgu0_.input_->bufferReady.disconnect();\n> > + imgu0_.output_->bufferReady.disconnect();\n> > + imgu0_.viewfinder_->bufferReady.disconnect();\n> > + imgu0_.param_->bufferReady.disconnect();\n> > + imgu0_.stat_->bufferReady.disconnect();\n> > inUseCamera_ = nullptr;\n> >\n> > return ret;\n> > @@ -834,13 +847,19 @@ void PipelineHandlerIPU3::stopDevice(Camera\n> *camera)\n> >\n> > data->ipa_->stop();\n> >\n> > - ret |= data->imgu_->stop();\n> > + ret |= imgu0_.stop();\n> > ret |= data->cio2_.stop();\n> > if (ret)\n> > LOG(IPU3, Warning) << \"Failed to stop camera \" <<\n> camera->id();\n> >\n> > freeBuffers(camera);\n> >\n> > + data->imgu0_->input_->bufferReady.disconnect();\n> > + data->imgu0_->output_->bufferReady.disconnect();\n> > + data->imgu0_->viewfinder_->bufferReady.disconnect();\n> > + data->imgu0_->param_->bufferReady.disconnect();\n> > + data->imgu0_->stat_->bufferReady.disconnect();\n> > +\n>\n>\n> I think we should better abstract the signal connections/disconnections\n> to IMGU somehow, if we are going to use them in ::start().\n>\n> Probably the abstractions can come through IPU3CameraData ?\n>\n>\nFor connections, |input_->bufferReady| is bound to CIO2Device instead, and\nwe need to bind different methods on |imgu1_|'s signals, when StillCapture\nis enabled.\nI don't think there's an elegant way to abstract it...?\n\nFor disconnections, I think adding a helper function in ImgUDevice should\nbe simpler. WDYT? (Added in the next version)\n\n\n> > inUseCamera_ = nullptr;\n> > }\n> >\n> > @@ -1184,7 +1203,8 @@ int PipelineHandlerIPU3::registerCameras()\n> > * only, and assign imgu0 to the first one and imgu1 to the\n> > * second.\n> > */\n> > - data->imgu_ = numCameras ? &imgu1_ : &imgu0_;\n> > + data->imgu0_ = &imgu0_;\n> > + data->imgu1_ = &imgu1_;\n> >\n> > /*\n> > * Connect video devices' 'bufferReady' signals to their\n> > @@ -1198,16 +1218,6 @@ int PipelineHandlerIPU3::registerCameras()\n> > &IPU3CameraData::cio2BufferReady);\n> > data->cio2_.bufferAvailable.connect(\n> > data.get(), &IPU3CameraData::queuePendingRequests);\n>\n>\n> How about we move the cio2_ signals connection to\n> PIpelineHandlerIPU3::Start() as well? There are 2 signals w.r.t cio2_ :\n> bufferReady and bufferAvailable.\n>\n>\nHmm, but PipelineHandlerIPU3::Start() might be called multiple times, while\nwe only need to setup |cio2_|'s signals connection once. Right?\n\n\n>\n> > - data->imgu_->input_->bufferReady.connect(&data->cio2_,\n> > - &CIO2Device::tryReturnBuffer);\n> > - data->imgu_->output_->bufferReady.connect(data.get(),\n> > -\n> &IPU3CameraData::imguOutputBufferReady);\n> > - data->imgu_->viewfinder_->bufferReady.connect(data.get(),\n> > -\n> &IPU3CameraData::imguOutputBufferReady);\n> > - data->imgu_->param_->bufferReady.connect(data.get(),\n> > - &IPU3CameraData::paramBufferReady);\n> > - data->imgu_->stat_->bufferReady.connect(data.get(),\n> > - &IPU3CameraData::statBufferReady);\n> >\n> > /* Create and register the Camera instance. */\n> > const std::string &cameraId = cio2->sensor()->id();\n> > @@ -1300,14 +1310,14 @@ void IPU3CameraData::paramsBufferReady(unsigned\n> int id)\n> > FrameBuffer *outbuffer = it.second;\n> >\n> > if (stream == &outStream_)\n> > - imgu_->output_->queueBuffer(outbuffer);\n> > + imgu0_->output_->queueBuffer(outbuffer);\n> > else if (stream == &vfStream_)\n> > - imgu_->viewfinder_->queueBuffer(outbuffer);\n> > + imgu0_->viewfinder_->queueBuffer(outbuffer);\n> > }\n> >\n> > - imgu_->param_->queueBuffer(info->paramBuffer);\n> > - imgu_->stat_->queueBuffer(info->statBuffer);\n> > - imgu_->input_->queueBuffer(info->rawBuffer);\n> > + imgu0_->param_->queueBuffer(info->paramBuffer);\n> > + imgu0_->stat_->queueBuffer(info->statBuffer);\n> > + imgu0_->input_->queueBuffer(info->rawBuffer);\n> > }\n> >\n> > void IPU3CameraData::metadataReady(unsigned int id, const ControlList\n> &metadata)\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 B763FC3275\n\tfor ;\n\tTue, 2 Aug 2022 10:30:42 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id 736A863316;\n\tTue, 2 Aug 2022 12:30:42 +0200 (CEST)","from mail-lf1-x12e.google.com (mail-lf1-x12e.google.com\n\t[IPv6:2a00:1450:4864:20::12e])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTPS id 5819F603E7\n\tfor ;\n\tTue, 2 Aug 2022 12:30:40 +0200 (CEST)","by mail-lf1-x12e.google.com with SMTP id t1so21285552lft.8\n\tfor ;\n\tTue, 02 Aug 2022 03:30:40 -0700 (PDT)"],"DKIM-Signature":["v=1; a=rsa-sha256; c=relaxed/simple; d=libcamera.org;\n\ts=mail; t=1659436242;\n\tbh=2q/9adNXZKZ4FOGCWX6hRHBi9+B9CJG7nUCCZ/eSQrU=;\n\th=References:In-Reply-To:Date:To:Subject:List-Id:List-Unsubscribe:\n\tList-Archive:List-Post:List-Help:List-Subscribe:From:Reply-To:Cc:\n\tFrom;\n\tb=kjOiJvysGjPhYhpfWhcyDz5yjtorOxyYbUTjLR+6+QbTypqoSH53eawuFva0uZpED\n\t510U1DKJdQ12nQfe87YZiuT9aCmO+GLzRpUo30m8HlqUW/ctrILSDM9RaYBSCpDaOJ\n\t6/p8C4qGGAtr2O/lMlxKMGXXFVPerN7k/kjbcLJllv4+UrwBmYBoZWx34c1wAobjic\n\t7TXzpZ4KF6YVLloLK6jQfJgQEyXAh1yVAz+KY2PIjp1gjXPdHUgk5DGsVfd8hno+M1\n\tO+M79ow6KGdfYmMIVXg5AudPd4QWBYwKLa23VqnxZ7BB3A7njrGKniO8f9yDjhfpw2\n\tgcYcbdWb55bIQ==","v=1; a=rsa-sha256; c=relaxed/relaxed; d=chromium.org;\n\ts=google; \n\th=mime-version:references:in-reply-to:from:date:message-id:subject:to\n\t:cc; bh=LyqzRkN85xhMAoVPM3kaf9i+Zz7YC0zk4y2mr7CS1Ng=;\n\tb=P0vNzOTJyTz08HGD7+K0WFtm+Qk+BwoZrSD1LmGFE6MKGI0iz6FrLMbc9ObDHutSnc\n\tGVtF7hk0/ynY7d15VXy4T/wDEDhYrIMR3btbL6KMPqfPLm9LHJGSkOh41lll7b5NEyzU\n\tGADBpRHo6yvyMqTpjyEqeFDucYgAB1h50dcXo="],"Authentication-Results":"lancelot.ideasonboard.com; dkim=pass (1024-bit key; \n\tunprotected) header.d=chromium.org\n\theader.i=@chromium.org header.b=\"P0vNzOTJ\"; \n\tdkim-atps=neutral","X-Google-DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/relaxed;\n\td=1e100.net; s=20210112;\n\th=x-gm-message-state:mime-version:references:in-reply-to:from:date\n\t:message-id:subject:to:cc;\n\tbh=LyqzRkN85xhMAoVPM3kaf9i+Zz7YC0zk4y2mr7CS1Ng=;\n\tb=uAd18azHFtWooxKs0rCjiGMB1rqbkr/grp85ftqu9RIIElGCcOvFmqgQWtEy2PnKDJ\n\t0uG8wO/tc4/v5DeJ0wEnGciCXa8GQkxlgXEVg+dOHeTn/zltedq/VworGw/D0ryPBjlm\n\t9m1ew2vEn9o7/vpPtJRiFQrFHUT5L0oOAxsz1/Rr8ANzsFGmZR0zZTLb4spU9+K9Lo74\n\t7JzilAv44k0dsPWzK7RietBXHCTa3yV/8Xuzig+av/znBtAaXZjQSRNhy9ECV0qnG2aK\n\tNia1h2DuapjTC1s6RsNR1+tHVULY9fYxjBdn1/I+YS5eVXGQ4fNJxkEf8yu00+W6tr5S\n\tV6Xg==","X-Gm-Message-State":"AJIora8r6Nxr7kG3oWFNZWxD5zdArpNUPh13HNU3tb7/RREDZA5p7qO6\n\tMztxmB/V8hkY/S8Sr/oSnKWVBzUQiUVCSbyffRWobOGkwI0=","X-Google-Smtp-Source":"AGRyM1vpDsopVRMfQjFsAKJNkPkfa1eqLPSfEcLFWtcfOAiW0nZTqScAfh5xxJ7CMsfTSztqilBxnT0z+zHu4vJOW5M=","X-Received":"by 2002:a05:6512:ad6:b0:48a:20f9:c0e6 with SMTP id\n\tn22-20020a0565120ad600b0048a20f9c0e6mr6751983lfu.287.1659436239722;\n\tTue, 02 Aug 2022 03:30:39 -0700 (PDT)","MIME-Version":"1.0","References":"<20220629103018.4025635-1-chenghaoyang@google.com>\n\t<20220629103018.4025635-4-chenghaoyang@google.com>\n\t<348b6804-ec21-3084-e33c-f07cd54bddbc@ideasonboard.com>","In-Reply-To":"<348b6804-ec21-3084-e33c-f07cd54bddbc@ideasonboard.com>","Date":"Tue, 2 Aug 2022 18:30:28 +0800","Message-ID":"","To":"Umang Jain ","Content-Type":"multipart/alternative; boundary=\"00000000000098b2f905e53f9b93\"","Subject":"Re: [libcamera-devel] [PATCH v3 3/9] ipu3: Use imgu0 as default","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","From":"Cheng-Hao Yang via libcamera-devel\n\t","Reply-To":"Cheng-Hao Yang ","Cc":"libcamera-devel@lists.libcamera.org","Errors-To":"libcamera-devel-bounces@lists.libcamera.org","Sender":"\"libcamera-devel\" "}}]