{"id":9604,"url":"https://patchwork.libcamera.org/api/patches/9604/?format=json","web_url":"https://patchwork.libcamera.org/patch/9604/","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":"<20200914142149.63857-13-niklas.soderlund@ragnatech.se>","date":"2020-09-14T14:21:48","name":"[libcamera-devel,v2,12/13] libcamera: pipeline: rkisp1: Add format validation for self path","commit_ref":null,"pull_url":null,"state":"accepted","archived":false,"hash":"498d1d3c9621f087ac69727cd8abdc9b180237fc","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/9604/mbox/","series":[{"id":1289,"url":"https://patchwork.libcamera.org/api/series/1289/?format=json","web_url":"https://patchwork.libcamera.org/project/libcamera/list/?series=1289","date":"2020-09-14T14:21:36","name":"libcamera: pipeline: rkisp1: Extend to support two streams","version":2,"mbox":"https://patchwork.libcamera.org/series/1289/mbox/"}],"comments":"https://patchwork.libcamera.org/api/patches/9604/comments/","check":"pending","checks":"https://patchwork.libcamera.org/api/patches/9604/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 E3C33BF01C\n\tfor <parsemail@patchwork.libcamera.org>;\n\tMon, 14 Sep 2020 14:22:18 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id BFCD162E1F;\n\tMon, 14 Sep 2020 16:22:18 +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 B073D62E1B\n\tfor <libcamera-devel@lists.libcamera.org>;\n\tMon, 14 Sep 2020 16:22:15 +0200 (CEST)","from bismarck.berto.se (p54ac52a8.dip0.t-ipconnect.de\n\t[84.172.82.168]) by bin-vsp-out-02.atm.binero.net (Halon) with ESMTPA\n\tid af8e5a68-f695-11ea-a39b-005056917f90;\n\tMon, 14 Sep 2020 16:22:14 +0200 (CEST)"],"X-Halon-ID":"af8e5a68-f695-11ea-a39b-005056917f90","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":"Mon, 14 Sep 2020 16:21:48 +0200","Message-Id":"<20200914142149.63857-13-niklas.soderlund@ragnatech.se>","X-Mailer":"git-send-email 2.28.0","In-Reply-To":"<20200914142149.63857-1-niklas.soderlund@ragnatech.se>","References":"<20200914142149.63857-1-niklas.soderlund@ragnatech.se>","MIME-Version":"1.0","Subject":"[libcamera-devel] [PATCH v2 12/13] libcamera: pipeline: rkisp1: Add\n\tformat validation for 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>","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":"Extend the format validation to work with both man and self path. The\nheuristics honors that the first stream in the configuration have the\nhighest priority while still examining both streams for a best match.\n\nThis change extends the formats supported by the Cameras backed by this\npipeline to also include RGB888 and RGB656, that is only available on\nthe self path.\n\nIt is not possible to capture from the self path as the self stream is\nnot yet exposed to applications.\n\nSigned-off-by: Niklas Söderlund <niklas.soderlund@ragnatech.se>\n---\n* Changes since v1\n- Store formats in std::vector instead of std::array to avoid template\n  usage for validate function.\n---\n src/libcamera/pipeline/rkisp1/rkisp1.cpp | 222 +++++++++++++++++------\n 1 file changed, 171 insertions(+), 51 deletions(-)","diff":"diff --git a/src/libcamera/pipeline/rkisp1/rkisp1.cpp b/src/libcamera/pipeline/rkisp1/rkisp1.cpp\nindex bc961f8e78f2c979..851ff68f138b98dd 100644\n--- a/src/libcamera/pipeline/rkisp1/rkisp1.cpp\n+++ b/src/libcamera/pipeline/rkisp1/rkisp1.cpp\n@@ -9,6 +9,7 @@\n #include <array>\n #include <iomanip>\n #include <memory>\n+#include <numeric>\n #include <queue>\n \n #include <linux/media-bus-format.h>\n@@ -40,7 +41,7 @@ LOG_DEFINE_CATEGORY(RkISP1)\n namespace {\n \tconstexpr Size RKISP1_RSZ_MP_SRC_MIN { 32, 16 };\n \tconstexpr Size RKISP1_RSZ_MP_SRC_MAX { 4416, 3312 };\n-\tconstexpr std::array<PixelFormat, 7> RKISP1_RSZ_MP_FORMATS{\n+\tconst std::vector<PixelFormat> RKISP1_RSZ_MP_FORMATS{\n \t\tformats::YUYV,\n \t\tformats::YVYU,\n \t\tformats::VYUY,\n@@ -50,7 +51,21 @@ namespace {\n \t\tformats::NV12,\n \t\t/* \\todo Add support for 8-bit greyscale to DRM formats */\n \t};\n-}\n+\n+\tconstexpr Size RKISP1_RSZ_SP_SRC_MIN { 32, 16 };\n+\tconstexpr Size RKISP1_RSZ_SP_SRC_MAX { 1920, 1920 };\n+\tconst std::vector<PixelFormat> RKISP1_RSZ_SP_FORMATS{\n+\t\tformats::YUYV,\n+\t\tformats::YVYU,\n+\t\tformats::VYUY,\n+\t\tformats::NV16,\n+\t\tformats::NV61,\n+\t\tformats::NV21,\n+\t\tformats::NV12,\n+\t\tformats::BGR888,\n+\t\tformats::RGB565,\n+\t};\n+};\n \n class PipelineHandlerRkISP1;\n class RkISP1ActionQueueBuffers;\n@@ -181,13 +196,22 @@ public:\n private:\n \tstatic constexpr unsigned int RKISP1_BUFFER_COUNT = 4;\n \n+\tbool fitAnyPath(const StreamConfiguration &cfg);\n+\n+\tCameraConfiguration::Status validatePath(StreamConfiguration *cfg,\n+\t\t\t\t\t\t const std::vector<PixelFormat> &formats,\n+\t\t\t\t\t\t const Size &min, const Size &max,\n+\t\t\t\t\t\t V4L2VideoDevice *video);\n+\tCameraConfiguration::Status validateMainPath(StreamConfiguration *cfg);\n+\tCameraConfiguration::Status validateSelfPath(StreamConfiguration *cfg);\n+\n \t/*\n \t * The RkISP1CameraData instance is guaranteed to be valid as long as the\n \t * corresponding Camera instance is valid. In order to borrow a\n \t * reference to the camera data, store a new reference to the camera.\n \t */\n \tstd::shared_ptr<Camera> camera_;\n-\tconst RkISP1CameraData *data_;\n+\tRkISP1CameraData *data_;\n \n \tV4L2SubdeviceFormat sensorFormat_;\n };\n@@ -497,6 +521,75 @@ RkISP1CameraConfiguration::RkISP1CameraConfiguration(Camera *camera,\n \tdata_ = data;\n }\n \n+bool RkISP1CameraConfiguration::fitAnyPath(const StreamConfiguration &cfg)\n+{\n+\tif (std::find(RKISP1_RSZ_MP_FORMATS.begin(),\n+\t\t      RKISP1_RSZ_MP_FORMATS.end(), cfg.pixelFormat) ==\n+\t    RKISP1_RSZ_MP_FORMATS.end())\n+\t\treturn false;\n+\n+\tif (cfg.size < RKISP1_RSZ_MP_SRC_MIN || cfg.size > RKISP1_RSZ_MP_SRC_MAX)\n+\t\treturn false;\n+\n+\tif (std::find(RKISP1_RSZ_SP_FORMATS.begin(),\n+\t\t      RKISP1_RSZ_SP_FORMATS.end(), cfg.pixelFormat) ==\n+\t    RKISP1_RSZ_SP_FORMATS.end())\n+\t\treturn false;\n+\n+\tif (cfg.size < RKISP1_RSZ_SP_SRC_MIN || cfg.size > RKISP1_RSZ_SP_SRC_MAX)\n+\t\treturn false;\n+\n+\treturn true;\n+}\n+\n+CameraConfiguration::Status RkISP1CameraConfiguration::validatePath(\n+\tStreamConfiguration *cfg, const std::vector<PixelFormat> &formats,\n+\tconst Size &min, const Size &max, V4L2VideoDevice *video)\n+{\n+\tconst StreamConfiguration reqCfg = *cfg;\n+\tStatus status = Valid;\n+\n+\tif (std::find(formats.begin(), formats.end(), cfg->pixelFormat) ==\n+\t    formats.end())\n+\t\tcfg->pixelFormat = formats::NV12;\n+\n+\tcfg->size.boundTo(max);\n+\tcfg->size.expandTo(min);\n+\tcfg->bufferCount = RKISP1_BUFFER_COUNT;\n+\n+\tV4L2DeviceFormat format = {};\n+\tformat.fourcc = video->toV4L2PixelFormat(cfg->pixelFormat);\n+\tformat.size = cfg->size;\n+\n+\tint ret = video->tryFormat(&format);\n+\tif (ret)\n+\t\treturn Invalid;\n+\n+\tcfg->stride = format.planes[0].bpl;\n+\tcfg->frameSize = format.planes[0].size;\n+\n+\tif (cfg->pixelFormat != reqCfg.pixelFormat || cfg->size != reqCfg.size) {\n+\t\tLOG(RkISP1, Debug)\n+\t\t\t<< \"Adjusting format from \" << reqCfg.toString()\n+\t\t\t<< \" to \" << cfg->toString();\n+\t\tstatus = Adjusted;\n+\t}\n+\n+\treturn status;\n+}\n+\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+}\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+}\n+\n CameraConfiguration::Status RkISP1CameraConfiguration::validate()\n {\n \tconst CameraSensor *sensor = data_->sensor_;\n@@ -506,22 +599,86 @@ CameraConfiguration::Status RkISP1CameraConfiguration::validate()\n \t\treturn Invalid;\n \n \t/* Cap the number of entries to the available streams. */\n-\tif (config_.size() > 1) {\n-\t\tconfig_.resize(1);\n+\tif (config_.size() > 2) {\n+\t\tconfig_.resize(2);\n \t\tstatus = Adjusted;\n \t}\n \n-\tStreamConfiguration &cfg = config_[0];\n-\n-\t/* Adjust the pixel format. */\n-\tif (std::find(RKISP1_RSZ_MP_FORMATS.begin(), RKISP1_RSZ_MP_FORMATS.end(),\n-\t\t      cfg.pixelFormat) == RKISP1_RSZ_MP_FORMATS.end()) {\n-\t\tLOG(RkISP1, Debug) << \"Adjusting format to NV12\";\n-\t\tcfg.pixelFormat = formats::NV12,\n-\t\tstatus = Adjusted;\n+\t/*\n+\t * If there are more then one stream in the configuration figure out the\n+\t * order to evaluate them streams. The first stream have the highest\n+\t * priority but if both main path and self path can satisfy it evaluate\n+\t * second stream first.\n+\t */\n+\tstd::vector<unsigned int> order(config_.size());\n+\tstd::iota(order.begin(), order.end(), 0);\n+\tif (config_.size() == 2 && fitAnyPath(config_[0]))\n+\t\tstd::reverse(order.begin(), order.end());\n+\n+\tbool mainPathAvailable = true;\n+\tbool selfPathAvailable = true;\n+\tfor (unsigned int index : order) {\n+\t\tStreamConfiguration &cfg = config_[index];\n+\n+\t\t/* Try to match stream without adjusting configuration. */\n+\t\tif (mainPathAvailable) {\n+\t\t\tStreamConfiguration tryCfg = cfg;\n+\t\t\tif (validateMainPath(&tryCfg) == Valid) {\n+\t\t\t\tmainPathAvailable = false;\n+\t\t\t\tcfg = tryCfg;\n+\t\t\t\tcfg.setStream(&data_->mainPathStream_);\n+\t\t\t\tLOG(RkISP1, Debug) << \"Exact match main\";\n+\t\t\t\tcontinue;\n+\t\t\t}\n+\t\t}\n+\n+\t\tif (selfPathAvailable) {\n+\t\t\tStreamConfiguration tryCfg = cfg;\n+\t\t\tif (validateSelfPath(&tryCfg) == Valid) {\n+\t\t\t\tselfPathAvailable = false;\n+\t\t\t\tcfg = tryCfg;\n+\t\t\t\tcfg.setStream(&data_->selfPathStream_);\n+\t\t\t\tLOG(RkISP1, Debug) << \"Exact match self\";\n+\t\t\t\tcontinue;\n+\t\t\t}\n+\t\t}\n+\n+\t\t/* Try to match stream allowing adjusting configuration. */\n+\t\tif (mainPathAvailable) {\n+\t\t\tStreamConfiguration tryCfg = cfg;\n+\t\t\tif (validateMainPath(&tryCfg) == Adjusted) {\n+\t\t\t\tmainPathAvailable = false;\n+\t\t\t\tcfg = tryCfg;\n+\t\t\t\tcfg.setStream(&data_->mainPathStream_);\n+\t\t\t\tstatus = Adjusted;\n+\t\t\t\tLOG(RkISP1, Debug) << \"Adjust match main\";\n+\t\t\t\tcontinue;\n+\t\t\t}\n+\t\t}\n+\n+\t\tif (selfPathAvailable) {\n+\t\t\tStreamConfiguration tryCfg = cfg;\n+\t\t\tif (validateSelfPath(&tryCfg) == Adjusted) {\n+\t\t\t\tselfPathAvailable = false;\n+\t\t\t\tcfg = tryCfg;\n+\t\t\t\tcfg.setStream(&data_->selfPathStream_);\n+\t\t\t\tstatus = Adjusted;\n+\t\t\t\tLOG(RkISP1, Debug) << \"Adjust match self\";\n+\t\t\t\tcontinue;\n+\t\t\t}\n+\t\t}\n+\n+\t\t/* All paths rejected configuraiton. */\n+\t\tLOG(RkISP1, Debug) << \"Camera configuration not supported \"\n+\t\t\t\t   << cfg.toString();\n+\t\treturn Invalid;\n \t}\n \n \t/* Select the sensor format. */\n+\tSize maxSize;\n+\tfor (const StreamConfiguration &cfg : config_)\n+\t\tmaxSize = std::max(maxSize, cfg.size);\n+\n \tsensorFormat_ = sensor->getFormat({ MEDIA_BUS_FMT_SBGGR12_1X12,\n \t\t\t\t\t    MEDIA_BUS_FMT_SGBRG12_1X12,\n \t\t\t\t\t    MEDIA_BUS_FMT_SGRBG12_1X12,\n@@ -534,47 +691,10 @@ CameraConfiguration::Status RkISP1CameraConfiguration::validate()\n \t\t\t\t\t    MEDIA_BUS_FMT_SGBRG8_1X8,\n \t\t\t\t\t    MEDIA_BUS_FMT_SGRBG8_1X8,\n \t\t\t\t\t    MEDIA_BUS_FMT_SRGGB8_1X8 },\n-\t\t\t\t\t  cfg.size);\n+\t\t\t\t\t  maxSize);\n \tif (sensorFormat_.size.isNull())\n \t\tsensorFormat_.size = sensor->resolution();\n \n-\t/*\n-\t * Provide a suitable default that matches the sensor aspect\n-\t * ratio and clamp the size to the hardware bounds.\n-\t *\n-\t * \\todo: Check the hardware alignment constraints.\n-\t */\n-\tconst Size size = cfg.size;\n-\n-\tif (cfg.size.isNull()) {\n-\t\tcfg.size.width = 1280;\n-\t\tcfg.size.height = 1280 * sensorFormat_.size.height\n-\t\t\t\t/ sensorFormat_.size.width;\n-\t}\n-\n-\tcfg.size.boundTo(RKISP1_RSZ_MP_SRC_MAX);\n-\tcfg.size.expandTo(RKISP1_RSZ_MP_SRC_MIN);\n-\n-\tif (cfg.size != size) {\n-\t\tLOG(RkISP1, Debug)\n-\t\t\t<< \"Adjusting size from \" << size.toString()\n-\t\t\t<< \" to \" << cfg.size.toString();\n-\t\tstatus = Adjusted;\n-\t}\n-\n-\tcfg.bufferCount = RKISP1_BUFFER_COUNT;\n-\n-\tV4L2DeviceFormat format = {};\n-\tformat.fourcc = data_->mainPathVideo_->toV4L2PixelFormat(cfg.pixelFormat);\n-\tformat.size = cfg.size;\n-\n-\tint ret = data_->mainPathVideo_->tryFormat(&format);\n-\tif (ret)\n-\t\treturn Invalid;\n-\n-\tcfg.stride = format.planes[0].bpl;\n-\tcfg.frameSize = format.planes[0].size;\n-\n \treturn status;\n }\n \n","prefixes":["libcamera-devel","v2","12/13"]}