[{"id":15170,"web_url":"https://patchwork.libcamera.org/comment/15170/","msgid":"","date":"2021-02-15T10:48:13","subject":"Re: [libcamera-devel] [PATCH v2] libcamera: pipeline: rkisp1:\n\tconfigure IPA from configure method instead of start method","submitter":{"id":46,"url":"https://patchwork.libcamera.org/api/people/46/","name":"Dafna Hirschfeld","email":"dafna.hirschfeld@collabora.com"},"content":"On 15.02.21 10:38, Paul Elder wrote:\n> From: Dafna Hirschfeld \n> \n> Currently the call to the configure method of rkisp1 IPA\n> is called upon the 'start' of the pipeline. This should be done\n> in the 'configure' method instead.\n> \n> [Original patch]\n> Signed-off-by: Dafna Hirschfeld \n> Reviewed-by: Paul Elder \n> Reviewed-by: Laurent Pinchart \n> [v2]\n> Signed-off-by: Paul Elder \n> \n> ---\n> Hi Dafna,\n> \n> The IPA IPC series is ready to be merged but depends on this patch, so\n> I've taken the liberty to send a v2 to speed things up.\n\nsure, thanks,\n\nDafna\n\n> \n> Changes in v2:\n> - remove path isEnabled() checks from configure()\n> ---\n> src/libcamera/pipeline/rkisp1/rkisp1.cpp | 62 ++++++++++++------------\n> 1 file changed, 30 insertions(+), 32 deletions(-)\n> \n> diff --git a/src/libcamera/pipeline/rkisp1/rkisp1.cpp b/src/libcamera/pipeline/rkisp1/rkisp1.cpp\n> index e85979a7..7cb89eb0 100644\n> --- a/src/libcamera/pipeline/rkisp1/rkisp1.cpp\n> +++ b/src/libcamera/pipeline/rkisp1/rkisp1.cpp\n> @@ -607,11 +607,22 @@ int PipelineHandlerRkISP1::configure(Camera *camera, CameraConfiguration *c)\n> \t\t<< \"ISP output pad configured with \" << format.toString()\n> \t\t<< \" crop \" << rect.toString();\n> \n> +\tstd::map streamConfig;\n> +\n> \tfor (const StreamConfiguration &cfg : *config) {\n> -\t\tif (cfg.stream() == &data->mainPathStream_)\n> +\t\tif (cfg.stream() == &data->mainPathStream_) {\n> \t\t\tret = mainPath_.configure(cfg, format);\n> -\t\telse\n> +\t\t\tstreamConfig[0] = {\n> +\t\t\t\t.pixelFormat = cfg.pixelFormat,\n> +\t\t\t\t.size = cfg.size,\n> +\t\t\t};\n> +\t\t} else {\n> \t\t\tret = selfPath_.configure(cfg, format);\n> +\t\t\tstreamConfig[1] = {\n> +\t\t\t\t.pixelFormat = cfg.pixelFormat,\n> +\t\t\t\t.size = cfg.size,\n> +\t\t\t};\n> +\t\t}\n> \n> \t\tif (ret)\n> \t\t\treturn ret;\n> @@ -629,6 +640,23 @@ int PipelineHandlerRkISP1::configure(Camera *camera, CameraConfiguration *c)\n> \tif (ret)\n> \t\treturn ret;\n> \n> +\t/* Inform IPA of stream configuration and sensor controls. */\n> +\tCameraSensorInfo sensorInfo = {};\n> +\tret = data->sensor_->sensorInfo(&sensorInfo);\n> +\tif (ret) {\n> +\t\t/* \\todo Turn this into a hard failure. */\n> +\t\tLOG(RkISP1, Warning) << \"Camera sensor information not available\";\n> +\t\tsensorInfo = {};\n> +\t\tret = 0;\n> +\t}\n> +\n> +\tstd::map entityControls;\n> +\tentityControls.emplace(0, data->sensor_->controls());\n> +\n> +\tIPAOperationData ipaConfig;\n> +\tdata->ipa_->configure(sensorInfo, streamConfig, entityControls,\n> +\t\t\t ipaConfig, nullptr);\n> +\n> \treturn 0;\n> }\n> \n> @@ -759,8 +787,6 @@ int PipelineHandlerRkISP1::start(Camera *camera, [[maybe_unused]] ControlList *c\n> \t\treturn ret;\n> \t}\n> \n> -\tstd::map streamConfig;\n> -\n> \tif (data->mainPath_->isEnabled()) {\n> \t\tret = mainPath_.start();\n> \t\tif (ret) {\n> @@ -770,11 +796,6 @@ int PipelineHandlerRkISP1::start(Camera *camera, [[maybe_unused]] ControlList *c\n> \t\t\tfreeBuffers(camera);\n> \t\t\treturn ret;\n> \t\t}\n> -\n> -\t\tstreamConfig[0] = {\n> -\t\t\t.pixelFormat = data->mainPathStream_.configuration().pixelFormat,\n> -\t\t\t.size = data->mainPathStream_.configuration().size,\n> -\t\t};\n> \t}\n> \n> \tif (data->selfPath_->isEnabled()) {\n> @@ -787,34 +808,11 @@ int PipelineHandlerRkISP1::start(Camera *camera, [[maybe_unused]] ControlList *c\n> \t\t\tfreeBuffers(camera);\n> \t\t\treturn ret;\n> \t\t}\n> -\n> -\t\tstreamConfig[1] = {\n> -\t\t\t.pixelFormat = data->selfPathStream_.configuration().pixelFormat,\n> -\t\t\t.size = data->selfPathStream_.configuration().size,\n> -\t\t};\n> \t}\n> \n> \tisp_->setFrameStartEnabled(true);\n> \n> \tactiveCamera_ = camera;\n> -\n> -\t/* Inform IPA of stream configuration and sensor controls. */\n> -\tCameraSensorInfo sensorInfo = {};\n> -\tret = data->sensor_->sensorInfo(&sensorInfo);\n> -\tif (ret) {\n> -\t\t/* \\todo Turn this in an hard failure. */\n> -\t\tLOG(RkISP1, Warning) << \"Camera sensor information not available\";\n> -\t\tsensorInfo = {};\n> -\t\tret = 0;\n> -\t}\n> -\n> -\tstd::map entityControls;\n> -\tentityControls.emplace(0, data->sensor_->controls());\n> -\n> -\tIPAOperationData ipaConfig;\n> -\tdata->ipa_->configure(sensorInfo, streamConfig, entityControls,\n> -\t\t\t ipaConfig, nullptr);\n> -\n> \treturn ret;\n> }\n> \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 BC56BBD160\n\tfor ;\n\tMon, 15 Feb 2021 10:48:18 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id 15CC1637C4;\n\tMon, 15 Feb 2021 11:48:18 +0100 (CET)","from bhuna.collabora.co.uk (bhuna.collabora.co.uk [46.235.227.227])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTPS id C9E4E6375F\n\tfor ;\n\tMon, 15 Feb 2021 11:48:16 +0100 (CET)","from [IPv6:2003:c7:cf1c:ce00:b857:ee62:703c:33b9]\n\t(p200300c7cf1cce00b857ee62703c33b9.dip0.t-ipconnect.de\n\t[IPv6:2003:c7:cf1c:ce00:b857:ee62:703c:33b9])\n\t(using TLSv1.2 with cipher ECDHE-RSA-AES128-GCM-SHA256 (128/128\n\tbits))\n\t(No client certificate requested) (Authenticated sender: dafna)\n\tby bhuna.collabora.co.uk (Postfix) with ESMTPSA id 3AF751F44A94;\n\tMon, 15 Feb 2021 10:48:16 +0000 (GMT)"],"To":"Paul Elder ,\n\tlibcamera-devel@lists.libcamera.org","References":"<20210215093843.144886-1-paul.elder@ideasonboard.com>","From":"Dafna Hirschfeld ","Message-ID":"","Date":"Mon, 15 Feb 2021 11:48:13 +0100","User-Agent":"Mozilla/5.0 (X11; Linux x86_64; rv:68.0) Gecko/20100101\n\tThunderbird/68.10.0","MIME-Version":"1.0","In-Reply-To":"<20210215093843.144886-1-paul.elder@ideasonboard.com>","Content-Language":"en-US","Subject":"Re: [libcamera-devel] [PATCH v2] libcamera: pipeline: rkisp1:\n\tconfigure IPA from configure method instead of start method","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","Content-Transfer-Encoding":"7bit","Content-Type":"text/plain; charset=\"us-ascii\"; Format=\"flowed\"","Errors-To":"libcamera-devel-bounces@lists.libcamera.org","Sender":"\"libcamera-devel\" "}}]