{"id":12745,"url":"https://patchwork.libcamera.org/api/patches/12745/?format=json","web_url":"https://patchwork.libcamera.org/patch/12745/","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":"<20210629104500.51672-3-naush@raspberrypi.com>","date":"2021-06-29T10:45:00","name":"[libcamera-devel,v3,2/2] ipa: raspberrypi: Generalise the SMIA metadata parser","commit_ref":null,"pull_url":null,"state":"accepted","archived":false,"hash":"46f11b0e3d54d0748b1c3e62697425ce7d2c3083","submitter":{"id":34,"url":"https://patchwork.libcamera.org/api/people/34/?format=json","name":"Naushir Patuck","email":"naush@raspberrypi.com"},"delegate":null,"mbox":"https://patchwork.libcamera.org/patch/12745/mbox/","series":[{"id":2191,"url":"https://patchwork.libcamera.org/api/series/2191/?format=json","web_url":"https://patchwork.libcamera.org/project/libcamera/list/?series=2191","date":"2021-06-29T10:44:58","name":"Raspberry Pi: Metadata parsing improvements (II)","version":3,"mbox":"https://patchwork.libcamera.org/series/2191/mbox/"}],"comments":"https://patchwork.libcamera.org/api/patches/12745/comments/","check":"pending","checks":"https://patchwork.libcamera.org/api/patches/12745/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 360B6C3220\n\tfor <parsemail@patchwork.libcamera.org>;\n\tTue, 29 Jun 2021 10:45:16 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id 8BFF0684EB;\n\tTue, 29 Jun 2021 12:45:15 +0200 (CEST)","from mail-wr1-x42b.google.com (mail-wr1-x42b.google.com\n\t[IPv6:2a00:1450:4864:20::42b])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTPS id 052D1684ED\n\tfor <libcamera-devel@lists.libcamera.org>;\n\tTue, 29 Jun 2021 12:45:11 +0200 (CEST)","by mail-wr1-x42b.google.com with SMTP id i8so2999788wrc.0\n\tfor <libcamera-devel@lists.libcamera.org>;\n\tTue, 29 Jun 2021 03:45:11 -0700 (PDT)","from naush-laptop.pitowers.org\n\t([2a00:1098:3142:14:d9cf:b3d0:bdee:72b2])\n\tby smtp.gmail.com with ESMTPSA id\n\tl20sm16669930wmq.3.2021.06.29.03.45.10\n\t(version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256);\n\tTue, 29 Jun 2021 03:45:10 -0700 (PDT)"],"Authentication-Results":"lancelot.ideasonboard.com;\n\tdkim=fail reason=\"signature verification failed\" (2048-bit key;\n\tunprotected) header.d=raspberrypi.com header.i=@raspberrypi.com\n\theader.b=\"CcuEQkYX\"; dkim-atps=neutral","DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/relaxed;\n\td=raspberrypi.com; s=google;\n\th=from:to:cc:subject:date:message-id:in-reply-to:references\n\t:mime-version:content-transfer-encoding;\n\tbh=EH6+B9Ybzch2uo+Fet1PHYrGxOFnIwUE1GDbzuQF4P4=;\n\tb=CcuEQkYXmwXC6ELxAZ77PXivDW1u7HZv/5eUefVEGa/Zt18ScPJctQo4kmjmM5keCS\n\tSG1fFgD9CGLoWKG/YAFpqX9TJaxLIl6t0FhZenD3LSlYJ5PWBOOZuqI1FdIzEvpRjvvP\n\tgm+xLRu87NAQXnX8VqZfcRW1GRFUv0fBJCkspS4Y/D5jJ4usZlMvBwwPFgoUhy16dX0a\n\tHa7otIsI6nY+hR2rZAjYCSXfXRYEugDujSeSBbsk6w/6gRi8QtfyYVk7glsE+JAVqDvt\n\t40DoeSYuQ/IrRmTRjqWFlkhqCl74IGvArDHOhyQe4pHiQ0aqK4m4CEw4kS1pN+6FWcJa\n\tZDNA==","X-Google-DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/relaxed;\n\td=1e100.net; s=20161025;\n\th=x-gm-message-state:from:to:cc:subject:date:message-id:in-reply-to\n\t:references:mime-version:content-transfer-encoding;\n\tbh=EH6+B9Ybzch2uo+Fet1PHYrGxOFnIwUE1GDbzuQF4P4=;\n\tb=uZo/+gAJEWyVqMZPjh0F9QbfYu6s9kLqnpEcWPXenT36ZKRxM5QK4vG/comtCILol2\n\tytJpDXYPaC0+RA5NeDRXB+HS6AZ8IMoOSEF8kicDHWdJDY9iFeFm6Mh3GmuLQZ0ihFpX\n\t38fJekeXIu02a9QQhoU3hDLMT3ZJFQJ4QBCakk/sim15H9t2VwaX+1W2dvBGX04FhorG\n\tuxFv2i8Zr7jlU7MRW/uNvwmAN8Q+7kc1LFUQk8mRA5IQskbdD69y4yyTsqmep+ow0PzY\n\tmgupyQS+LD793ArSyObBEGf2Bnc/qPlUpYn//zROgEZherM8RzPE7l30bwxexij03ZWn\n\tNm6Q==","X-Gm-Message-State":"AOAM533mTubHkLBjErwm19uNsDjytGvvvt0X5S3jyHQU9OZ2fAVz3J5s\n\t7XIR077mJ4AuW7ERBUFJe73UZCVW3VEodQ==","X-Google-Smtp-Source":"ABdhPJxGxyf0Itroiql0dKpLf4dDcwyE/c7Nq2QLCi+0eTJrMygczLPveWR6X0HPWh7EjPThtqzQ3Q==","X-Received":"by 2002:adf:fd46:: with SMTP id\n\th6mr32813436wrs.420.1624963511105; \n\tTue, 29 Jun 2021 03:45:11 -0700 (PDT)","From":"Naushir Patuck <naush@raspberrypi.com>","To":"libcamera-devel@lists.libcamera.org","Date":"Tue, 29 Jun 2021 11:45:00 +0100","Message-Id":"<20210629104500.51672-3-naush@raspberrypi.com>","X-Mailer":"git-send-email 2.25.1","In-Reply-To":"<20210629104500.51672-1-naush@raspberrypi.com>","References":"<20210629104500.51672-1-naush@raspberrypi.com>","MIME-Version":"1.0","Content-Transfer-Encoding":"8bit","Subject":"[libcamera-devel] [PATCH v3 2/2] ipa: raspberrypi: Generalise the\n\tSMIA metadata parser","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":"Instead of having each CamHelper subclass the MdParserSmia, change the\nimplementation of MdParserSmia to be more generic. The MdParserSmia now gets\ngiven a list of registers to search for and helper functions are used to compute\nexposure lines and gain codes from these registers.\n\nUpdate the imx219 and imx477 CamHelpers by using this new mechanism.\n\nSigned-off-by: Naushir Patuck <naush@raspberrypi.com>\nReviewed-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>\n---\n src/ipa/raspberrypi/cam_helper.cpp        |  33 +++---\n src/ipa/raspberrypi/cam_helper.hpp        |   2 +\n src/ipa/raspberrypi/cam_helper_imx219.cpp | 114 ++++----------------\n src/ipa/raspberrypi/cam_helper_imx477.cpp | 122 ++++------------------\n src/ipa/raspberrypi/md_parser.hpp         |  41 +++++---\n src/ipa/raspberrypi/md_parser_smia.cpp    |  66 ++++++++++--\n 6 files changed, 144 insertions(+), 234 deletions(-)","diff":"diff --git a/src/ipa/raspberrypi/cam_helper.cpp b/src/ipa/raspberrypi/cam_helper.cpp\nindex 90498c37af98..e6d2258c66d7 100644\n--- a/src/ipa/raspberrypi/cam_helper.cpp\n+++ b/src/ipa/raspberrypi/cam_helper.cpp\n@@ -159,32 +159,34 @@ unsigned int CamHelper::MistrustFramesModeSwitch() const\n void CamHelper::parseEmbeddedData(Span<const uint8_t> buffer,\n \t\t\t\t  Metadata &metadata)\n {\n+\tMdParser::RegisterMap registers;\n+\tMetadata parsedMetadata;\n+\n \tif (buffer.empty())\n \t\treturn;\n \n-\tuint32_t exposureLines, gainCode;\n-\n-\tif (parser_->Parse(buffer) != MdParser::Status::OK ||\n-\t    parser_->GetExposureLines(exposureLines) != MdParser::Status::OK ||\n-\t    parser_->GetGainCode(gainCode) != MdParser::Status::OK) {\n+\tif (parser_->Parse(buffer, registers) != MdParser::Status::OK) {\n \t\tLOG(IPARPI, Error) << \"Embedded data buffer parsing failed\";\n \t\treturn;\n \t}\n \n+\tPopulateMetadata(registers, parsedMetadata);\n+\tmetadata.Merge(parsedMetadata);\n+\n \t/*\n-\t * Overwrite the exposure/gain values in the DeviceStatus, as\n-\t * we know better. Fetch it first in case any other fields were\n-\t * set meaningfully.\n+\t * Overwrite the exposure/gain values in the existing DeviceStatus with\n+\t * values from the parsed embedded buffer. Fetch it first in case any\n+\t * other fields were set meaningfully.\n \t */\n-\tDeviceStatus deviceStatus;\n-\n-\tif (metadata.Get(\"device.status\", deviceStatus) != 0) {\n+\tDeviceStatus deviceStatus, parsedDeviceStatus;\n+\tif (metadata.Get(\"device.status\", deviceStatus) ||\n+\t    parsedMetadata.Get(\"device.status\", parsedDeviceStatus)) {\n \t\tLOG(IPARPI, Error) << \"DeviceStatus not found\";\n \t\treturn;\n \t}\n \n-\tdeviceStatus.shutter_speed = Exposure(exposureLines);\n-\tdeviceStatus.analogue_gain = Gain(gainCode);\n+\tdeviceStatus.shutter_speed = parsedDeviceStatus.shutter_speed;\n+\tdeviceStatus.analogue_gain = parsedDeviceStatus.analogue_gain;\n \n \tLOG(IPARPI, Debug) << \"Metadata updated - Exposure : \"\n \t\t\t   << deviceStatus.shutter_speed\n@@ -194,6 +196,11 @@ void CamHelper::parseEmbeddedData(Span<const uint8_t> buffer,\n \tmetadata.Set(\"device.status\", deviceStatus);\n }\n \n+void CamHelper::PopulateMetadata([[maybe_unused]] const MdParser::RegisterMap &registers,\n+\t\t\t\t [[maybe_unused]] Metadata &metadata) const\n+{\n+}\n+\n RegisterCamHelper::RegisterCamHelper(char const *cam_name,\n \t\t\t\t     CamHelperCreateFunc create_func)\n {\ndiff --git a/src/ipa/raspberrypi/cam_helper.hpp b/src/ipa/raspberrypi/cam_helper.hpp\nindex fc3139e22be0..200cc83f3872 100644\n--- a/src/ipa/raspberrypi/cam_helper.hpp\n+++ b/src/ipa/raspberrypi/cam_helper.hpp\n@@ -92,6 +92,8 @@ public:\n protected:\n \tvoid parseEmbeddedData(libcamera::Span<const uint8_t> buffer,\n \t\t\t       Metadata &metadata);\n+\tvirtual void PopulateMetadata(const MdParser::RegisterMap &registers,\n+\t\t\t\t      Metadata &metadata) const;\n \n \tstd::unique_ptr<MdParser> parser_;\n \tCameraMode mode_;\ndiff --git a/src/ipa/raspberrypi/cam_helper_imx219.cpp b/src/ipa/raspberrypi/cam_helper_imx219.cpp\nindex c85044a5fa6d..4e4994188ff3 100644\n--- a/src/ipa/raspberrypi/cam_helper_imx219.cpp\n+++ b/src/ipa/raspberrypi/cam_helper_imx219.cpp\n@@ -23,21 +23,14 @@\n \n using namespace RPiController;\n \n-/* Metadata parser implementation specific to Sony IMX219 sensors. */\n-\n-class MdParserImx219 : public MdParserSmia\n-{\n-public:\n-\tMdParserImx219();\n-\tStatus Parse(libcamera::Span<const uint8_t> buffer) override;\n-\tStatus GetExposureLines(unsigned int &lines) override;\n-\tStatus GetGainCode(unsigned int &gain_code) override;\n-private:\n-\t/* Offset of the register's value in the metadata block. */\n-\tint reg_offsets_[3];\n-\t/* Value of the register, once read from the metadata block. */\n-\tint reg_values_[3];\n-};\n+/*\n+ * We care about one gain register and a pair of exposure registers. Their I2C\n+ * addresses from the Sony IMX219 datasheet:\n+ */\n+constexpr uint32_t gainReg = 0x157;\n+constexpr uint32_t expHiReg = 0x15a;\n+constexpr uint32_t expLoReg = 0x15b;\n+constexpr std::initializer_list<uint32_t> registerList = { expHiReg, expLoReg, gainReg };\n \n class CamHelperImx219 : public CamHelper\n {\n@@ -54,11 +47,14 @@ private:\n \t * in units of lines.\n \t */\n \tstatic constexpr int frameIntegrationDiff = 4;\n+\n+\tvoid PopulateMetadata(const MdParser::RegisterMap &registers,\n+\t\t\t      Metadata &metadata) const override;\n };\n \n CamHelperImx219::CamHelperImx219()\n #if ENABLE_EMBEDDED_DATA\n-\t: CamHelper(std::make_unique<MdParserImx219>(), frameIntegrationDiff)\n+\t: CamHelper(std::make_unique<MdParserSmia>(registerList), frameIntegrationDiff)\n #else\n \t: CamHelper({}, frameIntegrationDiff)\n #endif\n@@ -90,88 +86,20 @@ bool CamHelperImx219::SensorEmbeddedDataPresent() const\n \treturn ENABLE_EMBEDDED_DATA;\n }\n \n-static CamHelper *Create()\n+void CamHelperImx219::PopulateMetadata(const MdParser::RegisterMap &registers,\n+\t\t\t\t       Metadata &metadata) const\n {\n-\treturn new CamHelperImx219();\n-}\n+\tDeviceStatus deviceStatus;\n \n-static RegisterCamHelper reg(\"imx219\", &Create);\n+\tdeviceStatus.shutter_speed = Exposure(registers.at(expHiReg) * 256 + registers.at(expLoReg));\n+\tdeviceStatus.analogue_gain = Gain(registers.at(gainReg));\n \n-/*\n- * We care about one gain register and a pair of exposure registers. Their I2C\n- * addresses from the Sony IMX219 datasheet:\n- */\n-#define GAIN_REG 0x157\n-#define EXPHI_REG 0x15A\n-#define EXPLO_REG 0x15B\n-\n-/*\n- * Index of each into the reg_offsets and reg_values arrays. Must be in\n- * register address order.\n- */\n-#define GAIN_INDEX 0\n-#define EXPHI_INDEX 1\n-#define EXPLO_INDEX 2\n-\n-MdParserImx219::MdParserImx219()\n-{\n-\treg_offsets_[0] = reg_offsets_[1] = reg_offsets_[2] = -1;\n+\tmetadata.Set(\"device.status\", deviceStatus);\n }\n \n-MdParser::Status MdParserImx219::Parse(libcamera::Span<const uint8_t> buffer)\n-{\n-\tbool try_again = false;\n-\n-\tif (reset_) {\n-\t\t/*\n-\t\t * Search again through the metadata for the gain and exposure\n-\t\t * registers.\n-\t\t */\n-\t\tassert(bits_per_pixel_);\n-\t\t/* Need to be ordered */\n-\t\tuint32_t regs[3] = { GAIN_REG, EXPHI_REG, EXPLO_REG };\n-\t\treg_offsets_[0] = reg_offsets_[1] = reg_offsets_[2] = -1;\n-\t\tint ret = static_cast<int>(findRegs(buffer,\n-\t\t\t\t\t\t    regs, reg_offsets_, 3));\n-\t\t/*\n-\t\t * > 0 means \"worked partially but parse again next time\",\n-\t\t * < 0 means \"hard error\".\n-\t\t */\n-\t\tif (ret > 0)\n-\t\t\ttry_again = true;\n-\t\telse if (ret < 0)\n-\t\t\treturn ERROR;\n-\t}\n-\n-\tfor (int i = 0; i < 3; i++) {\n-\t\tif (reg_offsets_[i] == -1)\n-\t\t\tcontinue;\n-\n-\t\treg_values_[i] = buffer[reg_offsets_[i]];\n-\t}\n-\n-\t/* Re-parse next time if we were unhappy in some way. */\n-\treset_ = try_again;\n-\n-\treturn OK;\n-}\n-\n-MdParser::Status MdParserImx219::GetExposureLines(unsigned int &lines)\n+static CamHelper *Create()\n {\n-\tif (reg_offsets_[EXPHI_INDEX] == -1 || reg_offsets_[EXPLO_INDEX] == -1)\n-\t\treturn NOTFOUND;\n-\n-\tlines = reg_values_[EXPHI_INDEX] * 256 + reg_values_[EXPLO_INDEX];\n-\n-\treturn OK;\n+\treturn new CamHelperImx219();\n }\n \n-MdParser::Status MdParserImx219::GetGainCode(unsigned int &gain_code)\n-{\n-\tif (reg_offsets_[GAIN_INDEX] == -1)\n-\t\treturn NOTFOUND;\n-\n-\tgain_code = reg_values_[GAIN_INDEX];\n-\n-\treturn OK;\n-}\n+static RegisterCamHelper reg(\"imx219\", &Create);\ndiff --git a/src/ipa/raspberrypi/cam_helper_imx477.cpp b/src/ipa/raspberrypi/cam_helper_imx477.cpp\nindex d72a9be0438e..efd1a5893db8 100644\n--- a/src/ipa/raspberrypi/cam_helper_imx477.cpp\n+++ b/src/ipa/raspberrypi/cam_helper_imx477.cpp\n@@ -15,21 +15,15 @@\n \n using namespace RPiController;\n \n-/* Metadata parser implementation specific to Sony IMX477 sensors. */\n-\n-class MdParserImx477 : public MdParserSmia\n-{\n-public:\n-\tMdParserImx477();\n-\tStatus Parse(libcamera::Span<const uint8_t> buffer) override;\n-\tStatus GetExposureLines(unsigned int &lines) override;\n-\tStatus GetGainCode(unsigned int &gain_code) override;\n-private:\n-\t/* Offset of the register's value in the metadata block. */\n-\tint reg_offsets_[4];\n-\t/* Value of the register, once read from the metadata block. */\n-\tint reg_values_[4];\n-};\n+/*\n+ * We care about two gain registers and a pair of exposure registers. Their\n+ * I2C addresses from the Sony IMX477 datasheet:\n+ */\n+constexpr uint32_t expHiReg = 0x0202;\n+constexpr uint32_t expLoReg = 0x0203;\n+constexpr uint32_t gainHiReg = 0x0204;\n+constexpr uint32_t gainLoReg = 0x0205;\n+constexpr std::initializer_list<uint32_t> registerList = { expHiReg, expLoReg, gainHiReg, gainLoReg };\n \n class CamHelperImx477 : public CamHelper\n {\n@@ -47,10 +41,13 @@ private:\n \t * in units of lines.\n \t */\n \tstatic constexpr int frameIntegrationDiff = 22;\n+\n+\tvoid PopulateMetadata(const MdParser::RegisterMap &registers,\n+\t\t\t      Metadata &metadata) const override;\n };\n \n CamHelperImx477::CamHelperImx477()\n-\t: CamHelper(std::make_unique<MdParserImx477>(), frameIntegrationDiff)\n+\t: CamHelper(std::make_unique<MdParserSmia>(registerList), frameIntegrationDiff)\n {\n }\n \n@@ -77,95 +74,20 @@ bool CamHelperImx477::SensorEmbeddedDataPresent() const\n \treturn true;\n }\n \n-static CamHelper *Create()\n+void CamHelperImx477::PopulateMetadata(const MdParser::RegisterMap &registers,\n+\t\t\t\t       Metadata &metadata) const\n {\n-\treturn new CamHelperImx477();\n-}\n+\tDeviceStatus deviceStatus;\n \n-static RegisterCamHelper reg(\"imx477\", &Create);\n+\tdeviceStatus.shutter_speed = Exposure(registers.at(expHiReg) * 256 + registers.at(expLoReg));\n+\tdeviceStatus.analogue_gain = Gain(registers.at(gainHiReg) * 256 + registers.at(gainLoReg));\n \n-/*\n- * We care about two gain registers and a pair of exposure registers. Their\n- * I2C addresses from the Sony IMX477 datasheet:\n- */\n-#define EXPHI_REG 0x0202\n-#define EXPLO_REG 0x0203\n-#define GAINHI_REG 0x0204\n-#define GAINLO_REG 0x0205\n-\n-/*\n- * Index of each into the reg_offsets and reg_values arrays. Must be in register\n- * address order.\n- */\n-#define EXPHI_INDEX 0\n-#define EXPLO_INDEX 1\n-#define GAINHI_INDEX 2\n-#define GAINLO_INDEX 3\n-\n-MdParserImx477::MdParserImx477()\n-{\n-\treg_offsets_[0] = reg_offsets_[1] = reg_offsets_[2] = reg_offsets_[3] = -1;\n+\tmetadata.Set(\"device.status\", deviceStatus);\n }\n \n-MdParser::Status MdParserImx477::Parse(libcamera::Span<const uint8_t> buffer)\n-{\n-\tbool try_again = false;\n-\n-\tif (reset_) {\n-\t\t/*\n-\t\t * Search again through the metadata for the gain and exposure\n-\t\t * registers.\n-\t\t */\n-\t\tassert(bits_per_pixel_);\n-\t\t/* Need to be ordered */\n-\t\tuint32_t regs[4] = {\n-\t\t\tEXPHI_REG,\n-\t\t\tEXPLO_REG,\n-\t\t\tGAINHI_REG,\n-\t\t\tGAINLO_REG\n-\t\t};\n-\t\treg_offsets_[0] = reg_offsets_[1] = reg_offsets_[2] = reg_offsets_[3] = -1;\n-\t\tint ret = static_cast<int>(findRegs(buffer,\n-\t\t\t\t\t\t    regs, reg_offsets_, 4));\n-\t\t/*\n-\t\t * > 0 means \"worked partially but parse again next time\",\n-\t\t * < 0 means \"hard error\".\n-\t\t */\n-\t\tif (ret > 0)\n-\t\t\ttry_again = true;\n-\t\telse if (ret < 0)\n-\t\t\treturn ERROR;\n-\t}\n-\n-\tfor (int i = 0; i < 4; i++) {\n-\t\tif (reg_offsets_[i] == -1)\n-\t\t\tcontinue;\n-\n-\t\treg_values_[i] = buffer[reg_offsets_[i]];\n-\t}\n-\n-\t/* Re-parse next time if we were unhappy in some way. */\n-\treset_ = try_again;\n-\n-\treturn OK;\n-}\n-\n-MdParser::Status MdParserImx477::GetExposureLines(unsigned int &lines)\n+static CamHelper *Create()\n {\n-\tif (reg_offsets_[EXPHI_INDEX] == -1 || reg_offsets_[EXPLO_INDEX] == -1)\n-\t\treturn NOTFOUND;\n-\n-\tlines = reg_values_[EXPHI_INDEX] * 256 + reg_values_[EXPLO_INDEX];\n-\n-\treturn OK;\n+\treturn new CamHelperImx477();\n }\n \n-MdParser::Status MdParserImx477::GetGainCode(unsigned int &gain_code)\n-{\n-\tif (reg_offsets_[GAINHI_INDEX] == -1 || reg_offsets_[GAINLO_INDEX] == -1)\n-\t\treturn NOTFOUND;\n-\n-\tgain_code = reg_values_[GAINHI_INDEX] * 256 + reg_values_[GAINLO_INDEX];\n-\n-\treturn OK;\n-}\n+static RegisterCamHelper reg(\"imx477\", &Create);\ndiff --git a/src/ipa/raspberrypi/md_parser.hpp b/src/ipa/raspberrypi/md_parser.hpp\nindex 8497216f8db7..e3e2738537a8 100644\n--- a/src/ipa/raspberrypi/md_parser.hpp\n+++ b/src/ipa/raspberrypi/md_parser.hpp\n@@ -6,6 +6,9 @@\n  */\n #pragma once\n \n+#include <initializer_list>\n+#include <map>\n+#include <optional>\n #include <stdint.h>\n \n #include <libcamera/base/span.h>\n@@ -19,7 +22,7 @@\n  * application code doesn't have to worry which kind to instantiate. But for\n  * the sake of example let's suppose we're parsing imx219 metadata.\n  *\n- * MdParser *parser = new MdParserImx219();  // for example\n+ * MdParser *parser = new MdParserSmia({ expHiReg, expLoReg, gainReg });\n  * parser->SetBitsPerPixel(bpp);\n  * parser->SetLineLengthBytes(pitch);\n  * parser->SetNumLines(2);\n@@ -32,13 +35,11 @@\n  *\n  * Then on every frame:\n  *\n- * if (parser->Parse(buffer) != MdParser::OK)\n+ * RegisterMap registers;\n+ * if (parser->Parse(buffer, registers) != MdParser::OK)\n  *     much badness;\n- * unsigned int exposure_lines, gain_code\n- * if (parser->GetExposureLines(exposure_lines) != MdParser::OK)\n- *     exposure was not found;\n- * if (parser->GetGainCode(parser, gain_code) != MdParser::OK)\n- *     gain code was not found;\n+ * Metadata metadata;\n+ * CamHelper::PopulateMetadata(registers, metadata);\n  *\n  * (Note that the CamHelper class converts to/from exposure lines and time,\n  * and gain_code / actual gain.)\n@@ -59,6 +60,8 @@ namespace RPiController {\n class MdParser\n {\n public:\n+\tusing RegisterMap = std::map<uint32_t, uint32_t>;\n+\n \t/*\n \t * Parser status codes:\n \t * OK       - success\n@@ -98,9 +101,8 @@ public:\n \t\tline_length_bytes_ = num_bytes;\n \t}\n \n-\tvirtual Status Parse(libcamera::Span<const uint8_t> buffer) = 0;\n-\tvirtual Status GetExposureLines(unsigned int &lines) = 0;\n-\tvirtual Status GetGainCode(unsigned int &gain_code) = 0;\n+\tvirtual Status Parse(libcamera::Span<const uint8_t> buffer,\n+\t\t\t     RegisterMap &registers) = 0;\n \n protected:\n \tbool reset_;\n@@ -116,14 +118,18 @@ protected:\n  * md_parser_imx219.cpp for an example).\n  */\n \n-class MdParserSmia : public MdParser\n+class MdParserSmia final : public MdParser\n {\n public:\n-\tMdParserSmia() : MdParser()\n-\t{\n-\t}\n+\tMdParserSmia(std::initializer_list<uint32_t> registerList);\n+\n+\tMdParser::Status Parse(libcamera::Span<const uint8_t> buffer,\n+\t\t\t       RegisterMap &registers) override;\n+\n+private:\n+\t/* Maps register address to offset in the buffer. */\n+\tusing OffsetMap = std::map<uint32_t, std::optional<uint32_t>>;\n \n-protected:\n \t/*\n \t * Note that error codes > 0 are regarded as non-fatal; codes < 0\n \t * indicate a bad data buffer. Status codes are:\n@@ -141,8 +147,9 @@ protected:\n \t\tBAD_PADDING   = -5\n \t};\n \n-\tParseStatus findRegs(libcamera::Span<const uint8_t> buffer, uint32_t regs[],\n-\t\t\t     int offsets[], unsigned int num_regs);\n+\tParseStatus findRegs(libcamera::Span<const uint8_t> buffer);\n+\n+\tOffsetMap offsets_;\n };\n \n } // namespace RPi\ndiff --git a/src/ipa/raspberrypi/md_parser_smia.cpp b/src/ipa/raspberrypi/md_parser_smia.cpp\nindex 0a14875575a2..7f72fc03948f 100644\n--- a/src/ipa/raspberrypi/md_parser_smia.cpp\n+++ b/src/ipa/raspberrypi/md_parser_smia.cpp\n@@ -6,9 +6,11 @@\n  */\n #include <assert.h>\n \n+#include \"libcamera/base/log.h\"\n #include \"md_parser.hpp\"\n \n using namespace RPiController;\n+using namespace libcamera;\n \n /*\n  * This function goes through the embedded data to find the offsets (not\n@@ -26,18 +28,61 @@ constexpr unsigned int REG_LOW_BITS = 0xa5;\n constexpr unsigned int REG_VALUE = 0x5a;\n constexpr unsigned int REG_SKIP = 0x55;\n \n-MdParserSmia::ParseStatus MdParserSmia::findRegs(libcamera::Span<const uint8_t> buffer,\n-\t\t\t\t\t\t uint32_t regs[], int offsets[],\n-\t\t\t\t\t\t unsigned int num_regs)\n+MdParserSmia::MdParserSmia(std::initializer_list<uint32_t> registerList)\n {\n-\tassert(num_regs > 0);\n+\tfor (auto r : registerList)\n+\t\toffsets_[r] = {};\n+}\n+\n+MdParser::Status MdParserSmia::Parse(libcamera::Span<const uint8_t> buffer,\n+\t\t\t\t     RegisterMap &registers)\n+{\n+\tif (reset_) {\n+\t\t/*\n+\t\t * Search again through the metadata for all the registers\n+\t\t * requested.\n+\t\t */\n+\t\tASSERT(bits_per_pixel_);\n+\n+\t\tfor (const auto &[reg, offset] : offsets_)\n+\t\t\toffsets_[reg] = {};\n+\n+\t\tParseStatus ret = findRegs(buffer);\n+\t\t/*\n+\t\t * > 0 means \"worked partially but parse again next time\",\n+\t\t * < 0 means \"hard error\".\n+\t\t *\n+\t\t * In either case, we retry parsing on the next frame.\n+\t\t */\n+\t\tif (ret != PARSE_OK)\n+\t\t\treturn ERROR;\n+\n+\t\treset_ = false;\n+\t}\n+\n+\t/* Populate the register values requested. */\n+\tregisters.clear();\n+\tfor (const auto &[reg, offset] : offsets_) {\n+\t\tif (!offset) {\n+\t\t\treset_ = true;\n+\t\t\treturn NOTFOUND;\n+\t\t}\n+\t\tregisters[reg] = buffer[offset.value()];\n+\t}\n+\n+\treturn OK;\n+}\n+\n+MdParserSmia::ParseStatus MdParserSmia::findRegs(libcamera::Span<const uint8_t> buffer)\n+{\n+\tASSERT(offsets_.size());\n \n \tif (buffer[0] != LINE_START)\n \t\treturn NO_LINE_START;\n \n \tunsigned int current_offset = 1; /* after the LINE_START */\n \tunsigned int current_line_start = 0, current_line = 0;\n-\tunsigned int reg_num = 0, first_reg = 0;\n+\tunsigned int reg_num = 0, regs_done = 0;\n \n \twhile (1) {\n \t\tint tag = buffer[current_offset++];\n@@ -89,13 +134,12 @@ MdParserSmia::ParseStatus MdParserSmia::findRegs(libcamera::Span<const uint8_t>\n \t\t\telse if (tag == REG_SKIP)\n \t\t\t\treg_num++;\n \t\t\telse if (tag == REG_VALUE) {\n-\t\t\t\twhile (reg_num >=\n-\t\t\t\t       /* assumes registers are in order... */\n-\t\t\t\t       regs[first_reg]) {\n-\t\t\t\t\tif (reg_num == regs[first_reg])\n-\t\t\t\t\t\toffsets[first_reg] = current_offset - 1;\n+\t\t\t\tauto reg = offsets_.find(reg_num);\n+\n+\t\t\t\tif (reg != offsets_.end()) {\n+\t\t\t\t\toffsets_[reg_num] = current_offset - 1;\n \n-\t\t\t\t\tif (++first_reg == num_regs)\n+\t\t\t\t\tif (++regs_done == offsets_.size())\n \t\t\t\t\t\treturn PARSE_OK;\n \t\t\t\t}\n \t\t\t\treg_num++;\n","prefixes":["libcamera-devel","v3","2/2"]}