[libcamera-devel,v3,1/2] ipa: raspberrypi: Use a unique_ptr for the metadata parser
diff mbox series

Message ID 20210629104500.51672-2-naush@raspberrypi.com
State Accepted
Headers show
Series
  • Raspberry Pi: Metadata parsing improvements (II)
Related show

Commit Message

Naushir Patuck June 29, 2021, 10:44 a.m. UTC 
The derived CamHelper class now allocates a metadata parser object through a
unique_ptr that is passed to the base class constructor. This automates the
lifetime management of the parser object.

Signed-off-by: Naushir Patuck <naush@raspberrypi.com>
Reviewed-by: David Plowman <david.plowman@raspberrypi.com>
---
 src/ipa/raspberrypi/cam_helper.cpp        | 5 ++---
 src/ipa/raspberrypi/cam_helper.hpp        | 5 +++--
 src/ipa/raspberrypi/cam_helper_imx219.cpp | 4 ++--
 src/ipa/raspberrypi/cam_helper_imx290.cpp | 2 +-
 src/ipa/raspberrypi/cam_helper_imx477.cpp | 2 +-
 src/ipa/raspberrypi/cam_helper_ov5647.cpp | 2 +-
 src/ipa/raspberrypi/cam_helper_ov9281.cpp | 2 +-
 7 files changed, 11 insertions(+), 11 deletions(-)

Comments

Kieran Bingham June 29, 2021, 10:48 a.m. UTC | #1 
Hi Naush,

On 29/06/2021 11:44, Naushir Patuck wrote:
> The derived CamHelper class now allocates a metadata parser object through a
> unique_ptr that is passed to the base class constructor. This automates the
> lifetime management of the parser object.
> 
> Signed-off-by: Naushir Patuck <naush@raspberrypi.com>
> Reviewed-by: David Plowman <david.plowman@raspberrypi.com>

Looks OK here.

Reviewed-by: Kieran Bingham <kieran.bingham@ideasonboard.com>

