[libcamera-devel,v2,09/12] ipa: raspberrypi: Drop CameraHelper::GetOrientation()
diff mbox series

Message ID 20200704005227.21782-10-laurent.pinchart@ideasonboard.com
State Accepted
Headers show
Series
  • Support passing custom data to IPA configure()
Related show

Commit Message

Laurent Pinchart July 4, 2020, 12:52 a.m. UTC 
The camera sensor orientation is now handled by the pipeline handler.
Drop hardcoded per-sensor orientations from the IPA.

Signed-off-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
---
 src/ipa/raspberrypi/cam_helper.cpp        | 6 ------
 src/ipa/raspberrypi/cam_helper.hpp        | 7 -------
 src/ipa/raspberrypi/cam_helper_imx219.cpp | 7 -------
 src/ipa/raspberrypi/cam_helper_imx477.cpp | 7 -------
 src/ipa/raspberrypi/raspberrypi.cpp       | 6 ------
 5 files changed, 33 deletions(-)

Comments

Jacopo Mondi July 6, 2020, 9:17 a.m. UTC | #1 
replying to my question on the previous patch: yes!
-.-'

On Sat, Jul 04, 2020 at 03:52:24AM +0300, Laurent Pinchart wrote:
> The camera sensor orientation is now handled by the pipeline handler.
> Drop hardcoded per-sensor orientations from the IPA.
>
> Signed-off-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>

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

Thanks
  j

