[libcamera-devel,v2,18/24] ipa: Pass ControlInfoMap references to IPAInterface::configure()
diff mbox series

Message ID 20191108205409.18845-19-laurent.pinchart@ideasonboard.com
State Accepted
Headers show
Series
  • Control serialization and IPA C API
Related show

Commit Message

Laurent Pinchart Nov. 8, 2019, 8:54 p.m. UTC 
The IPAInterface::configure() operation receives a map of ControlInfoMap
instances. Pass const references instead to avoid copies when not
required (the callee can still make manual copies), and to allow for the
future serialization layer to keep references to the original object.

Signed-off-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
---
 include/ipa/ipa_interface.h              | 2 +-
 src/ipa/ipa_vimc.cpp                     | 2 +-
 src/ipa/rkisp1/rkisp1.cpp                | 4 ++--
 src/libcamera/pipeline/rkisp1/rkisp1.cpp | 2 +-
 src/libcamera/proxy/ipa_proxy_linux.cpp  | 2 +-
 5 files changed, 6 insertions(+), 6 deletions(-)

Comments

Jacopo Mondi Nov. 15, 2019, 4:58 p.m. UTC | #1 
Hi Laurent,

On Fri, Nov 08, 2019 at 10:54:03PM +0200, Laurent Pinchart wrote:
> The IPAInterface::configure() operation receives a map of ControlInfoMap
> instances. Pass const references instead to avoid copies when not
> required (the callee can still make manual copies), and to allow for the
> future serialization layer to keep references to the original object.

Much better
Reviewed-by: Jacopo Mondi <jacopo@jmondi.org>

Thanks
   j
>
> Signed-off-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
> ---
>  include/ipa/ipa_interface.h              | 2 +-
>  src/ipa/ipa_vimc.cpp                     | 2 +-
>  src/ipa/rkisp1/rkisp1.cpp                | 4 ++--
>  src/libcamera/pipeline/rkisp1/rkisp1.cpp | 2 +-
>  src/libcamera/proxy/ipa_proxy_linux.cpp  | 2 +-
>  5 files changed, 6 insertions(+), 6 deletions(-)
>
> diff --git a/include/ipa/ipa_interface.h b/include/ipa/ipa_interface.h
> index 8fd658af5fdb..35dc4b4e3165 100644
> --- a/include/ipa/ipa_interface.h
> +++ b/include/ipa/ipa_interface.h
> @@ -43,7 +43,7 @@ public:
>  	virtual int init() = 0;
>
>  	virtual void configure(const std::map<unsigned int, IPAStream> &streamConfig,
> -			       const std::map<unsigned int, ControlInfoMap> &entityControls) = 0;
> +			       const std::map<unsigned int, const ControlInfoMap &> &entityControls) = 0;
>
>  	virtual void mapBuffers(const std::vector<IPABuffer> &buffers) = 0;
>  	virtual void unmapBuffers(const std::vector<unsigned int> &ids) = 0;
> diff --git a/src/ipa/ipa_vimc.cpp b/src/ipa/ipa_vimc.cpp
> index 9fd5212b0381..50ca8dd805fb 100644
> --- a/src/ipa/ipa_vimc.cpp
> +++ b/src/ipa/ipa_vimc.cpp
> @@ -31,7 +31,7 @@ public:
>
>  	int init() override;
>  	void configure(const std::map<unsigned int, IPAStream> &streamConfig,
> -		       const std::map<unsigned int, ControlInfoMap> &entityControls) override {}
> +		       const std::map<unsigned int, const ControlInfoMap &> &entityControls) override {}
>  	void mapBuffers(const std::vector<IPABuffer> &buffers) override {}
>  	void unmapBuffers(const std::vector<unsigned int> &ids) override {}
>  	void processEvent(const IPAOperationData &event) override {}
> diff --git a/src/ipa/rkisp1/rkisp1.cpp b/src/ipa/rkisp1/rkisp1.cpp
> index d741d5677d0e..41babf0c4140 100644
> --- a/src/ipa/rkisp1/rkisp1.cpp
> +++ b/src/ipa/rkisp1/rkisp1.cpp
> @@ -33,7 +33,7 @@ public:
>  	int init() override { return 0; }
>
>  	void configure(const std::map<unsigned int, IPAStream> &streamConfig,
> -		       const std::map<unsigned int, ControlInfoMap> &entityControls) override;
> +		       const std::map<unsigned int, const ControlInfoMap &> &entityControls) override;
>  	void mapBuffers(const std::vector<IPABuffer> &buffers) override;
>  	void unmapBuffers(const std::vector<unsigned int> &ids) override;
>  	void processEvent(const IPAOperationData &event) override;
> @@ -62,7 +62,7 @@ private:
>  };
>
>  void IPARkISP1::configure(const std::map<unsigned int, IPAStream> &streamConfig,
> -			  const std::map<unsigned int, ControlInfoMap> &entityControls)
> +			  const std::map<unsigned int, const ControlInfoMap &> &entityControls)
>  {
>  	if (entityControls.empty())
>  		return;
> diff --git a/src/libcamera/pipeline/rkisp1/rkisp1.cpp b/src/libcamera/pipeline/rkisp1/rkisp1.cpp
> index b21cf92435e7..4a583a7a1d7e 100644
> --- a/src/libcamera/pipeline/rkisp1/rkisp1.cpp
> +++ b/src/libcamera/pipeline/rkisp1/rkisp1.cpp
> @@ -777,7 +777,7 @@ int PipelineHandlerRkISP1::start(Camera *camera)
>  		.size = data->stream_.configuration().size,
>  	};
>
> -	std::map<unsigned int, ControlInfoMap> entityControls;
> +	std::map<unsigned int, const ControlInfoMap &> entityControls;
>  	entityControls.emplace(0, data->sensor_->controls());
>
>  	data->ipa_->configure(streamConfig, entityControls);
> diff --git a/src/libcamera/proxy/ipa_proxy_linux.cpp b/src/libcamera/proxy/ipa_proxy_linux.cpp
> index 27b6639d6312..c7218fb47249 100644
> --- a/src/libcamera/proxy/ipa_proxy_linux.cpp
> +++ b/src/libcamera/proxy/ipa_proxy_linux.cpp
> @@ -28,7 +28,7 @@ public:
>
>  	int init() override { return 0; }
>  	void configure(const std::map<unsigned int, IPAStream> &streamConfig,
> -		       const std::map<unsigned int, ControlInfoMap> &entityControls) override {}
> +		       const std::map<unsigned int, const ControlInfoMap &> &entityControls) override {}
>  	void mapBuffers(const std::vector<IPABuffer> &buffers) override {}
>  	void unmapBuffers(const std::vector<unsigned int> &ids) override {}
>  	void processEvent(const IPAOperationData &event) override {}
> --
> Regards,
>
> Laurent Pinchart
>
> _______________________________________________
> libcamera-devel mailing list
> libcamera-devel@lists.libcamera.org
> https://lists.libcamera.org/listinfo/libcamera-devel
Niklas Söderlund Nov. 18, 2019, 9:55 p.m. UTC | #2 
Hi Laurent,

