[v3] v4l2: v4l2_camera_proxy: Fix for getting default FrameDurationLimits

25629 diff mbox series

The default values for controls::FrameDurationLimits is now an array but
the v4l2 proxy is fetching it as a scalar value, causing a runtime
error. Fix this by templating the getter with the correct
Span<const int64_t, 2> type.

This fix also requires the RPi initial default value for FrameDurationLimits
to be specified as a Span<const int64_t, 2>.

As a drive-by, remove the hard-coded 33ms min and 120ms max frame
duration values in the initial defaults, and use the defaultMinFrameDuration
and defaultMaxFrameDuration const values. This change is inconsequential
to runtime operation as these always get overridden on the first camera
configure call.

Fixes: 4e9be7d11b9df ("ipa: ipu3, mali-c55, rkisp1, rpi: Fix reporting non-scalar controls")
Closes: https://github.com/raspberrypi/libcamera/issues/321
Signed-off-by: Naushir Patuck <naush@raspberrypi.com>
Reviewed-by: Barnabás Pőcze <barnabas.pocze@ideasonboard.com>
---
 src/ipa/rpi/common/ipa_base.cpp | 6 ++++--
 src/v4l2/v4l2_camera_proxy.cpp  | 4 ++--
 2 files changed, 6 insertions(+), 4 deletions(-)

Quoting Naushir Patuck (2026-01-06 18:37:30)
> The default values for controls::FrameDurationLimits is now an array but
> the v4l2 proxy is fetching it as a scalar value, causing a runtime
> error. Fix this by templating the getter with the correct
> Span<const int64_t, 2> type.
> 
> This fix also requires the RPi initial default value for FrameDurationLimits
> to be specified as a Span<const int64_t, 2>.
> 
> As a drive-by, remove the hard-coded 33ms min and 120ms max frame
> duration values in the initial defaults, and use the defaultMinFrameDuration
> and defaultMaxFrameDuration const values. This change is inconsequential
> to runtime operation as these always get overridden on the first camera
> configure call.
> 
> Fixes: 4e9be7d11b9df ("ipa: ipu3, mali-c55, rkisp1, rpi: Fix reporting non-scalar controls")
> Closes: https://github.com/raspberrypi/libcamera/issues/321
> Signed-off-by: Naushir Patuck <naush@raspberrypi.com>
> Reviewed-by: Barnabás Pőcze <barnabas.pocze@ideasonboard.com>
> ---
>  src/ipa/rpi/common/ipa_base.cpp | 6 ++++--
>  src/v4l2/v4l2_camera_proxy.cpp  | 4 ++--

I too would've split this in two but I've seen your rationale in v2 so ok.

Looks good to me.

Reviewed-by: Paul Elder <paul.elder@ideasonboard.com>

