[libcamera-devel] [PATCH 5/6] ipa: raspberrypi: Embed the metadata parser in the sensor CamHelper classes

[Naushir Patuck]

On Mon, 14 Jun 2021, 10:53 am Naushir Patuck, <naush at raspberrypi.com> wrote:

> This avoids the need for any dynamic allocations and lifetime management.
> The
> base CamHelper class still accesses the parser through a pointer that is
> setup
> by the derived class constructor.
>
> Signed-off-by: Naushir Patuck <naush at raspberrypi.com>
> ---
>  src/ipa/raspberrypi/cam_helper.cpp        | 5 ++---
>  src/ipa/raspberrypi/cam_helper.hpp        | 2 +-
>  src/ipa/raspberrypi/cam_helper_imx219.cpp | 9 +++++----
>  src/ipa/raspberrypi/cam_helper_imx290.cpp | 2 +-
>  src/ipa/raspberrypi/cam_helper_imx477.cpp | 5 ++++-
>  src/ipa/raspberrypi/cam_helper_ov5647.cpp | 2 +-
>  6 files changed, 14 insertions(+), 11 deletions(-)
>
> diff --git a/src/ipa/raspberrypi/cam_helper.cpp
> b/src/ipa/raspberrypi/cam_helper.cpp
> index 062e94c4fef3..ab66e2ddef3e 100644
> --- a/src/ipa/raspberrypi/cam_helper.cpp
> +++ b/src/ipa/raspberrypi/cam_helper.cpp
> @@ -40,15 +40,14 @@ CamHelper *CamHelper::Create(std::string const
> &cam_name)
>         return nullptr;
>  }
>
> -CamHelper::CamHelper(MdParser *parser, unsigned int frameIntegrationDiff)
> -       : parser_(parser), initialized_(false),
> +CamHelper::CamHelper(unsigned int frameIntegrationDiff)
> +       : parser_(nullptr), initialized_(false),
>           frameIntegrationDiff_(frameIntegrationDiff)
>  {
>  }
>
>  CamHelper::~CamHelper()
>  {
> -       delete parser_;
>  }
>
>  void CamHelper::Prepare(Span<const uint8_t> buffer,
> diff --git a/src/ipa/raspberrypi/cam_helper.hpp
> b/src/ipa/raspberrypi/cam_helper.hpp
> index f53f5c39b01c..460839079741 100644
> --- a/src/ipa/raspberrypi/cam_helper.hpp
> +++ b/src/ipa/raspberrypi/cam_helper.hpp
> @@ -67,7 +67,7 @@ class CamHelper
>  {
>  public:
>         static CamHelper *Create(std::string const &cam_name);
> -       CamHelper(MdParser *parser, unsigned int frameIntegrationDiff);
> +       CamHelper(unsigned int frameIntegrationDiff);
>         virtual ~CamHelper();
>         void SetCameraMode(const CameraMode &mode);
>         virtual void Prepare(libcamera::Span<const uint8_t> buffer,
> diff --git a/src/ipa/raspberrypi/cam_helper_imx219.cpp
> b/src/ipa/raspberrypi/cam_helper_imx219.cpp
> index ec218dce5456..36dbe8cd941a 100644
> --- a/src/ipa/raspberrypi/cam_helper_imx219.cpp
> +++ b/src/ipa/raspberrypi/cam_helper_imx219.cpp
> @@ -54,15 +54,16 @@ private:
>          * in units of lines.
>          */
>         static constexpr int frameIntegrationDiff = 4;
> +
> +       MdParserImx219 imx219_parser;
>  };
>

>  CamHelperImx219::CamHelperImx219()
> +       : CamHelper(frameIntegrationDiff)
> +{
>  #if ENABLE_EMBEDDED_DATA
> -       : CamHelper(new MdParserImx219(), frameIntegrationDiff)
> -#else
> -       : CamHelper(nullptr, frameIntegrationDiff)
> +       parser_ = &imx219_parser;
>  #endif
> -{
>  }
>


Looking at this again, I think it might be better to pass the parser
pointer thorough the CamHelper constructor as before. I'll change that back
for v2.



>
>
>  uint32_t CamHelperImx219::GainCode(double gain) const
> diff --git a/src/ipa/raspberrypi/cam_helper_imx290.cpp
> b/src/ipa/raspberrypi/cam_helper_imx290.cpp
> index 6f412e403f16..dea57b84bf0b 100644
> --- a/src/ipa/raspberrypi/cam_helper_imx290.cpp
> +++ b/src/ipa/raspberrypi/cam_helper_imx290.cpp
> @@ -30,7 +30,7 @@ private:
>  };
>
>  CamHelperImx290::CamHelperImx290()
> -       : CamHelper(nullptr, frameIntegrationDiff)
> +       : CamHelper(frameIntegrationDiff)
>  {
>  }
>
> diff --git a/src/ipa/raspberrypi/cam_helper_imx477.cpp
> b/src/ipa/raspberrypi/cam_helper_imx477.cpp
> index 25b36bce0dac..038a8583d311 100644
> --- a/src/ipa/raspberrypi/cam_helper_imx477.cpp
> +++ b/src/ipa/raspberrypi/cam_helper_imx477.cpp
> @@ -47,11 +47,14 @@ private:
>          * in units of lines.
>          */
>         static constexpr int frameIntegrationDiff = 22;
> +
> +       MdParserImx477 imx477_parser;
>  };
>
>  CamHelperImx477::CamHelperImx477()
> -       : CamHelper(new MdParserImx477(), frameIntegrationDiff)
> +       : CamHelper(frameIntegrationDiff)
>  {
> +       parser_ = &imx477_parser;
>  }
>
>  uint32_t CamHelperImx477::GainCode(double gain) const
> diff --git a/src/ipa/raspberrypi/cam_helper_ov5647.cpp
> b/src/ipa/raspberrypi/cam_helper_ov5647.cpp
> index 12be6bf931a8..fff648279d2a 100644
> --- a/src/ipa/raspberrypi/cam_helper_ov5647.cpp
> +++ b/src/ipa/raspberrypi/cam_helper_ov5647.cpp
> @@ -38,7 +38,7 @@ private:
>   */
>
>  CamHelperOv5647::CamHelperOv5647()
> -       : CamHelper(nullptr, frameIntegrationDiff)
> +       : CamHelper(frameIntegrationDiff)
>  {
>  }
>
> --
> 2.25.1
>
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <https://lists.libcamera.org/pipermail/libcamera-devel/attachments/20210614/b235abd3/attachment-0001.htm>