> ---
>  src/ipa/raspberrypi/cam_helper.cpp        | 5 ++---
>  src/ipa/raspberrypi/cam_helper.hpp        | 5 +++--
>  src/ipa/raspberrypi/cam_helper_imx219.cpp | 4 ++--
>  src/ipa/raspberrypi/cam_helper_imx290.cpp | 2 +-
>  src/ipa/raspberrypi/cam_helper_imx477.cpp | 2 +-
>  src/ipa/raspberrypi/cam_helper_ov5647.cpp | 2 +-
>  src/ipa/raspberrypi/cam_helper_ov9281.cpp | 2 +-
>  7 files changed, 11 insertions(+), 11 deletions(-)
> 
> diff --git a/src/ipa/raspberrypi/cam_helper.cpp b/src/ipa/raspberrypi/cam_helper.cpp
> index 062e94c4fef3..90498c37af98 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(std::unique_ptr<MdParser> parser, unsigned int frameIntegrationDiff)
> +	: parser_(std::move(parser)), 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 b19c95f67453..fc3139e22be0 100644
> --- a/src/ipa/raspberrypi/cam_helper.hpp
> +++ b/src/ipa/raspberrypi/cam_helper.hpp
> @@ -6,6 +6,7 @@
>   */
>  #pragma once
>  
> +#include <memory>
>  #include <string>
>  
>  #include <libcamera/base/span.h>
> @@ -67,7 +68,7 @@ class CamHelper
>  {
>  public:
>  	static CamHelper *Create(std::string const &cam_name);
> -	CamHelper(MdParser *parser, unsigned int frameIntegrationDiff);
> +	CamHelper(std::unique_ptr<MdParser> parser, unsigned int frameIntegrationDiff);
>  	virtual ~CamHelper();
>  	void SetCameraMode(const CameraMode &mode);
>  	virtual void Prepare(libcamera::Span<const uint8_t> buffer,
> @@ -92,7 +93,7 @@ protected:
>  	void parseEmbeddedData(libcamera::Span<const uint8_t> buffer,
>  			       Metadata &metadata);
>  
> -	MdParser *parser_;
> +	std::unique_ptr<MdParser> parser_;
>  	CameraMode mode_;
>  
>  private:
> diff --git a/src/ipa/raspberrypi/cam_helper_imx219.cpp b/src/ipa/raspberrypi/cam_helper_imx219.cpp
> index ec218dce5456..c85044a5fa6d 100644
> --- a/src/ipa/raspberrypi/cam_helper_imx219.cpp
> +++ b/src/ipa/raspberrypi/cam_helper_imx219.cpp
> @@ -58,9 +58,9 @@ private:
>  
>  CamHelperImx219::CamHelperImx219()
>  #if ENABLE_EMBEDDED_DATA
> -	: CamHelper(new MdParserImx219(), frameIntegrationDiff)
> +	: CamHelper(std::make_unique<MdParserImx219>(), frameIntegrationDiff)
>  #else
> -	: CamHelper(nullptr, frameIntegrationDiff)
> +	: CamHelper({}, frameIntegrationDiff)
>  #endif
>  {
>  }
> diff --git a/src/ipa/raspberrypi/cam_helper_imx290.cpp b/src/ipa/raspberrypi/cam_helper_imx290.cpp
> index 6f412e403f16..871c1f8eaec4 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..d72a9be0438e 100644
> --- a/src/ipa/raspberrypi/cam_helper_imx477.cpp
> +++ b/src/ipa/raspberrypi/cam_helper_imx477.cpp
> @@ -50,7 +50,7 @@ private:
>  };
>  
>  CamHelperImx477::CamHelperImx477()
> -	: CamHelper(new MdParserImx477(), frameIntegrationDiff)
> +	: CamHelper(std::make_unique<MdParserImx477>(), frameIntegrationDiff)
>  {
>  }
>  
> diff --git a/src/ipa/raspberrypi/cam_helper_ov5647.cpp b/src/ipa/raspberrypi/cam_helper_ov5647.cpp
> index 12be6bf931a8..702c2d07f73a 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)
>  {
>  }
>  
> diff --git a/src/ipa/raspberrypi/cam_helper_ov9281.cpp b/src/ipa/raspberrypi/cam_helper_ov9281.cpp
> index 54091f835565..9de868c31dc0 100644
> --- a/src/ipa/raspberrypi/cam_helper_ov9281.cpp
> +++ b/src/ipa/raspberrypi/cam_helper_ov9281.cpp
> @@ -34,7 +34,7 @@ private:
>   */
>  
>  CamHelperOv9281::CamHelperOv9281()
> -	: CamHelper(nullptr, frameIntegrationDiff)
> +	: CamHelper({}, frameIntegrationDiff)
>  {
>  }
>  
>
Laurent Pinchart June 29, 2021, 2:27 p.m. UTC | #2 
Hi Naush,

Thank you for the patch.

On Tue, Jun 29, 2021 at 11:44:59AM +0100, Naushir Patuck wrote:
> The derived CamHelper class now allocates a metadata parser object through a
> unique_ptr that is passed to the base class constructor. This automates the
> lifetime management of the parser object.
> 
> Signed-off-by: Naushir Patuck <naush@raspberrypi.com>
> Reviewed-by: David Plowman <david.plowman@raspberrypi.com>

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

