[{"id":12510,"web_url":"https://patchwork.libcamera.org/comment/12510/","msgid":"<20200915004827.GU15543@pendragon.ideasonboard.com>","date":"2020-09-15T00:48:27","subject":"Re: [libcamera-devel] [PATCH v2 08/13] libcamera: pipeline: rkisp1:\n\tPrefix main path video and resizer","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 Mon, Sep 14, 2020 at 04:21:44PM +0200, Niklas Söderlund wrote:\n> In preparation of supporting both the main and self path prefix the main\n> path specific variables with mainPath.\n> \n> Signed-off-by: Niklas Söderlund \n> ---\n> src/libcamera/pipeline/rkisp1/rkisp1.cpp | 82 ++++++++++++------------\n> 1 file changed, 41 insertions(+), 41 deletions(-)\n> \n> diff --git a/src/libcamera/pipeline/rkisp1/rkisp1.cpp b/src/libcamera/pipeline/rkisp1/rkisp1.cpp\n> index 45c5e40186df693d..4e1295486c184178 100644\n> --- a/src/libcamera/pipeline/rkisp1/rkisp1.cpp\n> +++ b/src/libcamera/pipeline/rkisp1/rkisp1.cpp\n> @@ -68,7 +68,7 @@ struct RkISP1FrameInfo {\n> \n> \tFrameBuffer *paramBuffer;\n> \tFrameBuffer *statBuffer;\n> -\tFrameBuffer *videoBuffer;\n> +\tFrameBuffer *mainPathBuffer;\n> \n> \tbool paramFilled;\n> \tbool paramDequeued;\n> @@ -133,7 +133,7 @@ class RkISP1CameraData : public CameraData\n> public:\n> \tRkISP1CameraData(PipelineHandler *pipe, V4L2VideoDevice *video)\n> \t\t: CameraData(pipe), sensor_(nullptr), frame_(0),\n> -\t\t frameInfo_(pipe), video_(video)\n> +\t\t frameInfo_(pipe), mainPathVideo_(video)\n> \t{\n> \t}\n> \n> @@ -144,14 +144,14 @@ public:\n> \n> \tint loadIPA();\n> \n> -\tStream stream_;\n> +\tStream mainPathStream_;\n> \tCameraSensor *sensor_;\n> \tunsigned int frame_;\n> \tstd::vector ipaBuffers_;\n> \tRkISP1Frames frameInfo_;\n> \tRkISP1Timeline timeline_;\n> \n> -\tV4L2VideoDevice *video_;\n> +\tV4L2VideoDevice *mainPathVideo_;\n> \n> private:\n> \tvoid queueFrameAction(unsigned int frame,\n> @@ -227,8 +227,8 @@ private:\n> \n> \tMediaDevice *media_;\n> \tV4L2Subdevice *isp_;\n> -\tV4L2Subdevice *resizer_;\n> -\tV4L2VideoDevice *video_;\n> +\tV4L2Subdevice *mainPathResizer_;\n> +\tV4L2VideoDevice *mainPathVideo_;\n\nWith the assumption you will follow up with a patch that creates a\nRkISP1Path (or RkISP1Stream) class, stored in the RkISP1PipelineHandler\nclass, as discussed in the review of v1,\n\nReviewed-by: Laurent Pinchart \n\n> \tV4L2VideoDevice *param_;\n> \tV4L2VideoDevice *stat_;\n> \n> @@ -261,8 +261,8 @@ RkISP1FrameInfo *RkISP1Frames::create(const RkISP1CameraData *data, Request *req\n> \t}\n> \tFrameBuffer *statBuffer = pipe_->availableStatBuffers_.front();\n> \n> -\tFrameBuffer *videoBuffer = request->findBuffer(&data->stream_);\n> -\tif (!videoBuffer) {\n> +\tFrameBuffer *mainPathBuffer = request->findBuffer(&data->mainPathStream_);\n> +\tif (!mainPathBuffer) {\n> \t\tLOG(RkISP1, Error)\n> \t\t\t<< \"Attempt to queue request with invalid stream\";\n> \t\treturn nullptr;\n> @@ -276,7 +276,7 @@ RkISP1FrameInfo *RkISP1Frames::create(const RkISP1CameraData *data, Request *req\n> \tinfo->frame = frame;\n> \tinfo->request = request;\n> \tinfo->paramBuffer = paramBuffer;\n> -\tinfo->videoBuffer = videoBuffer;\n> +\tinfo->mainPathBuffer = mainPathBuffer;\n> \tinfo->statBuffer = statBuffer;\n> \tinfo->paramFilled = false;\n> \tinfo->paramDequeued = false;\n> @@ -335,7 +335,7 @@ RkISP1FrameInfo *RkISP1Frames::find(FrameBuffer *buffer)\n> \n> \t\tif (info->paramBuffer == buffer ||\n> \t\t info->statBuffer == buffer ||\n> -\t\t info->videoBuffer == buffer)\n> +\t\t info->mainPathBuffer == buffer)\n> \t\t\treturn info;\n> \t}\n> \n> @@ -407,7 +407,7 @@ protected:\n> \n> \t\tpipe_->param_->queueBuffer(info->paramBuffer);\n> \t\tpipe_->stat_->queueBuffer(info->statBuffer);\n> -\t\tpipe_->video_->queueBuffer(info->videoBuffer);\n> +\t\tpipe_->mainPathVideo_->queueBuffer(info->mainPathBuffer);\n> \t}\n> \n> private:\n> @@ -546,10 +546,10 @@ CameraConfiguration::Status RkISP1CameraConfiguration::validate()\n> \tcfg.bufferCount = RKISP1_BUFFER_COUNT;\n> \n> \tV4L2DeviceFormat format = {};\n> -\tformat.fourcc = data_->video_->toV4L2PixelFormat(cfg.pixelFormat);\n> +\tformat.fourcc = data_->mainPathVideo_->toV4L2PixelFormat(cfg.pixelFormat);\n> \tformat.size = cfg.size;\n> \n> -\tint ret = data_->video_->tryFormat(&format);\n> +\tint ret = data_->mainPathVideo_->tryFormat(&format);\n> \tif (ret)\n> \t\treturn Invalid;\n> \n> @@ -560,8 +560,8 @@ CameraConfiguration::Status RkISP1CameraConfiguration::validate()\n> }\n> \n> PipelineHandlerRkISP1::PipelineHandlerRkISP1(CameraManager *manager)\n> -\t: PipelineHandler(manager), isp_(nullptr), resizer_(nullptr),\n> -\t video_(nullptr), param_(nullptr), stat_(nullptr)\n> +\t: PipelineHandler(manager), isp_(nullptr), mainPathResizer_(nullptr),\n> +\t mainPathVideo_(nullptr), param_(nullptr), stat_(nullptr)\n> {\n> }\n> \n> @@ -569,8 +569,8 @@ PipelineHandlerRkISP1::~PipelineHandlerRkISP1()\n> {\n> \tdelete param_;\n> \tdelete stat_;\n> -\tdelete video_;\n> -\tdelete resizer_;\n> +\tdelete mainPathVideo_;\n> +\tdelete mainPathResizer_;\n> \tdelete isp_;\n> }\n> \n> @@ -651,7 +651,7 @@ int PipelineHandlerRkISP1::configure(Camera *camera, CameraConfiguration *c)\n> \n> \tLOG(RkISP1, Debug) << \"ISP output pad configured with \" << format.toString();\n> \n> -\tret = resizer_->setFormat(0, &format);\n> +\tret = mainPathResizer_->setFormat(0, &format);\n> \tif (ret < 0)\n> \t\treturn ret;\n> \n> @@ -661,7 +661,7 @@ int PipelineHandlerRkISP1::configure(Camera *camera, CameraConfiguration *c)\n> \n> \tLOG(RkISP1, Debug) << \"Configuring resizer output pad with \" << format.toString();\n> \n> -\tret = resizer_->setFormat(1, &format);\n> +\tret = mainPathResizer_->setFormat(1, &format);\n> \tif (ret < 0)\n> \t\treturn ret;\n> \n> @@ -669,16 +669,16 @@ int PipelineHandlerRkISP1::configure(Camera *camera, CameraConfiguration *c)\n> \n> \tconst PixelFormatInfo &info = PixelFormatInfo::info(cfg.pixelFormat);\n> \tV4L2DeviceFormat outputFormat = {};\n> -\toutputFormat.fourcc = video_->toV4L2PixelFormat(cfg.pixelFormat);\n> +\toutputFormat.fourcc = mainPathVideo_->toV4L2PixelFormat(cfg.pixelFormat);\n> \toutputFormat.size = cfg.size;\n> \toutputFormat.planesCount = info.numPlanes();\n> \n> -\tret = video_->setFormat(&outputFormat);\n> +\tret = mainPathVideo_->setFormat(&outputFormat);\n> \tif (ret)\n> \t\treturn ret;\n> \n> \tif (outputFormat.size != cfg.size ||\n> -\t outputFormat.fourcc != video_->toV4L2PixelFormat(cfg.pixelFormat)) {\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> @@ -696,7 +696,7 @@ int PipelineHandlerRkISP1::configure(Camera *camera, CameraConfiguration *c)\n> \tif (ret)\n> \t\treturn ret;\n> \n> -\tcfg.setStream(&data->stream_);\n> +\tcfg.setStream(&data->mainPathStream_);\n> \n> \treturn 0;\n> }\n> @@ -705,17 +705,17 @@ int PipelineHandlerRkISP1::exportFrameBuffers([[maybe_unused]] Camera *camera, S\n> \t\t\t\t\t std::vector> *buffers)\n> {\n> \tunsigned int count = stream->configuration().bufferCount;\n> -\treturn video_->exportBuffers(count, buffers);\n> +\treturn mainPathVideo_->exportBuffers(count, buffers);\n> }\n> \n> int PipelineHandlerRkISP1::allocateBuffers(Camera *camera)\n> {\n> \tRkISP1CameraData *data = cameraData(camera);\n> -\tunsigned int count = data->stream_.configuration().bufferCount;\n> +\tunsigned int count = data->mainPathStream_.configuration().bufferCount;\n> \tunsigned int ipaBufferId = 1;\n> \tint ret;\n> \n> -\tret = video_->importBuffers(count);\n> +\tret = mainPathVideo_->importBuffers(count);\n> \tif (ret < 0)\n> \t\tgoto error;\n> \n> @@ -748,7 +748,7 @@ int PipelineHandlerRkISP1::allocateBuffers(Camera *camera)\n> error:\n> \tparamBuffers_.clear();\n> \tstatBuffers_.clear();\n> -\tvideo_->releaseBuffers();\n> +\tmainPathVideo_->releaseBuffers();\n> \n> \treturn ret;\n> }\n> @@ -779,8 +779,8 @@ int PipelineHandlerRkISP1::freeBuffers(Camera *camera)\n> \tif (stat_->releaseBuffers())\n> \t\tLOG(RkISP1, Error) << \"Failed to release stat buffers\";\n> \n> -\tif (video_->releaseBuffers())\n> -\t\tLOG(RkISP1, Error) << \"Failed to release video buffers\";\n> +\tif (mainPathVideo_->releaseBuffers())\n> +\t\tLOG(RkISP1, Error) << \"Failed to release main path buffers\";\n> \n> \treturn 0;\n> }\n> @@ -824,7 +824,7 @@ int PipelineHandlerRkISP1::start(Camera *camera)\n> \t\treturn ret;\n> \t}\n> \n> -\tret = video_->streamOn();\n> +\tret = mainPathVideo_->streamOn();\n> \tif (ret) {\n> \t\tparam_->streamOff();\n> \t\tstat_->streamOff();\n> @@ -849,8 +849,8 @@ int PipelineHandlerRkISP1::start(Camera *camera)\n> \n> \tstd::map streamConfig;\n> \tstreamConfig[0] = {\n> -\t\t.pixelFormat = data->stream_.configuration().pixelFormat,\n> -\t\t.size = data->stream_.configuration().size,\n> +\t\t.pixelFormat = data->mainPathStream_.configuration().pixelFormat,\n> +\t\t.size = data->mainPathStream_.configuration().size,\n> \t};\n> \n> \tstd::map entityControls;\n> @@ -868,7 +868,7 @@ void PipelineHandlerRkISP1::stop(Camera *camera)\n> \tRkISP1CameraData *data = cameraData(camera);\n> \tint ret;\n> \n> -\tret = video_->streamOff();\n> +\tret = mainPathVideo_->streamOff();\n> \tif (ret)\n> \t\tLOG(RkISP1, Warning)\n> \t\t\t<< \"Failed to stop camera \" << camera->id();\n> @@ -953,7 +953,7 @@ int PipelineHandlerRkISP1::initLinks(const Camera *camera,\n> \n> \tfor (const StreamConfiguration &cfg : config) {\n> \t\tMediaLink *link = nullptr;\n> -\t\tif (cfg.stream() == &data->stream_)\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\n> @@ -975,7 +975,7 @@ int PipelineHandlerRkISP1::createCamera(MediaEntity *sensor)\n> \tint ret;\n> \n> \tstd::unique_ptr data =\n> -\t\tstd::make_unique(this, video_);\n> +\t\tstd::make_unique(this, mainPathVideo_);\n> \n> \tControlInfoMap::Map ctrls;\n> \tctrls.emplace(std::piecewise_construct,\n> @@ -996,7 +996,7 @@ int PipelineHandlerRkISP1::createCamera(MediaEntity *sensor)\n> \tif (ret)\n> \t\treturn ret;\n> \n> -\tstd::set streams{ &data->stream_ };\n> +\tstd::set streams{ &data->mainPathStream_ };\n> \tstd::shared_ptr camera =\n> \t\tCamera::create(this, data->sensor_->id(), streams);\n> \tregisterCamera(std::move(camera), std::move(data));\n> @@ -1026,13 +1026,13 @@ bool PipelineHandlerRkISP1::match(DeviceEnumerator *enumerator)\n> \tif (isp_->open() < 0)\n> \t\treturn false;\n> \n> -\tresizer_ = V4L2Subdevice::fromEntityName(media_, \"rkisp1_resizer_mainpath\");\n> -\tif (resizer_->open() < 0)\n> +\tmainPathResizer_ = V4L2Subdevice::fromEntityName(media_, \"rkisp1_resizer_mainpath\");\n> +\tif (mainPathResizer_->open() < 0)\n> \t\treturn false;\n> \n> \t/* Locate and open the capture video node. */\n> -\tvideo_ = V4L2VideoDevice::fromEntityName(media_, \"rkisp1_mainpath\");\n> -\tif (video_->open() < 0)\n> +\tmainPathVideo_ = V4L2VideoDevice::fromEntityName(media_, \"rkisp1_mainpath\");\n> +\tif (mainPathVideo_->open() < 0)\n> \t\treturn false;\n> \n> \tstat_ = V4L2VideoDevice::fromEntityName(media_, \"rkisp1_stats\");\n> @@ -1043,7 +1043,7 @@ bool PipelineHandlerRkISP1::match(DeviceEnumerator *enumerator)\n> \tif (param_->open() < 0)\n> \t\treturn false;\n> \n> -\tvideo_->bufferReady.connect(this, &PipelineHandlerRkISP1::bufferReady);\n> +\tmainPathVideo_->bufferReady.connect(this, &PipelineHandlerRkISP1::bufferReady);\n> \tstat_->bufferReady.connect(this, &PipelineHandlerRkISP1::statReady);\n> \tparam_->bufferReady.connect(this, &PipelineHandlerRkISP1::paramReady);\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 14465C3B5B\n\tfor ;\n\tTue, 15 Sep 2020 00:48:59 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id 6F5B162D5B;\n\tTue, 15 Sep 2020 02:48:58 +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 2B21362901\n\tfor ;\n\tTue, 15 Sep 2020 02:48:57 +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 58DAD275;\n\tTue, 15 Sep 2020 02:48:55 +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=\"rqjXHup7\"; dkim-atps=neutral","DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com;\n\ts=mail; t=1600130936;\n\tbh=53qXJu/hxx376etB2DcGQSyBKVjOOBWna1BoGafzkUc=;\n\th=Date:From:To:Cc:Subject:References:In-Reply-To:From;\n\tb=rqjXHup7kLNl2mCX5vqIeOv1Yzjl0hk1sjsPLrDJl5b/fexYao4y4n5ljisbbXDP0\n\tTyiyfv6ReQMDcfM91tfdNMJZ1Nwx8PamzoJAzEX1ruy2swXYunrcqDwPomtH4bQF4w\n\tEKr6/0V4hp9KRlFQnZ5yAbeU2WQ/Z2EP5T7GX4f0=","Date":"Tue, 15 Sep 2020 03:48:27 +0300","From":"Laurent Pinchart ","To":"Niklas =?utf-8?q?S=C3=B6derlund?= ","Message-ID":"<20200915004827.GU15543@pendragon.ideasonboard.com>","References":"<20200914142149.63857-1-niklas.soderlund@ragnatech.se>\n\t<20200914142149.63857-9-niklas.soderlund@ragnatech.se>","MIME-Version":"1.0","Content-Disposition":"inline","In-Reply-To":"<20200914142149.63857-9-niklas.soderlund@ragnatech.se>","Subject":"Re: [libcamera-devel] [PATCH v2 08/13] libcamera: pipeline: rkisp1:\n\tPrefix main path video and resizer","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","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\" "}}]