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

[David Plowman]

Hi Naush

Thanks for this patch. Looks good to me!

Reviewed-by: David Plowman <david.plowman at raspberrypi.com>

Best regards
David

On Tue, 16 Feb 2021 at 10:32, Naushir Patuck <naush at raspberrypi.com> 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_)
> +               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 9e9ea80b93ad..0baf7b7e8d4a 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',
> --
> 2.25.1
>
> _______________________________________________
> libcamera-devel mailing list
> libcamera-devel at lists.libcamera.org
> https://lists.libcamera.org/listinfo/libcamera-devel