[libcamera-devel,v7,3/8] ipa: raspberrypi: Add an operator<< to struct DeviceStatus
diff mbox series

Message ID 20210712100209.447893-4-naush@raspberrypi.com
State Accepted
Commit f24d83720f0ecd5656699f5013d7b5532e6e72c2
Headers show
Series
  • ipa: raspberrypi: Allow long exposure modes for imx477.
Related show

Commit Message

Naushir Patuck July 12, 2021, 10:02 a.m. UTC 
Add an operator<< overload to log all fields in DeviceStatus, and remove the
manual logging statements in the IPA and CamHelper.

Signed-off-by: Naushir Patuck <naush@raspberrypi.com>
Reviewed-by: David Plowman <david.plowman@raspberrypi.com>
Reviewed-by: Jacopo Mondi <jacopo@jmondi.org>
---
 src/ipa/raspberrypi/cam_helper.cpp            |  5 +----
 .../raspberrypi/controller/device_status.cpp  | 20 +++++++++++++++++++
 .../raspberrypi/controller/device_status.h    |  4 ++++
 src/ipa/raspberrypi/meson.build               |  1 +
 src/ipa/raspberrypi/raspberrypi.cpp           |  5 +----
 5 files changed, 27 insertions(+), 8 deletions(-)
 create mode 100644 src/ipa/raspberrypi/controller/device_status.cpp

Comments

Kieran Bingham July 12, 2021, 12:51 p.m. UTC | #1 
Hi Naush,

On 12/07/2021 11:02, Naushir Patuck wrote:
> Add an operator<< overload to log all fields in DeviceStatus, and remove the
> manual logging statements in the IPA and CamHelper.
> 
> Signed-off-by: Naushir Patuck <naush@raspberrypi.com>
> Reviewed-by: David Plowman <david.plowman@raspberrypi.com>
> Reviewed-by: Jacopo Mondi <jacopo@jmondi.org>

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


