<div dir="ltr"><div dir="ltr">Hi Laurent,<div><br></div></div>Thank you for these fixups!<div>Apart from one minor thing below, all looks good.</div><div><div><br></div><div><div class="gmail_quote"><div dir="ltr" class="gmail_attr">On Tue, 29 Jun 2021 at 16:48, Laurent Pinchart <<a href="mailto:laurent.pinchart@ideasonboard.com">laurent.pinchart@ideasonboard.com</a>> wrote:<br></div><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left:1px solid rgb(204,204,204);padding-left:1ex">From: Naushir Patuck <<a href="mailto:naush@raspberrypi.com" target="_blank">naush@raspberrypi.com</a>><br>
<br>
Instead of having each CamHelper subclass the MdParserSmia, change the<br>
implementation of MdParserSmia to be more generic. The MdParserSmia now gets<br>
given a list of registers to search for and helper functions are used to compute<br>
exposure lines and gain codes from these registers.<br>
<br>
Update the imx219 and imx477 CamHelpers by using this new mechanism.<br>
<br>
Signed-off-by: Naushir Patuck <<a href="mailto:naush@raspberrypi.com" target="_blank">naush@raspberrypi.com</a>><br>
Reviewed-by: Laurent Pinchart <<a href="mailto:laurent.pinchart@ideasonboard.com" target="_blank">laurent.pinchart@ideasonboard.com</a>><br>
Signed-off-by: Laurent Pinchart <<a href="mailto:laurent.pinchart@ideasonboard.com" target="_blank">laurent.pinchart@ideasonboard.com</a>><br>
---<br>
src/ipa/raspberrypi/cam_helper.cpp | 33 +++---<br>
src/ipa/raspberrypi/cam_helper.hpp | 2 +<br>
src/ipa/raspberrypi/cam_helper_imx219.cpp | 118 ++++----------------<br>
src/ipa/raspberrypi/cam_helper_imx477.cpp | 126 +++++-----------------<br>
src/ipa/raspberrypi/md_parser.hpp | 41 ++++---<br>
src/ipa/raspberrypi/md_parser_smia.cpp | 67 +++++++++---<br>
6 files changed, 148 insertions(+), 239 deletions(-)<br>
<br>
diff --git a/src/ipa/raspberrypi/cam_helper.cpp b/src/ipa/raspberrypi/cam_helper.cpp<br>
index 90498c37af98..e6d2258c66d7 100644<br>
--- a/src/ipa/raspberrypi/cam_helper.cpp<br>
+++ b/src/ipa/raspberrypi/cam_helper.cpp<br>
@@ -159,32 +159,34 @@ unsigned int CamHelper::MistrustFramesModeSwitch() const<br>
void CamHelper::parseEmbeddedData(Span<const uint8_t> buffer,<br>
Metadata &metadata)<br>
{<br>
+ MdParser::RegisterMap registers;<br>
+ Metadata parsedMetadata;<br>
+<br>
if (buffer.empty())<br>
return;<br>
<br>
- uint32_t exposureLines, gainCode;<br>
-<br>
- if (parser_->Parse(buffer) != MdParser::Status::OK ||<br>
- parser_->GetExposureLines(exposureLines) != MdParser::Status::OK ||<br>
- parser_->GetGainCode(gainCode) != MdParser::Status::OK) {<br>
+ if (parser_->Parse(buffer, registers) != MdParser::Status::OK) {<br>
LOG(IPARPI, Error) << "Embedded data buffer parsing failed";<br>
return;<br>
}<br>
<br>
+ PopulateMetadata(registers, parsedMetadata);<br>
+ metadata.Merge(parsedMetadata);<br>
+<br>
/*<br>
- * Overwrite the exposure/gain values in the DeviceStatus, as<br>
- * we know better. Fetch it first in case any other fields were<br>
- * set meaningfully.<br>
+ * Overwrite the exposure/gain values in the existing DeviceStatus with<br>
+ * values from the parsed embedded buffer. Fetch it first in case any<br>
+ * other fields were set meaningfully.<br>
*/<br>
- DeviceStatus deviceStatus;<br>
-<br>
- if (metadata.Get("device.status", deviceStatus) != 0) {<br>
+ DeviceStatus deviceStatus, parsedDeviceStatus;<br>
+ if (metadata.Get("device.status", deviceStatus) ||<br>
+ parsedMetadata.Get("device.status", parsedDeviceStatus)) {<br>
LOG(IPARPI, Error) << "DeviceStatus not found";<br>
return;<br>
}<br>
<br>
- deviceStatus.shutter_speed = Exposure(exposureLines);<br>
- deviceStatus.analogue_gain = Gain(gainCode);<br>
+ deviceStatus.shutter_speed = parsedDeviceStatus.shutter_speed;<br>
+ deviceStatus.analogue_gain = parsedDeviceStatus.analogue_gain;<br>
<br>
LOG(IPARPI, Debug) << "Metadata updated - Exposure : "<br>
<< deviceStatus.shutter_speed<br>
@@ -194,6 +196,11 @@ void CamHelper::parseEmbeddedData(Span<const uint8_t> buffer,<br>
metadata.Set("device.status", deviceStatus);<br>
}<br>
<br>
+void CamHelper::PopulateMetadata([[maybe_unused]] const MdParser::RegisterMap ®isters,<br>
+ [[maybe_unused]] Metadata &metadata) const<br>
+{<br>
+}<br>
+<br>
RegisterCamHelper::RegisterCamHelper(char const *cam_name,<br>
CamHelperCreateFunc create_func)<br>
{<br>
diff --git a/src/ipa/raspberrypi/cam_helper.hpp b/src/ipa/raspberrypi/cam_helper.hpp<br>
index fc3139e22be0..200cc83f3872 100644<br>
--- a/src/ipa/raspberrypi/cam_helper.hpp<br>
+++ b/src/ipa/raspberrypi/cam_helper.hpp<br>
@@ -92,6 +92,8 @@ public:<br>
protected:<br>
void parseEmbeddedData(libcamera::Span<const uint8_t> buffer,<br>
Metadata &metadata);<br>
+ virtual void PopulateMetadata(const MdParser::RegisterMap ®isters,<br>
+ Metadata &metadata) const;<br>
<br>
std::unique_ptr<MdParser> parser_;<br>
CameraMode mode_;<br>
diff --git a/src/ipa/raspberrypi/cam_helper_imx219.cpp b/src/ipa/raspberrypi/cam_helper_imx219.cpp<br>
index c85044a5fa6d..4d68e01fce71 100644<br>
--- a/src/ipa/raspberrypi/cam_helper_imx219.cpp<br>
+++ b/src/ipa/raspberrypi/cam_helper_imx219.cpp<br>
@@ -23,21 +23,14 @@<br>
<br>
using namespace RPiController;<br>
<br>
-/* Metadata parser implementation specific to Sony IMX219 sensors. */<br>
-<br>
-class MdParserImx219 : public MdParserSmia<br>
-{<br>
-public:<br>
- MdParserImx219();<br>
- Status Parse(libcamera::Span<const uint8_t> buffer) override;<br>
- Status GetExposureLines(unsigned int &lines) override;<br>
- Status GetGainCode(unsigned int &gain_code) override;<br>
-private:<br>
- /* Offset of the register's value in the metadata block. */<br>
- int reg_offsets_[3];<br>
- /* Value of the register, once read from the metadata block. */<br>
- int reg_values_[3];<br>
-};<br>
+/*<br>
+ * We care about one gain register and a pair of exposure registers. Their I2C<br>
+ * addresses from the Sony IMX219 datasheet:<br>
+ */<br>
+constexpr uint32_t gainReg = 0x157;<br>
+constexpr uint32_t expHiReg = 0x15a;<br>
+constexpr uint32_t expLoReg = 0x15b;<br>
+constexpr std::initializer_list<uint32_t> registerList [[maybe_unused]] = { expHiReg, expLoReg, gainReg };<br>
<br>
class CamHelperImx219 : public CamHelper<br>
{<br>
@@ -54,11 +47,14 @@ private:<br>
* in units of lines.<br>
*/<br>
static constexpr int frameIntegrationDiff = 4;<br>
+<br>
+ void PopulateMetadata(const MdParser::RegisterMap ®isters,<br>
+ Metadata &metadata) const override;<br>
};<br>
<br>
CamHelperImx219::CamHelperImx219()<br>
#if ENABLE_EMBEDDED_DATA<br>
- : CamHelper(std::make_unique<MdParserImx219>(), frameIntegrationDiff)<br>
+ : CamHelper(std::make_unique<MdParserSmia>(registerList), frameIntegrationDiff)<br>
#else<br>
: CamHelper({}, frameIntegrationDiff)<br>
#endif<br>
@@ -90,88 +86,20 @@ bool CamHelperImx219::SensorEmbeddedDataPresent() const<br>
return ENABLE_EMBEDDED_DATA;<br>
}<br>
<br>
+void CamHelperImx219::PopulateMetadata(const MdParser::RegisterMap ®isters,<br>
+ Metadata &metadata) const<br>
+{<br>
+ DeviceStatus deviceStatus;<br>
+<br>
+ deviceStatus.shutter_speed = Exposure(<a href="http://registers.at" rel="noreferrer" target="_blank">registers.at</a>(expHiReg) * 256 + <a href="http://registers.at" rel="noreferrer" target="_blank">registers.at</a>(expLoReg));<br>
+ deviceStatus.analogue_gain = Gain(<a href="http://registers.at" rel="noreferrer" target="_blank">registers.at</a>(gainReg));<br>
+<br>
+ metadata.Set("device.status", deviceStatus);<br>
+}<br>
+<br>
static CamHelper *Create()<br>
{<br>
return new CamHelperImx219();<br>
}<br>
<br>
static RegisterCamHelper reg("imx219", &Create);<br>
-<br>
-/*<br>
- * We care about one gain register and a pair of exposure registers. Their I2C<br>
- * addresses from the Sony IMX219 datasheet:<br>
- */<br>
-#define GAIN_REG 0x157<br>
-#define EXPHI_REG 0x15A<br>
-#define EXPLO_REG 0x15B<br>
-<br>
-/*<br>
- * Index of each into the reg_offsets and reg_values arrays. Must be in<br>
- * register address order.<br>
- */<br>
-#define GAIN_INDEX 0<br>
-#define EXPHI_INDEX 1<br>
-#define EXPLO_INDEX 2<br>
-<br>
-MdParserImx219::MdParserImx219()<br>
-{<br>
- reg_offsets_[0] = reg_offsets_[1] = reg_offsets_[2] = -1;<br>
-}<br>
-<br>
-MdParser::Status MdParserImx219::Parse(libcamera::Span<const uint8_t> buffer)<br>
-{<br>
- bool try_again = false;<br>
-<br>
- if (reset_) {<br>
- /*<br>
- * Search again through the metadata for the gain and exposure<br>
- * registers.<br>
- */<br>
- assert(bits_per_pixel_);<br>
- /* Need to be ordered */<br>
- uint32_t regs[3] = { GAIN_REG, EXPHI_REG, EXPLO_REG };<br>
- reg_offsets_[0] = reg_offsets_[1] = reg_offsets_[2] = -1;<br>
- int ret = static_cast<int>(findRegs(buffer,<br>
- regs, reg_offsets_, 3));<br>
- /*<br>
- * > 0 means "worked partially but parse again next time",<br>
- * < 0 means "hard error".<br>
- */<br>
- if (ret > 0)<br>
- try_again = true;<br>
- else if (ret < 0)<br>
- return ERROR;<br>
- }<br>
-<br>
- for (int i = 0; i < 3; i++) {<br>
- if (reg_offsets_[i] == -1)<br>
- continue;<br>
-<br>
- reg_values_[i] = buffer[reg_offsets_[i]];<br>
- }<br>
-<br>
- /* Re-parse next time if we were unhappy in some way. */<br>
- reset_ = try_again;<br>
-<br>
- return OK;<br>
-}<br>
-<br>
-MdParser::Status MdParserImx219::GetExposureLines(unsigned int &lines)<br>
-{<br>
- if (reg_offsets_[EXPHI_INDEX] == -1 || reg_offsets_[EXPLO_INDEX] == -1)<br>
- return NOTFOUND;<br>
-<br>
- lines = reg_values_[EXPHI_INDEX] * 256 + reg_values_[EXPLO_INDEX];<br>
-<br>
- return OK;<br>
-}<br>
-<br>
-MdParser::Status MdParserImx219::GetGainCode(unsigned int &gain_code)<br>
-{<br>
- if (reg_offsets_[GAIN_INDEX] == -1)<br>
- return NOTFOUND;<br>
-<br>
- gain_code = reg_values_[GAIN_INDEX];<br>
-<br>
- return OK;<br>
-}<br>
diff --git a/src/ipa/raspberrypi/cam_helper_imx477.cpp b/src/ipa/raspberrypi/cam_helper_imx477.cpp<br>
index d72a9be0438e..4098fde6f322 100644<br>
--- a/src/ipa/raspberrypi/cam_helper_imx477.cpp<br>
+++ b/src/ipa/raspberrypi/cam_helper_imx477.cpp<br>
@@ -15,21 +15,15 @@<br>
<br>
using namespace RPiController;<br>
<br>
-/* Metadata parser implementation specific to Sony IMX477 sensors. */<br>
-<br>
-class MdParserImx477 : public MdParserSmia<br>
-{<br>
-public:<br>
- MdParserImx477();<br>
- Status Parse(libcamera::Span<const uint8_t> buffer) override;<br>
- Status GetExposureLines(unsigned int &lines) override;<br>
- Status GetGainCode(unsigned int &gain_code) override;<br>
-private:<br>
- /* Offset of the register's value in the metadata block. */<br>
- int reg_offsets_[4];<br>
- /* Value of the register, once read from the metadata block. */<br>
- int reg_values_[4];<br>
-};<br>
+/*<br>
+ * We care about two gain registers and a pair of exposure registers. Their<br>
+ * I2C addresses from the Sony IMX477 datasheet:<br>
+ */<br>
+constexpr uint32_t expHiReg = 0x0202;<br>
+constexpr uint32_t expLoReg = 0x0203;<br>
+constexpr uint32_t gainHiReg = 0x0204;<br>
+constexpr uint32_t gainLoReg = 0x0205;<br>
+constexpr std::initializer_list<uint32_t> registerList [[maybe_unused]] = { expHiReg, expLoReg, gainHiReg, gainLoReg };<br></blockquote><div><br></div><div>This does not need to have [[maybe_unused]] as we do not switch metadata parsing off</div><div>in the imx477.</div><div><br></div><div>Not sure I am able to tag my own patch, but...</div><div><br></div><div>Reviewed-by: Naushir Patuck <<a href="mailto:naush@raspberrypi.com">naush@raspberrypi.com</a>></div><div><br></div><div>:)</div><div> </div><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left:1px solid rgb(204,204,204);padding-left:1ex">
<br>
class CamHelperImx477 : public CamHelper<br>
{<br>
@@ -47,10 +41,13 @@ private:<br>
* in units of lines.<br>
*/<br>
static constexpr int frameIntegrationDiff = 22;<br>
+<br>
+ void PopulateMetadata(const MdParser::RegisterMap ®isters,<br>
+ Metadata &metadata) const override;<br>
};<br>
<br>
CamHelperImx477::CamHelperImx477()<br>
- : CamHelper(std::make_unique<MdParserImx477>(), frameIntegrationDiff)<br>
+ : CamHelper(std::make_unique<MdParserSmia>(registerList), frameIntegrationDiff)<br>
{<br>
}<br>
<br>
@@ -77,95 +74,20 @@ bool CamHelperImx477::SensorEmbeddedDataPresent() const<br>
return true;<br>
}<br>
<br>
+void CamHelperImx477::PopulateMetadata(const MdParser::RegisterMap ®isters,<br>
+ Metadata &metadata) const<br>
+{<br>
+ DeviceStatus deviceStatus;<br>
+<br>
+ deviceStatus.shutter_speed = Exposure(<a href="http://registers.at" rel="noreferrer" target="_blank">registers.at</a>(expHiReg) * 256 + <a href="http://registers.at" rel="noreferrer" target="_blank">registers.at</a>(expLoReg));<br>
+ deviceStatus.analogue_gain = Gain(<a href="http://registers.at" rel="noreferrer" target="_blank">registers.at</a>(gainHiReg) * 256 + <a href="http://registers.at" rel="noreferrer" target="_blank">registers.at</a>(gainLoReg));<br>
+<br>
+ metadata.Set("device.status", deviceStatus);<br>
+}<br>
+<br>
static CamHelper *Create()<br>
{<br>
return new CamHelperImx477();<br>
}<br>
<br>
static RegisterCamHelper reg("imx477", &Create);<br>
-<br>
-/*<br>
- * We care about two gain registers and a pair of exposure registers. Their<br>
- * I2C addresses from the Sony IMX477 datasheet:<br>
- */<br>
-#define EXPHI_REG 0x0202<br>
-#define EXPLO_REG 0x0203<br>
-#define GAINHI_REG 0x0204<br>
-#define GAINLO_REG 0x0205<br>
-<br>
-/*<br>
- * Index of each into the reg_offsets and reg_values arrays. Must be in register<br>
- * address order.<br>
- */<br>
-#define EXPHI_INDEX 0<br>
-#define EXPLO_INDEX 1<br>
-#define GAINHI_INDEX 2<br>
-#define GAINLO_INDEX 3<br>
-<br>
-MdParserImx477::MdParserImx477()<br>
-{<br>
- reg_offsets_[0] = reg_offsets_[1] = reg_offsets_[2] = reg_offsets_[3] = -1;<br>
-}<br>
-<br>
-MdParser::Status MdParserImx477::Parse(libcamera::Span<const uint8_t> buffer)<br>
-{<br>
- bool try_again = false;<br>
-<br>
- if (reset_) {<br>
- /*<br>
- * Search again through the metadata for the gain and exposure<br>
- * registers.<br>
- */<br>
- assert(bits_per_pixel_);<br>
- /* Need to be ordered */<br>
- uint32_t regs[4] = {<br>
- EXPHI_REG,<br>
- EXPLO_REG,<br>
- GAINHI_REG,<br>
- GAINLO_REG<br>
- };<br>
- reg_offsets_[0] = reg_offsets_[1] = reg_offsets_[2] = reg_offsets_[3] = -1;<br>
- int ret = static_cast<int>(findRegs(buffer,<br>
- regs, reg_offsets_, 4));<br>
- /*<br>
- * > 0 means "worked partially but parse again next time",<br>
- * < 0 means "hard error".<br>
- */<br>
- if (ret > 0)<br>
- try_again = true;<br>
- else if (ret < 0)<br>
- return ERROR;<br>
- }<br>
-<br>
- for (int i = 0; i < 4; i++) {<br>
- if (reg_offsets_[i] == -1)<br>
- continue;<br>
-<br>
- reg_values_[i] = buffer[reg_offsets_[i]];<br>
- }<br>
-<br>
- /* Re-parse next time if we were unhappy in some way. */<br>
- reset_ = try_again;<br>
-<br>
- return OK;<br>
-}<br>
-<br>
-MdParser::Status MdParserImx477::GetExposureLines(unsigned int &lines)<br>
-{<br>
- if (reg_offsets_[EXPHI_INDEX] == -1 || reg_offsets_[EXPLO_INDEX] == -1)<br>
- return NOTFOUND;<br>
-<br>
- lines = reg_values_[EXPHI_INDEX] * 256 + reg_values_[EXPLO_INDEX];<br>
-<br>
- return OK;<br>
-}<br>
-<br>
-MdParser::Status MdParserImx477::GetGainCode(unsigned int &gain_code)<br>
-{<br>
- if (reg_offsets_[GAINHI_INDEX] == -1 || reg_offsets_[GAINLO_INDEX] == -1)<br>
- return NOTFOUND;<br>
-<br>
- gain_code = reg_values_[GAINHI_INDEX] * 256 + reg_values_[GAINLO_INDEX];<br>
-<br>
- return OK;<br>
-}<br>
diff --git a/src/ipa/raspberrypi/md_parser.hpp b/src/ipa/raspberrypi/md_parser.hpp<br>
index 8497216f8db7..e3e2738537a8 100644<br>
--- a/src/ipa/raspberrypi/md_parser.hpp<br>
+++ b/src/ipa/raspberrypi/md_parser.hpp<br>
@@ -6,6 +6,9 @@<br>
*/<br>
#pragma once<br>
<br>
+#include <initializer_list><br>
+#include <map><br>
+#include <optional><br>
#include <stdint.h><br>
<br>
#include <libcamera/base/span.h><br>
@@ -19,7 +22,7 @@<br>
* application code doesn't have to worry which kind to instantiate. But for<br>
* the sake of example let's suppose we're parsing imx219 metadata.<br>
*<br>
- * MdParser *parser = new MdParserImx219(); // for example<br>
+ * MdParser *parser = new MdParserSmia({ expHiReg, expLoReg, gainReg });<br>
* parser->SetBitsPerPixel(bpp);<br>
* parser->SetLineLengthBytes(pitch);<br>
* parser->SetNumLines(2);<br>
@@ -32,13 +35,11 @@<br>
*<br>
* Then on every frame:<br>
*<br>
- * if (parser->Parse(buffer) != MdParser::OK)<br>
+ * RegisterMap registers;<br>
+ * if (parser->Parse(buffer, registers) != MdParser::OK)<br>
* much badness;<br>
- * unsigned int exposure_lines, gain_code<br>
- * if (parser->GetExposureLines(exposure_lines) != MdParser::OK)<br>
- * exposure was not found;<br>
- * if (parser->GetGainCode(parser, gain_code) != MdParser::OK)<br>
- * gain code was not found;<br>
+ * Metadata metadata;<br>
+ * CamHelper::PopulateMetadata(registers, metadata);<br>
*<br>
* (Note that the CamHelper class converts to/from exposure lines and time,<br>
* and gain_code / actual gain.)<br>
@@ -59,6 +60,8 @@ namespace RPiController {<br>
class MdParser<br>
{<br>
public:<br>
+ using RegisterMap = std::map<uint32_t, uint32_t>;<br>
+<br>
/*<br>
* Parser status codes:<br>
* OK - success<br>
@@ -98,9 +101,8 @@ public:<br>
line_length_bytes_ = num_bytes;<br>
}<br>
<br>
- virtual Status Parse(libcamera::Span<const uint8_t> buffer) = 0;<br>
- virtual Status GetExposureLines(unsigned int &lines) = 0;<br>
- virtual Status GetGainCode(unsigned int &gain_code) = 0;<br>
+ virtual Status Parse(libcamera::Span<const uint8_t> buffer,<br>
+ RegisterMap ®isters) = 0;<br>
<br>
protected:<br>
bool reset_;<br>
@@ -116,14 +118,18 @@ protected:<br>
* md_parser_imx219.cpp for an example).<br>
*/<br>
<br>
-class MdParserSmia : public MdParser<br>
+class MdParserSmia final : public MdParser<br>
{<br>
public:<br>
- MdParserSmia() : MdParser()<br>
- {<br>
- }<br>
+ MdParserSmia(std::initializer_list<uint32_t> registerList);<br>
+<br>
+ MdParser::Status Parse(libcamera::Span<const uint8_t> buffer,<br>
+ RegisterMap ®isters) override;<br>
+<br>
+private:<br>
+ /* Maps register address to offset in the buffer. */<br>
+ using OffsetMap = std::map<uint32_t, std::optional<uint32_t>>;<br>
<br>
-protected:<br>
/*<br>
* Note that error codes > 0 are regarded as non-fatal; codes < 0<br>
* indicate a bad data buffer. Status codes are:<br>
@@ -141,8 +147,9 @@ protected:<br>
BAD_PADDING = -5<br>
};<br>
<br>
- ParseStatus findRegs(libcamera::Span<const uint8_t> buffer, uint32_t regs[],<br>
- int offsets[], unsigned int num_regs);<br>
+ ParseStatus findRegs(libcamera::Span<const uint8_t> buffer);<br>
+<br>
+ OffsetMap offsets_;<br>
};<br>
<br>
} // namespace RPi<br>
diff --git a/src/ipa/raspberrypi/md_parser_smia.cpp b/src/ipa/raspberrypi/md_parser_smia.cpp<br>
index 0a14875575a2..ea5eac414b36 100644<br>
--- a/src/ipa/raspberrypi/md_parser_smia.cpp<br>
+++ b/src/ipa/raspberrypi/md_parser_smia.cpp<br>
@@ -4,11 +4,12 @@<br>
*<br>
* md_parser_smia.cpp - SMIA specification based embedded data parser<br>
*/<br>
-#include <assert.h><br>
<br>
+#include <libcamera/base/log.h><br>
#include "md_parser.hpp"<br>
<br>
using namespace RPiController;<br>
+using namespace libcamera;<br>
<br>
/*<br>
* This function goes through the embedded data to find the offsets (not<br>
@@ -26,18 +27,61 @@ constexpr unsigned int REG_LOW_BITS = 0xa5;<br>
constexpr unsigned int REG_VALUE = 0x5a;<br>
constexpr unsigned int REG_SKIP = 0x55;<br>
<br>
-MdParserSmia::ParseStatus MdParserSmia::findRegs(libcamera::Span<const uint8_t> buffer,<br>
- uint32_t regs[], int offsets[],<br>
- unsigned int num_regs)<br>
+MdParserSmia::MdParserSmia(std::initializer_list<uint32_t> registerList)<br>
{<br>
- assert(num_regs > 0);<br>
+ for (auto r : registerList)<br>
+ offsets_[r] = {};<br>
+}<br>
+<br>
+MdParser::Status MdParserSmia::Parse(libcamera::Span<const uint8_t> buffer,<br>
+ RegisterMap ®isters)<br>
+{<br>
+ if (reset_) {<br>
+ /*<br>
+ * Search again through the metadata for all the registers<br>
+ * requested.<br>
+ */<br>
+ ASSERT(bits_per_pixel_);<br>
+<br>
+ for (const auto &kv : offsets_)<br>
+ offsets_[kv.first] = {};<br>
+<br>
+ ParseStatus ret = findRegs(buffer);<br>
+ /*<br>
+ * > 0 means "worked partially but parse again next time",<br>
+ * < 0 means "hard error".<br>
+ *<br>
+ * In either case, we retry parsing on the next frame.<br>
+ */<br>
+ if (ret != PARSE_OK)<br>
+ return ERROR;<br>
+<br>
+ reset_ = false;<br>
+ }<br>
+<br>
+ /* Populate the register values requested. */<br>
+ registers.clear();<br>
+ for (const auto &[reg, offset] : offsets_) {<br>
+ if (!offset) {<br>
+ reset_ = true;<br>
+ return NOTFOUND;<br>
+ }<br>
+ registers[reg] = buffer[offset.value()];<br>
+ }<br>
+<br>
+ return OK;<br>
+}<br>
+<br>
+MdParserSmia::ParseStatus MdParserSmia::findRegs(libcamera::Span<const uint8_t> buffer)<br>
+{<br>
+ ASSERT(offsets_.size());<br>
<br>
if (buffer[0] != LINE_START)<br>
return NO_LINE_START;<br>
<br>
unsigned int current_offset = 1; /* after the LINE_START */<br>
unsigned int current_line_start = 0, current_line = 0;<br>
- unsigned int reg_num = 0, first_reg = 0;<br>
+ unsigned int reg_num = 0, regs_done = 0;<br>
<br>
while (1) {<br>
int tag = buffer[current_offset++];<br>
@@ -89,13 +133,12 @@ MdParserSmia::ParseStatus MdParserSmia::findRegs(libcamera::Span<const uint8_t><br>
else if (tag == REG_SKIP)<br>
reg_num++;<br>
else if (tag == REG_VALUE) {<br>
- while (reg_num >=<br>
- /* assumes registers are in order... */<br>
- regs[first_reg]) {<br>
- if (reg_num == regs[first_reg])<br>
- offsets[first_reg] = current_offset - 1;<br>
+ auto reg = offsets_.find(reg_num);<br>
<br>
- if (++first_reg == num_regs)<br>
+ if (reg != offsets_.end()) {<br>
+ offsets_[reg_num] = current_offset - 1;<br>
+<br>
+ if (++regs_done == offsets_.size())<br>
return PARSE_OK;<br>
}<br>
reg_num++;<br>
-- <br>
Regards,<br>
<br>
Laurent Pinchart<br>
<br>
</blockquote></div></div></div></div>