[libcamera-devel] [PATCH v4 1/2] ipa: raspberrypi: Make sensor embedded data parser use Span class
Laurent Pinchart
laurent.pinchart at ideasonboard.com
Sat May 8 00:20:39 CEST 2021
Hi David,
Thank you for the patch.
On Fri, May 07, 2021 at 12:37:27PM +0100, David Plowman wrote:
> Improve MdParser::Parse() by taking a Span, pointing to const data
> that it should not change, as its input buffer.
Looks good to me. I may have kept the variables named 'data' instead of
'buffer', but it doesn't matter much.
> Signed-off-by: David Plowman <david.plowman at raspberrypi.com>
Reviewed-by: Laurent Pinchart <laurent.pinchart at ideasonboard.com>
> ---
> src/ipa/raspberrypi/cam_helper_imx219.cpp | 8 ++++----
> src/ipa/raspberrypi/cam_helper_imx477.cpp | 8 ++++----
> src/ipa/raspberrypi/md_parser.cpp | 23 ++++++++++++-----------
> src/ipa/raspberrypi/md_parser.hpp | 20 ++++++++------------
> 4 files changed, 28 insertions(+), 31 deletions(-)
>
> diff --git a/src/ipa/raspberrypi/cam_helper_imx219.cpp b/src/ipa/raspberrypi/cam_helper_imx219.cpp
> index 566ecbca..e550fba6 100644
> --- a/src/ipa/raspberrypi/cam_helper_imx219.cpp
> +++ b/src/ipa/raspberrypi/cam_helper_imx219.cpp
> @@ -29,7 +29,7 @@ class MdParserImx219 : public MdParserSmia
> {
> public:
> MdParserImx219();
> - Status Parse(void *data) override;
> + Status Parse(libcamera::Span<const uint8_t> buffer) override;
> Status GetExposureLines(unsigned int &lines) override;
> Status GetGainCode(unsigned int &gain_code) override;
> private:
> @@ -118,7 +118,7 @@ MdParserImx219::MdParserImx219()
> reg_offsets_[0] = reg_offsets_[1] = reg_offsets_[2] = -1;
> }
>
> -MdParser::Status MdParserImx219::Parse(void *data)
> +MdParser::Status MdParserImx219::Parse(libcamera::Span<const uint8_t> buffer)
> {
> bool try_again = false;
>
> @@ -132,7 +132,7 @@ MdParser::Status MdParserImx219::Parse(void *data)
> /* 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(static_cast<uint8_t *>(data),
> + int ret = static_cast<int>(findRegs(buffer,
> regs, reg_offsets_, 3));
> /*
> * > 0 means "worked partially but parse again next time",
> @@ -148,7 +148,7 @@ MdParser::Status MdParserImx219::Parse(void *data)
> if (reg_offsets_[i] == -1)
> continue;
>
> - reg_values_[i] = static_cast<uint8_t *>(data)[reg_offsets_[i]];
> + reg_values_[i] = buffer[reg_offsets_[i]];
> }
>
> /* Re-parse next time if we were unhappy in some way. */
> diff --git a/src/ipa/raspberrypi/cam_helper_imx477.cpp b/src/ipa/raspberrypi/cam_helper_imx477.cpp
> index 73a5ca7d..a4a58c15 100644
> --- a/src/ipa/raspberrypi/cam_helper_imx477.cpp
> +++ b/src/ipa/raspberrypi/cam_helper_imx477.cpp
> @@ -21,7 +21,7 @@ class MdParserImx477 : public MdParserSmia
> {
> public:
> MdParserImx477();
> - Status Parse(void *data) override;
> + Status Parse(libcamera::Span<const uint8_t> buffer) override;
> Status GetExposureLines(unsigned int &lines) override;
> Status GetGainCode(unsigned int &gain_code) override;
> private:
> @@ -107,7 +107,7 @@ MdParserImx477::MdParserImx477()
> reg_offsets_[0] = reg_offsets_[1] = reg_offsets_[2] = reg_offsets_[3] = -1;
> }
>
> -MdParser::Status MdParserImx477::Parse(void *data)
> +MdParser::Status MdParserImx477::Parse(libcamera::Span<const uint8_t> buffer)
> {
> bool try_again = false;
>
> @@ -126,7 +126,7 @@ MdParser::Status MdParserImx477::Parse(void *data)
> GAINLO_REG
> };
> reg_offsets_[0] = reg_offsets_[1] = reg_offsets_[2] = reg_offsets_[3] = -1;
> - int ret = static_cast<int>(findRegs(static_cast<uint8_t *>(data),
> + int ret = static_cast<int>(findRegs(buffer,
> regs, reg_offsets_, 4));
> /*
> * > 0 means "worked partially but parse again next time",
> @@ -142,7 +142,7 @@ MdParser::Status MdParserImx477::Parse(void *data)
> if (reg_offsets_[i] == -1)
> continue;
>
> - reg_values_[i] = static_cast<uint8_t *>(data)[reg_offsets_[i]];
> + reg_values_[i] = buffer[reg_offsets_[i]];
> }
>
> /* Re-parse next time if we were unhappy in some way. */
> diff --git a/src/ipa/raspberrypi/md_parser.cpp b/src/ipa/raspberrypi/md_parser.cpp
> index d82c102c..852a1d34 100644
> --- a/src/ipa/raspberrypi/md_parser.cpp
> +++ b/src/ipa/raspberrypi/md_parser.cpp
> @@ -27,12 +27,13 @@ using namespace RPiController;
> #define REG_VALUE 0x5a
> #define REG_SKIP 0x55
>
> -MdParserSmia::ParseStatus MdParserSmia::findRegs(unsigned char *data,
> +MdParserSmia::ParseStatus MdParserSmia::findRegs(libcamera::Span<const uint8_t> buffer,
> uint32_t regs[], int offsets[],
> unsigned int num_regs)
> {
> assert(num_regs > 0);
> - if (data[0] != LINE_START)
> +
> + if (buffer[0] != LINE_START)
> return NO_LINE_START;
>
> unsigned int current_offset = 1; // after the LINE_START
> @@ -40,15 +41,15 @@ MdParserSmia::ParseStatus MdParserSmia::findRegs(unsigned char *data,
> unsigned int reg_num = 0, first_reg = 0;
> ParseStatus retcode = PARSE_OK;
> while (1) {
> - int tag = data[current_offset++];
> + int tag = buffer[current_offset++];
> if ((bits_per_pixel_ == 10 &&
> (current_offset + 1 - current_line_start) % 5 == 0) ||
> (bits_per_pixel_ == 12 &&
> (current_offset + 1 - current_line_start) % 3 == 0)) {
> - if (data[current_offset++] != REG_SKIP)
> + if (buffer[current_offset++] != REG_SKIP)
> return BAD_DUMMY;
> }
> - int data_byte = data[current_offset++];
> + int data_byte = buffer[current_offset++];
> //printf("Offset %u, tag 0x%02x data_byte 0x%02x\n", current_offset-1, tag, data_byte);
> if (tag == LINE_END_TAG) {
> if (data_byte != LINE_END_TAG)
> @@ -59,18 +60,18 @@ MdParserSmia::ParseStatus MdParserSmia::findRegs(unsigned char *data,
> current_offset =
> current_line_start + line_length_bytes_;
> // Require whole line to be in the buffer (if buffer size set).
> - if (buffer_size_bytes_ &&
> + if (buffer.size() &&
> current_offset + line_length_bytes_ >
> - buffer_size_bytes_)
> + buffer.size())
> return MISSING_REGS;
> - if (data[current_offset] != LINE_START)
> + if (buffer[current_offset] != LINE_START)
> return NO_LINE_START;
> } else {
> // allow a zero line length to mean "hunt for the next line"
> - while (data[current_offset] != LINE_START &&
> - current_offset < buffer_size_bytes_)
> + while (buffer[current_offset] != LINE_START &&
> + current_offset < buffer.size())
> current_offset++;
> - if (current_offset == buffer_size_bytes_)
> + if (current_offset == buffer.size())
> return NO_LINE_START;
> }
> // inc current_offset to after LINE_START
> diff --git a/src/ipa/raspberrypi/md_parser.hpp b/src/ipa/raspberrypi/md_parser.hpp
> index 926f676e..8e22b1d7 100644
> --- a/src/ipa/raspberrypi/md_parser.hpp
> +++ b/src/ipa/raspberrypi/md_parser.hpp
> @@ -8,6 +8,8 @@
>
> #include <stdint.h>
>
> +#include <libcamera/span.h>
> +
> /* Camera metadata parser class. Usage as shown below.
>
> Setup:
> @@ -21,17 +23,15 @@ parser->SetBitsPerPixel(bpp);
> parser->SetLineLengthBytes(pitch);
> parser->SetNumLines(2);
>
> -Note 1: if you don't know how many lines there are, you can use SetBufferSize
> -instead to limit the total buffer size.
> +Note 1: if you don't know how many lines there are, the size of the input
> +buffer is used as a limit instead.
>
> Note 2: if you don't know the line length, you can leave the line length unset
> -(or set to zero) and the parser will hunt for the line start instead. In this
> -case SetBufferSize *must* be used so that the parser won't run off the end of
> -the buffer.
> +(or set to zero) and the parser will hunt for the line start instead.
>
> Then on every frame:
>
> -if (parser->Parse(data) != MdParser::OK)
> +if (parser->Parse(buffer) != MdParser::OK)
> much badness;
> unsigned int exposure_lines, gain_code
> if (parser->GetExposureLines(exposure_lines) != MdParser::OK)
> @@ -75,11 +75,7 @@ public:
> {
> line_length_bytes_ = num_bytes;
> }
> - void SetBufferSize(unsigned int num_bytes)
> - {
> - buffer_size_bytes_ = num_bytes;
> - }
> - virtual Status Parse(void *data) = 0;
> + 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;
>
> @@ -116,7 +112,7 @@ protected:
> BAD_LINE_END = -4,
> BAD_PADDING = -5
> };
> - ParseStatus findRegs(unsigned char *data, uint32_t regs[],
> + ParseStatus findRegs(libcamera::Span<const uint8_t> buffer, uint32_t regs[],
> int offsets[], unsigned int num_regs);
> };
>
--
Regards,
Laurent Pinchart
More information about the libcamera-devel
mailing list