<div dir="ltr"><div dir="ltr">Hi Laurent,<div><br></div><div>Thank you for your feedback.</div></div><br><div class="gmail_quote"><div dir="ltr" class="gmail_attr">On Tue, 15 Jun 2021 at 04:09, Laurent Pinchart <<a href="mailto:laurent.pinchart@ideasonboard.com" target="_blank">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">Hi Naush,<br>
<br>
Thank you for the patch.<br>
<br>
On Mon, Jun 14, 2021 at 10:53:40AM +0100, Naushir Patuck wrote:<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>
> As a drive-by change, fixup a possible buffer overrun in the parsing code.<br>
<br>
Could that be split to a separate patch ?</blockquote><div><br></div><div>Sure, will do. </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>
> Signed-off-by: Naushir Patuck <<a href="mailto:naush@raspberrypi.com" target="_blank">naush@raspberrypi.com</a>><br>
> ---<br>
>  src/ipa/raspberrypi/cam_helper_imx219.cpp | 117 ++++----------------<br>
>  src/ipa/raspberrypi/cam_helper_imx477.cpp | 125 ++++------------------<br>
>  src/ipa/raspberrypi/md_parser.hpp         |  42 ++++++--<br>
>  src/ipa/raspberrypi/md_parser_smia.cpp    |  89 ++++++++++++---<br>
>  4 files changed, 153 insertions(+), 220 deletions(-)<br>
> <br>
> diff --git a/src/ipa/raspberrypi/cam_helper_imx219.cpp b/src/ipa/raspberrypi/cam_helper_imx219.cpp<br>
> index 36dbe8cd941a..72c1042ad6be 100644<br>
> --- a/src/ipa/raspberrypi/cam_helper_imx219.cpp<br>
> +++ b/src/ipa/raspberrypi/cam_helper_imx219.cpp<br>
> @@ -23,21 +23,13 @@<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>
<br>
Lowercase for hex constants please.<br></blockquote><div><br></div><div>Ack.</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>
> +constexpr uint32_t expLoReg = 0x15B;<br>
>  <br>
>  class CamHelperImx219 : public CamHelper<br>
>  {<br>
> @@ -55,11 +47,23 @@ private:<br>
>        */<br>
>       static constexpr int frameIntegrationDiff = 4;<br>
>  <br>
> -     MdParserImx219 imx219_parser;<br>
> +     MdParserSmia imx219_parser;<br>
> +<br>
> +     static uint32_t ParseExposureLines(const MdParserSmia::RegMap &map)<br>
> +     {<br>
> +             return <a href="http://map.at" rel="noreferrer" target="_blank">map.at</a>(expHiReg).value * 256 + <a href="http://map.at" rel="noreferrer" target="_blank">map.at</a>(expLoReg).value;<br>
> +     }<br>
> +<br>
> +     static uint32_t ParseGainCode(const MdParserSmia::RegMap &map)<br>
> +     {<br>
> +             return <a href="http://map.at" rel="noreferrer" target="_blank">map.at</a>(gainReg).value;<br>
> +     }<br>
>  };<br>
>  <br>
>  CamHelperImx219::CamHelperImx219()<br>
> -     : CamHelper(frameIntegrationDiff)<br>
> +     : CamHelper(frameIntegrationDiff),<br>
> +       imx219_parser({ expHiReg, expLoReg, gainReg },<br>
> +                     ParseGainCode, ParseExposureLines)<br>
<br>
I'm not thrilled by this to be honest, as we'll have to add more<br>
functions when we'll want to parse more than just the exposure time and<br>
gain. Could we use a mechanism that would scale better ? Decoupling the<br>
data parser from the data interpreter would be one way to achieve this.<br>
You could pass the list of registers you're interested in to the<br>
MdParserSmia constructor. It would the parse the embedded data in<br>
findRegs() (which should be renamed) and return a map of register<br>
address to register value. The CamHelper would then be responsible for<br>
extracting the register values and converting them.<br></blockquote><div><br></div><div>Yes, I do agree that this is somewhat restrictive.  Here's a way to decouple</div><div>the parsing and supplying values:</div><div><br></div><div>1) MdParser::Parse and MdParserSmia::Parse now return the register to offset</div><div>map, instead of keeping this internal:</div><br>Status Parse(libcamera::Span<const uint8_t> buffer, RegMap &map);</div><div class="gmail_quote"><br></div>2) CamHelper::parseEmbeddedData calls MdParser::Parse() as it currently does,<div>and this will now call a new virtual method CamHelper::fillMetadata() that will</div><div>fill our metadata buffer with whatever embedded data bits we think useful.</div><div>This way, all the sensor specific dealing is hidden behind the sensor derived </div><div>CamHelper class.</div><div><br></div><div>I did not get your point about renaming findRegs(), could you elaborate on that</div><div>a bit?</div><div><br></div><div>I'll prepare this change and we can discuss further once I have some code</div><div>written.</div><div><br></div><div>Regards,</div><div>Naush</div><div><div class="gmail_quote"><br><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left:1px solid rgb(204,204,204);padding-left:1ex">
<br>
>  {<br>
>  #if ENABLE_EMBEDDED_DATA<br>
>       parser_ = &imx219_parser;<br>
> @@ -97,82 +101,3 @@ static CamHelper *Create()<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 038a8583d311..7a1100c25afc 100644<br>
> --- a/src/ipa/raspberrypi/cam_helper_imx477.cpp<br>
> +++ b/src/ipa/raspberrypi/cam_helper_imx477.cpp<br>
> @@ -15,21 +15,14 @@<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>
>  <br>
>  class CamHelperImx477 : public CamHelper<br>
>  {<br>
> @@ -48,11 +41,23 @@ private:<br>
>        */<br>
>       static constexpr int frameIntegrationDiff = 22;<br>
>  <br>
> -     MdParserImx477 imx477_parser;<br>
> +     MdParserSmia imx477_parser;<br>
> +<br>
> +     static uint32_t ParseExposureLines(const MdParserSmia::RegMap &map)<br>
> +     {<br>
> +             return <a href="http://map.at" rel="noreferrer" target="_blank">map.at</a>(expHiReg).value * 256 + <a href="http://map.at" rel="noreferrer" target="_blank">map.at</a>(expLoReg).value;<br>
> +     }<br>
> +<br>
> +     static uint32_t ParseGainCode(const MdParserSmia::RegMap &map)<br>
> +     {<br>
> +             return <a href="http://map.at" rel="noreferrer" target="_blank">map.at</a>(gainHiReg).value * 256 + <a href="http://map.at" rel="noreferrer" target="_blank">map.at</a>(gainLoReg).value;<br>
> +     }<br>
>  };<br>
>  <br>
>  CamHelperImx477::CamHelperImx477()<br>
> -     : CamHelper(frameIntegrationDiff)<br>
> +     : CamHelper(frameIntegrationDiff),<br>
> +       imx477_parser({ expHiReg, expLoReg, gainHiReg, gainLoReg },<br>
> +                     ParseGainCode, ParseExposureLines)<br>
>  {<br>
>       parser_ = &imx477_parser;<br>
>  }<br>
> @@ -86,89 +91,3 @@ static CamHelper *Create()<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 25ba0e7c9400..6bbcdec0830b 100644<br>
> --- a/src/ipa/raspberrypi/md_parser.hpp<br>
> +++ b/src/ipa/raspberrypi/md_parser.hpp<br>
> @@ -6,6 +6,10 @@<br>
>   */<br>
>  #pragma once<br>
>  <br>
> +#include <functional><br>
> +#include <map><br>
> +#include <vector><br>
> +<br>
>  #include <libcamera/span.h><br>
>  <br>
>  /* Camera metadata parser class. Usage as shown below.<br>
> @@ -16,7 +20,8 @@<br>
>   * application code doesn't have to worry which to 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>
> +                                    ParseGainCode, ParseExposureLines));<br>
>   * parser->SetBitsPerPixel(bpp);<br>
>   * parser->SetLineLengthBytes(pitch);<br>
>   * parser->SetNumLines(2);<br>
> @@ -113,14 +118,32 @@ 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>
> +     struct Register {<br>
> +             Register()<br>
> +                     : offset(0), value(0), found(false)<br>
> +             {<br>
> +             }<br>
> +<br>
> +             uint32_t offset;<br>
> +             uint32_t value;<br>
> +             bool found;<br>
> +     };<br>
>  <br>
> -protected:<br>
> +     /* Maps register address to offset in the buffer. */<br>
> +     using RegMap = std::map<uint32_t, Register>;<br>
> +     using GetFn = std::function<uint32_t(const RegMap&)>;<br>
> +<br>
> +     MdParserSmia(const std::vector<uint32_t> &regs, GetFn gain_fn,<br>
> +                  GetFn exposureFn);<br>
> +<br>
> +     MdParser::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>
> +<br>
> +private:<br>
>       /*<br>
>        * Note that error codes > 0 are regarded as non-fatal; codes < 0<br>
>        * indicate a bad data buffer. Status codes are:<br>
> @@ -138,8 +161,11 @@ 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>
> +     RegMap map_;<br>
> +     GetFn gain_fn_;<br>
> +     GetFn exposure_fn_;<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 65ffbe00c76e..f4748dd535d0 100644<br>
> --- a/src/ipa/raspberrypi/md_parser_smia.cpp<br>
> +++ b/src/ipa/raspberrypi/md_parser_smia.cpp<br>
> @@ -8,9 +8,11 @@<br>
>  #include <map><br>
>  #include <string><br>
>  <br>
> +#include "libcamera/internal/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>
> @@ -28,18 +30,79 @@ 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(const std::vector<uint32_t> &registers,<br>
> +                        GetFn gain_fn, GetFn exposure_fn)<br>
> +     : gain_fn_(gain_fn), exposure_fn_(exposure_fn)<br>
>  {<br>
> -     assert(num_regs > 0);<br>
> +     for (auto r : registers)<br>
> +             map_[r] = {};<br>
> +}<br>
> +<br>
> +MdParser::Status MdParserSmia::Parse(libcamera::Span<const uint8_t> buffer)<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>
> +<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>
> +     for (auto &kv : map_) {<br>
> +             Register &reg = kv.second;<br>
> +<br>
> +             if (!reg.found) {<br>
> +                     reset_ = true;<br>
> +                     return NOTFOUND;<br>
> +             }<br>
> +<br>
> +             reg.value = buffer[reg.offset];<br>
> +     }<br>
> +<br>
> +     return OK;<br>
> +}<br>
> +<br>
> +MdParser::Status MdParserSmia::GetExposureLines(unsigned int &lines)<br>
> +{<br>
> +     if (reset_)<br>
> +             return NOTFOUND;<br>
> +<br>
> +     lines = exposure_fn_(map_);<br>
> +     return OK;<br>
> +}<br>
> +<br>
> +MdParser::Status MdParserSmia::GetGainCode(unsigned int &gain_code)<br>
> +{<br>
> +     if (reset_)<br>
> +             return NOTFOUND;<br>
> +<br>
> +     gain_code = gain_fn_(map_);<br>
> +     return OK;<br>
> +}<br>
> +<br>
> +MdParserSmia::ParseStatus MdParserSmia::findRegs(libcamera::Span<const uint8_t> buffer)<br>
> +{<br>
> +     ASSERT(map_.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>
> @@ -73,8 +136,8 @@ MdParserSmia::ParseStatus MdParserSmia::findRegs(libcamera::Span<const uint8_t><br>
>                                       return NO_LINE_START;<br>
>                       } else {<br>
>                               /* allow a zero line length to mean "hunt for the next line" */<br>
> -                             while (buffer[current_offset] != LINE_START &&<br>
> -                                    current_offset < buffer.size())<br>
> +                             while (current_offset < buffer.size() &&<br>
> +                                    buffer[current_offset] != LINE_START)<br>
>                                       current_offset++;<br>
>  <br>
>                               if (current_offset == buffer.size())<br>
> @@ -91,13 +154,13 @@ 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 = map_.find(reg_num);<br>
> +<br>
> +                             if (reg != map_.end()) {<br>
> +                                     map_[reg_num].offset = current_offset - 1;<br>
> +                                     map_[reg_num].found = true;<br>
>  <br>
> -                                     if (++first_reg == num_regs)<br>
> +                                     if (++regs_done == map_.size())<br>
>                                               return PARSE_OK;<br>
>                               }<br>
>                               reg_num++;<br>
<br>
-- <br>
Regards,<br>
<br>
Laurent Pinchart<br>
</blockquote></div></div></div>