Patch Detail
Show a patch.
GET /api/1.1/patches/22126/?format=api
{ "id": 22126, "url": "https://patchwork.libcamera.org/api/1.1/patches/22126/?format=api", "web_url": "https://patchwork.libcamera.org/patch/22126/", "project": { "id": 1, "url": "https://patchwork.libcamera.org/api/1.1/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": "<20241127144655.1074720-3-isaac.scott@ideasonboard.com>", "date": "2024-11-27T14:46:55", "name": "[RFC,2/2] libcamera: bitdepth: Adapt camera_sensor_helper to use BitDepth", "commit_ref": null, "pull_url": null, "state": "new", "archived": false, "hash": "29bdf82c17928268f74a8d711eeb8b4e1153625b", "submitter": { "id": 215, "url": "https://patchwork.libcamera.org/api/1.1/people/215/?format=api", "name": "Isaac Scott", "email": "isaac.scott@ideasonboard.com" }, "delegate": null, "mbox": "https://patchwork.libcamera.org/patch/22126/mbox/", "series": [ { "id": 4830, "url": "https://patchwork.libcamera.org/api/1.1/series/4830/?format=api", "web_url": "https://patchwork.libcamera.org/project/libcamera/list/?series=4830", "date": "2024-11-27T14:46:53", "name": "Add BitDepthValue for simplified bit depth conversion", "version": 1, "mbox": "https://patchwork.libcamera.org/series/4830/mbox/" } ], "comments": "https://patchwork.libcamera.org/api/patches/22126/comments/", "check": "pending", "checks": "https://patchwork.libcamera.org/api/patches/22126/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 4C394C3200\n\tfor <parsemail@patchwork.libcamera.org>;\n\tWed, 27 Nov 2024 14:47:12 +0000 (UTC)", "from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id 0A9C3660E2;\n\tWed, 27 Nov 2024 15:47:10 +0100 (CET)", "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 9B08E660CC\n\tfor <libcamera-devel@lists.libcamera.org>;\n\tWed, 27 Nov 2024 15:47:04 +0100 (CET)", "from isaac-ThinkPad-T16-Gen-2.lan\n\t(cpc89244-aztw30-2-0-cust6594.18-1.cable.virginm.net [86.31.185.195])\n\tby perceval.ideasonboard.com (Postfix) with ESMTPSA id 7DD49132B;\n\tWed, 27 Nov 2024 15:46:41 +0100 (CET)" ], "Authentication-Results": "lancelot.ideasonboard.com; dkim=pass (1024-bit key;\n\tunprotected) header.d=ideasonboard.com header.i=@ideasonboard.com\n\theader.b=\"pnELYpN4\"; dkim-atps=neutral", "DKIM-Signature": "v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com;\n\ts=mail; t=1732718801;\n\tbh=4X8G55S0q85DqIdRvLB6LYno0H0+XBO/XrvQQYQ6zSc=;\n\th=From:To:Cc:Subject:Date:In-Reply-To:References:From;\n\tb=pnELYpN4h5Nqfm8y9jBPFLja3yhBI4OXQw+y/0y69wfKRA10DW81oy++i9lHGTLPv\n\tRiDwz+PUc0flWqAB9ANzx8GU/+/YiiUhKskGZVDf0i5Iuj0DSuff4eRCNcaOrxc7SO\n\tB7WPXxUbGLVtJao5q41iCMZ/FwTzts9e29cOwihM=", "From": "Isaac Scott <isaac.scott@ideasonboard.com>", "To": "libcamera devel <libcamera-devel@lists.libcamera.org>", "Cc": "Isaac Scott <isaac.scott@ideasonboard.com>", "Subject": "[RFC PATCH 2/2] libcamera: bitdepth: Adapt camera_sensor_helper to\n\tuse BitDepth", "Date": "Wed, 27 Nov 2024 14:46:55 +0000", "Message-ID": "<20241127144655.1074720-3-isaac.scott@ideasonboard.com>", "X-Mailer": "git-send-email 2.43.0", "In-Reply-To": "<20241127144655.1074720-1-isaac.scott@ideasonboard.com>", "References": "<20241127144655.1074720-1-isaac.scott@ideasonboard.com>", "MIME-Version": "1.0", "Content-Transfer-Encoding": "8bit", "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>", "Errors-To": "libcamera-devel-bounces@lists.libcamera.org", "Sender": "\"libcamera-devel\" <libcamera-devel-bounces@lists.libcamera.org>" }, "content": "Adapt the camera_sensor_helper class to use BitDepth instead of bit\nshifting for black levels as an example of how the BitDepth\nimplementation can be used.\n\nSigned-off-by: Isaac Scott <isaac.scott@ideasonboard.com>\n---\n src/ipa/libipa/camera_sensor_helper.cpp | 18 +++++++++---------\n src/ipa/libipa/camera_sensor_helper.h | 5 +++--\n src/ipa/simple/algorithms/awb.cpp | 3 ++-\n src/ipa/simple/algorithms/blc.cpp | 10 ++++++----\n src/ipa/simple/ipa_context.h | 5 +++--\n src/ipa/simple/soft_simple.cpp | 5 ++---\n 6 files changed, 25 insertions(+), 21 deletions(-)", "diff": "diff --git a/src/ipa/libipa/camera_sensor_helper.cpp b/src/ipa/libipa/camera_sensor_helper.cpp\nindex c6169bdc..1031dbd1 100644\n--- a/src/ipa/libipa/camera_sensor_helper.cpp\n+++ b/src/ipa/libipa/camera_sensor_helper.cpp\n@@ -407,7 +407,7 @@ public:\n \tCameraSensorHelperAr0144()\n \t{\n \t\t/* Power-on default value: 168 at 12bits. */\n-\t\tblackLevel_ = 2688;\n+\t\tblackLevel_ = 168_12bit;\n \t}\n \n \tuint32_t gainCode(double gain) const override\n@@ -525,7 +525,7 @@ public:\n \tCameraSensorHelperImx214()\n \t{\n \t\t/* From datasheet: 64 at 10bits. */\n-\t\tblackLevel_ = 4096;\n+\t\tblackLevel_ = 64_10bit;\n \t\tgainType_ = AnalogueGainLinear;\n \t\tgainConstants_.linear = { 0, 512, -1, 512 };\n \t}\n@@ -538,7 +538,7 @@ public:\n \tCameraSensorHelperImx219()\n \t{\n \t\t/* From datasheet: 64 at 10bits. */\n-\t\tblackLevel_ = 4096;\n+\t\tblackLevel_ = 64_10bit;\n \t\tgainType_ = AnalogueGainLinear;\n \t\tgainConstants_.linear = { 0, 256, -1, 256 };\n \t}\n@@ -551,7 +551,7 @@ public:\n \tCameraSensorHelperImx258()\n \t{\n \t\t/* From datasheet: 0x40 at 10bits. */\n-\t\tblackLevel_ = 4096;\n+\t\tblackLevel_ = 0x40_10bit;\n \t\tgainType_ = AnalogueGainLinear;\n \t\tgainConstants_.linear = { 0, 512, -1, 512 };\n \t}\n@@ -564,7 +564,7 @@ public:\n \tCameraSensorHelperImx283()\n \t{\n \t\t/* From datasheet: 0x32 at 10bits. */\n-\t\tblackLevel_ = 3200;\n+\t\tblackLevel_ = 0x32_10bit;\n \t\tgainType_ = AnalogueGainLinear;\n \t\tgainConstants_.linear = { 0, 2048, -1, 2048 };\n \t}\n@@ -604,7 +604,7 @@ public:\n \tCameraSensorHelperImx335()\n \t{\n \t\t/* From datasheet: 0x32 at 10bits. */\n-\t\tblackLevel_ = 3200;\n+\t\tblackLevel_ = 0x32_10bit;\n \t\tgainType_ = AnalogueGainExponential;\n \t\tgainConstants_.exp = { 1.0, expGainDb(0.3) };\n \t}\n@@ -665,7 +665,7 @@ public:\n \tCameraSensorHelperOv4689()\n \t{\n \t\t/* From datasheet: 0x40 at 12bits. */\n-\t\tblackLevel_ = 1024;\n+\t\tblackLevel_ = 0x40_12bit;\n \t\tgainType_ = AnalogueGainLinear;\n \t\tgainConstants_.linear = { 1, 0, 0, 128 };\n \t}\n@@ -678,7 +678,7 @@ public:\n \tCameraSensorHelperOv5640()\n \t{\n \t\t/* From datasheet: 0x10 at 10bits. */\n-\t\tblackLevel_ = 1024;\n+\t\tblackLevel_ = 0x10_10bit;\n \t\tgainType_ = AnalogueGainLinear;\n \t\tgainConstants_.linear = { 1, 0, 0, 16 };\n \t}\n@@ -713,7 +713,7 @@ public:\n \tCameraSensorHelperOv5675()\n \t{\n \t\t/* From Linux kernel driver: 0x40 at 10bits. */\n-\t\tblackLevel_ = 4096;\n+\t\tblackLevel_ = 0x40_10bit;\n \t\tgainType_ = AnalogueGainLinear;\n \t\tgainConstants_.linear = { 1, 0, 0, 128 };\n \t}\ndiff --git a/src/ipa/libipa/camera_sensor_helper.h b/src/ipa/libipa/camera_sensor_helper.h\nindex 75868205..b72606bf 100644\n--- a/src/ipa/libipa/camera_sensor_helper.h\n+++ b/src/ipa/libipa/camera_sensor_helper.h\n@@ -14,6 +14,7 @@\n #include <vector>\n \n #include <libcamera/base/class.h>\n+#include \"bitdepth.h\"\n \n namespace libcamera {\n \n@@ -25,7 +26,7 @@ public:\n \tCameraSensorHelper() = default;\n \tvirtual ~CameraSensorHelper() = default;\n \n-\tstd::optional<int16_t> blackLevel() const { return blackLevel_; }\n+\tstd::optional<BitDepthValue<16>> blackLevel() const { return blackLevel_; }\n \tvirtual uint32_t gainCode(double gain) const;\n \tvirtual double gain(uint32_t gainCode) const;\n \n@@ -52,7 +53,7 @@ protected:\n \t\tAnalogueGainExpConstants exp;\n \t};\n \n-\tstd::optional<int16_t> blackLevel_;\n+\tstd::optional<BitDepthValue<16>> blackLevel_;\n \tAnalogueGainType gainType_;\n \tAnalogueGainConstants gainConstants_;\n \ndiff --git a/src/ipa/simple/algorithms/awb.cpp b/src/ipa/simple/algorithms/awb.cpp\nindex 195de41d..cfce5869 100644\n--- a/src/ipa/simple/algorithms/awb.cpp\n+++ b/src/ipa/simple/algorithms/awb.cpp\n@@ -12,6 +12,7 @@\n \n #include <libcamera/base/log.h>\n \n+#include \"libipa/bitdepth.h\"\n #include \"simple/ipa_context.h\"\n \n namespace libcamera {\n@@ -36,7 +37,7 @@ void Awb::process(IPAContext &context,\n \t\t [[maybe_unused]] ControlList &metadata)\n {\n \tconst SwIspStats::Histogram &histogram = stats->yHistogram;\n-\tconst uint8_t blackLevel = context.activeState.blc.level;\n+\tconst BitDepthValue<8> blackLevel = context.activeState.blc.level;\n \n \t/*\n \t * Black level must be subtracted to get the correct AWB ratios, they\ndiff --git a/src/ipa/simple/algorithms/blc.cpp b/src/ipa/simple/algorithms/blc.cpp\nindex b4e32fe1..7c5d3f6d 100644\n--- a/src/ipa/simple/algorithms/blc.cpp\n+++ b/src/ipa/simple/algorithms/blc.cpp\n@@ -10,6 +10,7 @@\n #include <numeric>\n \n #include <libcamera/base/log.h>\n+#include \"libipa/bitdepth.h\"\n \n namespace libcamera {\n \n@@ -29,7 +30,7 @@ int BlackLevel::init(IPAContext &context, const YamlObject &tuningData)\n \t\t * Convert 16 bit values from the tuning file to 8 bit black\n \t\t * level for the SoftISP.\n \t\t */\n-\t\tcontext.configuration.black.level = blackLevel.value() >> 8;\n+\t\tcontext.configuration.black.level->convert<8>();\n \t}\n \treturn 0;\n }\n@@ -38,7 +39,7 @@ int BlackLevel::configure(IPAContext &context,\n \t\t\t [[maybe_unused]] const IPAConfigInfo &configInfo)\n {\n \tcontext.activeState.blc.level =\n-\t\tcontext.configuration.black.level.value_or(255);\n+\t\tcontext.configuration.black.level.value_or(255_8bit);\n \treturn 0;\n }\n \n@@ -68,14 +69,15 @@ void BlackLevel::process(IPAContext &context,\n \tconst unsigned int pixelThreshold = ignoredPercentage * total;\n \tconst unsigned int histogramRatio = 256 / SwIspStats::kYHistogramSize;\n \tconst unsigned int currentBlackIdx =\n-\t\tcontext.activeState.blc.level / histogramRatio;\n+\t\tcontext.activeState.blc.level.value() / histogramRatio;\n \n \tfor (unsigned int i = 0, seen = 0;\n \t i < currentBlackIdx && i < SwIspStats::kYHistogramSize;\n \t i++) {\n \t\tseen += histogram[i];\n \t\tif (seen >= pixelThreshold) {\n-\t\t\tcontext.activeState.blc.level = i * histogramRatio;\n+\t\t\tcontext.activeState.blc.level =\n+\t\t\t\tBitDepthValue<8>(i * histogramRatio);\n \t\t\texposure_ = frameContext.sensor.exposure;\n \t\t\tgain_ = frameContext.sensor.gain;\n \t\t\tLOG(IPASoftBL, Debug)\ndiff --git a/src/ipa/simple/ipa_context.h b/src/ipa/simple/ipa_context.h\nindex fd121eeb..be3b967a 100644\n--- a/src/ipa/simple/ipa_context.h\n+++ b/src/ipa/simple/ipa_context.h\n@@ -12,6 +12,7 @@\n #include <stdint.h>\n \n #include <libipa/fc_queue.h>\n+#include \"libipa/bitdepth.h\"\n \n namespace libcamera {\n \n@@ -24,13 +25,13 @@ struct IPASessionConfiguration {\n \t\tdouble againMin, againMax, againMinStep;\n \t} agc;\n \tstruct {\n-\t\tstd::optional<uint8_t> level;\n+\t\tstd::optional<BitDepthValue<8>> level;\n \t} black;\n };\n \n struct IPAActiveState {\n \tstruct {\n-\t\tuint8_t level;\n+\t\tBitDepthValue<8> level;\n \t} blc;\n \n \tstruct {\ndiff --git a/src/ipa/simple/soft_simple.cpp b/src/ipa/simple/soft_simple.cpp\nindex ac2a9421..292fa81f 100644\n--- a/src/ipa/simple/soft_simple.cpp\n+++ b/src/ipa/simple/soft_simple.cpp\n@@ -211,11 +211,10 @@ int IPASoftSimple::configure(const IPAConfigInfo &configInfo)\n \t\t\t/*\n \t\t\t * The black level from camHelper_ is a 16 bit value, software ISP\n \t\t\t * works with 8 bit pixel values, both regardless of the actual\n-\t\t\t * sensor pixel width. Hence we obtain the pixel-based black value\n-\t\t\t * by dividing the value from the helper by 256.\n+\t\t\t * sensor pixel width.\n \t\t\t */\n \t\t\tcontext_.configuration.black.level =\n-\t\t\t\tcamHelper_->blackLevel().value() / 256;\n+\t\t\t\tcamHelper_->blackLevel();\n \t\t}\n \t} else {\n \t\t/*\n", "prefixes": [ "RFC", "2/2" ] }