>  2 files changed, 6 insertions(+), 4 deletions(-)
> 
> diff --git a/src/ipa/rpi/common/ipa_base.cpp b/src/ipa/rpi/common/ipa_base.cpp
> index 14aba4500ae4..322694985e72 100644
> --- a/src/ipa/rpi/common/ipa_base.cpp
> +++ b/src/ipa/rpi/common/ipa_base.cpp
> @@ -84,8 +84,10 @@ const ControlInfoMap::Map ipaControls{
>         { &controls::Sharpness, ControlInfo(0.0f, 16.0f, 1.0f) },
>         { &controls::ScalerCrop, ControlInfo(Rectangle{}, Rectangle(65535, 65535, 65535, 65535), Rectangle{}) },
>         { &controls::FrameDurationLimits,
> -         ControlInfo(INT64_C(33333), INT64_C(120000),
> -                     static_cast<int64_t>(defaultMinFrameDuration.get<std::micro>())) },
> +         ControlInfo(static_cast<int64_t>(defaultMinFrameDuration.get<std::micro>()),
> +                     static_cast<int64_t>(defaultMaxFrameDuration.get<std::micro>()),
> +                     Span<const int64_t, 2>{ { static_cast<int64_t>(defaultMinFrameDuration.get<std::micro>()),
> +                                               static_cast<int64_t>(defaultMinFrameDuration.get<std::micro>()) } }) },
>         { &controls::draft::NoiseReductionMode, ControlInfo(controls::draft::NoiseReductionModeValues) },
>         { &controls::rpi::StatsOutputEnable, ControlInfo(false, true, false) },
>  };
> diff --git a/src/v4l2/v4l2_camera_proxy.cpp b/src/v4l2/v4l2_camera_proxy.cpp
> index 559ffc6170b1..03cd4810cc05 100644
> --- a/src/v4l2/v4l2_camera_proxy.cpp
> +++ b/src/v4l2/v4l2_camera_proxy.cpp
> @@ -200,9 +200,9 @@ void V4L2CameraProxy::setFmtFromConfig(const StreamConfiguration &streamConfig)
>         const auto &it = controls.find(&controls::FrameDurationLimits);
>  
>         if (it != controls.end()) {
> -               const int64_t duration = it->second.def().get<int64_t>();
> +               Span<const int64_t, 2> duration = it->second.def().get<Span<const int64_t, 2>>();
>  
> -               v4l2TimePerFrame_.numerator = duration;
> +               v4l2TimePerFrame_.numerator = duration[0];
>                 v4l2TimePerFrame_.denominator = 1000000;
>         } else {
>                 /*
> -- 
> 2.51.0
>

Hi Naush

Thanks for the patch!

On Tue, 6 Jan 2026 at 09:37, Naushir Patuck <naush@raspberrypi.com> wrote:
>
> The default values for controls::FrameDurationLimits is now an array but
> the v4l2 proxy is fetching it as a scalar value, causing a runtime
> error. Fix this by templating the getter with the correct
> Span<const int64_t, 2> type.
>
> This fix also requires the RPi initial default value for FrameDurationLimits
> to be specified as a Span<const int64_t, 2>.
>
> As a drive-by, remove the hard-coded 33ms min and 120ms max frame
> duration values in the initial defaults, and use the defaultMinFrameDuration
> and defaultMaxFrameDuration const values. This change is inconsequential
> to runtime operation as these always get overridden on the first camera
> configure call.
>
> Fixes: 4e9be7d11b9df ("ipa: ipu3, mali-c55, rkisp1, rpi: Fix reporting non-scalar controls")
> Closes: https://github.com/raspberrypi/libcamera/issues/321
> Signed-off-by: Naushir Patuck <naush@raspberrypi.com>
> Reviewed-by: Barnabás Pőcze <barnabas.pocze@ideasonboard.com>

Reviewed-by: David Plowman <david.plowman@raspberrypi.com>

Thanks
David

> ---
>  src/ipa/rpi/common/ipa_base.cpp | 6 ++++--
>  src/v4l2/v4l2_camera_proxy.cpp  | 4 ++--
>  2 files changed, 6 insertions(+), 4 deletions(-)
>
> diff --git a/src/ipa/rpi/common/ipa_base.cpp b/src/ipa/rpi/common/ipa_base.cpp
> index 14aba4500ae4..322694985e72 100644
> --- a/src/ipa/rpi/common/ipa_base.cpp
> +++ b/src/ipa/rpi/common/ipa_base.cpp
> @@ -84,8 +84,10 @@ const ControlInfoMap::Map ipaControls{
>         { &controls::Sharpness, ControlInfo(0.0f, 16.0f, 1.0f) },
>         { &controls::ScalerCrop, ControlInfo(Rectangle{}, Rectangle(65535, 65535, 65535, 65535), Rectangle{}) },
>         { &controls::FrameDurationLimits,
> -         ControlInfo(INT64_C(33333), INT64_C(120000),
> -                     static_cast<int64_t>(defaultMinFrameDuration.get<std::micro>())) },
> +         ControlInfo(static_cast<int64_t>(defaultMinFrameDuration.get<std::micro>()),
> +                     static_cast<int64_t>(defaultMaxFrameDuration.get<std::micro>()),
> +                     Span<const int64_t, 2>{ { static_cast<int64_t>(defaultMinFrameDuration.get<std::micro>()),
> +                                               static_cast<int64_t>(defaultMinFrameDuration.get<std::micro>()) } }) },
>         { &controls::draft::NoiseReductionMode, ControlInfo(controls::draft::NoiseReductionModeValues) },
>         { &controls::rpi::StatsOutputEnable, ControlInfo(false, true, false) },
>  };
> diff --git a/src/v4l2/v4l2_camera_proxy.cpp b/src/v4l2/v4l2_camera_proxy.cpp
> index 559ffc6170b1..03cd4810cc05 100644
> --- a/src/v4l2/v4l2_camera_proxy.cpp
> +++ b/src/v4l2/v4l2_camera_proxy.cpp
> @@ -200,9 +200,9 @@ void V4L2CameraProxy::setFmtFromConfig(const StreamConfiguration &streamConfig)
>         const auto &it = controls.find(&controls::FrameDurationLimits);
>
>         if (it != controls.end()) {
> -               const int64_t duration = it->second.def().get<int64_t>();
> +               Span<const int64_t, 2> duration = it->second.def().get<Span<const int64_t, 2>>();
>
> -               v4l2TimePerFrame_.numerator = duration;
> +               v4l2TimePerFrame_.numerator = duration[0];
>                 v4l2TimePerFrame_.denominator = 1000000;
>         } else {
>                 /*
> --
> 2.51.0
>