> ---
>  src/ipa/raspberrypi/cam_helper.cpp            |  5 +----
>  .../raspberrypi/controller/device_status.cpp  | 20 +++++++++++++++++++
>  .../raspberrypi/controller/device_status.h    |  4 ++++
>  src/ipa/raspberrypi/meson.build               |  1 +
>  src/ipa/raspberrypi/raspberrypi.cpp           |  5 +----
>  5 files changed, 27 insertions(+), 8 deletions(-)
>  create mode 100644 src/ipa/raspberrypi/controller/device_status.cpp
> 
> diff --git a/src/ipa/raspberrypi/cam_helper.cpp b/src/ipa/raspberrypi/cam_helper.cpp
> index e6d2258c66d7..1ec3f03e1aa3 100644
> --- a/src/ipa/raspberrypi/cam_helper.cpp
> +++ b/src/ipa/raspberrypi/cam_helper.cpp
> @@ -188,10 +188,7 @@ void CamHelper::parseEmbeddedData(Span<const uint8_t> buffer,
>  	deviceStatus.shutter_speed = parsedDeviceStatus.shutter_speed;
>  	deviceStatus.analogue_gain = parsedDeviceStatus.analogue_gain;
>  
> -	LOG(IPARPI, Debug) << "Metadata updated - Exposure : "
> -			   << deviceStatus.shutter_speed
> -			   << " Gain : "
> -			   << deviceStatus.analogue_gain;
> +	LOG(IPARPI, Debug) << "Metadata updated - " << deviceStatus;
>  
>  	metadata.Set("device.status", deviceStatus);
>  }
> diff --git a/src/ipa/raspberrypi/controller/device_status.cpp b/src/ipa/raspberrypi/controller/device_status.cpp
> new file mode 100644
> index 000000000000..7b8218ca67d5
> --- /dev/null
> +++ b/src/ipa/raspberrypi/controller/device_status.cpp
> @@ -0,0 +1,20 @@
> +/* SPDX-License-Identifier: BSD-2-Clause */
> +/*
> + * Copyright (C) 2021, Raspberry Pi (Trading) Limited
> + *
> + * device_status.cpp - device (image sensor) status
> + */
> +#include "device_status.h"
> +
> +using namespace libcamera; /* for the Duration operator<< overload */
> +
> +std::ostream &operator<<(std::ostream &out, const DeviceStatus &d)
> +{
> +	out << "Exposure: " << d.shutter_speed
> +	    << " Gain: " << d.analogue_gain
> +	    << " Aperture: " << d.aperture
> +	    << " Lens: " << d.lens_position
> +	    << " Flash: " << d.flash_intensity;
> +
> +	return out;
> +}
> diff --git a/src/ipa/raspberrypi/controller/device_status.h b/src/ipa/raspberrypi/controller/device_status.h
> index 73df7ce228dd..ec4bbe738b35 100644
> --- a/src/ipa/raspberrypi/controller/device_status.h
> +++ b/src/ipa/raspberrypi/controller/device_status.h
> @@ -6,6 +6,8 @@
>   */
>  #pragma once
>  
> +#include <iostream>
> +
>  #include <libcamera/base/utils.h>
>  
>  /*
> @@ -20,6 +22,8 @@ struct DeviceStatus {
>  	{
>  	}
>  
> +	friend std::ostream &operator<<(std::ostream &out, const DeviceStatus &d);
> +
>  	/* time shutter is open */
>  	libcamera::utils::Duration shutter_speed;
>  	double analogue_gain;
> diff --git a/src/ipa/raspberrypi/meson.build b/src/ipa/raspberrypi/meson.build
> index c98550180d7b..1af31e4aa021 100644
> --- a/src/ipa/raspberrypi/meson.build
> +++ b/src/ipa/raspberrypi/meson.build
> @@ -40,6 +40,7 @@ rpi_ipa_sources = files([
>      'controller/rpi/contrast.cpp',
>      'controller/rpi/sdn.cpp',
>      'controller/pwl.cpp',
> +    'controller/device_status.cpp',
>  ])
>  
>  mod = shared_module(ipa_name,
> diff --git a/src/ipa/raspberrypi/raspberrypi.cpp b/src/ipa/raspberrypi/raspberrypi.cpp
> index 4d09a84f6532..f51c970befb5 100644
> --- a/src/ipa/raspberrypi/raspberrypi.cpp
> +++ b/src/ipa/raspberrypi/raspberrypi.cpp
> @@ -1019,10 +1019,7 @@ void IPARPi::fillDeviceStatus(const ControlList &sensorControls)
>  	deviceStatus.shutter_speed = helper_->Exposure(exposureLines);
>  	deviceStatus.analogue_gain = helper_->Gain(gainCode);
>  
> -	LOG(IPARPI, Debug) << "Metadata - Exposure : "
> -			   << deviceStatus.shutter_speed
> -			   << " Gain : "
> -			   << deviceStatus.analogue_gain;
> +	LOG(IPARPI, Debug) << "Metadata - " << deviceStatus;
>  
>  	rpiMetadata_.Set("device.status", deviceStatus);
>  }
>
Laurent Pinchart July 12, 2021, 12:54 p.m. UTC | #2 
Hi Naush,

Thank you for the patch.

