[libcamera-devel] [PATCH v5 2/8] ipa: raspberrypi: Add a constructor struct DeviceStatus

[Kieran Bingham]

Hi Naush,

On 09/07/2021 10:56, Naushir Patuck wrote:
> The constructor sets all fields to 0. This replaces the memset(0) and default
> value initialisation usage in the agc and lux controllers respectively.

I guess this makes everything a bit more explicit.

Is it to fix coverity warnings? or just a general improvement?

> Signed-off-by: Naushir Patuck <naush at raspberrypi.com>
> Reviewed-by: David Plowman <david.plowman at raspberrypi.com>
> ---
>  src/ipa/raspberrypi/controller/device_status.h | 6 ++++++
>  src/ipa/raspberrypi/controller/rpi/agc.cpp     | 1 -
>  src/ipa/raspberrypi/controller/rpi/lux.cpp     | 9 +--------
>  3 files changed, 7 insertions(+), 9 deletions(-)
> 
> diff --git a/src/ipa/raspberrypi/controller/device_status.h b/src/ipa/raspberrypi/controller/device_status.h
> index 733378dbfa27..73df7ce228dd 100644
> --- a/src/ipa/raspberrypi/controller/device_status.h
> +++ b/src/ipa/raspberrypi/controller/device_status.h
> @@ -14,6 +14,12 @@
>   */
>  
>  struct DeviceStatus {
> +	DeviceStatus()
> +		: shutter_speed(std::chrono::seconds(0)), analogue_gain(0.0),
> +		  lens_position(0.0), aperture(0.0), flash_intensity(0.0)
> +	{
> +	}
> +
>  	/* time shutter is open */
>  	libcamera::utils::Duration shutter_speed;
>  	double analogue_gain;
> diff --git a/src/ipa/raspberrypi/controller/rpi/agc.cpp b/src/ipa/raspberrypi/controller/rpi/agc.cpp
> index 6c3a4eb2a04d..f57783f821ec 100644
> --- a/src/ipa/raspberrypi/controller/rpi/agc.cpp
> +++ b/src/ipa/raspberrypi/controller/rpi/agc.cpp
> @@ -172,7 +172,6 @@ Agc::Agc(Controller *controller)
>  	// it's not been calculated yet (i.e. Process hasn't yet run).
>  	memset(&status_, 0, sizeof(status_));
>  	status_.ev = ev_;
> -	memset(&last_device_status_, 0, sizeof(last_device_status_));
>  }
>  
>  char const *Agc::Name() const
> diff --git a/src/ipa/raspberrypi/controller/rpi/lux.cpp b/src/ipa/raspberrypi/controller/rpi/lux.cpp
> index f58d69397e1c..6367b17dc7f4 100644
> --- a/src/ipa/raspberrypi/controller/rpi/lux.cpp
> +++ b/src/ipa/raspberrypi/controller/rpi/lux.cpp
> @@ -60,14 +60,7 @@ void Lux::Prepare(Metadata *image_metadata)
>  
>  void Lux::Process(StatisticsPtr &stats, Metadata *image_metadata)
>  {
> -	// set some initial values to shut the compiler up
> -	DeviceStatus device_status = {
> -		.shutter_speed = 1.0ms,
> -		.analogue_gain = 1.0,

These are now going to be set to 0, I assume that's not an issue if
previously they're just default values to satisfy the compiler...

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

> -		.lens_position = 0.0,
> -		.aperture = 0.0,
> -		.flash_intensity = 0.0
> -	};
> +	DeviceStatus device_status;
>  	if (image_metadata->Get("device.status", device_status) == 0) {
>  		double current_gain = device_status.analogue_gain;
>  		double current_aperture = device_status.aperture;
>