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