> ---
>  src/ipa/raspberrypi/cam_helper.cpp        | 5 ++---
>  src/ipa/raspberrypi/cam_helper.hpp        | 5 +++--
>  src/ipa/raspberrypi/cam_helper_imx219.cpp | 4 ++--
>  src/ipa/raspberrypi/cam_helper_imx290.cpp | 2 +-
>  src/ipa/raspberrypi/cam_helper_imx477.cpp | 2 +-
>  src/ipa/raspberrypi/cam_helper_ov5647.cpp | 2 +-
>  src/ipa/raspberrypi/cam_helper_ov9281.cpp | 2 +-
>  7 files changed, 11 insertions(+), 11 deletions(-)
> 
> diff --git a/src/ipa/raspberrypi/cam_helper.cpp b/src/ipa/raspberrypi/cam_helper.cpp
> index 062e94c4fef3..90498c37af98 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(std::unique_ptr<MdParser> parser, unsigned int frameIntegrationDiff)
> +	: parser_(std::move(parser)), 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 b19c95f67453..fc3139e22be0 100644
> --- a/src/ipa/raspberrypi/cam_helper.hpp
> +++ b/src/ipa/raspberrypi/cam_helper.hpp
> @@ -6,6 +6,7 @@
>   */
>  #pragma once
>  
> +#include <memory>
>  #include <string>
>  
>  #include <libcamera/base/span.h>
> @@ -67,7 +68,7 @@ class CamHelper
>  {
>  public:
>  	static CamHelper *Create(std::string const &cam_name);
> -	CamHelper(MdParser *parser, unsigned int frameIntegrationDiff);
> +	CamHelper(std::unique_ptr<MdParser> parser, unsigned int frameIntegrationDiff);
>  	virtual ~CamHelper();
>  	void SetCameraMode(const CameraMode &mode);
>  	virtual void Prepare(libcamera::Span<const uint8_t> buffer,
> @@ -92,7 +93,7 @@ protected:
>  	void parseEmbeddedData(libcamera::Span<const uint8_t> buffer,
>  			       Metadata &metadata);
>  
> -	MdParser *parser_;
> +	std::unique_ptr<MdParser> parser_;
>  	CameraMode mode_;
>  
>  private:
> diff --git a/src/ipa/raspberrypi/cam_helper_imx219.cpp b/src/ipa/raspberrypi/cam_helper_imx219.cpp
> index ec218dce5456..c85044a5fa6d 100644
> --- a/src/ipa/raspberrypi/cam_helper_imx219.cpp
> +++ b/src/ipa/raspberrypi/cam_helper_imx219.cpp
> @@ -58,9 +58,9 @@ private:
>  
>  CamHelperImx219::CamHelperImx219()
>  #if ENABLE_EMBEDDED_DATA
> -	: CamHelper(new MdParserImx219(), frameIntegrationDiff)
> +	: CamHelper(std::make_unique<MdParserImx219>(), frameIntegrationDiff)
>  #else
> -	: CamHelper(nullptr, frameIntegrationDiff)
> +	: CamHelper({}, frameIntegrationDiff)
>  #endif
>  {
>  }
> diff --git a/src/ipa/raspberrypi/cam_helper_imx290.cpp b/src/ipa/raspberrypi/cam_helper_imx290.cpp
> index 6f412e403f16..871c1f8eaec4 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..d72a9be0438e 100644
> --- a/src/ipa/raspberrypi/cam_helper_imx477.cpp
> +++ b/src/ipa/raspberrypi/cam_helper_imx477.cpp
> @@ -50,7 +50,7 @@ private:
>  };
>  
>  CamHelperImx477::CamHelperImx477()
> -	: CamHelper(new MdParserImx477(), frameIntegrationDiff)
> +	: CamHelper(std::make_unique<MdParserImx477>(), frameIntegrationDiff)
>  {
>  }
>  
> diff --git a/src/ipa/raspberrypi/cam_helper_ov5647.cpp b/src/ipa/raspberrypi/cam_helper_ov5647.cpp
> index 12be6bf931a8..702c2d07f73a 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)
>  {
>  }
>  
> diff --git a/src/ipa/raspberrypi/cam_helper_ov9281.cpp b/src/ipa/raspberrypi/cam_helper_ov9281.cpp
> index 54091f835565..9de868c31dc0 100644
> --- a/src/ipa/raspberrypi/cam_helper_ov9281.cpp
> +++ b/src/ipa/raspberrypi/cam_helper_ov9281.cpp
> @@ -34,7 +34,7 @@ private:
>   */
>  
>  CamHelperOv9281::CamHelperOv9281()
> -	: CamHelper(nullptr, frameIntegrationDiff)
> +	: CamHelper({}, frameIntegrationDiff)
>  {
>  }
>