On Mon, Jul 12, 2021 at 11:02:04AM +0100, Naushir Patuck wrote:
> Add an operator<< overload to log all fields in DeviceStatus, and remove the
> manual logging statements in the IPA and CamHelper.
> 
> Signed-off-by: Naushir Patuck <naush@raspberrypi.com>
> Reviewed-by: David Plowman <david.plowman@raspberrypi.com>
> Reviewed-by: Jacopo Mondi <jacopo@jmondi.org>
> ---
>  src/ipa/raspberrypi/cam_helper.cpp            |  5 +----
>  .../raspberrypi/controller/device_status.cpp  | 20 +++++++++++++++++++
>  .../raspberrypi/controller/device_status.h    |  4 ++++
>  src/ipa/raspberrypi/meson.build               |  1 +
>  src/ipa/raspberrypi/raspberrypi.cpp           |  5 +----
>  5 files changed, 27 insertions(+), 8 deletions(-)
>  create mode 100644 src/ipa/raspberrypi/controller/device_status.cpp
> 
> diff --git a/src/ipa/raspberrypi/cam_helper.cpp b/src/ipa/raspberrypi/cam_helper.cpp
> index e6d2258c66d7..1ec3f03e1aa3 100644
> --- a/src/ipa/raspberrypi/cam_helper.cpp
> +++ b/src/ipa/raspberrypi/cam_helper.cpp
> @@ -188,10 +188,7 @@ void CamHelper::parseEmbeddedData(Span<const uint8_t> buffer,
>  	deviceStatus.shutter_speed = parsedDeviceStatus.shutter_speed;
>  	deviceStatus.analogue_gain = parsedDeviceStatus.analogue_gain;
>  
> -	LOG(IPARPI, Debug) << "Metadata updated - Exposure : "
> -			   << deviceStatus.shutter_speed
> -			   << " Gain : "
> -			   << deviceStatus.analogue_gain;
> +	LOG(IPARPI, Debug) << "Metadata updated - " << deviceStatus;
>  
>  	metadata.Set("device.status", deviceStatus);
>  }
> diff --git a/src/ipa/raspberrypi/controller/device_status.cpp b/src/ipa/raspberrypi/controller/device_status.cpp
> new file mode 100644
> index 000000000000..7b8218ca67d5
> --- /dev/null
> +++ b/src/ipa/raspberrypi/controller/device_status.cpp
> @@ -0,0 +1,20 @@
> +/* SPDX-License-Identifier: BSD-2-Clause */
> +/*
> + * Copyright (C) 2021, Raspberry Pi (Trading) Limited
> + *
> + * device_status.cpp - device (image sensor) status
> + */
> +#include "device_status.h"
> +
> +using namespace libcamera; /* for the Duration operator<< overload */
> +
> +std::ostream &operator<<(std::ostream &out, const DeviceStatus &d)
> +{
> +	out << "Exposure: " << d.shutter_speed
> +	    << " Gain: " << d.analogue_gain
> +	    << " Aperture: " << d.aperture
> +	    << " Lens: " << d.lens_position
> +	    << " Flash: " << d.flash_intensity;
> +
> +	return out;
> +}
> diff --git a/src/ipa/raspberrypi/controller/device_status.h b/src/ipa/raspberrypi/controller/device_status.h
> index 73df7ce228dd..ec4bbe738b35 100644
> --- a/src/ipa/raspberrypi/controller/device_status.h
> +++ b/src/ipa/raspberrypi/controller/device_status.h
> @@ -6,6 +6,8 @@
>   */
>  #pragma once
>  
> +#include <iostream>
> +
>  #include <libcamera/base/utils.h>
>  
>  /*
> @@ -20,6 +22,8 @@ struct DeviceStatus {
>  	{
>  	}
>  
> +	friend std::ostream &operator<<(std::ostream &out, const DeviceStatus &d);
> +
>  	/* time shutter is open */
>  	libcamera::utils::Duration shutter_speed;
>  	double analogue_gain;
> diff --git a/src/ipa/raspberrypi/meson.build b/src/ipa/raspberrypi/meson.build
> index c98550180d7b..1af31e4aa021 100644
> --- a/src/ipa/raspberrypi/meson.build
> +++ b/src/ipa/raspberrypi/meson.build
> @@ -40,6 +40,7 @@ rpi_ipa_sources = files([
>      'controller/rpi/contrast.cpp',
>      'controller/rpi/sdn.cpp',
>      'controller/pwl.cpp',
> +    'controller/device_status.cpp',

When you get a chance, sorting this alphabetically would be nice.

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

>  ])
>  
>  mod = shared_module(ipa_name,
> diff --git a/src/ipa/raspberrypi/raspberrypi.cpp b/src/ipa/raspberrypi/raspberrypi.cpp
> index 4d09a84f6532..f51c970befb5 100644
> --- a/src/ipa/raspberrypi/raspberrypi.cpp
> +++ b/src/ipa/raspberrypi/raspberrypi.cpp
> @@ -1019,10 +1019,7 @@ void IPARPi::fillDeviceStatus(const ControlList &sensorControls)
>  	deviceStatus.shutter_speed = helper_->Exposure(exposureLines);
>  	deviceStatus.analogue_gain = helper_->Gain(gainCode);
>  
> -	LOG(IPARPI, Debug) << "Metadata - Exposure : "
> -			   << deviceStatus.shutter_speed
> -			   << " Gain : "
> -			   << deviceStatus.analogue_gain;
> +	LOG(IPARPI, Debug) << "Metadata - " << deviceStatus;
>  
>  	rpiMetadata_.Set("device.status", deviceStatus);
>  }

Patch
diff mbox series 

