[libcamera-devel,4/4] ipa: raspberrypi: Remove unneeded Span casts
diff mbox series

Message ID 20220810002906.5406-5-laurent.pinchart@ideasonboard.com
State Superseded
Headers show
Series
  • libcamera: Improve handling of fixed-size array controls
Related show

Commit Message

Laurent Pinchart Aug. 10, 2022, 12:29 a.m. UTC 
Commit 09c1b081baa2 ("libcamera: controls: Generate and use fixed-sized
Span types") added explicit Span casts for fixed extent spans that were
required due to the ControlList::set() function that takes an
std::initializer_list not being able to infer a control size from
template arguments. This has now been fixed, so the casts are not needed
anymore. Drop them.

Signed-off-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
---
 src/ipa/raspberrypi/raspberrypi.cpp | 19 +++++++++----------
 1 file changed, 9 insertions(+), 10 deletions(-)

Comments

Jacopo Mondi Aug. 10, 2022, 8:51 a.m. UTC | #1 
Hi Laurent,

On Wed, Aug 10, 2022 at 03:29:06AM +0300, Laurent Pinchart via libcamera-devel wrote:
> Commit 09c1b081baa2 ("libcamera: controls: Generate and use fixed-sized
> Span types") added explicit Span casts for fixed extent spans that were
> required due to the ControlList::set() function that takes an
> std::initializer_list not being able to infer a control size from
> template arguments. This has now been fixed, so the casts are not needed
> anymore. Drop them.
>
> Signed-off-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>

That's certainly nicer!!

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

Thanks
  j

> ---
>  src/ipa/raspberrypi/raspberrypi.cpp | 19 +++++++++----------
>  1 file changed, 9 insertions(+), 10 deletions(-)
>
> diff --git a/src/ipa/raspberrypi/raspberrypi.cpp b/src/ipa/raspberrypi/raspberrypi.cpp
> index 69c73f8c780a..6befdd71433d 100644
> --- a/src/ipa/raspberrypi/raspberrypi.cpp
> +++ b/src/ipa/raspberrypi/raspberrypi.cpp
> @@ -567,19 +567,18 @@ void IPARPi::reportMetadata()
>
>  	AwbStatus *awbStatus = rpiMetadata_.getLocked<AwbStatus>("awb.status");
>  	if (awbStatus) {
> -		libcameraMetadata_.set(controls::ColourGains,
> -				       Span<const float, 2>({ static_cast<float>(awbStatus->gainR),
> -							      static_cast<float>(awbStatus->gainB) }));
> +		libcameraMetadata_.set(controls::ColourGains, { static_cast<float>(awbStatus->gainR),
> +								static_cast<float>(awbStatus->gainB) });
>  		libcameraMetadata_.set(controls::ColourTemperature, awbStatus->temperatureK);
>  	}
>
>  	BlackLevelStatus *blackLevelStatus = rpiMetadata_.getLocked<BlackLevelStatus>("black_level.status");
>  	if (blackLevelStatus)
>  		libcameraMetadata_.set(controls::SensorBlackLevels,
> -				       Span<const int32_t, 4>({ static_cast<int32_t>(blackLevelStatus->blackLevelR),
> -								static_cast<int32_t>(blackLevelStatus->blackLevelG),
> -								static_cast<int32_t>(blackLevelStatus->blackLevelG),
> -								static_cast<int32_t>(blackLevelStatus->blackLevelB) }));
> +				       { static_cast<int32_t>(blackLevelStatus->blackLevelR),
> +					 static_cast<int32_t>(blackLevelStatus->blackLevelG),
> +					 static_cast<int32_t>(blackLevelStatus->blackLevelG),
> +					 static_cast<int32_t>(blackLevelStatus->blackLevelB) });
>
>  	FocusStatus *focusStatus = rpiMetadata_.getLocked<FocusStatus>("focus.status");
>  	if (focusStatus && focusStatus->num == 12) {
> @@ -884,7 +883,7 @@ void IPARPi::queueRequest(const ControlList &controls)
>  			if (gains[0] != 0.0f && gains[1] != 0.0f)
>  				/* A gain of 0.0f will switch back to auto mode. */
>  				libcameraMetadata_.set(controls::ColourGains,
> -						       Span<const float, 2>({ gains[0], gains[1] }));
> +						       { gains[0], gains[1] });
>  			break;
>  		}
>
> @@ -1168,8 +1167,8 @@ void IPARPi::applyFrameDurations(Duration minFrameDuration, Duration maxFrameDur
>
>  	/* Return the validated limits via metadata. */
>  	libcameraMetadata_.set(controls::FrameDurationLimits,
> -			       Span<const int64_t, 2>({ static_cast<int64_t>(minFrameDuration_.get<std::micro>()),
> -							static_cast<int64_t>(maxFrameDuration_.get<std::micro>()) }));
> +			       { static_cast<int64_t>(minFrameDuration_.get<std::micro>()),
> +				 static_cast<int64_t>(maxFrameDuration_.get<std::micro>()) });
>
>  	/*
>  	 * Calculate the maximum exposure time possible for the AGC to use.
> --
> Regards,
>
> Laurent Pinchart
>
Christian Rauch Aug. 13, 2022, 10:59 p.m. UTC | #2 
Hi Laurent,

I also like this simplification of the API.

Reviewed-by: Christian Rauch <Rauch.Christian@gmx.de>


Am 10.08.22 um 02:29 schrieb Laurent Pinchart:
> Commit 09c1b081baa2 ("libcamera: controls: Generate and use fixed-sized
> Span types") added explicit Span casts for fixed extent spans that were
> required due to the ControlList::set() function that takes an
> std::initializer_list not being able to infer a control size from
> template arguments. This has now been fixed, so the casts are not needed
> anymore. Drop them.
>
> Signed-off-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
> ---
>  src/ipa/raspberrypi/raspberrypi.cpp | 19 +++++++++----------
>  1 file changed, 9 insertions(+), 10 deletions(-)
>
> diff --git a/src/ipa/raspberrypi/raspberrypi.cpp b/src/ipa/raspberrypi/raspberrypi.cpp
> index 69c73f8c780a..6befdd71433d 100644
> --- a/src/ipa/raspberrypi/raspberrypi.cpp
> +++ b/src/ipa/raspberrypi/raspberrypi.cpp
> @@ -567,19 +567,18 @@ void IPARPi::reportMetadata()
>
>  	AwbStatus *awbStatus = rpiMetadata_.getLocked<AwbStatus>("awb.status");
>  	if (awbStatus) {
> -		libcameraMetadata_.set(controls::ColourGains,
> -				       Span<const float, 2>({ static_cast<float>(awbStatus->gainR),
> -							      static_cast<float>(awbStatus->gainB) }));
> +		libcameraMetadata_.set(controls::ColourGains, { static_cast<float>(awbStatus->gainR),
> +								static_cast<float>(awbStatus->gainB) });
>  		libcameraMetadata_.set(controls::ColourTemperature, awbStatus->temperatureK);
>  	}
>
>  	BlackLevelStatus *blackLevelStatus = rpiMetadata_.getLocked<BlackLevelStatus>("black_level.status");
>  	if (blackLevelStatus)
>  		libcameraMetadata_.set(controls::SensorBlackLevels,
> -				       Span<const int32_t, 4>({ static_cast<int32_t>(blackLevelStatus->blackLevelR),
> -								static_cast<int32_t>(blackLevelStatus->blackLevelG),
> -								static_cast<int32_t>(blackLevelStatus->blackLevelG),
> -								static_cast<int32_t>(blackLevelStatus->blackLevelB) }));
> +				       { static_cast<int32_t>(blackLevelStatus->blackLevelR),
> +					 static_cast<int32_t>(blackLevelStatus->blackLevelG),
> +					 static_cast<int32_t>(blackLevelStatus->blackLevelG),
> +					 static_cast<int32_t>(blackLevelStatus->blackLevelB) });
>
>  	FocusStatus *focusStatus = rpiMetadata_.getLocked<FocusStatus>("focus.status");
>  	if (focusStatus && focusStatus->num == 12) {
> @@ -884,7 +883,7 @@ void IPARPi::queueRequest(const ControlList &controls)
>  			if (gains[0] != 0.0f && gains[1] != 0.0f)
>  				/* A gain of 0.0f will switch back to auto mode. */
>  				libcameraMetadata_.set(controls::ColourGains,
> -						       Span<const float, 2>({ gains[0], gains[1] }));
> +						       { gains[0], gains[1] });
>  			break;
>  		}
>
> @@ -1168,8 +1167,8 @@ void IPARPi::applyFrameDurations(Duration minFrameDuration, Duration maxFrameDur
>
>  	/* Return the validated limits via metadata. */
>  	libcameraMetadata_.set(controls::FrameDurationLimits,
> -			       Span<const int64_t, 2>({ static_cast<int64_t>(minFrameDuration_.get<std::micro>()),
> -							static_cast<int64_t>(maxFrameDuration_.get<std::micro>()) }));
> +			       { static_cast<int64_t>(minFrameDuration_.get<std::micro>()),
> +				 static_cast<int64_t>(maxFrameDuration_.get<std::micro>()) });
>
>  	/*
>  	 * Calculate the maximum exposure time possible for the AGC to use.

Patch
diff mbox series 

diff --git a/src/ipa/raspberrypi/raspberrypi.cpp b/src/ipa/raspberrypi/raspberrypi.cpp
index 69c73f8c780a..6befdd71433d 100644
--- a/src/ipa/raspberrypi/raspberrypi.cpp
+++ b/src/ipa/raspberrypi/raspberrypi.cpp
@@ -567,19 +567,18 @@  void IPARPi::reportMetadata()
 
 	AwbStatus *awbStatus = rpiMetadata_.getLocked<AwbStatus>("awb.status");
 	if (awbStatus) {
-		libcameraMetadata_.set(controls::ColourGains,
-				       Span<const float, 2>({ static_cast<float>(awbStatus->gainR),
-							      static_cast<float>(awbStatus->gainB) }));
+		libcameraMetadata_.set(controls::ColourGains, { static_cast<float>(awbStatus->gainR),
+								static_cast<float>(awbStatus->gainB) });
 		libcameraMetadata_.set(controls::ColourTemperature, awbStatus->temperatureK);
 	}
 
 	BlackLevelStatus *blackLevelStatus = rpiMetadata_.getLocked<BlackLevelStatus>("black_level.status");
 	if (blackLevelStatus)
 		libcameraMetadata_.set(controls::SensorBlackLevels,
-				       Span<const int32_t, 4>({ static_cast<int32_t>(blackLevelStatus->blackLevelR),
-								static_cast<int32_t>(blackLevelStatus->blackLevelG),
-								static_cast<int32_t>(blackLevelStatus->blackLevelG),
-								static_cast<int32_t>(blackLevelStatus->blackLevelB) }));
+				       { static_cast<int32_t>(blackLevelStatus->blackLevelR),
+					 static_cast<int32_t>(blackLevelStatus->blackLevelG),
+					 static_cast<int32_t>(blackLevelStatus->blackLevelG),
+					 static_cast<int32_t>(blackLevelStatus->blackLevelB) });
 
 	FocusStatus *focusStatus = rpiMetadata_.getLocked<FocusStatus>("focus.status");
 	if (focusStatus && focusStatus->num == 12) {
@@ -884,7 +883,7 @@  void IPARPi::queueRequest(const ControlList &controls)
 			if (gains[0] != 0.0f && gains[1] != 0.0f)
 				/* A gain of 0.0f will switch back to auto mode. */
 				libcameraMetadata_.set(controls::ColourGains,
-						       Span<const float, 2>({ gains[0], gains[1] }));
+						       { gains[0], gains[1] });
 			break;
 		}
 
@@ -1168,8 +1167,8 @@  void IPARPi::applyFrameDurations(Duration minFrameDuration, Duration maxFrameDur
 
 	/* Return the validated limits via metadata. */
 	libcameraMetadata_.set(controls::FrameDurationLimits,
-			       Span<const int64_t, 2>({ static_cast<int64_t>(minFrameDuration_.get<std::micro>()),
-							static_cast<int64_t>(maxFrameDuration_.get<std::micro>()) }));
+			       { static_cast<int64_t>(minFrameDuration_.get<std::micro>()),
+				 static_cast<int64_t>(maxFrameDuration_.get<std::micro>()) });
 
 	/*
 	 * Calculate the maximum exposure time possible for the AGC to use.