[libcamera-devel,v4,17/37] libcamera: PipelineHandler: Remove IPA from base class
diff mbox series

Message ID 20201106103707.49660-18-paul.elder@ideasonboard.com
State Superseded
Headers show
Series
  • IPA isolation implementation
Related show

Commit Message

Paul Elder Nov. 6, 2020, 10:36 a.m. UTC 
Since pipline handlers now have their own IPA interface types, it can no
longer be defined in the base class, and each pipline handler
implementation must declare it and its type themselves. Remove it from
the base class.

Signed-off-by: Paul Elder <paul.elder@ideasonboard.com>
Reviewed-by: Niklas Söderlund <niklas.soderlund@ragnatech.se>

---
No change in v4

No change in v3

Changes in v2:
- remove documentation
---
 include/libcamera/internal/pipeline_handler.h | 1 -
 src/libcamera/pipeline_handler.cpp            | 8 --------
 2 files changed, 9 deletions(-)

Comments

Jacopo Mondi Nov. 17, 2020, 4:12 p.m. UTC | #1 
Hi Paul,

On Fri, Nov 06, 2020 at 07:36:47PM +0900, Paul Elder wrote:
> Since pipline handlers now have their own IPA interface types, it can no
> longer be defined in the base class, and each pipline handler
> implementation must declare it and its type themselves. Remove it from
> the base class.
>
> Signed-off-by: Paul Elder <paul.elder@ideasonboard.com>
> Reviewed-by: Niklas Söderlund <niklas.soderlund@ragnatech.se>

nice!

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

Thanks
  j