Thanks for your patch.

On 2019-11-08 22:54:03 +0200, Laurent Pinchart wrote:
> The IPAInterface::configure() operation receives a map of ControlInfoMap
> instances. Pass const references instead to avoid copies when not
> required (the callee can still make manual copies), and to allow for the
> future serialization layer to keep references to the original object.
> 
> Signed-off-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>

Reviewed-by: Niklas Söderlund <niklas.soderlund@ragnatech.se>

> ---
>  include/ipa/ipa_interface.h              | 2 +-
>  src/ipa/ipa_vimc.cpp                     | 2 +-
>  src/ipa/rkisp1/rkisp1.cpp                | 4 ++--
>  src/libcamera/pipeline/rkisp1/rkisp1.cpp | 2 +-
>  src/libcamera/proxy/ipa_proxy_linux.cpp  | 2 +-
>  5 files changed, 6 insertions(+), 6 deletions(-)
> 
> diff --git a/include/ipa/ipa_interface.h b/include/ipa/ipa_interface.h
> index 8fd658af5fdb..35dc4b4e3165 100644
> --- a/include/ipa/ipa_interface.h
> +++ b/include/ipa/ipa_interface.h
> @@ -43,7 +43,7 @@ public:
>  	virtual int init() = 0;
>  
>  	virtual void configure(const std::map<unsigned int, IPAStream> &streamConfig,
> -			       const std::map<unsigned int, ControlInfoMap> &entityControls) = 0;
> +			       const std::map<unsigned int, const ControlInfoMap &> &entityControls) = 0;
>  
>  	virtual void mapBuffers(const std::vector<IPABuffer> &buffers) = 0;
>  	virtual void unmapBuffers(const std::vector<unsigned int> &ids) = 0;
> diff --git a/src/ipa/ipa_vimc.cpp b/src/ipa/ipa_vimc.cpp
> index 9fd5212b0381..50ca8dd805fb 100644
> --- a/src/ipa/ipa_vimc.cpp
> +++ b/src/ipa/ipa_vimc.cpp
> @@ -31,7 +31,7 @@ public:
>  
>  	int init() override;
>  	void configure(const std::map<unsigned int, IPAStream> &streamConfig,
> -		       const std::map<unsigned int, ControlInfoMap> &entityControls) override {}
> +		       const std::map<unsigned int, const ControlInfoMap &> &entityControls) override {}
>  	void mapBuffers(const std::vector<IPABuffer> &buffers) override {}
>  	void unmapBuffers(const std::vector<unsigned int> &ids) override {}
>  	void processEvent(const IPAOperationData &event) override {}
> diff --git a/src/ipa/rkisp1/rkisp1.cpp b/src/ipa/rkisp1/rkisp1.cpp
> index d741d5677d0e..41babf0c4140 100644
> --- a/src/ipa/rkisp1/rkisp1.cpp
> +++ b/src/ipa/rkisp1/rkisp1.cpp
> @@ -33,7 +33,7 @@ public:
>  	int init() override { return 0; }
>  
>  	void configure(const std::map<unsigned int, IPAStream> &streamConfig,
> -		       const std::map<unsigned int, ControlInfoMap> &entityControls) override;
> +		       const std::map<unsigned int, const ControlInfoMap &> &entityControls) override;
>  	void mapBuffers(const std::vector<IPABuffer> &buffers) override;
>  	void unmapBuffers(const std::vector<unsigned int> &ids) override;
>  	void processEvent(const IPAOperationData &event) override;
> @@ -62,7 +62,7 @@ private:
>  };
>  
>  void IPARkISP1::configure(const std::map<unsigned int, IPAStream> &streamConfig,
> -			  const std::map<unsigned int, ControlInfoMap> &entityControls)
> +			  const std::map<unsigned int, const ControlInfoMap &> &entityControls)
>  {
>  	if (entityControls.empty())
>  		return;
> diff --git a/src/libcamera/pipeline/rkisp1/rkisp1.cpp b/src/libcamera/pipeline/rkisp1/rkisp1.cpp
> index b21cf92435e7..4a583a7a1d7e 100644
> --- a/src/libcamera/pipeline/rkisp1/rkisp1.cpp
> +++ b/src/libcamera/pipeline/rkisp1/rkisp1.cpp
> @@ -777,7 +777,7 @@ int PipelineHandlerRkISP1::start(Camera *camera)
>  		.size = data->stream_.configuration().size,
>  	};
>  
> -	std::map<unsigned int, ControlInfoMap> entityControls;
> +	std::map<unsigned int, const ControlInfoMap &> entityControls;
>  	entityControls.emplace(0, data->sensor_->controls());
>  
>  	data->ipa_->configure(streamConfig, entityControls);
> diff --git a/src/libcamera/proxy/ipa_proxy_linux.cpp b/src/libcamera/proxy/ipa_proxy_linux.cpp
> index 27b6639d6312..c7218fb47249 100644
> --- a/src/libcamera/proxy/ipa_proxy_linux.cpp
> +++ b/src/libcamera/proxy/ipa_proxy_linux.cpp
> @@ -28,7 +28,7 @@ public:
>  
>  	int init() override { return 0; }
>  	void configure(const std::map<unsigned int, IPAStream> &streamConfig,
> -		       const std::map<unsigned int, ControlInfoMap> &entityControls) override {}
> +		       const std::map<unsigned int, const ControlInfoMap &> &entityControls) override {}
>  	void mapBuffers(const std::vector<IPABuffer> &buffers) override {}
>  	void unmapBuffers(const std::vector<unsigned int> &ids) override {}
>  	void processEvent(const IPAOperationData &event) override {}
> -- 
> Regards,
> 
> Laurent Pinchart
> 
> _______________________________________________
> libcamera-devel mailing list
> libcamera-devel@lists.libcamera.org
> https://lists.libcamera.org/listinfo/libcamera-devel

