[libcamera-devel] [PATCH v3 2/4] ipa: raspberrypi: Remove MdParserRPi

[Laurent Pinchart]

Hi Naush,

Thank you for the patch.

On Thu, Feb 18, 2021 at 05:01:24PM +0000, Naushir Patuck wrote:
> With the recent change to pass a ControlList to the IPA with exposure
> and gain values used for a frame, RPiController::MdParserRPi is not
> needed any more. Remove all traces of it.
> 
> The derived CamHelper objects now pass nullptr values for the parser to
> the base CamHelper class when sensors do not use metadata.
> 
> Signed-off-by: Naushir Patuck <naush at raspberrypi.com>
> ---
>  src/ipa/raspberrypi/cam_helper.cpp        |  9 ++++--
>  src/ipa/raspberrypi/cam_helper_imx219.cpp |  4 +--
>  src/ipa/raspberrypi/cam_helper_ov5647.cpp |  3 +-
>  src/ipa/raspberrypi/md_parser_rpi.cpp     | 37 -----------------------
>  src/ipa/raspberrypi/md_parser_rpi.hpp     | 32 --------------------
>  src/ipa/raspberrypi/meson.build           |  1 -
>  6 files changed, 8 insertions(+), 78 deletions(-)
>  delete mode 100644 src/ipa/raspberrypi/md_parser_rpi.cpp
>  delete mode 100644 src/ipa/raspberrypi/md_parser_rpi.hpp
> 
> diff --git a/src/ipa/raspberrypi/cam_helper.cpp b/src/ipa/raspberrypi/cam_helper.cpp
> index 93d1b7b0296a..c9cdc39c5932 100644
> --- a/src/ipa/raspberrypi/cam_helper.cpp
> +++ b/src/ipa/raspberrypi/cam_helper.cpp
> @@ -42,7 +42,8 @@ CamHelper::CamHelper(MdParser *parser, unsigned int frameIntegrationDiff)
>  
>  CamHelper::~CamHelper()
>  {
> -	delete parser_;
> +	if (parser_)

You can drop the check, delete on a null pointer is a no-op.

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

> +		delete parser_;
>  }
>  
>  uint32_t CamHelper::ExposureLines(double exposure_us) const
> @@ -88,8 +89,10 @@ uint32_t CamHelper::GetVBlanking(double &exposure, double minFrameDuration,
>  void CamHelper::SetCameraMode(const CameraMode &mode)
>  {
>  	mode_ = mode;
> -	parser_->SetBitsPerPixel(mode.bitdepth);
> -	parser_->SetLineLengthBytes(0); /* We use SetBufferSize. */
> +	if (parser_) {
> +		parser_->SetBitsPerPixel(mode.bitdepth);
> +		parser_->SetLineLengthBytes(0); /* We use SetBufferSize. */
> +	}
>  	initialized_ = true;
>  }
>  
> diff --git a/src/ipa/raspberrypi/cam_helper_imx219.cpp b/src/ipa/raspberrypi/cam_helper_imx219.cpp
> index 95b8e698fe3b..0e454d0de2dc 100644
> --- a/src/ipa/raspberrypi/cam_helper_imx219.cpp
> +++ b/src/ipa/raspberrypi/cam_helper_imx219.cpp
> @@ -19,8 +19,6 @@
>  #include "cam_helper.hpp"
>  #if ENABLE_EMBEDDED_DATA
>  #include "md_parser.hpp"
> -#else
> -#include "md_parser_rpi.hpp"
>  #endif
>  
>  using namespace RPiController;
> @@ -62,7 +60,7 @@ CamHelperImx219::CamHelperImx219()
>  #if ENABLE_EMBEDDED_DATA
>  	: CamHelper(new MdParserImx219(), frameIntegrationDiff)
>  #else
> -	: CamHelper(new MdParserRPi(), frameIntegrationDiff)
> +	: CamHelper(nullptr, frameIntegrationDiff)
>  #endif
>  {
>  }
> diff --git a/src/ipa/raspberrypi/cam_helper_ov5647.cpp b/src/ipa/raspberrypi/cam_helper_ov5647.cpp
> index a7f417324048..75486e900d31 100644
> --- a/src/ipa/raspberrypi/cam_helper_ov5647.cpp
> +++ b/src/ipa/raspberrypi/cam_helper_ov5647.cpp
> @@ -8,7 +8,6 @@
>  #include <assert.h>
>  
>  #include "cam_helper.hpp"
> -#include "md_parser_rpi.hpp"
>  
>  using namespace RPiController;
>  
> @@ -38,7 +37,7 @@ private:
>   */
>  
>  CamHelperOv5647::CamHelperOv5647()
> -	: CamHelper(new MdParserRPi(), frameIntegrationDiff)
> +	: CamHelper(nullptr, frameIntegrationDiff)
>  {
>  }
>  
> diff --git a/src/ipa/raspberrypi/md_parser_rpi.cpp b/src/ipa/raspberrypi/md_parser_rpi.cpp
> deleted file mode 100644
> index 2b0bcfc5f034..000000000000
> --- a/src/ipa/raspberrypi/md_parser_rpi.cpp
> +++ /dev/null
> @@ -1,37 +0,0 @@
> -/* SPDX-License-Identifier: BSD-2-Clause */
> -/*
> - * Copyright (C) 2020, Raspberry Pi (Trading) Limited
> - *
> - * md_parser_rpi.cpp - Metadata parser for generic Raspberry Pi metadata
> - */
> -
> -#include <string.h>
> -
> -#include "md_parser_rpi.hpp"
> -
> -using namespace RPiController;
> -
> -MdParserRPi::MdParserRPi()
> -{
> -}
> -
> -MdParser::Status MdParserRPi::Parse(void *data)
> -{
> -	if (buffer_size_bytes_ < sizeof(rpiMetadata))
> -		return ERROR;
> -
> -	memcpy(&metadata, data, sizeof(rpiMetadata));
> -	return OK;
> -}
> -
> -MdParser::Status MdParserRPi::GetExposureLines(unsigned int &lines)
> -{
> -	lines = metadata.exposure;
> -	return OK;
> -}
> -
> -MdParser::Status MdParserRPi::GetGainCode(unsigned int &gain_code)
> -{
> -	gain_code = metadata.gain;
> -	return OK;
> -}
> diff --git a/src/ipa/raspberrypi/md_parser_rpi.hpp b/src/ipa/raspberrypi/md_parser_rpi.hpp
> deleted file mode 100644
> index 52f54f008056..000000000000
> --- a/src/ipa/raspberrypi/md_parser_rpi.hpp
> +++ /dev/null
> @@ -1,32 +0,0 @@
> -/* SPDX-License-Identifier: BSD-2-Clause */
> -/*
> - * Copyright (C) 2019, Raspberry Pi (Trading) Limited
> - *
> - * md_parser_rpi.hpp - Raspberry Pi metadata parser interface
> - */
> -#pragma once
> -
> -#include "md_parser.hpp"
> -
> -namespace RPiController {
> -
> -class MdParserRPi : public MdParser
> -{
> -public:
> -	MdParserRPi();
> -	Status Parse(void *data) override;
> -	Status GetExposureLines(unsigned int &lines) override;
> -	Status GetGainCode(unsigned int &gain_code) override;
> -
> -private:
> -	// This must be the same struct that is filled into the metadata buffer
> -	// in the pipeline handler.
> -	struct rpiMetadata
> -	{
> -		uint32_t exposure;
> -		uint32_t gain;
> -	};
> -	rpiMetadata metadata;
> -};
> -
> -}
> diff --git a/src/ipa/raspberrypi/meson.build b/src/ipa/raspberrypi/meson.build
> index 9af755259747..59e49686bc09 100644
> --- a/src/ipa/raspberrypi/meson.build
> +++ b/src/ipa/raspberrypi/meson.build
> @@ -17,7 +17,6 @@ rpi_ipa_includes = [
>  rpi_ipa_sources = files([
>      'raspberrypi.cpp',
>      'md_parser.cpp',
> -    'md_parser_rpi.cpp',
>      'cam_helper.cpp',
>      'cam_helper_ov5647.cpp',
>      'cam_helper_imx219.cpp',

-- 
Regards,

Laurent Pinchart