[libcamera-devel,v3,3/4] apply explicit fixed-sized Span type casts
diff mbox series

Message ID 20220408014231.231083-4-Rauch.Christian@gmx.de
State Superseded
Headers show
Series
  • generate and use fixed-sized Span Control types
Related show

Commit Message

Christian Rauch April 8, 2022, 1:42 a.m. UTC 
The change of types of some Controls from variable- to fixed-sized requires
explicit casting of FrameDurationLimits, ColourGains and SensorBlackLevels.

Signed-off-by: Christian Rauch <Rauch.Christian@gmx.de>
---
 src/ipa/raspberrypi/raspberrypi.cpp            | 18 +++++++++---------
 .../pipeline/raspberrypi/raspberrypi.cpp       |  2 +-
 src/qcam/dng_writer.cpp                        |  6 +++---
 3 files changed, 13 insertions(+), 13 deletions(-)

--
2.25.1

Comments

Laurent Pinchart April 19, 2022, 8:46 p.m. UTC | #1 
Hi Christian,

Thank you for the patch.

On Fri, Apr 08, 2022 at 02:42:30AM +0100, Christian Rauch via libcamera-devel wrote:
> The change of types of some Controls from variable- to fixed-sized requires
> explicit casting of FrameDurationLimits, ColourGains and SensorBlackLevels.
> 
> Signed-off-by: Christian Rauch <Rauch.Christian@gmx.de>
> ---
>  src/ipa/raspberrypi/raspberrypi.cpp            | 18 +++++++++---------
>  .../pipeline/raspberrypi/raspberrypi.cpp       |  2 +-
>  src/qcam/dng_writer.cpp                        |  6 +++---
>  3 files changed, 13 insertions(+), 13 deletions(-)
> 
> diff --git a/src/ipa/raspberrypi/raspberrypi.cpp b/src/ipa/raspberrypi/raspberrypi.cpp
> index 89767a9d..5a5cdf66 100644
> --- a/src/ipa/raspberrypi/raspberrypi.cpp
> +++ b/src/ipa/raspberrypi/raspberrypi.cpp
> @@ -500,18 +500,18 @@ void IPARPi::reportMetadata()
> 
>  	AwbStatus *awbStatus = rpiMetadata_.GetLocked<AwbStatus>("awb.status");
>  	if (awbStatus) {
> -		libcameraMetadata_.set(controls::ColourGains, { static_cast<float>(awbStatus->gain_r),
> -								static_cast<float>(awbStatus->gain_b) });
> +		libcameraMetadata_.set(controls::ColourGains, Span<const float, 2>({ static_cast<float>(awbStatus->gain_r),
> +										     static_cast<float>(awbStatus->gain_b) }));

Those lines are too long. I'm bothered here, as I think it's very nice
to use fixed-extent spans to catch errors at compilation time, but the
cast is ugly :-( This is caused by the explicit constructor for
fixed-extent spans, which we can't change if we want to remain
compatible with std::span.

I was also wondering it we could drop the static_cast<float> to
compensate for the additional explicit constructor, but that generates
other errors:

../../src/ipa/raspberrypi/raspberrypi.cpp:505:72: error: non-constant-expression cannot be narrowed from type 'double' to 'libcamera::Span<const float, 2>::element_type' (aka 'const float') in initializer list [-Wc++11-narrowing]
                libcameraMetadata_.set(controls::ColourGains, Span<const float, 2>({ awbStatus->gain_r,
                                                                                     ^~~~~~~~~~~~~~~~~
../../src/ipa/raspberrypi/raspberrypi.cpp:505:72: note: insert an explicit cast to silence this issue
                libcameraMetadata_.set(controls::ColourGains, Span<const float, 2>({ awbStatus->gain_r,
                                                                                     ^~~~~~~~~~~~~~~~~
                                                                                     static_cast<element_type>( )
../../src/ipa/raspberrypi/raspberrypi.cpp:506:16: error: non-constant-expression cannot be narrowed from type 'double' to 'libcamera::Span<const float, 2>::element_type' (aka 'const float') in initializer list [-Wc++11-narrowing]
                                                                                     awbStatus->gain_b }));
                                                                                     ^~~~~~~~~~~~~~~~~
../../src/ipa/raspberrypi/raspberrypi.cpp:506:16: note: insert an explicit cast to silence this issue
                                                                                     awbStatus->gain_b }));
                                                                                     ^~~~~~~~~~~~~~~~~
                                                                                     static_cast<element_type>( )
2 errors generated.

I suppose stars don't always align.

With proper line wraps, such as

		libcameraMetadata_.set(controls::ColourGains,
				       Span<const float, 2>({ static_cast<float>(awbStatus->gain_r),
							      static_cast<float>(awbStatus->gain_b) }));

or maybe

		libcameraMetadata_.set(controls::ColourGains, Span<const float, 2>({
				       static_cast<float>(awbStatus->gain_r),
				       static_cast<float>(awbStatus->gain_b) }));

I'm OK with this patch. I'd like to hear from others though. Bonus
points if someone can find a nicer way to express all this. We could,
for instance, add new ControlList::set() overloads specialized for
spans.

>  		libcameraMetadata_.set(controls::ColourTemperature, awbStatus->temperature_K);
>  	}
> 
>  	BlackLevelStatus *blackLevelStatus = rpiMetadata_.GetLocked<BlackLevelStatus>("black_level.status");
>  	if (blackLevelStatus)
>  		libcameraMetadata_.set(controls::SensorBlackLevels,
> -				       { static_cast<int32_t>(blackLevelStatus->black_level_r),
> -					 static_cast<int32_t>(blackLevelStatus->black_level_g),
> -					 static_cast<int32_t>(blackLevelStatus->black_level_g),
> -					 static_cast<int32_t>(blackLevelStatus->black_level_b) });
> +				       Span<const int32_t, 4>({ static_cast<int32_t>(blackLevelStatus->black_level_r),
> +								static_cast<int32_t>(blackLevelStatus->black_level_g),
> +								static_cast<int32_t>(blackLevelStatus->black_level_g),
> +								static_cast<int32_t>(blackLevelStatus->black_level_b) }));
> 
>  	FocusStatus *focusStatus = rpiMetadata_.GetLocked<FocusStatus>("focus.status");
>  	if (focusStatus && focusStatus->num == 12) {
> @@ -816,7 +816,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,
> -						       { gains[0], gains[1] });
> +						       Span<const float, 2>({ gains[0], gains[1] }));
>  			break;
>  		}
> 
> @@ -1100,8 +1100,8 @@ void IPARPi::applyFrameDurations(Duration minFrameDuration, Duration maxFrameDur
> 
>  	/* Return the validated limits via metadata. */
>  	libcameraMetadata_.set(controls::FrameDurationLimits,
> -			       { static_cast<int64_t>(minFrameDuration_.get<std::micro>()),
> -				 static_cast<int64_t>(maxFrameDuration_.get<std::micro>()) });
> +			       Span<const int64_t, 2>({ 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.
> diff --git a/src/libcamera/pipeline/raspberrypi/raspberrypi.cpp b/src/libcamera/pipeline/raspberrypi/raspberrypi.cpp
> index d6148724..0fa294d4 100644
> --- a/src/libcamera/pipeline/raspberrypi/raspberrypi.cpp
> +++ b/src/libcamera/pipeline/raspberrypi/raspberrypi.cpp
> @@ -1696,7 +1696,7 @@ void RPiCameraData::statsMetadataComplete(uint32_t bufferId, const ControlList &
>  	 * V4L2_CID_NOTIFY_GAINS control (which means notifyGainsUnity_ is set).
>  	 */
>  	if (notifyGainsUnity_ && controls.contains(libcamera::controls::ColourGains)) {
> -		libcamera::Span<const float> colourGains = controls.get(libcamera::controls::ColourGains);
> +		libcamera::Span<const float, 2> colourGains = controls.get(libcamera::controls::ColourGains);
>  		/* The control wants linear gains in the order B, Gb, Gr, R. */
>  		ControlList ctrls(sensor_->controls());
>  		std::array<int32_t, 4> gains{
> diff --git a/src/qcam/dng_writer.cpp b/src/qcam/dng_writer.cpp
> index 34c8df5a..2fb527d8 100644
> --- a/src/qcam/dng_writer.cpp
> +++ b/src/qcam/dng_writer.cpp
> @@ -438,7 +438,7 @@ int DNGWriter::write(const char *filename, const Camera *camera,
>  	const double eps = 1e-2;
> 
>  	if (metadata.contains(controls::ColourGains)) {
> -		Span<const float> const &colourGains = metadata.get(controls::ColourGains);
> +		Span<const float, 2> const &colourGains = metadata.get(controls::ColourGains);
>  		if (colourGains[0] > eps && colourGains[1] > eps) {
>  			wbGain = Matrix3d::diag(colourGains[0], 1, colourGains[1]);
>  			neutral[0] = 1.0 / colourGains[0]; /* red */
> @@ -446,7 +446,7 @@ int DNGWriter::write(const char *filename, const Camera *camera,
>  		}
>  	}
>  	if (metadata.contains(controls::ColourCorrectionMatrix)) {
> -		Span<const float> const &coeffs = metadata.get(controls::ColourCorrectionMatrix);
> +		Span<const float, 9> const &coeffs = metadata.get(controls::ColourCorrectionMatrix);
>  		Matrix3d ccmSupplied(coeffs);
>  		if (ccmSupplied.determinant() > eps)
>  			ccm = ccmSupplied;
> @@ -515,7 +515,7 @@ int DNGWriter::write(const char *filename, const Camera *camera,
>  	uint32_t whiteLevel = (1 << info->bitsPerSample) - 1;
> 
>  	if (metadata.contains(controls::SensorBlackLevels)) {
> -		Span<const int32_t> levels = metadata.get(controls::SensorBlackLevels);
> +		Span<const int32_t, 4> levels = metadata.get(controls::SensorBlackLevels);
> 
>  		/*
>  		 * The black levels control is specified in R, Gr, Gb, B order.
Christian Rauch April 19, 2022, 10:47 p.m. UTC | #2 
Hi Laurent,

In short: We can skip a lot of casts, if those values would already be
in the "target type".

Am 19.04.22 um 21:46 schrieb Laurent Pinchart:
> Hi Christian,
>
> Thank you for the patch.
>
> On Fri, Apr 08, 2022 at 02:42:30AM +0100, Christian Rauch via libcamera-devel wrote:
>> The change of types of some Controls from variable- to fixed-sized requires
>> explicit casting of FrameDurationLimits, ColourGains and SensorBlackLevels.
>>
>> Signed-off-by: Christian Rauch <Rauch.Christian@gmx.de>
>> ---
>>  src/ipa/raspberrypi/raspberrypi.cpp            | 18 +++++++++---------
>>  .../pipeline/raspberrypi/raspberrypi.cpp       |  2 +-
>>  src/qcam/dng_writer.cpp                        |  6 +++---
>>  3 files changed, 13 insertions(+), 13 deletions(-)
>>
>> diff --git a/src/ipa/raspberrypi/raspberrypi.cpp b/src/ipa/raspberrypi/raspberrypi.cpp
>> index 89767a9d..5a5cdf66 100644
>> --- a/src/ipa/raspberrypi/raspberrypi.cpp
>> +++ b/src/ipa/raspberrypi/raspberrypi.cpp
>> @@ -500,18 +500,18 @@ void IPARPi::reportMetadata()
>>
>>  	AwbStatus *awbStatus = rpiMetadata_.GetLocked<AwbStatus>("awb.status");
>>  	if (awbStatus) {
>> -		libcameraMetadata_.set(controls::ColourGains, { static_cast<float>(awbStatus->gain_r),
>> -								static_cast<float>(awbStatus->gain_b) });
>> +		libcameraMetadata_.set(controls::ColourGains, Span<const float, 2>({ static_cast<float>(awbStatus->gain_r),
>> +										     static_cast<float>(awbStatus->gain_b) }));
>
> Those lines are too long. I'm bothered here, as I think it's very nice
> to use fixed-extent spans to catch errors at compilation time, but the
> cast is ugly :-( This is caused by the explicit constructor for
> fixed-extent spans, which we can't change if we want to remain
> compatible with std::span.
>
> I was also wondering it we could drop the static_cast<float> to
> compensate for the additional explicit constructor, but that generates
> other errors:
>
> ../../src/ipa/raspberrypi/raspberrypi.cpp:505:72: error: non-constant-expression cannot be narrowed from type 'double' to 'libcamera::Span<const float, 2>::element_type' (aka 'const float') in initializer list [-Wc++11-narrowing]
>                 libcameraMetadata_.set(controls::ColourGains, Span<const float, 2>({ awbStatus->gain_r,
>                                                                                      ^~~~~~~~~~~~~~~~~
> ../../src/ipa/raspberrypi/raspberrypi.cpp:505:72: note: insert an explicit cast to silence this issue
>                 libcameraMetadata_.set(controls::ColourGains, Span<const float, 2>({ awbStatus->gain_r,
>                                                                                      ^~~~~~~~~~~~~~~~~
>                                                                                      static_cast<element_type>( )
> ../../src/ipa/raspberrypi/raspberrypi.cpp:506:16: error: non-constant-expression cannot be narrowed from type 'double' to 'libcamera::Span<const float, 2>::element_type' (aka 'const float') in initializer list [-Wc++11-narrowing]
>                                                                                      awbStatus->gain_b }));
>                                                                                      ^~~~~~~~~~~~~~~~~
> ../../src/ipa/raspberrypi/raspberrypi.cpp:506:16: note: insert an explicit cast to silence this issue
>                                                                                      awbStatus->gain_b }));
>                                                                                      ^~~~~~~~~~~~~~~~~
>                                                                                      static_cast<element_type>( )
> 2 errors generated.
>
> I suppose stars don't always align.

The "AwbStatus" stores its values as "double". If you either change that
struct to "float" or the "ColourGains" Control to "double", then you can
skip the explicit casts. You still have to call the explicit "Span"
constructor.

There are a couple of other places where these "static_cast" are required.

>
> With proper line wraps, such as
>
> 		libcameraMetadata_.set(controls::ColourGains,
> 				       Span<const float, 2>({ static_cast<float>(awbStatus->gain_r),
> 							      static_cast<float>(awbStatus->gain_b) }));
>
> or maybe
>
> 		libcameraMetadata_.set(controls::ColourGains, Span<const float, 2>({
> 				       static_cast<float>(awbStatus->gain_r),
> 				       static_cast<float>(awbStatus->gain_b) }));
>
> I'm OK with this patch. I'd like to hear from others though. Bonus
> points if someone can find a nicer way to express all this. We could,
> for instance, add new ControlList::set() overloads specialized for
> spans.
>
>>  		libcameraMetadata_.set(controls::ColourTemperature, awbStatus->temperature_K);
>>  	}
>>
>>  	BlackLevelStatus *blackLevelStatus = rpiMetadata_.GetLocked<BlackLevelStatus>("black_level.status");
>>  	if (blackLevelStatus)
>>  		libcameraMetadata_.set(controls::SensorBlackLevels,
>> -				       { static_cast<int32_t>(blackLevelStatus->black_level_r),
>> -					 static_cast<int32_t>(blackLevelStatus->black_level_g),
>> -					 static_cast<int32_t>(blackLevelStatus->black_level_g),
>> -					 static_cast<int32_t>(blackLevelStatus->black_level_b) });
>> +				       Span<const int32_t, 4>({ static_cast<int32_t>(blackLevelStatus->black_level_r),
>> +								static_cast<int32_t>(blackLevelStatus->black_level_g),
>> +								static_cast<int32_t>(blackLevelStatus->black_level_g),
>> +								static_cast<int32_t>(blackLevelStatus->black_level_b) }));
>>
>>  	FocusStatus *focusStatus = rpiMetadata_.GetLocked<FocusStatus>("focus.status");
>>  	if (focusStatus && focusStatus->num == 12) {
>> @@ -816,7 +816,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,
>> -						       { gains[0], gains[1] });
>> +						       Span<const float, 2>({ gains[0], gains[1] }));
>>  			break;
>>  		}
>>
>> @@ -1100,8 +1100,8 @@ void IPARPi::applyFrameDurations(Duration minFrameDuration, Duration maxFrameDur
>>
>>  	/* Return the validated limits via metadata. */
>>  	libcameraMetadata_.set(controls::FrameDurationLimits,
>> -			       { static_cast<int64_t>(minFrameDuration_.get<std::micro>()),
>> -				 static_cast<int64_t>(maxFrameDuration_.get<std::micro>()) });
>> +			       Span<const int64_t, 2>({ 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.
>> diff --git a/src/libcamera/pipeline/raspberrypi/raspberrypi.cpp b/src/libcamera/pipeline/raspberrypi/raspberrypi.cpp
>> index d6148724..0fa294d4 100644
>> --- a/src/libcamera/pipeline/raspberrypi/raspberrypi.cpp
>> +++ b/src/libcamera/pipeline/raspberrypi/raspberrypi.cpp
>> @@ -1696,7 +1696,7 @@ void RPiCameraData::statsMetadataComplete(uint32_t bufferId, const ControlList &
>>  	 * V4L2_CID_NOTIFY_GAINS control (which means notifyGainsUnity_ is set).
>>  	 */
>>  	if (notifyGainsUnity_ && controls.contains(libcamera::controls::ColourGains)) {
>> -		libcamera::Span<const float> colourGains = controls.get(libcamera::controls::ColourGains);
>> +		libcamera::Span<const float, 2> colourGains = controls.get(libcamera::controls::ColourGains);
>>  		/* The control wants linear gains in the order B, Gb, Gr, R. */
>>  		ControlList ctrls(sensor_->controls());
>>  		std::array<int32_t, 4> gains{
>> diff --git a/src/qcam/dng_writer.cpp b/src/qcam/dng_writer.cpp
>> index 34c8df5a..2fb527d8 100644
>> --- a/src/qcam/dng_writer.cpp
>> +++ b/src/qcam/dng_writer.cpp
>> @@ -438,7 +438,7 @@ int DNGWriter::write(const char *filename, const Camera *camera,
>>  	const double eps = 1e-2;
>>
>>  	if (metadata.contains(controls::ColourGains)) {
>> -		Span<const float> const &colourGains = metadata.get(controls::ColourGains);
>> +		Span<const float, 2> const &colourGains = metadata.get(controls::ColourGains);
>>  		if (colourGains[0] > eps && colourGains[1] > eps) {
>>  			wbGain = Matrix3d::diag(colourGains[0], 1, colourGains[1]);
>>  			neutral[0] = 1.0 / colourGains[0]; /* red */
>> @@ -446,7 +446,7 @@ int DNGWriter::write(const char *filename, const Camera *camera,
>>  		}
>>  	}
>>  	if (metadata.contains(controls::ColourCorrectionMatrix)) {
>> -		Span<const float> const &coeffs = metadata.get(controls::ColourCorrectionMatrix);
>> +		Span<const float, 9> const &coeffs = metadata.get(controls::ColourCorrectionMatrix);
>>  		Matrix3d ccmSupplied(coeffs);
>>  		if (ccmSupplied.determinant() > eps)
>>  			ccm = ccmSupplied;
>> @@ -515,7 +515,7 @@ int DNGWriter::write(const char *filename, const Camera *camera,
>>  	uint32_t whiteLevel = (1 << info->bitsPerSample) - 1;
>>
>>  	if (metadata.contains(controls::SensorBlackLevels)) {
>> -		Span<const int32_t> levels = metadata.get(controls::SensorBlackLevels);
>> +		Span<const int32_t, 4> levels = metadata.get(controls::SensorBlackLevels);
>>
>>  		/*
>>  		 * The black levels control is specified in R, Gr, Gb, B order.
>

Patch
diff mbox series 

diff --git a/src/ipa/raspberrypi/raspberrypi.cpp b/src/ipa/raspberrypi/raspberrypi.cpp
index 89767a9d..5a5cdf66 100644
--- a/src/ipa/raspberrypi/raspberrypi.cpp
+++ b/src/ipa/raspberrypi/raspberrypi.cpp
@@ -500,18 +500,18 @@  void IPARPi::reportMetadata()

 	AwbStatus *awbStatus = rpiMetadata_.GetLocked<AwbStatus>("awb.status");
 	if (awbStatus) {
-		libcameraMetadata_.set(controls::ColourGains, { static_cast<float>(awbStatus->gain_r),
-								static_cast<float>(awbStatus->gain_b) });
+		libcameraMetadata_.set(controls::ColourGains, Span<const float, 2>({ static_cast<float>(awbStatus->gain_r),
+										     static_cast<float>(awbStatus->gain_b) }));
 		libcameraMetadata_.set(controls::ColourTemperature, awbStatus->temperature_K);
 	}

 	BlackLevelStatus *blackLevelStatus = rpiMetadata_.GetLocked<BlackLevelStatus>("black_level.status");
 	if (blackLevelStatus)
 		libcameraMetadata_.set(controls::SensorBlackLevels,
-				       { static_cast<int32_t>(blackLevelStatus->black_level_r),
-					 static_cast<int32_t>(blackLevelStatus->black_level_g),
-					 static_cast<int32_t>(blackLevelStatus->black_level_g),
-					 static_cast<int32_t>(blackLevelStatus->black_level_b) });
+				       Span<const int32_t, 4>({ static_cast<int32_t>(blackLevelStatus->black_level_r),
+								static_cast<int32_t>(blackLevelStatus->black_level_g),
+								static_cast<int32_t>(blackLevelStatus->black_level_g),
+								static_cast<int32_t>(blackLevelStatus->black_level_b) }));

 	FocusStatus *focusStatus = rpiMetadata_.GetLocked<FocusStatus>("focus.status");
 	if (focusStatus && focusStatus->num == 12) {
@@ -816,7 +816,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,
-						       { gains[0], gains[1] });
+						       Span<const float, 2>({ gains[0], gains[1] }));
 			break;
 		}

@@ -1100,8 +1100,8 @@  void IPARPi::applyFrameDurations(Duration minFrameDuration, Duration maxFrameDur

 	/* Return the validated limits via metadata. */
 	libcameraMetadata_.set(controls::FrameDurationLimits,
-			       { static_cast<int64_t>(minFrameDuration_.get<std::micro>()),
-				 static_cast<int64_t>(maxFrameDuration_.get<std::micro>()) });
+			       Span<const int64_t, 2>({ 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.
diff --git a/src/libcamera/pipeline/raspberrypi/raspberrypi.cpp b/src/libcamera/pipeline/raspberrypi/raspberrypi.cpp
index d6148724..0fa294d4 100644
--- a/src/libcamera/pipeline/raspberrypi/raspberrypi.cpp
+++ b/src/libcamera/pipeline/raspberrypi/raspberrypi.cpp
@@ -1696,7 +1696,7 @@  void RPiCameraData::statsMetadataComplete(uint32_t bufferId, const ControlList &
 	 * V4L2_CID_NOTIFY_GAINS control (which means notifyGainsUnity_ is set).
 	 */
 	if (notifyGainsUnity_ && controls.contains(libcamera::controls::ColourGains)) {
-		libcamera::Span<const float> colourGains = controls.get(libcamera::controls::ColourGains);
+		libcamera::Span<const float, 2> colourGains = controls.get(libcamera::controls::ColourGains);
 		/* The control wants linear gains in the order B, Gb, Gr, R. */
 		ControlList ctrls(sensor_->controls());
 		std::array<int32_t, 4> gains{
diff --git a/src/qcam/dng_writer.cpp b/src/qcam/dng_writer.cpp
index 34c8df5a..2fb527d8 100644
--- a/src/qcam/dng_writer.cpp
+++ b/src/qcam/dng_writer.cpp
@@ -438,7 +438,7 @@  int DNGWriter::write(const char *filename, const Camera *camera,
 	const double eps = 1e-2;

 	if (metadata.contains(controls::ColourGains)) {
-		Span<const float> const &colourGains = metadata.get(controls::ColourGains);
+		Span<const float, 2> const &colourGains = metadata.get(controls::ColourGains);
 		if (colourGains[0] > eps && colourGains[1] > eps) {
 			wbGain = Matrix3d::diag(colourGains[0], 1, colourGains[1]);
 			neutral[0] = 1.0 / colourGains[0]; /* red */
@@ -446,7 +446,7 @@  int DNGWriter::write(const char *filename, const Camera *camera,
 		}
 	}
 	if (metadata.contains(controls::ColourCorrectionMatrix)) {
-		Span<const float> const &coeffs = metadata.get(controls::ColourCorrectionMatrix);
+		Span<const float, 9> const &coeffs = metadata.get(controls::ColourCorrectionMatrix);
 		Matrix3d ccmSupplied(coeffs);
 		if (ccmSupplied.determinant() > eps)
 			ccm = ccmSupplied;
@@ -515,7 +515,7 @@  int DNGWriter::write(const char *filename, const Camera *camera,
 	uint32_t whiteLevel = (1 << info->bitsPerSample) - 1;

 	if (metadata.contains(controls::SensorBlackLevels)) {
-		Span<const int32_t> levels = metadata.get(controls::SensorBlackLevels);
+		Span<const int32_t, 4> levels = metadata.get(controls::SensorBlackLevels);

 		/*
 		 * The black levels control is specified in R, Gr, Gb, B order.