{"id":8450,"url":"https://patchwork.libcamera.org/api/patches/8450/?format=json","web_url":"https://patchwork.libcamera.org/patch/8450/","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":"<20200627030043.2088585-9-niklas.soderlund@ragnatech.se>","date":"2020-06-27T03:00:38","name":"[libcamera-devel,08/13] libcamera: ipu3: imgu: Use specific functions to configure each sink","commit_ref":null,"pull_url":null,"state":"superseded","archived":false,"hash":"6fe976a9132c7b761c577fcdf5bc387ea93a0946","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/8450/mbox/","series":[{"id":1046,"url":"https://patchwork.libcamera.org/api/series/1046/?format=json","web_url":"https://patchwork.libcamera.org/project/libcamera/list/?series=1046","date":"2020-06-27T03:00:30","name":"libcamera: ipu3: Refactoring of ImgU","version":1,"mbox":"https://patchwork.libcamera.org/series/1046/mbox/"}],"comments":"https://patchwork.libcamera.org/api/patches/8450/comments/","check":"pending","checks":"https://patchwork.libcamera.org/api/patches/8450/checks/","tags":{},"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 06EF0C2E66\n\tfor ;\n\tSat, 27 Jun 2020 03:01:00 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id B22E160B03;\n\tSat, 27 Jun 2020 05:00:59 +0200 (CEST)","from bin-mail-out-05.binero.net (bin-mail-out-05.binero.net\n\t[195.74.38.228])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTPS id 31EAD609C8\n\tfor ;\n\tSat, 27 Jun 2020 05:00:56 +0200 (CEST)","from bismarck.berto.se (p4fca2eca.dip0.t-ipconnect.de\n\t[79.202.46.202]) by bin-vsp-out-03.atm.binero.net (Halon) with ESMTPA\n\tid 578059bb-b822-11ea-86ee-0050569116f7;\n\tSat, 27 Jun 2020 05:00:22 +0200 (CEST)"],"X-Halon-ID":"578059bb-b822-11ea-86ee-0050569116f7","Authorized-sender":"niklas@soderlund.pp.se","From":"=?utf-8?q?Niklas_S=C3=B6derlund?= ","To":"libcamera-devel@lists.libcamera.org","Date":"Sat, 27 Jun 2020 05:00:38 +0200","Message-Id":"<20200627030043.2088585-9-niklas.soderlund@ragnatech.se>","X-Mailer":"git-send-email 2.27.0","In-Reply-To":"<20200627030043.2088585-1-niklas.soderlund@ragnatech.se>","References":"<20200627030043.2088585-1-niklas.soderlund@ragnatech.se>","MIME-Version":"1.0","Subject":"[libcamera-devel] [PATCH 08/13] libcamera: ipu3: imgu: Use specific\n\tfunctions to configure each sink","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-Type":"text/plain; charset=\"utf-8\"","Content-Transfer-Encoding":"base64","Errors-To":"libcamera-devel-bounces@lists.libcamera.org","Sender":"\"libcamera-devel\" "},"content":"When the IPU3 pipeline only provided streams to applications that came\nfrom the ImgU it made sens to have a generic function to configure all\nthe different outputs. With the addition of the RAW stream this begins\nto be cumbersome to read and make sens of in the PipelineHandlerIPU3\ncode. Replace the generic function that takes a specific argument for\nwhich sink to configure with a specific function for each sink.\n\nThis makes the code easier to follow as it's always clear which of the\nImgU sinks are being configured without knowing the content of a\ngenerically named variable. It also paves way for future improvements.\n\nSigned-off-by: Niklas Söderlund \n---\n src/libcamera/pipeline/ipu3/imgu.cpp | 34 ++++++++++++++++---------\n src/libcamera/pipeline/ipu3/imgu.h | 11 +++++++-\n src/libcamera/pipeline/ipu3/ipu3.cpp | 38 +++++++++++++++-------------\n 3 files changed, 53 insertions(+), 30 deletions(-)","diff":"diff --git a/src/libcamera/pipeline/ipu3/imgu.cpp b/src/libcamera/pipeline/ipu3/imgu.cpp\nindex 5d11539605f26303..d54e844d9bfc5147 100644\n--- a/src/libcamera/pipeline/ipu3/imgu.cpp\n+++ b/src/libcamera/pipeline/ipu3/imgu.cpp\n@@ -139,19 +139,28 @@ int ImgUDevice::configureInput(const Size &size,\n \treturn 0;\n }\n \n-/**\n- * \\brief Configure the ImgU unit \\a id video output\n- * \\param[in] output The ImgU output device to configure\n- * \\param[in] cfg The requested configuration\n- * \\return 0 on success or a negative error code otherwise\n- */\n-int ImgUDevice::configureOutput(ImgUOutput *output,\n-\t\t\t\tconst StreamConfiguration &cfg,\n+int ImgUDevice::configureOutput(const StreamConfiguration &cfg,\n \t\t\t\tV4L2DeviceFormat *outputFormat)\n {\n-\tV4L2VideoDevice *dev = output->dev;\n-\tunsigned int pad = output->pad;\n+\treturn configureVideoDevice(output_.dev, PAD_OUTPUT, cfg, outputFormat);\n+}\n \n+int ImgUDevice::configureViewfinder(const StreamConfiguration &cfg,\n+\t\t\t\t V4L2DeviceFormat *outputFormat)\n+{\n+\treturn configureVideoDevice(viewfinder_.dev, PAD_VF, cfg, outputFormat);\n+}\n+\n+int ImgUDevice::configureStat(const StreamConfiguration &cfg,\n+\t\t\t V4L2DeviceFormat *outputFormat)\n+{\n+\treturn configureVideoDevice(stat_.dev, PAD_STAT, cfg, outputFormat);\n+}\n+\n+int ImgUDevice::configureVideoDevice(V4L2VideoDevice *dev, unsigned int pad,\n+\t\t\t\t const StreamConfiguration &cfg,\n+\t\t\t\t V4L2DeviceFormat *outputFormat)\n+{\n \tV4L2SubdeviceFormat imguFormat = {};\n \timguFormat.mbus_code = MEDIA_BUS_FMT_FIXED;\n \timguFormat.size = cfg.size;\n@@ -161,7 +170,7 @@ int ImgUDevice::configureOutput(ImgUOutput *output,\n \t\treturn ret;\n \n \t/* No need to apply format to the stat node. */\n-\tif (output == &stat_)\n+\tif (dev == stat_.dev)\n \t\treturn 0;\n \n \t*outputFormat = {};\n@@ -173,7 +182,8 @@ int ImgUDevice::configureOutput(ImgUOutput *output,\n \tif (ret)\n \t\treturn ret;\n \n-\tLOG(IPU3, Debug) << \"ImgU \" << output->name << \" format = \"\n+\tconst char *name = dev == output_.dev ? \"output\" : \"viewfinder\";\n+\tLOG(IPU3, Debug) << \"ImgU \" << name << \" format = \"\n \t\t\t << outputFormat->toString();\n \n \treturn 0;\ndiff --git a/src/libcamera/pipeline/ipu3/imgu.h b/src/libcamera/pipeline/ipu3/imgu.h\nindex b6b08b4fef2d3a9d..f8fd54089753b40e 100644\n--- a/src/libcamera/pipeline/ipu3/imgu.h\n+++ b/src/libcamera/pipeline/ipu3/imgu.h\n@@ -49,9 +49,14 @@ public:\n \t}\n \n \tint init(MediaDevice *media, unsigned int index);\n+\n \tint configureInput(const Size &size, V4L2DeviceFormat *inputFormat);\n-\tint configureOutput(ImgUOutput *output, const StreamConfiguration &cfg,\n+\tint configureOutput(const StreamConfiguration &cfg,\n \t\t\t V4L2DeviceFormat *outputFormat);\n+\tint configureViewfinder(const StreamConfiguration &cfg,\n+\t\t\t\tV4L2DeviceFormat *outputFormat);\n+\tint configureStat(const StreamConfiguration &cfg,\n+\t\t\t V4L2DeviceFormat *outputFormat);\n \n \tint allocateBuffers(unsigned int bufferCount);\n \tvoid freeBuffers();\n@@ -78,6 +83,10 @@ private:\n \t\t const std::string &sink, unsigned int sinkPad,\n \t\t bool enable);\n \n+\tint configureVideoDevice(V4L2VideoDevice *dev, unsigned int pad,\n+\t\t\t\t const StreamConfiguration &cfg,\n+\t\t\t\t V4L2DeviceFormat *outputFormat);\n+\n \tstd::string name_;\n \tMediaDevice *media_;\n };\ndiff --git a/src/libcamera/pipeline/ipu3/ipu3.cpp b/src/libcamera/pipeline/ipu3/ipu3.cpp\nindex b41a789e8dc2a7b2..ae7a01b81dacf498 100644\n--- a/src/libcamera/pipeline/ipu3/ipu3.cpp\n+++ b/src/libcamera/pipeline/ipu3/ipu3.cpp\n@@ -504,19 +504,25 @@ int PipelineHandlerIPU3::configure(Camera *camera, CameraConfiguration *c)\n \t\tstream->active_ = true;\n \t\tcfg.setStream(stream);\n \n-\t\t/*\n-\t\t * The RAW still capture stream just copies buffers from the\n-\t\t * internal queue and doesn't need any specific configuration.\n-\t\t */\n-\t\tif (stream->raw_) {\n+\t\tif (stream == outStream) {\n+\t\t\tret = imgu->configureOutput(cfg, &outputFormat);\n+\t\t\tif (ret)\n+\t\t\t\treturn ret;\n+\n+\t\t\tcfg.stride = outputFormat.planes[0].bpl;\n+\t\t} else if (stream == vfStream) {\n+\t\t\tret = imgu->configureViewfinder(cfg, &outputFormat);\n+\t\t\tif (ret)\n+\t\t\t\treturn ret;\n+\n+\t\t\tcfg.stride = outputFormat.planes[0].bpl;\n+\t\t} else {\n+\t\t\t/*\n+\t\t\t * The RAW still capture stream just copies buffers from\n+\t\t\t * the internal queue and doesn't need any specific\n+\t\t\t * configuration.\n+\t\t\t */\n \t\t\tcfg.stride = cio2Format.planes[0].bpl;\n-\t\t} else {\n-\t\t\tret = imgu->configureOutput(stream->device_, cfg,\n-\t\t\t\t\t\t &outputFormat);\n-\t\t\tif (ret)\n-\t\t\t\treturn ret;\n-\n-\t\t\tcfg.stride = outputFormat.planes[0].bpl;\n \t\t}\n \t}\n \n@@ -526,15 +532,13 @@ int PipelineHandlerIPU3::configure(Camera *camera, CameraConfiguration *c)\n \t * be at least one active stream in the configuration request).\n \t */\n \tif (!outStream->active_) {\n-\t\tret = imgu->configureOutput(outStream->device_, config->at(0),\n-\t\t\t\t\t &outputFormat);\n+\t\tret = imgu->configureOutput(config->at(0), &outputFormat);\n \t\tif (ret)\n \t\t\treturn ret;\n \t}\n \n \tif (!vfStream->active_) {\n-\t\tret = imgu->configureOutput(vfStream->device_, config->at(0),\n-\t\t\t\t\t &outputFormat);\n+\t\tret = imgu->configureViewfinder(config->at(0), &outputFormat);\n \t\tif (ret)\n \t\t\treturn ret;\n \t}\n@@ -546,7 +550,7 @@ int PipelineHandlerIPU3::configure(Camera *camera, CameraConfiguration *c)\n \tStreamConfiguration statCfg = {};\n \tstatCfg.size = cio2Format.size;\n \n-\tret = imgu->configureOutput(&imgu->stat_, statCfg, &outputFormat);\n+\tret = imgu->configureStat(statCfg, &outputFormat);\n \tif (ret)\n \t\treturn ret;\n \n","prefixes":["libcamera-devel","08/13"]}