> ---
>  src/ipa/raspberrypi/cam_helper.cpp        | 6 ------
>  src/ipa/raspberrypi/cam_helper.hpp        | 7 -------
>  src/ipa/raspberrypi/cam_helper_imx219.cpp | 7 -------
>  src/ipa/raspberrypi/cam_helper_imx477.cpp | 7 -------
>  src/ipa/raspberrypi/raspberrypi.cpp       | 6 ------
>  5 files changed, 33 deletions(-)
>
> diff --git a/src/ipa/raspberrypi/cam_helper.cpp b/src/ipa/raspberrypi/cam_helper.cpp
> index a0c73f99390f..b1343eb2cf4e 100644
> --- a/src/ipa/raspberrypi/cam_helper.cpp
> +++ b/src/ipa/raspberrypi/cam_helper.cpp
> @@ -106,12 +106,6 @@ unsigned int CamHelper::MistrustFramesModeSwitch() const
>  	return 0;
>  }
>
> -CamTransform CamHelper::GetOrientation() const
> -{
> -	/* Most sensors will be mounted the "right" way up? */
> -	return CamTransform_IDENTITY;
> -}
> -
>  RegisterCamHelper::RegisterCamHelper(char const *cam_name,
>  				     CamHelperCreateFunc create_func)
>  {
> diff --git a/src/ipa/raspberrypi/cam_helper.hpp b/src/ipa/raspberrypi/cam_helper.hpp
> index 6877f4735031..97ce3e92ac2f 100644
> --- a/src/ipa/raspberrypi/cam_helper.hpp
> +++ b/src/ipa/raspberrypi/cam_helper.hpp
> @@ -58,12 +58,6 @@ namespace RPi {
>  //    (other than start-up), for which control algorithms should not run
>  //    (for example, metadata may be unreliable).
>
> -// Bitfield to represent the default orientation of the camera.
> -typedef int CamTransform;
> -static constexpr CamTransform CamTransform_IDENTITY = 0;
> -static constexpr CamTransform CamTransform_HFLIP    = 1;
> -static constexpr CamTransform CamTransform_VFLIP    = 2;
> -
>  class CamHelper
>  {
>  public:
> @@ -82,7 +76,6 @@ public:
>  	virtual unsigned int HideFramesModeSwitch() const;
>  	virtual unsigned int MistrustFramesStartup() const;
>  	virtual unsigned int MistrustFramesModeSwitch() const;
> -	virtual CamTransform GetOrientation() const;
>  protected:
>  	MdParser *parser_;
>  	CameraMode mode_;
> diff --git a/src/ipa/raspberrypi/cam_helper_imx219.cpp b/src/ipa/raspberrypi/cam_helper_imx219.cpp
> index 35c6597c2016..1b9ce382f330 100644
> --- a/src/ipa/raspberrypi/cam_helper_imx219.cpp
> +++ b/src/ipa/raspberrypi/cam_helper_imx219.cpp
> @@ -49,7 +49,6 @@ public:
>  	double Gain(uint32_t gain_code) const override;
>  	unsigned int MistrustFramesModeSwitch() const override;
>  	bool SensorEmbeddedDataPresent() const override;
> -	CamTransform GetOrientation() const override;
>  };
>
>  CamHelperImx219::CamHelperImx219()
> @@ -86,12 +85,6 @@ bool CamHelperImx219::SensorEmbeddedDataPresent() const
>  	return ENABLE_EMBEDDED_DATA;
>  }
>
> -CamTransform CamHelperImx219::GetOrientation() const
> -{
> -	/* Camera is "upside down" on this board. */
> -	return CamTransform_HFLIP | CamTransform_VFLIP;
> -}
> -
>  static CamHelper *Create()
>  {
>  	return new CamHelperImx219();
> diff --git a/src/ipa/raspberrypi/cam_helper_imx477.cpp b/src/ipa/raspberrypi/cam_helper_imx477.cpp
> index 695444567bb2..a53b47c013cb 100644
> --- a/src/ipa/raspberrypi/cam_helper_imx477.cpp
> +++ b/src/ipa/raspberrypi/cam_helper_imx477.cpp
> @@ -38,7 +38,6 @@ public:
>  	uint32_t GainCode(double gain) const override;
>  	double Gain(uint32_t gain_code) const override;
>  	bool SensorEmbeddedDataPresent() const override;
> -	CamTransform GetOrientation() const override;
>  };
>
>  CamHelperImx477::CamHelperImx477()
> @@ -61,12 +60,6 @@ bool CamHelperImx477::SensorEmbeddedDataPresent() const
>  	return true;
>  }
>
> -CamTransform CamHelperImx477::GetOrientation() const
> -{
> -	/* Camera is "upside down" on this board. */
> -	return CamTransform_HFLIP | CamTransform_VFLIP;
> -}
> -
>  static CamHelper *Create()
>  {
>  	return new CamHelperImx477();
> diff --git a/src/ipa/raspberrypi/raspberrypi.cpp b/src/ipa/raspberrypi/raspberrypi.cpp
> index 860be22ddb5d..b40ca9e44776 100644
> --- a/src/ipa/raspberrypi/raspberrypi.cpp
> +++ b/src/ipa/raspberrypi/raspberrypi.cpp
> @@ -215,7 +215,6 @@ void IPARPi::configure(const CameraSensorInfo &sensorInfo,
>  		int gainDelay, exposureDelay, sensorMetadata;
>  		helper_->GetDelays(exposureDelay, gainDelay);
>  		sensorMetadata = helper_->SensorEmbeddedDataPresent();
> -		RPi::CamTransform orientation = helper_->GetOrientation();
>
>  		IPAOperationData op;
>  		op.operation = RPI_IPA_ACTION_SET_SENSOR_CONFIG;
> @@ -223,11 +222,6 @@ void IPARPi::configure(const CameraSensorInfo &sensorInfo,
>  		op.data.push_back(exposureDelay);
>  		op.data.push_back(sensorMetadata);
>
> -		ControlList ctrls(unicam_ctrls_);
> -		ctrls.set(V4L2_CID_HFLIP, (int32_t) !!(orientation & RPi::CamTransform_HFLIP));
> -		ctrls.set(V4L2_CID_VFLIP, (int32_t) !!(orientation & RPi::CamTransform_VFLIP));
> -		op.controls.push_back(ctrls);
> -
>  		queueFrameAction.emit(0, op);
>  	}
>
> --
> Regards,
>
> Laurent Pinchart
>
> _______________________________________________
> libcamera-devel mailing list
> libcamera-devel@lists.libcamera.org
> https://lists.libcamera.org/listinfo/libcamera-devel
Naushir Patuck July 8, 2020, 10:25 a.m. UTC | #2 
Hi Laurent,


On Sat, 4 Jul 2020 at 01:52, Laurent Pinchart
<laurent.pinchart@ideasonboard.com> wrote:
>
> The camera sensor orientation is now handled by the pipeline handler.
> Drop hardcoded per-sensor orientations from the IPA.
>
> Signed-off-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>

Reviewed-by: Naushir Patuck <naush@raspberrypi.com>

> ---
>  src/ipa/raspberrypi/cam_helper.cpp        | 6 ------
>  src/ipa/raspberrypi/cam_helper.hpp        | 7 -------
>  src/ipa/raspberrypi/cam_helper_imx219.cpp | 7 -------
>  src/ipa/raspberrypi/cam_helper_imx477.cpp | 7 -------
>  src/ipa/raspberrypi/raspberrypi.cpp       | 6 ------
>  5 files changed, 33 deletions(-)
>
> diff --git a/src/ipa/raspberrypi/cam_helper.cpp b/src/ipa/raspberrypi/cam_helper.cpp
> index a0c73f99390f..b1343eb2cf4e 100644
> --- a/src/ipa/raspberrypi/cam_helper.cpp
> +++ b/src/ipa/raspberrypi/cam_helper.cpp
> @@ -106,12 +106,6 @@ unsigned int CamHelper::MistrustFramesModeSwitch() const
>         return 0;
>  }
>
> -CamTransform CamHelper::GetOrientation() const
> -{
> -       /* Most sensors will be mounted the "right" way up? */
> -       return CamTransform_IDENTITY;
> -}
> -
>  RegisterCamHelper::RegisterCamHelper(char const *cam_name,
>                                      CamHelperCreateFunc create_func)
>  {
> diff --git a/src/ipa/raspberrypi/cam_helper.hpp b/src/ipa/raspberrypi/cam_helper.hpp
> index 6877f4735031..97ce3e92ac2f 100644
> --- a/src/ipa/raspberrypi/cam_helper.hpp
> +++ b/src/ipa/raspberrypi/cam_helper.hpp
> @@ -58,12 +58,6 @@ namespace RPi {
>  //    (other than start-up), for which control algorithms should not run
>  //    (for example, metadata may be unreliable).
>
> -// Bitfield to represent the default orientation of the camera.
> -typedef int CamTransform;
> -static constexpr CamTransform CamTransform_IDENTITY = 0;
> -static constexpr CamTransform CamTransform_HFLIP    = 1;
> -static constexpr CamTransform CamTransform_VFLIP    = 2;
> -
>  class CamHelper
>  {
>  public:
> @@ -82,7 +76,6 @@ public:
>         virtual unsigned int HideFramesModeSwitch() const;
>         virtual unsigned int MistrustFramesStartup() const;
>         virtual unsigned int MistrustFramesModeSwitch() const;
> -       virtual CamTransform GetOrientation() const;
>  protected:
>         MdParser *parser_;
>         CameraMode mode_;
> diff --git a/src/ipa/raspberrypi/cam_helper_imx219.cpp b/src/ipa/raspberrypi/cam_helper_imx219.cpp
> index 35c6597c2016..1b9ce382f330 100644
> --- a/src/ipa/raspberrypi/cam_helper_imx219.cpp
> +++ b/src/ipa/raspberrypi/cam_helper_imx219.cpp
> @@ -49,7 +49,6 @@ public:
>         double Gain(uint32_t gain_code) const override;
>         unsigned int MistrustFramesModeSwitch() const override;
>         bool SensorEmbeddedDataPresent() const override;
> -       CamTransform GetOrientation() const override;
>  };
>
>  CamHelperImx219::CamHelperImx219()
> @@ -86,12 +85,6 @@ bool CamHelperImx219::SensorEmbeddedDataPresent() const
>         return ENABLE_EMBEDDED_DATA;
>  }
>
> -CamTransform CamHelperImx219::GetOrientation() const
> -{
> -       /* Camera is "upside down" on this board. */
> -       return CamTransform_HFLIP | CamTransform_VFLIP;
> -}
> -
>  static CamHelper *Create()
>  {
>         return new CamHelperImx219();
> diff --git a/src/ipa/raspberrypi/cam_helper_imx477.cpp b/src/ipa/raspberrypi/cam_helper_imx477.cpp
> index 695444567bb2..a53b47c013cb 100644
> --- a/src/ipa/raspberrypi/cam_helper_imx477.cpp
> +++ b/src/ipa/raspberrypi/cam_helper_imx477.cpp
> @@ -38,7 +38,6 @@ public:
>         uint32_t GainCode(double gain) const override;
>         double Gain(uint32_t gain_code) const override;
>         bool SensorEmbeddedDataPresent() const override;
> -       CamTransform GetOrientation() const override;
>  };
>
>  CamHelperImx477::CamHelperImx477()
> @@ -61,12 +60,6 @@ bool CamHelperImx477::SensorEmbeddedDataPresent() const
>         return true;
>  }
>
> -CamTransform CamHelperImx477::GetOrientation() const
> -{
> -       /* Camera is "upside down" on this board. */
> -       return CamTransform_HFLIP | CamTransform_VFLIP;
> -}
> -
>  static CamHelper *Create()
>  {
>         return new CamHelperImx477();
> diff --git a/src/ipa/raspberrypi/raspberrypi.cpp b/src/ipa/raspberrypi/raspberrypi.cpp
> index 860be22ddb5d..b40ca9e44776 100644
> --- a/src/ipa/raspberrypi/raspberrypi.cpp
> +++ b/src/ipa/raspberrypi/raspberrypi.cpp
> @@ -215,7 +215,6 @@ void IPARPi::configure(const CameraSensorInfo &sensorInfo,
>                 int gainDelay, exposureDelay, sensorMetadata;
>                 helper_->GetDelays(exposureDelay, gainDelay);
>                 sensorMetadata = helper_->SensorEmbeddedDataPresent();
> -               RPi::CamTransform orientation = helper_->GetOrientation();
>
>                 IPAOperationData op;
>                 op.operation = RPI_IPA_ACTION_SET_SENSOR_CONFIG;
> @@ -223,11 +222,6 @@ void IPARPi::configure(const CameraSensorInfo &sensorInfo,
>                 op.data.push_back(exposureDelay);
>                 op.data.push_back(sensorMetadata);
>
> -               ControlList ctrls(unicam_ctrls_);
> -               ctrls.set(V4L2_CID_HFLIP, (int32_t) !!(orientation & RPi::CamTransform_HFLIP));
> -               ctrls.set(V4L2_CID_VFLIP, (int32_t) !!(orientation & RPi::CamTransform_VFLIP));
> -               op.controls.push_back(ctrls);
> -
>                 queueFrameAction.emit(0, op);
>         }
>
> --
> Regards,
>
> Laurent Pinchart
>

Patch
diff mbox series 

diff --git a/src/ipa/raspberrypi/cam_helper.cpp b/src/ipa/raspberrypi/cam_helper.cpp
index a0c73f99390f..b1343eb2cf4e 100644
--- a/src/ipa/raspberrypi/cam_helper.cpp
+++ b/src/ipa/raspberrypi/cam_helper.cpp
@@ -106,12 +106,6 @@  unsigned int CamHelper::MistrustFramesModeSwitch() const
 	return 0;
 }
 
-CamTransform CamHelper::GetOrientation() const
-{
-	/* Most sensors will be mounted the "right" way up? */
-	return CamTransform_IDENTITY;
-}
-
 RegisterCamHelper::RegisterCamHelper(char const *cam_name,
 				     CamHelperCreateFunc create_func)
 {
diff --git a/src/ipa/raspberrypi/cam_helper.hpp b/src/ipa/raspberrypi/cam_helper.hpp
index 6877f4735031..97ce3e92ac2f 100644
--- a/src/ipa/raspberrypi/cam_helper.hpp
+++ b/src/ipa/raspberrypi/cam_helper.hpp
@@ -58,12 +58,6 @@  namespace RPi {
 //    (other than start-up), for which control algorithms should not run
 //    (for example, metadata may be unreliable).
 
-// Bitfield to represent the default orientation of the camera.
-typedef int CamTransform;
-static constexpr CamTransform CamTransform_IDENTITY = 0;
-static constexpr CamTransform CamTransform_HFLIP    = 1;
-static constexpr CamTransform CamTransform_VFLIP    = 2;
-
 class CamHelper
 {
 public:
@@ -82,7 +76,6 @@  public:
 	virtual unsigned int HideFramesModeSwitch() const;
 	virtual unsigned int MistrustFramesStartup() const;
 	virtual unsigned int MistrustFramesModeSwitch() const;
-	virtual CamTransform GetOrientation() const;
 protected:
 	MdParser *parser_;
 	CameraMode mode_;
diff --git a/src/ipa/raspberrypi/cam_helper_imx219.cpp b/src/ipa/raspberrypi/cam_helper_imx219.cpp
index 35c6597c2016..1b9ce382f330 100644
--- a/src/ipa/raspberrypi/cam_helper_imx219.cpp
+++ b/src/ipa/raspberrypi/cam_helper_imx219.cpp
@@ -49,7 +49,6 @@  public:
 	double Gain(uint32_t gain_code) const override;
 	unsigned int MistrustFramesModeSwitch() const override;
 	bool SensorEmbeddedDataPresent() const override;
-	CamTransform GetOrientation() const override;
 };
 
 CamHelperImx219::CamHelperImx219()
@@ -86,12 +85,6 @@  bool CamHelperImx219::SensorEmbeddedDataPresent() const
 	return ENABLE_EMBEDDED_DATA;
 }
 
-CamTransform CamHelperImx219::GetOrientation() const
-{
-	/* Camera is "upside down" on this board. */
-	return CamTransform_HFLIP | CamTransform_VFLIP;
-}
-
 static CamHelper *Create()
 {
 	return new CamHelperImx219();
diff --git a/src/ipa/raspberrypi/cam_helper_imx477.cpp b/src/ipa/raspberrypi/cam_helper_imx477.cpp
index 695444567bb2..a53b47c013cb 100644
--- a/src/ipa/raspberrypi/cam_helper_imx477.cpp
+++ b/src/ipa/raspberrypi/cam_helper_imx477.cpp
@@ -38,7 +38,6 @@  public:
 	uint32_t GainCode(double gain) const override;
 	double Gain(uint32_t gain_code) const override;
 	bool SensorEmbeddedDataPresent() const override;
-	CamTransform GetOrientation() const override;
 };
 
 CamHelperImx477::CamHelperImx477()
@@ -61,12 +60,6 @@  bool CamHelperImx477::SensorEmbeddedDataPresent() const
 	return true;
 }
 
-CamTransform CamHelperImx477::GetOrientation() const
-{
-	/* Camera is "upside down" on this board. */
-	return CamTransform_HFLIP | CamTransform_VFLIP;
-}
-
 static CamHelper *Create()
 {
 	return new CamHelperImx477();
diff --git a/src/ipa/raspberrypi/raspberrypi.cpp b/src/ipa/raspberrypi/raspberrypi.cpp
index 860be22ddb5d..b40ca9e44776 100644
--- a/src/ipa/raspberrypi/raspberrypi.cpp
+++ b/src/ipa/raspberrypi/raspberrypi.cpp
@@ -215,7 +215,6 @@  void IPARPi::configure(const CameraSensorInfo &sensorInfo,
 		int gainDelay, exposureDelay, sensorMetadata;
 		helper_->GetDelays(exposureDelay, gainDelay);
 		sensorMetadata = helper_->SensorEmbeddedDataPresent();
-		RPi::CamTransform orientation = helper_->GetOrientation();
 
 		IPAOperationData op;
 		op.operation = RPI_IPA_ACTION_SET_SENSOR_CONFIG;
@@ -223,11 +222,6 @@  void IPARPi::configure(const CameraSensorInfo &sensorInfo,
 		op.data.push_back(exposureDelay);
 		op.data.push_back(sensorMetadata);
 
-		ControlList ctrls(unicam_ctrls_);
-		ctrls.set(V4L2_CID_HFLIP, (int32_t) !!(orientation & RPi::CamTransform_HFLIP));
-		ctrls.set(V4L2_CID_VFLIP, (int32_t) !!(orientation & RPi::CamTransform_VFLIP));
-		op.controls.push_back(ctrls);
-
 		queueFrameAction.emit(0, op);
 	}