>
> ---
> No change in v4
>
> No change in v3
>
> Changes in v2:
> - remove documentation
> ---
>  include/libcamera/internal/pipeline_handler.h | 1 -
>  src/libcamera/pipeline_handler.cpp            | 8 --------
>  2 files changed, 9 deletions(-)
>
> diff --git a/include/libcamera/internal/pipeline_handler.h b/include/libcamera/internal/pipeline_handler.h
> index c12c8904..57ad3052 100644
> --- a/include/libcamera/internal/pipeline_handler.h
> +++ b/include/libcamera/internal/pipeline_handler.h
> @@ -47,7 +47,6 @@ public:
>  	std::list<Request *> queuedRequests_;
>  	ControlInfoMap controlInfo_;
>  	ControlList properties_;
> -	std::unique_ptr<IPAProxy> ipa_;
>
>  private:
>  	CameraData(const CameraData &) = delete;
> diff --git a/src/libcamera/pipeline_handler.cpp b/src/libcamera/pipeline_handler.cpp
> index 894200ee..b893b0e7 100644
> --- a/src/libcamera/pipeline_handler.cpp
> +++ b/src/libcamera/pipeline_handler.cpp
> @@ -106,14 +106,6 @@ LOG_DEFINE_CATEGORY(Pipeline)
>   * when creating the camera, and shall not be modified afterwards.
>   */
>
> -/**
> - * \var CameraData::ipa_
> - * \brief The IPA module used by the camera
> - *
> - * Reference to the Image Processing Algorithms (IPA) operating on the camera's
> - * stream(s). If no IPA exists for the camera, this field is set to nullptr.
> - */
> -
>  /**
>   * \class PipelineHandler
>   * \brief Create and manage cameras based on a set of media devices
> --
> 2.27.0
>
> _______________________________________________
> libcamera-devel mailing list
> libcamera-devel@lists.libcamera.org
> https://lists.libcamera.org/listinfo/libcamera-devel
Jacopo Mondi Nov. 17, 2020, 4:18 p.m. UTC | #2 
Hi Paul,

On Fri, Nov 06, 2020 at 07:36:47PM +0900, Paul Elder wrote:
> Since pipline handlers now have their own IPA interface types, it can no
> longer be defined in the base class, and each pipline handler
> implementation must declare it and its type themselves. Remove it from
> the base class.
>
> Signed-off-by: Paul Elder <paul.elder@ideasonboard.com>
> Reviewed-by: Niklas Söderlund <niklas.soderlund@ragnatech.se>
>

This patch in isolation will break compilation, but I don't think
there's many ways around it

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

Thanks
  j

> ---
> No change in v4
>
> No change in v3
>
> Changes in v2:
> - remove documentation
> ---
>  include/libcamera/internal/pipeline_handler.h | 1 -
>  src/libcamera/pipeline_handler.cpp            | 8 --------
>  2 files changed, 9 deletions(-)
>
> diff --git a/include/libcamera/internal/pipeline_handler.h b/include/libcamera/internal/pipeline_handler.h
> index c12c8904..57ad3052 100644
> --- a/include/libcamera/internal/pipeline_handler.h
> +++ b/include/libcamera/internal/pipeline_handler.h
> @@ -47,7 +47,6 @@ public:
>  	std::list<Request *> queuedRequests_;
>  	ControlInfoMap controlInfo_;
>  	ControlList properties_;
> -	std::unique_ptr<IPAProxy> ipa_;
>
>  private:
>  	CameraData(const CameraData &) = delete;
> diff --git a/src/libcamera/pipeline_handler.cpp b/src/libcamera/pipeline_handler.cpp
> index 894200ee..b893b0e7 100644
> --- a/src/libcamera/pipeline_handler.cpp
> +++ b/src/libcamera/pipeline_handler.cpp
> @@ -106,14 +106,6 @@ LOG_DEFINE_CATEGORY(Pipeline)
>   * when creating the camera, and shall not be modified afterwards.
>   */
>
> -/**
> - * \var CameraData::ipa_
> - * \brief The IPA module used by the camera
> - *
> - * Reference to the Image Processing Algorithms (IPA) operating on the camera's
> - * stream(s). If no IPA exists for the camera, this field is set to nullptr.
> - */
> -
>  /**
>   * \class PipelineHandler
>   * \brief Create and manage cameras based on a set of media devices
> --
> 2.27.0
>
> _______________________________________________
> libcamera-devel mailing list
> libcamera-devel@lists.libcamera.org
> https://lists.libcamera.org/listinfo/libcamera-devel
Laurent Pinchart Nov. 25, 2020, 3:36 p.m. UTC | #3 
Hi Paul,

Thank you for the patch.

On Fri, Nov 06, 2020 at 07:36:47PM +0900, Paul Elder wrote:
> Since pipline handlers now have their own IPA interface types, it can no

s/pipline/pipeline/

> longer be defined in the base class, and each pipline handler

Here too.

> implementation must declare it and its type themselves. Remove it from
> the base class.
> 
> Signed-off-by: Paul Elder <paul.elder@ideasonboard.com>
> Reviewed-by: Niklas Söderlund <niklas.soderlund@ragnatech.se>

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

> ---
> No change in v4
> 
> No change in v3
> 
> Changes in v2:
> - remove documentation
> ---
>  include/libcamera/internal/pipeline_handler.h | 1 -
>  src/libcamera/pipeline_handler.cpp            | 8 --------
>  2 files changed, 9 deletions(-)
> 
> diff --git a/include/libcamera/internal/pipeline_handler.h b/include/libcamera/internal/pipeline_handler.h
> index c12c8904..57ad3052 100644
> --- a/include/libcamera/internal/pipeline_handler.h
> +++ b/include/libcamera/internal/pipeline_handler.h
> @@ -47,7 +47,6 @@ public:
>  	std::list<Request *> queuedRequests_;
>  	ControlInfoMap controlInfo_;
>  	ControlList properties_;
> -	std::unique_ptr<IPAProxy> ipa_;
>  
>  private:
>  	CameraData(const CameraData &) = delete;
> diff --git a/src/libcamera/pipeline_handler.cpp b/src/libcamera/pipeline_handler.cpp
> index 894200ee..b893b0e7 100644
> --- a/src/libcamera/pipeline_handler.cpp
> +++ b/src/libcamera/pipeline_handler.cpp
> @@ -106,14 +106,6 @@ LOG_DEFINE_CATEGORY(Pipeline)
>   * when creating the camera, and shall not be modified afterwards.
>   */
>  
> -/**
> - * \var CameraData::ipa_
> - * \brief The IPA module used by the camera
> - *
> - * Reference to the Image Processing Algorithms (IPA) operating on the camera's
> - * stream(s). If no IPA exists for the camera, this field is set to nullptr.
> - */
> -
>  /**
>   * \class PipelineHandler
>   * \brief Create and manage cameras based on a set of media devices

Patch
diff mbox series 

diff --git a/include/libcamera/internal/pipeline_handler.h b/include/libcamera/internal/pipeline_handler.h
index c12c8904..57ad3052 100644
--- a/include/libcamera/internal/pipeline_handler.h
+++ b/include/libcamera/internal/pipeline_handler.h
@@ -47,7 +47,6 @@  public:
 	std::list<Request *> queuedRequests_;
 	ControlInfoMap controlInfo_;
 	ControlList properties_;
-	std::unique_ptr<IPAProxy> ipa_;
 
 private:
 	CameraData(const CameraData &) = delete;
diff --git a/src/libcamera/pipeline_handler.cpp b/src/libcamera/pipeline_handler.cpp
index 894200ee..b893b0e7 100644
--- a/src/libcamera/pipeline_handler.cpp
+++ b/src/libcamera/pipeline_handler.cpp
@@ -106,14 +106,6 @@  LOG_DEFINE_CATEGORY(Pipeline)
  * when creating the camera, and shall not be modified afterwards.
  */
 
-/**
- * \var CameraData::ipa_
- * \brief The IPA module used by the camera
- *
- * Reference to the Image Processing Algorithms (IPA) operating on the camera's
- * stream(s). If no IPA exists for the camera, this field is set to nullptr.
- */
-
 /**
  * \class PipelineHandler
  * \brief Create and manage cameras based on a set of media devices