[{"id":4322,"web_url":"https://patchwork.libcamera.org/comment/4322/","msgid":"<20200326212734.GC20581@pendragon.ideasonboard.com>","date":"2020-03-26T21:27:34","subject":"Re: [libcamera-devel] [RFCv2 5/7] libcamera: pipeline: rkisp1: Call\n\tIPA start() and stop()","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 Thu, Mar 26, 2020 at 05:08:17PM +0100, Niklas Söderlund wrote:\n> Call the IPA start()/stop() functions before/after the camera is\n> started. This makes sure the IPA functions properly once thread\n> management is moved into the start/stop interface.\n> \n> Signed-off-by: Niklas Söderlund \n> ---\n> src/libcamera/pipeline/rkisp1/rkisp1.cpp | 24 ++++++++++++++++++++++--\n> 1 file changed, 22 insertions(+), 2 deletions(-)\n> \n> diff --git a/src/libcamera/pipeline/rkisp1/rkisp1.cpp b/src/libcamera/pipeline/rkisp1/rkisp1.cpp\n> index f49b3a7f0945ac92..3287fd3a18204cbc 100644\n> --- a/src/libcamera/pipeline/rkisp1/rkisp1.cpp\n> +++ b/src/libcamera/pipeline/rkisp1/rkisp1.cpp\n> @@ -113,8 +113,8 @@ class RkISP1CameraData : public CameraData\n> {\n> public:\n> \tRkISP1CameraData(PipelineHandler *pipe)\n> -\t\t: CameraData(pipe), sensor_(nullptr), frame_(0),\n> -\t\t frameInfo_(pipe)\n> +\t\t: CameraData(pipe), running_(false), sensor_(nullptr),\n> +\t\t frame_(0), frameInfo_(pipe)\n> \t{\n> \t}\n> \n> @@ -125,6 +125,7 @@ public:\n> \n> \tint loadIPA();\n> \n> +\tbool running_;\n> \tStream stream_;\n> \tCameraSensor *sensor_;\n> \tunsigned int frame_;\n> @@ -394,6 +395,9 @@ int RkISP1CameraData::loadIPA()\n> void RkISP1CameraData::queueFrameAction(unsigned int frame,\n> \t\t\t\t\tconst IPAOperationData &action)\n> {\n> +\tif (!running_)\n> +\t\treturn;\n> +\n> \tswitch (action.operation) {\n> \tcase RKISP1_IPA_ACTION_V4L2_SET: {\n> \t\tconst ControlList &controls = action.controls[0];\n> @@ -764,10 +768,19 @@ int PipelineHandlerRkISP1::start(Camera *camera)\n> \tif (ret)\n> \t\treturn ret;\n> \n> +\tret = data->ipa_->start();\n> +\tif (ret) {\n> +\t\tfreeBuffers(camera);\n> +\t\tLOG(RkISP1, Error)\n> +\t\t\t<< \"Failed to start IPA \" << camera->name();\n> +\t\treturn ret;\n> +\t}\n> +\n\nI wonder if it would simplify error handling if you moved this last (the\nstop could go first at stop time then).\n\nReviewed-by: Laurent Pinchart \n\n> \tdata->frame_ = 0;\n> \n> \tret = param_->streamOn();\n> \tif (ret) {\n> +\t\tdata->ipa_->stop();\n> \t\tfreeBuffers(camera);\n> \t\tLOG(RkISP1, Error)\n> \t\t\t<< \"Failed to start parameters \" << camera->name();\n> @@ -777,6 +790,7 @@ int PipelineHandlerRkISP1::start(Camera *camera)\n> \tret = stat_->streamOn();\n> \tif (ret) {\n> \t\tparam_->streamOff();\n> +\t\tdata->ipa_->stop();\n> \t\tfreeBuffers(camera);\n> \t\tLOG(RkISP1, Error)\n> \t\t\t<< \"Failed to start statistics \" << camera->name();\n> @@ -787,12 +801,14 @@ int PipelineHandlerRkISP1::start(Camera *camera)\n> \tif (ret) {\n> \t\tparam_->streamOff();\n> \t\tstat_->streamOff();\n> +\t\tdata->ipa_->stop();\n> \t\tfreeBuffers(camera);\n> \n> \t\tLOG(RkISP1, Error)\n> \t\t\t<< \"Failed to start camera \" << camera->name();\n> \t}\n> \n> +\tdata->running_ = true;\n> \tactiveCamera_ = camera;\n> \n> \t/* Inform IPA of stream configuration and sensor controls. */\n> @@ -830,6 +846,10 @@ void PipelineHandlerRkISP1::stop(Camera *camera)\n> \t\tLOG(RkISP1, Warning)\n> \t\t\t<< \"Failed to stop parameters \" << camera->name();\n> \n> +\tdata->running_ = false;\n> +\n> +\tdata->ipa_->stop();\n> +\n> \tdata->timeline_.reset();\n> \n> \tfreeBuffers(camera);","headers":{"Return-Path":"","Received":["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 EF19260412\n\tfor ;\n\tThu, 26 Mar 2020 22:27:38 +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 71F132DC;\n\tThu, 26 Mar 2020 22:27:38 +0100 (CET)"],"Authentication-Results":"lancelot.ideasonboard.com; dkim=pass (1024-bit key; \n\tunprotected) header.d=ideasonboard.com\n\theader.i=@ideasonboard.com\n\theader.b=\"rRxq9K72\"; dkim-atps=neutral","DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com;\n\ts=mail; t=1585258058;\n\tbh=5vnNB+Dlux93eL46j26TqPtmAhxpJKNfErv6pjKT+vo=;\n\th=Date:From:To:Cc:Subject:References:In-Reply-To:From;\n\tb=rRxq9K72HlEF7d0JFlwok9+hDTDoANMHrcViHbz6Mwew7p8Cn769jcKCE4T4N/kq2\n\th8PQqafLhkK2Bn1w80AADGzMHfLFUu87SsPQxWzKP6/v/BQVMRwCq8SjG7Bi6isut+\n\tT0Vqh5MRakjpFS4tfj553MWs4QQYlPZcQV8+3Tr0=","Date":"Thu, 26 Mar 2020 23:27:34 +0200","From":"Laurent Pinchart ","To":"Niklas =?utf-8?q?S=C3=B6derlund?= ","Cc":"libcamera-devel@lists.libcamera.org","Message-ID":"<20200326212734.GC20581@pendragon.ideasonboard.com>","References":"<20200326160819.4088361-1-niklas.soderlund@ragnatech.se>\n\t<20200326160819.4088361-6-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":"<20200326160819.4088361-6-niklas.soderlund@ragnatech.se>","User-Agent":"Mutt/1.10.1 (2018-07-13)","Subject":"Re: [libcamera-devel] [RFCv2 5/7] libcamera: pipeline: rkisp1: Call\n\tIPA start() and stop()","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":"Thu, 26 Mar 2020 21:27:39 -0000"}}]