Patch
diff mbox series 

diff --git a/include/ipa/ipa_interface.h b/include/ipa/ipa_interface.h
index 8fd658af5fdb..35dc4b4e3165 100644
--- a/include/ipa/ipa_interface.h
+++ b/include/ipa/ipa_interface.h
@@ -43,7 +43,7 @@  public:
 	virtual int init() = 0;
 
 	virtual void configure(const std::map<unsigned int, IPAStream> &streamConfig,
-			       const std::map<unsigned int, ControlInfoMap> &entityControls) = 0;
+			       const std::map<unsigned int, const ControlInfoMap &> &entityControls) = 0;
 
 	virtual void mapBuffers(const std::vector<IPABuffer> &buffers) = 0;
 	virtual void unmapBuffers(const std::vector<unsigned int> &ids) = 0;
diff --git a/src/ipa/ipa_vimc.cpp b/src/ipa/ipa_vimc.cpp
index 9fd5212b0381..50ca8dd805fb 100644
--- a/src/ipa/ipa_vimc.cpp
+++ b/src/ipa/ipa_vimc.cpp
@@ -31,7 +31,7 @@  public:
 
 	int init() override;
 	void configure(const std::map<unsigned int, IPAStream> &streamConfig,
-		       const std::map<unsigned int, ControlInfoMap> &entityControls) override {}
+		       const std::map<unsigned int, const ControlInfoMap &> &entityControls) override {}
 	void mapBuffers(const std::vector<IPABuffer> &buffers) override {}
 	void unmapBuffers(const std::vector<unsigned int> &ids) override {}
 	void processEvent(const IPAOperationData &event) override {}
