[libcamera-devel] [PATCH 1/6] ipa: raspberrypi: Non-functional formatting fixes to md_parser.hpp

[Laurent Pinchart]

On Mon, Jun 14, 2021 at 10:53:35AM +0100, Naushir Patuck wrote:
> Adjust source formatting to closer match libcamera guidelines:
> 
> - Remove unused header files.
> - Switch to C style comments.
> - Add whitespace for readability.
> 
> There are no functional changes in this commit.
> 
> Signed-off-by: Naushir Patuck <naush at raspberrypi.com>
> ---
>  src/ipa/raspberrypi/md_parser.hpp | 144 ++++++++++++++++++------------
>  1 file changed, 85 insertions(+), 59 deletions(-)
> 
> diff --git a/src/ipa/raspberrypi/md_parser.hpp b/src/ipa/raspberrypi/md_parser.hpp
> index 8e22b1d7ca43..86e0577614e0 100644
> --- a/src/ipa/raspberrypi/md_parser.hpp
> +++ b/src/ipa/raspberrypi/md_parser.hpp
> @@ -6,75 +6,94 @@
>   */
>  #pragma once
>  
> -#include <stdint.h>
> -
>  #include <libcamera/span.h>
>  
>  /* Camera metadata parser class. Usage as shown below.

This should be

/*
 * Camera metadata parser class. Usage as shown below.

I'll fix when applying.

Reviewed-by: Laurent Pinchart <laurent.pinchart at ideasonboard.com>

> -
> -Setup:
> -
> -Usually the metadata parser will be made as part of the CamHelper class so
> -application code doesn't have to worry which to kind to instantiate. But for
> -the sake of example let's suppose we're parsing imx219 metadata.
> -
> -MdParser *parser = new MdParserImx219();  // for example
> -parser->SetBitsPerPixel(bpp);
> -parser->SetLineLengthBytes(pitch);
> -parser->SetNumLines(2);
> -
> -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.
> -
> -Then on every frame:
> -
> -if (parser->Parse(buffer) != 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;
> -
> -(Note that the CamHelper class converts to/from exposure lines and time,
> -and gain_code / actual gain.)
> -
> -If you suspect your embedded data may have changed its layout, change any line
> -lengths, number of lines, bits per pixel etc. that are different, and
> -then:
> -
> -parser->Reset();
> -
> -before calling Parse again. */
> + *
> + * Setup:
> + *
> + * Usually the metadata parser will be made as part of the CamHelper class so
> + * application code doesn't have to worry which to kind to instantiate. But for
> + * the sake of example let's suppose we're parsing imx219 metadata.
> + *
> + * MdParser *parser = new MdParserImx219();  // for example
> + * parser->SetBitsPerPixel(bpp);
> + * parser->SetLineLengthBytes(pitch);
> + * parser->SetNumLines(2);
> + *
> + * 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.
> + *
> + * Then on every frame:
> + *
> + * if (parser->Parse(buffer) != 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;
> + *
> + * (Note that the CamHelper class converts to/from exposure lines and time,
> + * and gain_code / actual gain.)
> + *
> + * If you suspect your embedded data may have changed its layout, change any line
> + * lengths, number of lines, bits per pixel etc. that are different, and
> + * then:
> + *
> + * parser->Reset();
> + *
> + * before calling Parse again.
> + */
>  
>  namespace RPiController {
>  
> -// Abstract base class from which other metadata parsers are derived.
> +/* Abstract base class from which other metadata parsers are derived. */
>  
>  class MdParser
>  {
>  public:
> -	// Parser status codes:
> -	// OK       - success
> -	// NOTFOUND - value such as exposure or gain was not found
> -	// ERROR    - all other errors
> +	/*
> +	 * Parser status codes:
> +	 * OK       - success
> +	 * NOTFOUND - value such as exposure or gain was not found
> +	 * ERROR    - all other errors
> +	 */
>  	enum Status {
>  		OK = 0,
>  		NOTFOUND = 1,
>  		ERROR = 2
>  	};
> -	MdParser() : reset_(true) {}
> +
> +	MdParser() : reset_(true)
> +	{
> +	}
> +
>  	virtual ~MdParser() = default;
> -	void Reset() { reset_ = true; }
> -	void SetBitsPerPixel(int bpp) { bits_per_pixel_ = bpp; }
> -	void SetNumLines(unsigned int num_lines) { num_lines_ = num_lines; }
> +
> +	void Reset()
> +	{
> +		reset_ = true;
> +	}
> +
> +	void SetBitsPerPixel(int bpp)
> +	{
> +		bits_per_pixel_ = bpp;
> +	}
> +
> +	void SetNumLines(unsigned int num_lines)
> +	{
> +		num_lines_ = num_lines;
> +	}
> +
>  	void SetLineLengthBytes(unsigned int num_bytes)
>  	{
>  		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;
> @@ -87,22 +106,28 @@ protected:
>  	unsigned int buffer_size_bytes_;
>  };
>  
> -// This isn't a full implementation of a metadata parser for SMIA sensors,
> -// however, it does provide the findRegs method which will prove useful and make
> -// it easier to implement parsers for other SMIA-like sensors (see
> -// md_parser_imx219.cpp for an example).
> +/*
> + * This isn't a full implementation of a metadata parser for SMIA sensors,
> + * however, it does provide the findRegs method which will prove useful and make
> + * it easier to implement parsers for other SMIA-like sensors (see
> + * md_parser_imx219.cpp for an example).
> + */
>  
>  class MdParserSmia : public MdParser
>  {
>  public:
> -	MdParserSmia() : MdParser() {}
> +	MdParserSmia() : MdParser()
> +	{
> +	}
>  
>  protected:
> -	// Note that error codes > 0 are regarded as non-fatal; codes < 0
> -	// indicate a bad data buffer. Status codes are:
> -	// PARSE_OK     - found all registers, much happiness
> -	// MISSING_REGS - some registers found; should this be a hard error?
> -	// The remaining codes are all hard errors.
> +	/*
> +	 * Note that error codes > 0 are regarded as non-fatal; codes < 0
> +	 * indicate a bad data buffer. Status codes are:
> +	 * PARSE_OK     - found all registers, much happiness
> +	 * MISSING_REGS - some registers found; should this be a hard error?
> +	 * The remaining codes are all hard errors.
> +	 */
>  	enum ParseStatus {
>  		PARSE_OK      =  0,
>  		MISSING_REGS  =  1,
> @@ -112,6 +137,7 @@ protected:
>  		BAD_LINE_END  = -4,
>  		BAD_PADDING   = -5
>  	};
> +
>  	ParseStatus findRegs(libcamera::Span<const uint8_t> buffer, uint32_t regs[],
>  			     int offsets[], unsigned int num_regs);
>  };

-- 
Regards,

Laurent Pinchart