Patch
diff mbox series 

diff --git a/src/ipa/raspberrypi/cam_helper.cpp b/src/ipa/raspberrypi/cam_helper.cpp
index 062e94c4fef3..90498c37af98 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(std::unique_ptr<MdParser> parser, unsigned int frameIntegrationDiff)
+	: parser_(std::move(parser)), 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 b19c95f67453..fc3139e22be0 100644
--- a/src/ipa/raspberrypi/cam_helper.hpp
+++ b/src/ipa/raspberrypi/cam_helper.hpp
@@ -6,6 +6,7 @@ 
  */
 #pragma once
 
+#include <memory>
 #include <string>
 
 #include <libcamera/base/span.h>
@@ -67,7 +68,7 @@  class CamHelper
 {
 public:
 	static CamHelper *Create(std::string const &cam_name);
-	CamHelper(MdParser *parser, unsigned int frameIntegrationDiff);
+	CamHelper(std::unique_ptr<MdParser> parser, unsigned int frameIntegrationDiff);
 	virtual ~CamHelper();
 	void SetCameraMode(const CameraMode &mode);
 	virtual void Prepare(libcamera::Span<const uint8_t> buffer,
@@ -92,7 +93,7 @@  protected:
 	void parseEmbeddedData(libcamera::Span<const uint8_t> buffer,
 			       Metadata &metadata);
 
-	MdParser *parser_;
+	std::unique_ptr<MdParser> parser_;
 	CameraMode mode_;
 
 private:
diff --git a/src/ipa/raspberrypi/cam_helper_imx219.cpp b/src/ipa/raspberrypi/cam_helper_imx219.cpp
index ec218dce5456..c85044a5fa6d 100644
--- a/src/ipa/raspberrypi/cam_helper_imx219.cpp
+++ b/src/ipa/raspberrypi/cam_helper_imx219.cpp
@@ -58,9 +58,9 @@  private:
 
 CamHelperImx219::CamHelperImx219()
 #if ENABLE_EMBEDDED_DATA
-	: CamHelper(new MdParserImx219(), frameIntegrationDiff)
+	: CamHelper(std::make_unique<MdParserImx219>(), frameIntegrationDiff)
 #else
-	: CamHelper(nullptr, frameIntegrationDiff)
+	: CamHelper({}, frameIntegrationDiff)
 #endif
 {
 }
diff --git a/src/ipa/raspberrypi/cam_helper_imx290.cpp b/src/ipa/raspberrypi/cam_helper_imx290.cpp
index 6f412e403f16..871c1f8eaec4 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..d72a9be0438e 100644
--- a/src/ipa/raspberrypi/cam_helper_imx477.cpp
+++ b/src/ipa/raspberrypi/cam_helper_imx477.cpp
@@ -50,7 +50,7 @@  private:
 };
 
 CamHelperImx477::CamHelperImx477()
-	: CamHelper(new MdParserImx477(), frameIntegrationDiff)
+	: CamHelper(std::make_unique<MdParserImx477>(), frameIntegrationDiff)
 {
 }
 
diff --git a/src/ipa/raspberrypi/cam_helper_ov5647.cpp b/src/ipa/raspberrypi/cam_helper_ov5647.cpp
index 12be6bf931a8..702c2d07f73a 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)
 {
 }
 
diff --git a/src/ipa/raspberrypi/cam_helper_ov9281.cpp b/src/ipa/raspberrypi/cam_helper_ov9281.cpp
index 54091f835565..9de868c31dc0 100644
--- a/src/ipa/raspberrypi/cam_helper_ov9281.cpp
+++ b/src/ipa/raspberrypi/cam_helper_ov9281.cpp
@@ -34,7 +34,7 @@  private:
  */
 
 CamHelperOv9281::CamHelperOv9281()
-	: CamHelper(nullptr, frameIntegrationDiff)
+	: CamHelper({}, frameIntegrationDiff)
 {
 }