diff --git a/src/ipa/rkisp1/rkisp1.cpp b/src/ipa/rkisp1/rkisp1.cpp
index d741d5677d0e..41babf0c4140 100644
--- a/src/ipa/rkisp1/rkisp1.cpp
+++ b/src/ipa/rkisp1/rkisp1.cpp
@@ -33,7 +33,7 @@  public:
 	int init() override { return 0; }
 
 	void configure(const std::map<unsigned int, IPAStream> &streamConfig,
-		       const std::map<unsigned int, ControlInfoMap> &entityControls) override;
+		       const std::map<unsigned int, const ControlInfoMap &> &entityControls) override;
 	void mapBuffers(const std::vector<IPABuffer> &buffers) override;
 	void unmapBuffers(const std::vector<unsigned int> &ids) override;
 	void processEvent(const IPAOperationData &event) override;
@@ -62,7 +62,7 @@  private:
 };
 
 void IPARkISP1::configure(const std::map<unsigned int, IPAStream> &streamConfig,
-			  const std::map<unsigned int, ControlInfoMap> &entityControls)
+			  const std::map<unsigned int, const ControlInfoMap &> &entityControls)
 {
 	if (entityControls.empty())
 		return;
diff --git a/src/libcamera/pipeline/rkisp1/rkisp1.cpp b/src/libcamera/pipeline/rkisp1/rkisp1.cpp
index b21cf92435e7..4a583a7a1d7e 100644
--- a/src/libcamera/pipeline/rkisp1/rkisp1.cpp
+++ b/src/libcamera/pipeline/rkisp1/rkisp1.cpp
@@ -777,7 +777,7 @@  int PipelineHandlerRkISP1::start(Camera *camera)
 		.size = data->stream_.configuration().size,
 	};
 
-	std::map<unsigned int, ControlInfoMap> entityControls;
+	std::map<unsigned int, const ControlInfoMap &> entityControls;
 	entityControls.emplace(0, data->sensor_->controls());
 
 	data->ipa_->configure(streamConfig, entityControls);
diff --git a/src/libcamera/proxy/ipa_proxy_linux.cpp b/src/libcamera/proxy/ipa_proxy_linux.cpp
index 27b6639d6312..c7218fb47249 100644
--- a/src/libcamera/proxy/ipa_proxy_linux.cpp
+++ b/src/libcamera/proxy/ipa_proxy_linux.cpp
@@ -28,7 +28,7 @@  public:
 
 	int init() override { return 0; }
 	void configure(const std::map<unsigned int, IPAStream> &streamConfig,
-		       const std::map<unsigned int, ControlInfoMap> &entityControls) override {}
+		       const std::map<unsigned int, const ControlInfoMap &> &entityControls) override {}
 	void mapBuffers(const std::vector<IPABuffer> &buffers) override {}
 	void unmapBuffers(const std::vector<unsigned int> &ids) override {}
 	void processEvent(const IPAOperationData &event) override {}