[libcamera-devel] [PATCH 4/6] ipa: raspberrypi: Non-functional formatting fixes to md_parser_smia.cpp
Naushir Patuck
naush at raspberrypi.com
Tue Jun 15 10:54:41 CEST 2021
Hi Laurent,
On Tue, 15 Jun 2021 at 03:48, Laurent Pinchart <
laurent.pinchart at ideasonboard.com> wrote:
> Hi Naush,
>
> Thank you for the patch.
>
> On Mon, Jun 14, 2021 at 10:53:38AM +0100, Naushir Patuck wrote:
> > Adjust source formatting to closer match libcamera guidelines:
> >
> > - Switch to C style comments.
> > - Adjust whitespace for readability.
> > - Remove retcode local variable usage.
> >
> > There are no functional changes in this commit.
> >
> > Signed-off-by: Naushir Patuck <naush at raspberrypi.com>
> > ---
> > src/ipa/raspberrypi/md_parser_smia.cpp | 70 ++++++++++++++------------
> > 1 file changed, 38 insertions(+), 32 deletions(-)
> >
> > diff --git a/src/ipa/raspberrypi/md_parser_smia.cpp
> b/src/ipa/raspberrypi/md_parser_smia.cpp
> > index 852a1d347d11..65ffbe00c76e 100644
> > --- a/src/ipa/raspberrypi/md_parser_smia.cpp
> > +++ b/src/ipa/raspberrypi/md_parser_smia.cpp
> > @@ -1,31 +1,32 @@
> > /* SPDX-License-Identifier: BSD-2-Clause */
> > /*
> > - * Copyright (C) 2019, Raspberry Pi (Trading) Limited
> > + * Copyright (C) 2019-2021, Raspberry Pi (Trading) Limited
> > *
> > - * md_parser.cpp - image sensor metadata parsers
> > + * md_parser_smia.cpp - SMIA specification based embedded data parser
> > */
> > -
> > #include <assert.h>
> > #include <map>
> > -#include <string.h>
> > +#include <string>
>
> string doesn't seem to be needed, and neither is map.
>
> >
> > #include "md_parser.hpp"
> >
> > using namespace RPiController;
> >
> > -// This function goes through the embedded data to find the offsets (not
> > -// values!), in the data block, where the values of the given registers
> can
> > -// subsequently be found.
> > -
> > -// Embedded data tag bytes, from Sony IMX219 datasheet but general to
> all SMIA
> > -// sensors, I think.
> > +/*
> > + * This function goes through the embedded data to find the offsets (not
> > + * values!), in the data block, where the values of the given registers
> can
> > + * subsequently be found.
> > + *
> > + * Embedded data tag bytes, from Sony IMX219 datasheet but general to
> all SMIA
> > + * sensors, I think.
>
> Are they identical to the values used in the MIPI CCS specification ? If
> so, maybe we should rename the parser from SMIA to CCS, as the latter is
> a public specification (SMIA is as well but SMIA++ isn't, if I recall
> correctly). This can be done later.
>
Sure, renaming it to CCS seems sensible here.
Thanks,
Naush
> Reviewed-by: Laurent Pinchart <laurent.pinchart at ideasonboard.com>
>
> I can handle the small change above when applying.
>
> > + */
> >
> > -#define LINE_START 0x0a
> > -#define LINE_END_TAG 0x07
> > -#define REG_HI_BITS 0xaa
> > -#define REG_LOW_BITS 0xa5
> > -#define REG_VALUE 0x5a
> > -#define REG_SKIP 0x55
> > +constexpr unsigned int LINE_START = 0x0a;
> > +constexpr unsigned int LINE_END_TAG = 0x07;
> > +constexpr unsigned int REG_HI_BITS = 0xaa;
> > +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[],
> > @@ -36,12 +37,13 @@ MdParserSmia::ParseStatus
> MdParserSmia::findRegs(libcamera::Span<const uint8_t>
> > if (buffer[0] != LINE_START)
> > return NO_LINE_START;
> >
> > - unsigned int current_offset = 1; // after the 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;
> > - ParseStatus retcode = PARSE_OK;
> > +
> > while (1) {
> > int tag = buffer[current_offset++];
> > +
> > if ((bits_per_pixel_ == 10 &&
> > (current_offset + 1 - current_line_start) % 5 == 0) ||
> > (bits_per_pixel_ == 12 &&
> > @@ -49,34 +51,38 @@ MdParserSmia::ParseStatus
> MdParserSmia::findRegs(libcamera::Span<const uint8_t>
> > if (buffer[current_offset++] != REG_SKIP)
> > return BAD_DUMMY;
> > }
> > +
> > 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)
> > return BAD_LINE_END;
> > +
> > if (num_lines_ && ++current_line == num_lines_)
> > return MISSING_REGS;
> > +
> > if (line_length_bytes_) {
> > - current_offset =
> > - current_line_start +
> line_length_bytes_;
> > - // Require whole line to be in the buffer
> (if buffer size set).
> > + current_offset = current_line_start +
> line_length_bytes_;
> > +
> > + /* Require whole line to be in the buffer
> (if buffer size set). */
> > if (buffer.size() &&
> > - current_offset + line_length_bytes_ >
> > - buffer.size())
> > + current_offset + line_length_bytes_ >
> buffer.size())
> > return MISSING_REGS;
> > +
> > if (buffer[current_offset] != LINE_START)
> > return NO_LINE_START;
> > } else {
> > - // allow a zero line length to mean "hunt
> for the next line"
> > + /* allow a zero line length to mean "hunt
> for the next line" */
> > while (buffer[current_offset] !=
> LINE_START &&
> > current_offset < buffer.size())
> > current_offset++;
> > +
> > if (current_offset == buffer.size())
> > return NO_LINE_START;
> > }
> > - // inc current_offset to after LINE_START
> > - current_line_start =
> > - current_offset++;
> > +
> > + /* inc current_offset to after LINE_START */
> > + current_line_start = current_offset++;
> > } else {
> > if (tag == REG_HI_BITS)
> > reg_num = (reg_num & 0xff) | (data_byte <<
> 8);
> > @@ -86,13 +92,13 @@ MdParserSmia::ParseStatus
> MdParserSmia::findRegs(libcamera::Span<const uint8_t>
> > reg_num++;
> > else if (tag == REG_VALUE) {
> > while (reg_num >=
> > - // assumes registers are in order...
> > + /* assumes registers are in
> order... */
> > regs[first_reg]) {
> > if (reg_num == regs[first_reg])
> > - offsets[first_reg] =
> > - current_offset - 1;
> > + offsets[first_reg] =
> current_offset - 1;
> > +
> > if (++first_reg == num_regs)
> > - return retcode;
> > + return PARSE_OK;
> > }
> > reg_num++;
> > } else
>
> --
> Regards,
>
> Laurent Pinchart
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <https://lists.libcamera.org/pipermail/libcamera-devel/attachments/20210615/33ba6464/attachment-0001.htm>
More information about the libcamera-devel
mailing list