diff --git a/src/ipa/raspberrypi/cam_helper.cpp b/src/ipa/raspberrypi/cam_helper.cpp
index e6d2258c66d7..1ec3f03e1aa3 100644
--- a/src/ipa/raspberrypi/cam_helper.cpp
+++ b/src/ipa/raspberrypi/cam_helper.cpp
@@ -188,10 +188,7 @@  void CamHelper::parseEmbeddedData(Span<const uint8_t> buffer,
 	deviceStatus.shutter_speed = parsedDeviceStatus.shutter_speed;
 	deviceStatus.analogue_gain = parsedDeviceStatus.analogue_gain;
 
-	LOG(IPARPI, Debug) << "Metadata updated - Exposure : "
-			   << deviceStatus.shutter_speed
-			   << " Gain : "
-			   << deviceStatus.analogue_gain;
+	LOG(IPARPI, Debug) << "Metadata updated - " << deviceStatus;
 
 	metadata.Set("device.status", deviceStatus);
 }
diff --git a/src/ipa/raspberrypi/controller/device_status.cpp b/src/ipa/raspberrypi/controller/device_status.cpp
new file mode 100644
index 000000000000..7b8218ca67d5
--- /dev/null
+++ b/src/ipa/raspberrypi/controller/device_status.cpp
@@ -0,0 +1,20 @@ 
+/* SPDX-License-Identifier: BSD-2-Clause */
+/*
+ * Copyright (C) 2021, Raspberry Pi (Trading) Limited
+ *
+ * device_status.cpp - device (image sensor) status
+ */
+#include "device_status.h"
+
+using namespace libcamera; /* for the Duration operator<< overload */
+
+std::ostream &operator<<(std::ostream &out, const DeviceStatus &d)
+{
+	out << "Exposure: " << d.shutter_speed
+	    << " Gain: " << d.analogue_gain
+	    << " Aperture: " << d.aperture
+	    << " Lens: " << d.lens_position
+	    << " Flash: " << d.flash_intensity;
+
+	return out;
+}
diff --git a/src/ipa/raspberrypi/controller/device_status.h b/src/ipa/raspberrypi/controller/device_status.h
index 73df7ce228dd..ec4bbe738b35 100644
--- a/src/ipa/raspberrypi/controller/device_status.h
+++ b/src/ipa/raspberrypi/controller/device_status.h
@@ -6,6 +6,8 @@ 
  */
 #pragma once
 
+#include <iostream>
+
 #include <libcamera/base/utils.h>
 
 /*
@@ -20,6 +22,8 @@  struct DeviceStatus {
 	{
 	}
 
+	friend std::ostream &operator<<(std::ostream &out, const DeviceStatus &d);
+
 	/* time shutter is open */
 	libcamera::utils::Duration shutter_speed;
 	double analogue_gain;
diff --git a/src/ipa/raspberrypi/meson.build b/src/ipa/raspberrypi/meson.build
index c98550180d7b..1af31e4aa021 100644
--- a/src/ipa/raspberrypi/meson.build
+++ b/src/ipa/raspberrypi/meson.build
@@ -40,6 +40,7 @@  rpi_ipa_sources = files([
     'controller/rpi/contrast.cpp',
     'controller/rpi/sdn.cpp',
     'controller/pwl.cpp',
+    'controller/device_status.cpp',
 ])
 
 mod = shared_module(ipa_name,
diff --git a/src/ipa/raspberrypi/raspberrypi.cpp b/src/ipa/raspberrypi/raspberrypi.cpp
index 4d09a84f6532..f51c970befb5 100644
--- a/src/ipa/raspberrypi/raspberrypi.cpp
+++ b/src/ipa/raspberrypi/raspberrypi.cpp
@@ -1019,10 +1019,7 @@  void IPARPi::fillDeviceStatus(const ControlList &sensorControls)
 	deviceStatus.shutter_speed = helper_->Exposure(exposureLines);
 	deviceStatus.analogue_gain = helper_->Gain(gainCode);
 
-	LOG(IPARPI, Debug) << "Metadata - Exposure : "
-			   << deviceStatus.shutter_speed
-			   << " Gain : "
-			   << deviceStatus.analogue_gain;
+	LOG(IPARPI, Debug) << "Metadata - " << deviceStatus;
 
 	rpiMetadata_.Set("device.status", deviceStatus);
 }