Patch Detail
Show a patch.
GET /api/patches/9848/?format=api
{ "id": 9848, "url": "https://patchwork.libcamera.org/api/patches/9848/?format=api", "web_url": "https://patchwork.libcamera.org/patch/9848/", "project": { "id": 1, "url": "https://patchwork.libcamera.org/api/projects/1/?format=api", "name": "libcamera", "link_name": "libcamera", "list_id": "libcamera_core", "list_email": "libcamera-devel@lists.libcamera.org", "web_url": "", "scm_url": "", "webscm_url": "" }, "msgid": "<20200929014334.49719-4-niklas.soderlund@ragnatech.se>", "date": "2020-09-29T01:43:30", "name": "[libcamera-devel,v4,3/7] libcamera: pipeline: rkisp1: Move path configuration generation and validation to RkISP1Path", "commit_ref": null, "pull_url": null, "state": "accepted", "archived": false, "hash": "94800edd2e5631c9492c75899c9586569b225009", "submitter": { "id": 5, "url": "https://patchwork.libcamera.org/api/people/5/?format=api", "name": "Niklas Söderlund", "email": "niklas.soderlund@ragnatech.se" }, "delegate": null, "mbox": "https://patchwork.libcamera.org/patch/9848/mbox/", "series": [ { "id": 1333, "url": "https://patchwork.libcamera.org/api/series/1333/?format=api", "web_url": "https://patchwork.libcamera.org/project/libcamera/list/?series=1333", "date": "2020-09-29T01:43:27", "name": "libcamera: pipeline: rkisp1: Create RkISP1Path helper", "version": 4, "mbox": "https://patchwork.libcamera.org/series/1333/mbox/" } ], "comments": "https://patchwork.libcamera.org/api/patches/9848/comments/", "check": "pending", "checks": "https://patchwork.libcamera.org/api/patches/9848/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 B25D8C3B5B\n\tfor <parsemail@patchwork.libcamera.org>;\n\tTue, 29 Sep 2020 01:43:54 +0000 (UTC)", "from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id 3DA6362140;\n\tTue, 29 Sep 2020 03:43:54 +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 31C6D60BD4\n\tfor <libcamera-devel@lists.libcamera.org>;\n\tTue, 29 Sep 2020 03:43:52 +0200 (CEST)", "from bismarck.berto.se (p54ac52a8.dip0.t-ipconnect.de\n\t[84.172.82.168]) by bin-vsp-out-03.atm.binero.net (Halon) with ESMTPA\n\tid 37995844-01f5-11eb-b295-0050569116f7;\n\tTue, 29 Sep 2020 03:43:47 +0200 (CEST)" ], "X-Halon-ID": "37995844-01f5-11eb-b295-0050569116f7", "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": "Tue, 29 Sep 2020 03:43:30 +0200", "Message-Id": "<20200929014334.49719-4-niklas.soderlund@ragnatech.se>", "X-Mailer": "git-send-email 2.28.0", "In-Reply-To": "<20200929014334.49719-1-niklas.soderlund@ragnatech.se>", "References": "<20200929014334.49719-1-niklas.soderlund@ragnatech.se>", "MIME-Version": "1.0", "Subject": "[libcamera-devel] [PATCH v4 3/7] libcamera: pipeline: rkisp1: Move\n\tpath configuration generation and validation to RkISP1Path", "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": "Move the path configuration generation and validation to RkISP1Path.\nThis is done to increase code reuse and to encapsulate the main and self\npath differences inside the RkISP1Path class. There is no functional\nchange.\n\nSigned-off-by: Niklas Söderlund <niklas.soderlund@ragnatech.se>\nReviewed-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>\n---\n* Changes since v3\n- Include headers instead of forward declare Size and PixelFormat.\n- Keep array of PixelFormats in Span.\n---\n src/libcamera/pipeline/rkisp1/rkisp1.cpp | 122 ++----------------\n src/libcamera/pipeline/rkisp1/rkisp1_path.cpp | 94 +++++++++++++-\n src/libcamera/pipeline/rkisp1/rkisp1_path.h | 19 ++-\n 3 files changed, 119 insertions(+), 116 deletions(-)", "diff": "diff --git a/src/libcamera/pipeline/rkisp1/rkisp1.cpp b/src/libcamera/pipeline/rkisp1/rkisp1.cpp\nindex 13e0666d80050b51..6c81faa818e2b11a 100644\n--- a/src/libcamera/pipeline/rkisp1/rkisp1.cpp\n+++ b/src/libcamera/pipeline/rkisp1/rkisp1.cpp\n@@ -20,7 +20,6 @@\n #include <libcamera/formats.h>\n #include <libcamera/ipa/rkisp1.h>\n #include <libcamera/request.h>\n-#include <libcamera/span.h>\n #include <libcamera/stream.h>\n \n #include \"libcamera/internal/camera_sensor.h\"\n@@ -40,34 +39,6 @@ namespace libcamera {\n \n LOG_DEFINE_CATEGORY(RkISP1)\n \n-namespace {\n-constexpr Size RKISP1_RSZ_MP_SRC_MIN{ 32, 16 };\n-constexpr Size RKISP1_RSZ_MP_SRC_MAX{ 4416, 3312 };\n-constexpr std::array<PixelFormat, 7> RKISP1_RSZ_MP_FORMATS{\n-\tformats::YUYV,\n-\tformats::YVYU,\n-\tformats::VYUY,\n-\tformats::NV16,\n-\tformats::NV61,\n-\tformats::NV21,\n-\tformats::NV12,\n-\t/* \\todo Add support for 8-bit greyscale to DRM formats */\n-};\n-\n-constexpr Size RKISP1_RSZ_SP_SRC_MIN{ 32, 16 };\n-constexpr Size RKISP1_RSZ_SP_SRC_MAX{ 1920, 1920 };\n-constexpr std::array<PixelFormat, 7> RKISP1_RSZ_SP_FORMATS{\n-\tformats::YUYV,\n-\tformats::YVYU,\n-\tformats::VYUY,\n-\tformats::NV16,\n-\tformats::NV61,\n-\tformats::NV21,\n-\tformats::NV12,\n-\t/* \\todo Add support for BGR888 and RGB565 */\n-};\n-} /* namespace */\n-\n class PipelineHandlerRkISP1;\n class RkISP1ActionQueueBuffers;\n class RkISP1CameraData;\n@@ -194,14 +165,6 @@ public:\n \tconst V4L2SubdeviceFormat &sensorFormat() { return sensorFormat_; }\n \n private:\n-\tstatic constexpr unsigned int RKISP1_BUFFER_COUNT = 4;\n-\n-\tCameraConfiguration::Status validatePath(StreamConfiguration *cfg,\n-\t\t\t\t\t\t const Span<const 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 \tbool fitsAllPaths(const StreamConfiguration &cfg);\n \n \t/*\n@@ -514,64 +477,16 @@ RkISP1CameraConfiguration::RkISP1CameraConfiguration(Camera *camera,\n \tdata_ = data;\n }\n \n-CameraConfiguration::Status RkISP1CameraConfiguration::validatePath(\n-\tStreamConfiguration *cfg, const Span<const 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_->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_->selfPath_->video_);\n-}\n-\n bool RkISP1CameraConfiguration::fitsAllPaths(const StreamConfiguration &cfg)\n {\n \tStreamConfiguration config;\n \n \tconfig = cfg;\n-\tif (validateMainPath(&config) != Valid)\n+\tif (data_->mainPath_->validate(&config) != Valid)\n \t\treturn false;\n \n \tconfig = cfg;\n-\tif (validateSelfPath(&config) != Valid)\n+\tif (data_->selfPath_->validate(&config) != Valid)\n \t\treturn false;\n \n \treturn true;\n@@ -611,7 +526,7 @@ CameraConfiguration::Status RkISP1CameraConfiguration::validate()\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\tif (data_->mainPath_->validate(&tryCfg) == Valid) {\n \t\t\t\tmainPathAvailable = false;\n \t\t\t\tcfg = tryCfg;\n \t\t\t\tcfg.setStream(const_cast<Stream *>(&data_->mainPathStream_));\n@@ -621,7 +536,7 @@ CameraConfiguration::Status RkISP1CameraConfiguration::validate()\n \n \t\tif (selfPathAvailable) {\n \t\t\tStreamConfiguration tryCfg = cfg;\n-\t\t\tif (validateSelfPath(&tryCfg) == Valid) {\n+\t\t\tif (data_->selfPath_->validate(&tryCfg) == Valid) {\n \t\t\t\tselfPathAvailable = false;\n \t\t\t\tcfg = tryCfg;\n \t\t\t\tcfg.setStream(const_cast<Stream *>(&data_->selfPathStream_));\n@@ -632,7 +547,7 @@ CameraConfiguration::Status RkISP1CameraConfiguration::validate()\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\tif (data_->mainPath_->validate(&tryCfg) == Adjusted) {\n \t\t\t\tmainPathAvailable = false;\n \t\t\t\tcfg = tryCfg;\n \t\t\t\tcfg.setStream(const_cast<Stream *>(&data_->mainPathStream_));\n@@ -643,7 +558,7 @@ CameraConfiguration::Status RkISP1CameraConfiguration::validate()\n \n \t\tif (selfPathAvailable) {\n \t\t\tStreamConfiguration tryCfg = cfg;\n-\t\t\tif (validateSelfPath(&tryCfg) == Adjusted) {\n+\t\t\tif (data_->selfPath_->validate(&tryCfg) == Adjusted) {\n \t\t\t\tselfPathAvailable = false;\n \t\t\t\tcfg = tryCfg;\n \t\t\t\tcfg.setStream(const_cast<Stream *>(&data_->selfPathStream_));\n@@ -729,32 +644,17 @@ CameraConfiguration *PipelineHandlerRkISP1::generateConfiguration(Camera *camera\n \t\t\treturn nullptr;\n \t\t}\n \n-\t\tstd::map<PixelFormat, std::vector<SizeRange>> streamFormats;\n-\t\tSize maxResolution = data->sensor_->resolution();\n+\t\tStreamConfiguration cfg;\n \t\tif (useMainPath) {\n+\t\t\tcfg = data->mainPath_->generateConfiguration(\n+\t\t\t\tdata->sensor_->resolution());\n \t\t\tmainPathAvailable = false;\n-\t\t\tmaxResolution.boundTo(RKISP1_RSZ_MP_SRC_MAX);\n-\t\t\tfor (const PixelFormat &format : RKISP1_RSZ_MP_FORMATS)\n-\t\t\t\tstreamFormats[format] = { {\n-\t\t\t\t\tRKISP1_RSZ_MP_SRC_MIN,\n-\t\t\t\t\tmaxResolution,\n-\t\t\t\t} };\n \t\t} else {\n+\t\t\tcfg = data->selfPath_->generateConfiguration(\n+\t\t\t\tdata->sensor_->resolution());\n \t\t\tselfPathAvailable = false;\n-\t\t\tmaxResolution.boundTo(RKISP1_RSZ_SP_SRC_MAX);\n-\t\t\tfor (const PixelFormat &format : RKISP1_RSZ_SP_FORMATS)\n-\t\t\t\tstreamFormats[format] = { {\n-\t\t\t\t\tRKISP1_RSZ_SP_SRC_MIN,\n-\t\t\t\t\tmaxResolution,\n-\t\t\t\t} };\n \t\t}\n \n-\t\tStreamFormats formats(streamFormats);\n-\t\tStreamConfiguration cfg(formats);\n-\t\tcfg.pixelFormat = formats::NV12;\n-\t\tcfg.size = maxResolution;\n-\t\tcfg.bufferCount = 4;\n-\n \t\tconfig->addConfiguration(cfg);\n \t}\n \ndiff --git a/src/libcamera/pipeline/rkisp1/rkisp1_path.cpp b/src/libcamera/pipeline/rkisp1/rkisp1_path.cpp\nindex 66b5eebdcb804e24..96c18b352472a417 100644\n--- a/src/libcamera/pipeline/rkisp1/rkisp1_path.cpp\n+++ b/src/libcamera/pipeline/rkisp1/rkisp1_path.cpp\n@@ -7,6 +7,7 @@\n \n #include \"rkisp1_path.h\"\n \n+#include <libcamera/formats.h>\n #include <libcamera/stream.h>\n \n #include \"libcamera/internal/media_device.h\"\n@@ -17,8 +18,11 @@ namespace libcamera {\n \n LOG_DECLARE_CATEGORY(RkISP1)\n \n-RkISP1Path::RkISP1Path(const char *name)\n-\t: video_(nullptr), name_(name), resizer_(nullptr)\n+RkISP1Path::RkISP1Path(const char *name, const Span<const PixelFormat> &formats,\n+\t\t const Size &minResolution, const Size &maxResolution)\n+\t: video_(nullptr), name_(name), formats_(formats),\n+\t minResolution_(minResolution), maxResolution_(maxResolution),\n+\t resizer_(nullptr)\n {\n }\n \n@@ -44,6 +48,58 @@ bool RkISP1Path::init(MediaDevice *media)\n \treturn true;\n }\n \n+StreamConfiguration RkISP1Path::generateConfiguration(const Size &resolution)\n+{\n+\tSize maxResolution = resolution;\n+\tmaxResolution.boundTo(maxResolution_);\n+\n+\tstd::map<PixelFormat, std::vector<SizeRange>> streamFormats;\n+\tfor (const PixelFormat &format : formats_)\n+\t\tstreamFormats[format] = { { minResolution_, maxResolution } };\n+\n+\tStreamFormats formats(streamFormats);\n+\tStreamConfiguration cfg(formats);\n+\tcfg.pixelFormat = formats::NV12;\n+\tcfg.size = maxResolution;\n+\tcfg.bufferCount = RKISP1_BUFFER_COUNT;\n+\n+\treturn cfg;\n+}\n+\n+CameraConfiguration::Status RkISP1Path::validate(StreamConfiguration *cfg)\n+{\n+\tconst StreamConfiguration reqCfg = *cfg;\n+\tCameraConfiguration::Status status = CameraConfiguration::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(maxResolution_);\n+\tcfg->size.expandTo(minResolution_);\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 CameraConfiguration::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 = CameraConfiguration::Adjusted;\n+\t}\n+\n+\treturn status;\n+}\n+\n int RkISP1Path::configure(const StreamConfiguration &config,\n \t\t\t const V4L2SubdeviceFormat &inputFormat)\n {\n@@ -93,13 +149,43 @@ int RkISP1Path::configure(const StreamConfiguration &config,\n \treturn 0;\n }\n \n+namespace {\n+constexpr Size RKISP1_RSZ_MP_SRC_MIN{ 32, 16 };\n+constexpr Size RKISP1_RSZ_MP_SRC_MAX{ 4416, 3312 };\n+constexpr std::array<PixelFormat, 7> RKISP1_RSZ_MP_FORMATS{\n+\tformats::YUYV,\n+\tformats::YVYU,\n+\tformats::VYUY,\n+\tformats::NV16,\n+\tformats::NV61,\n+\tformats::NV21,\n+\tformats::NV12,\n+\t/* \\todo Add support for 8-bit greyscale to DRM formats */\n+};\n+\n+constexpr Size RKISP1_RSZ_SP_SRC_MIN{ 32, 16 };\n+constexpr Size RKISP1_RSZ_SP_SRC_MAX{ 1920, 1920 };\n+constexpr std::array<PixelFormat, 7> RKISP1_RSZ_SP_FORMATS{\n+\tformats::YUYV,\n+\tformats::YVYU,\n+\tformats::VYUY,\n+\tformats::NV16,\n+\tformats::NV61,\n+\tformats::NV21,\n+\tformats::NV12,\n+\t/* \\todo Add support for BGR888 and RGB565 */\n+};\n+} /* namespace */\n+\n RkISP1MainPath::RkISP1MainPath()\n-\t: RkISP1Path(\"main\")\n+\t: RkISP1Path(\"main\", RKISP1_RSZ_MP_FORMATS,\n+\t\t RKISP1_RSZ_MP_SRC_MIN, RKISP1_RSZ_MP_SRC_MAX)\n {\n }\n \n RkISP1SelfPath::RkISP1SelfPath()\n-\t: RkISP1Path(\"self\")\n+\t: RkISP1Path(\"self\", RKISP1_RSZ_SP_FORMATS,\n+\t\t RKISP1_RSZ_SP_SRC_MIN, RKISP1_RSZ_SP_SRC_MAX)\n {\n }\n \ndiff --git a/src/libcamera/pipeline/rkisp1/rkisp1_path.h b/src/libcamera/pipeline/rkisp1/rkisp1_path.h\nindex 6eb01529d2fddb1c..7433ad03387dce0b 100644\n--- a/src/libcamera/pipeline/rkisp1/rkisp1_path.h\n+++ b/src/libcamera/pipeline/rkisp1/rkisp1_path.h\n@@ -7,6 +7,13 @@\n #ifndef __LIBCAMERA_PIPELINE_RKISP1_PATH_H__\n #define __LIBCAMERA_PIPELINE_RKISP1_PATH_H__\n \n+#include <vector>\n+\n+#include <libcamera/camera.h>\n+#include <libcamera/geometry.h>\n+#include <libcamera/pixel_format.h>\n+#include <libcamera/span.h>\n+\n namespace libcamera {\n \n class MediaDevice;\n@@ -18,11 +25,15 @@ struct V4L2SubdeviceFormat;\n class RkISP1Path\n {\n public:\n-\tRkISP1Path(const char *name);\n+\tRkISP1Path(const char *name, const Span<const PixelFormat> &formats,\n+\t\t const Size &minResolution, const Size &maxResolution);\n \t~RkISP1Path();\n \n \tbool init(MediaDevice *media);\n \n+\tStreamConfiguration generateConfiguration(const Size &resolution);\n+\tCameraConfiguration::Status validate(StreamConfiguration *cfg);\n+\n \tint configure(const StreamConfiguration &config,\n \t\t const V4L2SubdeviceFormat &inputFormat);\n \n@@ -30,8 +41,14 @@ public:\n \tV4L2VideoDevice *video_;\n \n private:\n+\tstatic constexpr unsigned int RKISP1_BUFFER_COUNT = 4;\n+\n \tconst char *name_;\n \n+\tconst Span<const PixelFormat> formats_;\n+\tconst Size minResolution_;\n+\tconst Size maxResolution_;\n+\n \tV4L2Subdevice *resizer_;\n };\n \n", "prefixes": [ "libcamera-devel", "v4", "3/7" ] }