[{"id":12820,"web_url":"https://patchwork.libcamera.org/comment/12820/","msgid":"<20200928191707.GN23539@pendragon.ideasonboard.com>","date":"2020-09-28T19:17:07","subject":"Re: [libcamera-devel] [PATCH v3 11/22] libcamera: pipeline: rkisp1:\n\tConfigure self path","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, Sep 25, 2020 at 03:41:56AM +0200, Niklas Söderlund wrote:\n> Allow for both the main and self path streams to be configured. This\n> change adds the self path as an internal stream to the pipeline handler.\n> It is not exposed as a Camera stream so it can not yet be used.\n> \n> Signed-off-by: Niklas Söderlund <niklas.soderlund@ragnatech.se>\n\nReviewed-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>\n\n> ---\n> * Changes since v2\n> - Print which resizer is configured\n> - Use if ...; else if; ... instead of if ...; if ...; in\n>   exportFrameBuffers()\n> - Configure main and self streams with separate numbers of buffers while\n>   still allocating the max of the two for stats and params.\n> - Improve Warning logs in stop()`\n> ---\n>  src/libcamera/pipeline/rkisp1/rkisp1.cpp | 227 ++++++++++++++++-------\n>  1 file changed, 161 insertions(+), 66 deletions(-)\n> \n> diff --git a/src/libcamera/pipeline/rkisp1/rkisp1.cpp b/src/libcamera/pipeline/rkisp1/rkisp1.cpp\n> index fef4fa813190056e..28e99129498e4a0a 100644\n> --- a/src/libcamera/pipeline/rkisp1/rkisp1.cpp\n> +++ b/src/libcamera/pipeline/rkisp1/rkisp1.cpp\n> @@ -135,7 +135,8 @@ public:\n>  \t\t\t V4L2VideoDevice *selfPathVideo)\n>  \t\t: CameraData(pipe), sensor_(nullptr), frame_(0),\n>  \t\t  frameInfo_(pipe), mainPathVideo_(mainPathVideo),\n> -\t\t  selfPathVideo_(selfPathVideo)\n> +\t\t  selfPathVideo_(selfPathVideo), mainPathActive_(false),\n> +\t\t  selfPathActive_(false)\n>  \t{\n>  \t}\n>  \n> @@ -147,6 +148,7 @@ public:\n>  \tint loadIPA();\n>  \n>  \tStream mainPathStream_;\n> +\tStream selfPathStream_;\n>  \tCameraSensor *sensor_;\n>  \tunsigned int frame_;\n>  \tstd::vector<IPABuffer> ipaBuffers_;\n> @@ -156,6 +158,9 @@ public:\n>  \tV4L2VideoDevice *mainPathVideo_;\n>  \tV4L2VideoDevice *selfPathVideo_;\n>  \n> +\tbool mainPathActive_;\n> +\tbool selfPathActive_;\n> +\n>  private:\n>  \tvoid queueFrameAction(unsigned int frame,\n>  \t\t\t      const IPAOperationData &action);\n> @@ -612,7 +617,6 @@ int PipelineHandlerRkISP1::configure(Camera *camera, CameraConfiguration *c)\n>  \tRkISP1CameraConfiguration *config =\n>  \t\tstatic_cast<RkISP1CameraConfiguration *>(c);\n>  \tRkISP1CameraData *data = cameraData(camera);\n> -\tStreamConfiguration &cfg = config->at(0);\n>  \tCameraSensor *sensor = data->sensor_;\n>  \tint ret;\n>  \n> @@ -654,37 +658,63 @@ int PipelineHandlerRkISP1::configure(Camera *camera, CameraConfiguration *c)\n>  \n>  \tLOG(RkISP1, Debug) << \"ISP output pad configured with \" << format.toString();\n>  \n> -\tret = mainPathResizer_->setFormat(0, &format);\n> -\tif (ret < 0)\n> -\t\treturn ret;\n> -\n> -\tLOG(RkISP1, Debug) << \"Resizer input pad configured with \" << format.toString();\n> -\n> -\tformat.size = cfg.size;\n> -\n> -\tLOG(RkISP1, Debug) << \"Configuring resizer output pad with \" << format.toString();\n> -\n> -\tret = mainPathResizer_->setFormat(1, &format);\n> -\tif (ret < 0)\n> -\t\treturn ret;\n> -\n> -\tLOG(RkISP1, Debug) << \"Resizer output pad configured with \" << format.toString();\n> -\n> -\tconst PixelFormatInfo &info = PixelFormatInfo::info(cfg.pixelFormat);\n> -\tV4L2DeviceFormat outputFormat = {};\n> -\toutputFormat.fourcc = mainPathVideo_->toV4L2PixelFormat(cfg.pixelFormat);\n> -\toutputFormat.size = cfg.size;\n> -\toutputFormat.planesCount = info.numPlanes();\n> -\n> -\tret = mainPathVideo_->setFormat(&outputFormat);\n> -\tif (ret)\n> -\t\treturn ret;\n> -\n> -\tif (outputFormat.size != cfg.size ||\n> -\t    outputFormat.fourcc != mainPathVideo_->toV4L2PixelFormat(cfg.pixelFormat)) {\n> -\t\tLOG(RkISP1, Error)\n> -\t\t\t<< \"Unable to configure capture in \" << cfg.toString();\n> -\t\treturn -EINVAL;\n> +\tdata->mainPathActive_ = false;\n> +\tdata->selfPathActive_ = false;\n> +\tfor (const StreamConfiguration &cfg : *config) {\n> +\t\tV4L2SubdeviceFormat ispFormat = format;\n> +\t\tV4L2Subdevice *resizer;\n> +\t\tV4L2VideoDevice *video;\n> +\n> +\t\tif (cfg.stream() == &data->mainPathStream_) {\n> +\t\t\tresizer = mainPathResizer_;\n> +\t\t\tvideo = mainPathVideo_;\n> +\t\t\tdata->mainPathActive_ = true;\n> +\t\t} else {\n> +\t\t\tresizer = selfPathResizer_;\n> +\t\t\tvideo = selfPathVideo_;\n> +\t\t\tdata->selfPathActive_ = true;\n> +\t\t}\n> +\n> +\t\tret = resizer->setFormat(0, &ispFormat);\n> +\t\tif (ret < 0)\n> +\t\t\treturn ret;\n> +\n> +\t\tconst char *name = resizer == mainPathResizer_ ? \"main\" : \"self\";\n> +\n> +\t\tLOG(RkISP1, Debug)\n> +\t\t\t<< \"Configured \" << name << \" resizer input pad with \"\n> +\t\t\t<< ispFormat.toString();\n> +\n> +\t\tispFormat.size = cfg.size;\n> +\n> +\t\tLOG(RkISP1, Debug)\n> +\t\t\t<< \"Configuring \" << name << \" resizer output pad with \"\n> +\t\t\t<< ispFormat.toString();\n> +\n> +\t\tret = resizer->setFormat(1, &ispFormat);\n> +\t\tif (ret < 0)\n> +\t\t\treturn ret;\n> +\n> +\t\tLOG(RkISP1, Debug)\n> +\t\t\t<< \"Configured \" << name << \" resizer output pad with \"\n> +\t\t\t<< ispFormat.toString();\n> +\n> +\t\tconst PixelFormatInfo &info = PixelFormatInfo::info(cfg.pixelFormat);\n> +\t\tV4L2DeviceFormat outputFormat = {};\n> +\t\toutputFormat.fourcc = video->toV4L2PixelFormat(cfg.pixelFormat);\n> +\t\toutputFormat.size = cfg.size;\n> +\t\toutputFormat.planesCount = info.numPlanes();\n> +\n> +\t\tret = video->setFormat(&outputFormat);\n> +\t\tif (ret)\n> +\t\t\treturn ret;\n> +\n> +\t\tif (outputFormat.size != cfg.size ||\n> +\t\t    outputFormat.fourcc != video->toV4L2PixelFormat(cfg.pixelFormat)) {\n> +\t\t\tLOG(RkISP1, Error)\n> +\t\t\t\t<< \"Unable to configure capture in \" << cfg.toString();\n> +\t\t\treturn -EINVAL;\n> +\t\t}\n>  \t}\n>  \n>  \tV4L2DeviceFormat paramFormat = {};\n> @@ -699,34 +729,53 @@ int PipelineHandlerRkISP1::configure(Camera *camera, CameraConfiguration *c)\n>  \tif (ret)\n>  \t\treturn ret;\n>  \n> -\tcfg.setStream(&data->mainPathStream_);\n> -\n>  \treturn 0;\n>  }\n>  \n>  int PipelineHandlerRkISP1::exportFrameBuffers([[maybe_unused]] Camera *camera, Stream *stream,\n>  \t\t\t\t\t      std::vector<std::unique_ptr<FrameBuffer>> *buffers)\n>  {\n> +\tRkISP1CameraData *data = cameraData(camera);\n>  \tunsigned int count = stream->configuration().bufferCount;\n> -\treturn mainPathVideo_->exportBuffers(count, buffers);\n> +\n> +\tif (stream == &data->mainPathStream_)\n> +\t\treturn mainPathVideo_->exportBuffers(count, buffers);\n> +\telse if (stream == &data->selfPathStream_)\n> +\t\treturn selfPathVideo_->exportBuffers(count, buffers);\n> +\n> +\treturn -EINVAL;\n>  }\n>  \n>  int PipelineHandlerRkISP1::allocateBuffers(Camera *camera)\n>  {\n>  \tRkISP1CameraData *data = cameraData(camera);\n> -\tunsigned int count = data->mainPathStream_.configuration().bufferCount;\n>  \tunsigned int ipaBufferId = 1;\n>  \tint ret;\n>  \n> -\tret = mainPathVideo_->importBuffers(count);\n> -\tif (ret < 0)\n> -\t\tgoto error;\n> +\tunsigned int maxCount = std::max({\n> +\t\tdata->mainPathStream_.configuration().bufferCount,\n> +\t\tdata->selfPathStream_.configuration().bufferCount,\n> +\t});\n> +\n> +\tif (data->mainPathActive_) {\n> +\t\tret = mainPathVideo_->importBuffers(\n> +\t\t\tdata->mainPathStream_.configuration().bufferCount);\n> +\t\tif (ret < 0)\n> +\t\t\tgoto error;\n> +\t}\n> +\n> +\tif (data->selfPathActive_) {\n> +\t\tret = selfPathVideo_->importBuffers(\n> +\t\t\tdata->selfPathStream_.configuration().bufferCount);\n> +\t\tif (ret < 0)\n> +\t\t\tgoto error;\n> +\t}\n>  \n> -\tret = param_->allocateBuffers(count, &paramBuffers_);\n> +\tret = param_->allocateBuffers(maxCount, &paramBuffers_);\n>  \tif (ret < 0)\n>  \t\tgoto error;\n>  \n> -\tret = stat_->allocateBuffers(count, &statBuffers_);\n> +\tret = stat_->allocateBuffers(maxCount, &statBuffers_);\n>  \tif (ret < 0)\n>  \t\tgoto error;\n>  \n> @@ -752,6 +801,7 @@ error:\n>  \tparamBuffers_.clear();\n>  \tstatBuffers_.clear();\n>  \tmainPathVideo_->releaseBuffers();\n> +\tselfPathVideo_->releaseBuffers();\n>  \n>  \treturn ret;\n>  }\n> @@ -785,6 +835,9 @@ int PipelineHandlerRkISP1::freeBuffers(Camera *camera)\n>  \tif (mainPathVideo_->releaseBuffers())\n>  \t\tLOG(RkISP1, Error) << \"Failed to release main path buffers\";\n>  \n> +\tif (selfPathVideo_->releaseBuffers())\n> +\t\tLOG(RkISP1, Error) << \"Failed to release self path buffers\";\n> +\n>  \treturn 0;\n>  }\n>  \n> @@ -827,15 +880,47 @@ int PipelineHandlerRkISP1::start(Camera *camera)\n>  \t\treturn ret;\n>  \t}\n>  \n> -\tret = mainPathVideo_->streamOn();\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->id();\n> +\tstd::map<unsigned int, IPAStream> streamConfig;\n> +\n> +\tif (data->mainPathActive_) {\n> +\t\tret = mainPathVideo_->streamOn();\n> +\t\tif (ret) {\n> +\t\t\tparam_->streamOff();\n> +\t\t\tstat_->streamOff();\n> +\t\t\tdata->ipa_->stop();\n> +\t\t\tfreeBuffers(camera);\n> +\n> +\t\t\tLOG(RkISP1, Error)\n> +\t\t\t\t<< \"Failed to start main path \" << camera->id();\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->selfPathActive_) {\n> +\t\tret = selfPathVideo_->streamOn();\n> +\t\tif (ret) {\n> +\t\t\tif (data->mainPathActive_)\n> +\t\t\t\tmainPathVideo_->streamOff();\n> +\n> +\t\t\tparam_->streamOff();\n> +\t\t\tstat_->streamOff();\n> +\t\t\tdata->ipa_->stop();\n> +\t\t\tfreeBuffers(camera);\n> +\n> +\t\t\tLOG(RkISP1, Error)\n> +\t\t\t\t<< \"Failed to start self path \" << camera->id();\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>  \tactiveCamera_ = camera;\n> @@ -850,12 +935,6 @@ int PipelineHandlerRkISP1::start(Camera *camera)\n>  \t\tret = 0;\n>  \t}\n>  \n> -\tstd::map<unsigned int, IPAStream> streamConfig;\n> -\tstreamConfig[0] = {\n> -\t\t.pixelFormat = data->mainPathStream_.configuration().pixelFormat,\n> -\t\t.size = data->mainPathStream_.configuration().size,\n> -\t};\n> -\n>  \tstd::map<unsigned int, const ControlInfoMap &> entityControls;\n>  \tentityControls.emplace(0, data->sensor_->controls());\n>  \n> @@ -871,20 +950,31 @@ void PipelineHandlerRkISP1::stop(Camera *camera)\n>  \tRkISP1CameraData *data = cameraData(camera);\n>  \tint ret;\n>  \n> -\tret = mainPathVideo_->streamOff();\n> -\tif (ret)\n> -\t\tLOG(RkISP1, Warning)\n> -\t\t\t<< \"Failed to stop camera \" << camera->id();\n> +\tif (data->selfPathActive_) {\n> +\t\tret = selfPathVideo_->streamOff();\n> +\t\tif (ret)\n> +\t\t\tLOG(RkISP1, Warning)\n> +\t\t\t\t<< \"Failed to stop self path for \"\n> +\t\t\t\t<< camera->id();\n> +\t}\n> +\n> +\tif (data->mainPathActive_) {\n> +\t\tret = mainPathVideo_->streamOff();\n> +\t\tif (ret)\n> +\t\t\tLOG(RkISP1, Warning)\n> +\t\t\t\t<< \"Failed to stop main path for \"\n> +\t\t\t\t<< camera->id();\n> +\t}\n>  \n>  \tret = stat_->streamOff();\n>  \tif (ret)\n>  \t\tLOG(RkISP1, Warning)\n> -\t\t\t<< \"Failed to stop statistics \" << camera->id();\n> +\t\t\t<< \"Failed to stop statistics for \" << camera->id();\n>  \n>  \tret = param_->streamOff();\n>  \tif (ret)\n>  \t\tLOG(RkISP1, Warning)\n> -\t\t\t<< \"Failed to stop parameters \" << camera->id();\n> +\t\t\t<< \"Failed to stop parameters for \" << camera->id();\n>  \n>  \tdata->ipa_->stop();\n>  \n> @@ -955,11 +1045,16 @@ int PipelineHandlerRkISP1::initLinks(const Camera *camera,\n>  \t}\n>  \n>  \tfor (const StreamConfiguration &cfg : config) {\n> -\t\tif (cfg.stream() != &data->mainPathStream_)\n> +\t\tMediaLink *link;\n> +\t\tif (cfg.stream() == &data->mainPathStream_)\n> +\t\t\tlink = media_->link(\"rkisp1_isp\", 2,\n> +\t\t\t\t\t    \"rkisp1_resizer_mainpath\", 0);\n> +\t\telse if (cfg.stream() == &data->selfPathStream_)\n> +\t\t\tlink = media_->link(\"rkisp1_isp\", 2,\n> +\t\t\t\t\t    \"rkisp1_resizer_selfpath\", 0);\n> +\t\telse\n>  \t\t\treturn -EINVAL;\n>  \n> -\t\tMediaLink *link = media_->link(\"rkisp1_isp\", 2,\n> -\t\t\t\t\t       \"rkisp1_resizer_mainpath\", 0);\n>  \t\tif (!link)\n>  \t\t\treturn -ENODEV;\n>","headers":{"Return-Path":"<libcamera-devel-bounces@lists.libcamera.org>","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 462D9C3B5C\n\tfor <parsemail@patchwork.libcamera.org>;\n\tMon, 28 Sep 2020 19:17:44 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id C73A26138F;\n\tMon, 28 Sep 2020 21:17:43 +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 AE84860366\n\tfor <libcamera-devel@lists.libcamera.org>;\n\tMon, 28 Sep 2020 21:17:42 +0200 (CEST)","from pendragon.ideasonboard.com (62-78-145-57.bb.dnainternet.fi\n\t[62.78.145.57])\n\tby perceval.ideasonboard.com (Postfix) with ESMTPSA id 27901A58;\n\tMon, 28 Sep 2020 21:17:42 +0200 (CEST)"],"Authentication-Results":"lancelot.ideasonboard.com;\n\tdkim=fail reason=\"signature verification failed\" (1024-bit key;\n\tunprotected) header.d=ideasonboard.com header.i=@ideasonboard.com\n\theader.b=\"GX/7DdtD\"; dkim-atps=neutral","DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com;\n\ts=mail; t=1601320662;\n\tbh=gpqqNfJircXct/JNIIVI3BfK95xWsJOPk7Zdkq1OHgY=;\n\th=Date:From:To:Cc:Subject:References:In-Reply-To:From;\n\tb=GX/7DdtDt+gMRvGmt8wKqVZnaeBKfcrD3qnJOBcEy85ibMEdLnGemzC/19OJI22g3\n\t0OeBNnQPQ5mhvti7EdLftNCRoh2ydtmowDjogc7tHzYP0DL36++UwWnZjsAsvy0+i3\n\tp1hQMWxxFTbP8btSFk1ifdwNtHdY+O/gV6WGEUA0=","Date":"Mon, 28 Sep 2020 22:17:07 +0300","From":"Laurent Pinchart <laurent.pinchart@ideasonboard.com>","To":"Niklas =?utf-8?q?S=C3=B6derlund?= <niklas.soderlund@ragnatech.se>","Message-ID":"<20200928191707.GN23539@pendragon.ideasonboard.com>","References":"<20200925014207.1455796-1-niklas.soderlund@ragnatech.se>\n\t<20200925014207.1455796-12-niklas.soderlund@ragnatech.se>","MIME-Version":"1.0","Content-Disposition":"inline","In-Reply-To":"<20200925014207.1455796-12-niklas.soderlund@ragnatech.se>","Subject":"Re: [libcamera-devel] [PATCH v3 11/22] libcamera: pipeline: rkisp1:\n\tConfigure self path","X-BeenThere":"libcamera-devel@lists.libcamera.org","X-Mailman-Version":"2.1.29","Precedence":"list","List-Id":"<libcamera-devel.lists.libcamera.org>","List-Unsubscribe":"<https://lists.libcamera.org/options/libcamera-devel>,\n\t<mailto:libcamera-devel-request@lists.libcamera.org?subject=unsubscribe>","List-Archive":"<https://lists.libcamera.org/pipermail/libcamera-devel/>","List-Post":"<mailto:libcamera-devel@lists.libcamera.org>","List-Help":"<mailto:libcamera-devel-request@lists.libcamera.org?subject=help>","List-Subscribe":"<https://lists.libcamera.org/listinfo/libcamera-devel>,\n\t<mailto:libcamera-devel-request@lists.libcamera.org?subject=subscribe>","Cc":"libcamera-devel@lists.libcamera.org","Content-Type":"text/plain; charset=\"utf-8\"","Content-Transfer-Encoding":"base64","Errors-To":"libcamera-devel-bounces@lists.libcamera.org","Sender":"\"libcamera-devel\" <libcamera-devel-bounces@lists.libcamera.org>"}}]