{"id":9813,"url":"https://patchwork.libcamera.org/api/patches/9813/?format=json","web_url":"https://patchwork.libcamera.org/patch/9813/","project":{"id":1,"url":"https://patchwork.libcamera.org/api/projects/1/?format=json","name":"libcamera","link_name":"libcamera","list_id":"libcamera_core","list_email":"libcamera-devel@lists.libcamera.org","web_url":"","scm_url":"","webscm_url":""},"msgid":"<20200925014207.1455796-16-niklas.soderlund@ragnatech.se>","date":"2020-09-25T01:42:00","name":"[libcamera-devel,v3,15/22] libcamera: pipeline: rkisp1: Breakout basic path handling to own class","commit_ref":null,"pull_url":null,"state":"accepted","archived":false,"hash":"f99bd59c4b800283a440324a18e770d25ec3fee2","submitter":{"id":5,"url":"https://patchwork.libcamera.org/api/people/5/?format=json","name":"Niklas Söderlund","email":"niklas.soderlund@ragnatech.se"},"delegate":null,"mbox":"https://patchwork.libcamera.org/patch/9813/mbox/","series":[{"id":1325,"url":"https://patchwork.libcamera.org/api/series/1325/?format=json","web_url":"https://patchwork.libcamera.org/project/libcamera/list/?series=1325","date":"2020-09-25T01:41:45","name":"libcamera: pipeline: rkisp1: Extend to support two streams","version":3,"mbox":"https://patchwork.libcamera.org/series/1325/mbox/"}],"comments":"https://patchwork.libcamera.org/api/patches/9813/comments/","check":"pending","checks":"https://patchwork.libcamera.org/api/patches/9813/checks/","tags":{},"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 A195BC3B5C\n\tfor <parsemail@patchwork.libcamera.org>;\n\tFri, 25 Sep 2020 01:42:59 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id 4C5096303A;\n\tFri, 25 Sep 2020 03:42:59 +0200 (CEST)","from vsp-unauthed02.binero.net (vsp-unauthed02.binero.net\n\t[195.74.38.227])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTPS id 4AD2D63044\n\tfor <libcamera-devel@lists.libcamera.org>;\n\tFri, 25 Sep 2020 03:42:53 +0200 (CEST)","from bismarck.berto.se (p54ac52a8.dip0.t-ipconnect.de\n\t[84.172.82.168]) by bin-vsp-out-01.atm.binero.net (Halon) with ESMTPA\n\tid 6c711096-fed0-11ea-92dc-005056917a89;\n\tFri, 25 Sep 2020 03:42:51 +0200 (CEST)"],"X-Halon-ID":"6c711096-fed0-11ea-92dc-005056917a89","Authorized-sender":"niklas.soderlund@fsdn.se","From":"=?utf-8?q?Niklas_S=C3=B6derlund?= <niklas.soderlund@ragnatech.se>","To":"libcamera-devel@lists.libcamera.org","Date":"Fri, 25 Sep 2020 03:42:00 +0200","Message-Id":"<20200925014207.1455796-16-niklas.soderlund@ragnatech.se>","X-Mailer":"git-send-email 2.28.0","In-Reply-To":"<20200925014207.1455796-1-niklas.soderlund@ragnatech.se>","References":"<20200925014207.1455796-1-niklas.soderlund@ragnatech.se>","MIME-Version":"1.0","Subject":"[libcamera-devel] [PATCH v3 15/22] libcamera: pipeline: rkisp1:\n\tBreakout basic path handling to own class","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>","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>"},"content":"The self and main paths are very similar and the introduction of support\nfor two paths simulating sly (paths) have made it clear their handling\ncould be abstracted in a separate class.\n\nThis is the first step to create such an class by breaking out the\ninitialization and storage of the video and subdevices. There is no\nfunctional change in this patch.\n\nSigned-off-by: Niklas Söderlund <niklas.soderlund@ragnatech.se>\n---\n src/libcamera/pipeline/rkisp1/meson.build    |   1 +\n src/libcamera/pipeline/rkisp1/rkisp1.cpp     | 104 ++++++++-----------\n src/libcamera/pipeline/rkisp1/rkisp1path.cpp |  53 ++++++++++\n src/libcamera/pipeline/rkisp1/rkisp1path.h   |  46 ++++++++\n 4 files changed, 144 insertions(+), 60 deletions(-)\n create mode 100644 src/libcamera/pipeline/rkisp1/rkisp1path.cpp\n create mode 100644 src/libcamera/pipeline/rkisp1/rkisp1path.h","diff":"diff --git a/src/libcamera/pipeline/rkisp1/meson.build b/src/libcamera/pipeline/rkisp1/meson.build\nindex 1ab3964a6db190f0..eddf795e54575956 100644\n--- a/src/libcamera/pipeline/rkisp1/meson.build\n+++ b/src/libcamera/pipeline/rkisp1/meson.build\n@@ -2,5 +2,6 @@\n \n libcamera_sources += files([\n     'rkisp1.cpp',\n+    'rkisp1path.cpp',\n     'timeline.cpp',\n ])\ndiff --git a/src/libcamera/pipeline/rkisp1/rkisp1.cpp b/src/libcamera/pipeline/rkisp1/rkisp1.cpp\nindex 27a3c44da3805c5f..e738a7eb19264d79 100644\n--- a/src/libcamera/pipeline/rkisp1/rkisp1.cpp\n+++ b/src/libcamera/pipeline/rkisp1/rkisp1.cpp\n@@ -33,6 +33,7 @@\n #include \"libcamera/internal/v4l2_subdevice.h\"\n #include \"libcamera/internal/v4l2_videodevice.h\"\n \n+#include \"rkisp1path.h\"\n #include \"timeline.h\"\n \n namespace libcamera {\n@@ -147,12 +148,11 @@ public:\n class RkISP1CameraData : public CameraData\n {\n public:\n-\tRkISP1CameraData(PipelineHandler *pipe, V4L2VideoDevice *mainPathVideo,\n-\t\t\t V4L2VideoDevice *selfPathVideo)\n+\tRkISP1CameraData(PipelineHandler *pipe, RkISP1MainPath *mainPath,\n+\t\t\t RkISP1SelfPath *selfPath)\n \t\t: CameraData(pipe), sensor_(nullptr), frame_(0),\n-\t\t  frameInfo_(pipe), mainPathVideo_(mainPathVideo),\n-\t\t  selfPathVideo_(selfPathVideo), mainPathActive_(false),\n-\t\t  selfPathActive_(false)\n+\t\t  frameInfo_(pipe), mainPath_(mainPath), selfPath_(selfPath),\n+\t\t  mainPathActive_(false), selfPathActive_(false)\n \t{\n \t}\n \n@@ -171,8 +171,8 @@ public:\n \tRkISP1Frames frameInfo_;\n \tRkISP1Timeline timeline_;\n \n-\tV4L2VideoDevice *mainPathVideo_;\n-\tV4L2VideoDevice *selfPathVideo_;\n+\tRkISP1MainPath *mainPath_;\n+\tRkISP1SelfPath *selfPath_;\n \n \tbool mainPathActive_;\n \tbool selfPathActive_;\n@@ -259,13 +259,12 @@ private:\n \n \tMediaDevice *media_;\n \tV4L2Subdevice *isp_;\n-\tV4L2Subdevice *mainPathResizer_;\n-\tV4L2Subdevice *selfPathResizer_;\n-\tV4L2VideoDevice *mainPathVideo_;\n-\tV4L2VideoDevice *selfPathVideo_;\n \tV4L2VideoDevice *param_;\n \tV4L2VideoDevice *stat_;\n \n+\tRkISP1MainPath mainPath_;\n+\tRkISP1SelfPath selfPath_;\n+\n \tstd::vector<std::unique_ptr<FrameBuffer>> paramBuffers_;\n \tstd::vector<std::unique_ptr<FrameBuffer>> statBuffers_;\n \tstd::queue<FrameBuffer *> availableParamBuffers_;\n@@ -441,10 +440,10 @@ protected:\n \t\tpipe_->stat_->queueBuffer(info->statBuffer);\n \n \t\tif (info->mainPathBuffer)\n-\t\t\tpipe_->mainPathVideo_->queueBuffer(info->mainPathBuffer);\n+\t\t\tpipe_->mainPath_.video_->queueBuffer(info->mainPathBuffer);\n \n \t\tif (info->selfPathBuffer)\n-\t\t\tpipe_->selfPathVideo_->queueBuffer(info->selfPathBuffer);\n+\t\t\tpipe_->selfPath_.video_->queueBuffer(info->selfPathBuffer);\n \t}\n \n private:\n@@ -554,13 +553,13 @@ CameraConfiguration::Status RkISP1CameraConfiguration::validatePath(\n CameraConfiguration::Status RkISP1CameraConfiguration::validateMainPath(StreamConfiguration *cfg)\n {\n \treturn validatePath(cfg, RKISP1_RSZ_MP_FORMATS, RKISP1_RSZ_MP_SRC_MIN,\n-\t\t\t    RKISP1_RSZ_MP_SRC_MAX, data_->mainPathVideo_);\n+\t\t\t    RKISP1_RSZ_MP_SRC_MAX, data_->mainPath_->video_);\n }\n \n CameraConfiguration::Status RkISP1CameraConfiguration::validateSelfPath(StreamConfiguration *cfg)\n {\n \treturn validatePath(cfg, RKISP1_RSZ_SP_FORMATS, RKISP1_RSZ_SP_SRC_MIN,\n-\t\t\t    RKISP1_RSZ_SP_SRC_MAX, data_->selfPathVideo_);\n+\t\t\t    RKISP1_RSZ_SP_SRC_MAX, data_->selfPath_->video_);\n }\n \n bool RkISP1CameraConfiguration::fitsAllPaths(const StreamConfiguration &cfg)\n@@ -688,9 +687,8 @@ CameraConfiguration::Status RkISP1CameraConfiguration::validate()\n }\n \n PipelineHandlerRkISP1::PipelineHandlerRkISP1(CameraManager *manager)\n-\t: PipelineHandler(manager), isp_(nullptr), mainPathResizer_(nullptr),\n-\t  selfPathResizer_(nullptr), mainPathVideo_(nullptr),\n-\t  selfPathVideo_(nullptr), param_(nullptr), stat_(nullptr)\n+\t: PipelineHandler(manager), isp_(nullptr), param_(nullptr),\n+\t  stat_(nullptr)\n {\n }\n \n@@ -698,10 +696,6 @@ PipelineHandlerRkISP1::~PipelineHandlerRkISP1()\n {\n \tdelete param_;\n \tdelete stat_;\n-\tdelete mainPathVideo_;\n-\tdelete selfPathVideo_;\n-\tdelete mainPathResizer_;\n-\tdelete selfPathResizer_;\n \tdelete isp_;\n }\n \n@@ -821,12 +815,12 @@ int PipelineHandlerRkISP1::configure(Camera *camera, CameraConfiguration *c)\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\tresizer = mainPath_.resizer_;\n+\t\t\tvideo = mainPath_.video_;\n \t\t\tdata->mainPathActive_ = true;\n \t\t} else {\n-\t\t\tresizer = selfPathResizer_;\n-\t\t\tvideo = selfPathVideo_;\n+\t\t\tresizer = selfPath_.resizer_;\n+\t\t\tvideo = selfPath_.video_;\n \t\t\tdata->selfPathActive_ = true;\n \t\t}\n \n@@ -834,7 +828,7 @@ int PipelineHandlerRkISP1::configure(Camera *camera, CameraConfiguration *c)\n \t\tif (ret < 0)\n \t\t\treturn ret;\n \n-\t\tconst char *name = resizer == mainPathResizer_ ? \"main\" : \"self\";\n+\t\tconst char *name = resizer == mainPath_.resizer_ ? \"main\" : \"self\";\n \n \t\tLOG(RkISP1, Debug)\n \t\t\t<< \"Configured \" << name << \" resizer input pad with \"\n@@ -894,9 +888,9 @@ int PipelineHandlerRkISP1::exportFrameBuffers([[maybe_unused]] Camera *camera, S\n \tunsigned int count = stream->configuration().bufferCount;\n \n \tif (stream == &data->mainPathStream_)\n-\t\treturn mainPathVideo_->exportBuffers(count, buffers);\n+\t\treturn mainPath_.video_->exportBuffers(count, buffers);\n \telse if (stream == &data->selfPathStream_)\n-\t\treturn selfPathVideo_->exportBuffers(count, buffers);\n+\t\treturn selfPath_.video_->exportBuffers(count, buffers);\n \n \treturn -EINVAL;\n }\n@@ -913,14 +907,14 @@ int PipelineHandlerRkISP1::allocateBuffers(Camera *camera)\n \t});\n \n \tif (data->mainPathActive_) {\n-\t\tret = mainPathVideo_->importBuffers(\n+\t\tret = mainPath_.video_->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\tret = selfPath_.video_->importBuffers(\n \t\t\tdata->selfPathStream_.configuration().bufferCount);\n \t\tif (ret < 0)\n \t\t\tgoto error;\n@@ -955,8 +949,8 @@ int PipelineHandlerRkISP1::allocateBuffers(Camera *camera)\n error:\n \tparamBuffers_.clear();\n \tstatBuffers_.clear();\n-\tmainPathVideo_->releaseBuffers();\n-\tselfPathVideo_->releaseBuffers();\n+\tmainPath_.video_->releaseBuffers();\n+\tselfPath_.video_->releaseBuffers();\n \n \treturn ret;\n }\n@@ -987,10 +981,10 @@ int PipelineHandlerRkISP1::freeBuffers(Camera *camera)\n \tif (stat_->releaseBuffers())\n \t\tLOG(RkISP1, Error) << \"Failed to release stat buffers\";\n \n-\tif (mainPathVideo_->releaseBuffers())\n+\tif (mainPath_.video_->releaseBuffers())\n \t\tLOG(RkISP1, Error) << \"Failed to release main path buffers\";\n \n-\tif (selfPathVideo_->releaseBuffers())\n+\tif (selfPath_.video_->releaseBuffers())\n \t\tLOG(RkISP1, Error) << \"Failed to release self path buffers\";\n \n \treturn 0;\n@@ -1038,7 +1032,7 @@ int PipelineHandlerRkISP1::start(Camera *camera)\n \tstd::map<unsigned int, IPAStream> streamConfig;\n \n \tif (data->mainPathActive_) {\n-\t\tret = mainPathVideo_->streamOn();\n+\t\tret = mainPath_.video_->streamOn();\n \t\tif (ret) {\n \t\t\tparam_->streamOff();\n \t\t\tstat_->streamOff();\n@@ -1057,10 +1051,10 @@ int PipelineHandlerRkISP1::start(Camera *camera)\n \t}\n \n \tif (data->selfPathActive_) {\n-\t\tret = selfPathVideo_->streamOn();\n+\t\tret = selfPath_.video_->streamOn();\n \t\tif (ret) {\n \t\t\tif (data->mainPathActive_)\n-\t\t\t\tmainPathVideo_->streamOff();\n+\t\t\t\tmainPath_.video_->streamOff();\n \n \t\t\tparam_->streamOff();\n \t\t\tstat_->streamOff();\n@@ -1106,7 +1100,7 @@ void PipelineHandlerRkISP1::stop(Camera *camera)\n \tint ret;\n \n \tif (data->selfPathActive_) {\n-\t\tret = selfPathVideo_->streamOff();\n+\t\tret = selfPath_.video_->streamOff();\n \t\tif (ret)\n \t\t\tLOG(RkISP1, Warning)\n \t\t\t\t<< \"Failed to stop self path for \"\n@@ -1114,7 +1108,7 @@ void PipelineHandlerRkISP1::stop(Camera *camera)\n \t}\n \n \tif (data->mainPathActive_) {\n-\t\tret = mainPathVideo_->streamOff();\n+\t\tret = mainPath_.video_->streamOff();\n \t\tif (ret)\n \t\t\tLOG(RkISP1, Warning)\n \t\t\t\t<< \"Failed to stop main path for \"\n@@ -1226,8 +1220,7 @@ int PipelineHandlerRkISP1::createCamera(MediaEntity *sensor)\n \tint ret;\n \n \tstd::unique_ptr<RkISP1CameraData> data =\n-\t\tstd::make_unique<RkISP1CameraData>(this, mainPathVideo_,\n-\t\t\t\t\t\t   selfPathVideo_);\n+\t\tstd::make_unique<RkISP1CameraData>(this, &mainPath_, &selfPath_);\n \n \tControlInfoMap::Map ctrls;\n \tctrls.emplace(std::piecewise_construct,\n@@ -1281,23 +1274,7 @@ bool PipelineHandlerRkISP1::match(DeviceEnumerator *enumerator)\n \tif (isp_->open() < 0)\n \t\treturn false;\n \n-\tmainPathResizer_ = V4L2Subdevice::fromEntityName(media_, \"rkisp1_resizer_mainpath\");\n-\tif (mainPathResizer_->open() < 0)\n-\t\treturn false;\n-\n-\tselfPathResizer_ = V4L2Subdevice::fromEntityName(media_, \"rkisp1_resizer_selfpath\");\n-\tif (selfPathResizer_->open() < 0)\n-\t\treturn false;\n-\n \t/* Locate and open the capture video node. */\n-\tmainPathVideo_ = V4L2VideoDevice::fromEntityName(media_, \"rkisp1_mainpath\");\n-\tif (mainPathVideo_->open() < 0)\n-\t\treturn false;\n-\n-\tselfPathVideo_ = V4L2VideoDevice::fromEntityName(media_, \"rkisp1_selfpath\");\n-\tif (selfPathVideo_->open() < 0)\n-\t\treturn false;\n-\n \tstat_ = V4L2VideoDevice::fromEntityName(media_, \"rkisp1_stats\");\n \tif (stat_->open() < 0)\n \t\treturn false;\n@@ -1306,8 +1283,15 @@ bool PipelineHandlerRkISP1::match(DeviceEnumerator *enumerator)\n \tif (param_->open() < 0)\n \t\treturn false;\n \n-\tmainPathVideo_->bufferReady.connect(this, &PipelineHandlerRkISP1::bufferReady);\n-\tselfPathVideo_->bufferReady.connect(this, &PipelineHandlerRkISP1::bufferReady);\n+\t/* Locate and open the ISP main and self paths. */\n+\tif (!mainPath_.init(media_))\n+\t\treturn false;\n+\n+\tif (!selfPath_.init(media_))\n+\t\treturn false;\n+\n+\tmainPath_.video_->bufferReady.connect(this, &PipelineHandlerRkISP1::bufferReady);\n+\tselfPath_.video_->bufferReady.connect(this, &PipelineHandlerRkISP1::bufferReady);\n \tstat_->bufferReady.connect(this, &PipelineHandlerRkISP1::statReady);\n \tparam_->bufferReady.connect(this, &PipelineHandlerRkISP1::paramReady);\n \ndiff --git a/src/libcamera/pipeline/rkisp1/rkisp1path.cpp b/src/libcamera/pipeline/rkisp1/rkisp1path.cpp\nnew file mode 100644\nindex 0000000000000000..51a75df86baaaa7b\n--- /dev/null\n+++ b/src/libcamera/pipeline/rkisp1/rkisp1path.cpp\n@@ -0,0 +1,53 @@\n+/* SPDX-License-Identifier: LGPL-2.1-or-later */\n+/*\n+ * Copyright (C) 2020, Google Inc.\n+ *\n+ * rkisp1path.cpp - Rockchip ISP1 path helper\n+ */\n+\n+#include \"rkisp1path.h\"\n+\n+#include \"libcamera/internal/media_device.h\"\n+#include \"libcamera/internal/v4l2_subdevice.h\"\n+#include \"libcamera/internal/v4l2_videodevice.h\"\n+\n+namespace libcamera {\n+\n+RkISP1Path::RkISP1Path(const char *name)\n+\t: resizer_(nullptr), video_(nullptr), name_(name)\n+{\n+}\n+\n+RkISP1Path::~RkISP1Path()\n+{\n+\tdelete video_;\n+\tdelete resizer_;\n+}\n+\n+bool RkISP1Path::init(MediaDevice *media)\n+{\n+\tstd::string resizer = std::string(\"rkisp1_resizer_\") + name_ + \"path\";\n+\tstd::string video = std::string(\"rkisp1_\") + name_ + \"path\";\n+\n+\tresizer_ = V4L2Subdevice::fromEntityName(media, resizer);\n+\tif (resizer_->open() < 0)\n+\t\treturn false;\n+\n+\tvideo_ = V4L2VideoDevice::fromEntityName(media, video);\n+\tif (video_->open() < 0)\n+\t\treturn false;\n+\n+\treturn true;\n+}\n+\n+RkISP1MainPath::RkISP1MainPath()\n+\t: RkISP1Path(\"main\")\n+{\n+}\n+\n+RkISP1SelfPath::RkISP1SelfPath()\n+\t: RkISP1Path(\"self\")\n+{\n+}\n+\n+} /* namespace libcamera */\ndiff --git a/src/libcamera/pipeline/rkisp1/rkisp1path.h b/src/libcamera/pipeline/rkisp1/rkisp1path.h\nnew file mode 100644\nindex 0000000000000000..d3172e228a3f67bf\n--- /dev/null\n+++ b/src/libcamera/pipeline/rkisp1/rkisp1path.h\n@@ -0,0 +1,46 @@\n+/* SPDX-License-Identifier: LGPL-2.1-or-later */\n+/*\n+ * Copyright (C) 2020, Google Inc.\n+ *\n+ * rkisp1path.h - Rockchip ISP1 path helper\n+ */\n+#ifndef __LIBCAMERA_PIPELINE_RKISP1_PATH_H__\n+#define __LIBCAMERA_PIPELINE_RKISP1_PATH_H__\n+\n+namespace libcamera {\n+\n+class MediaDevice;\n+class V4L2Subdevice;\n+class V4L2VideoDevice;\n+\n+class RkISP1Path\n+{\n+public:\n+\tRkISP1Path(const char *name);\n+\t~RkISP1Path();\n+\n+\tbool init(MediaDevice *media);\n+\n+\t/* \\todo Make resizer and video private. */\n+\tV4L2Subdevice *resizer_;\n+\tV4L2VideoDevice *video_;\n+\n+private:\n+\tconst char *name_;\n+};\n+\n+class RkISP1MainPath : public RkISP1Path\n+{\n+public:\n+\tRkISP1MainPath();\n+};\n+\n+class RkISP1SelfPath : public RkISP1Path\n+{\n+public:\n+\tRkISP1SelfPath();\n+};\n+\n+} /* namespace libcamera */\n+\n+#endif /* __LIBCAMERA_PIPELINE_RKISP1_PATH_H__ */\n","prefixes":["libcamera-devel","v3","15/22"]}