[libcamera-devel] [PATCH v4 1/3] controls: Split FrameDurations into FrameDuration and FrameDurationLimits

[Hirokazu Honda]

Hi Paul, thank you for the patch.

On Wed, May 26, 2021 at 12:47 PM Paul Elder <paul.elder at ideasonboard.com>
wrote:

> We need a separate control to report the nominal frame duration, but
> it's also useful to report the min/max frame duration values that will
> be used. Split the FrameDurations control into FrameDuration and
> FrameDurationLimits respectively to support both of these.
>
> Signed-off-by: Paul Elder <paul.elder at ideasonboard.com>
> Reviewed-by: Laurent Pinchart <laurent.pinchart at ideasonboard.com>
>
>
Reviewed-by: Hirokazu Honda <hiroh at chromium.org>


> ---
> Changes in v4:
> - set FrameDurationLimits (instead of FrameDuration) in the ipu3
>   pipeline handler
>
> Changes in v3:
> - mention that the control is meant to be returned in metadata
> - s/nominal/instantaneous
> ---
>  include/libcamera/ipa/raspberrypi.h  | 2 +-
>  src/android/camera_device.cpp        | 2 +-
>  src/ipa/raspberrypi/raspberrypi.cpp  | 4 ++--
>  src/libcamera/control_ids.yaml       | 9 ++++++++-
>  src/libcamera/pipeline/ipu3/ipu3.cpp | 7 ++++---
>  5 files changed, 16 insertions(+), 8 deletions(-)
>
> diff --git a/include/libcamera/ipa/raspberrypi.h
> b/include/libcamera/ipa/raspberrypi.h
> index d10c1733..a8790000 100644
> --- a/include/libcamera/ipa/raspberrypi.h
> +++ b/include/libcamera/ipa/raspberrypi.h
> @@ -44,7 +44,7 @@ static const ControlInfoMap Controls = {
>         { &controls::Sharpness, ControlInfo(0.0f, 16.0f, 1.0f) },
>         { &controls::ColourCorrectionMatrix, ControlInfo(-16.0f, 16.0f) },
>         { &controls::ScalerCrop, ControlInfo(Rectangle{}, Rectangle(65535,
> 65535, 65535, 65535), Rectangle{}) },
> -       { &controls::FrameDurations, ControlInfo(INT64_C(1000),
> INT64_C(1000000000)) },
> +       { &controls::FrameDurationLimits, ControlInfo(INT64_C(1000),
> INT64_C(1000000000)) },
>         { &controls::draft::NoiseReductionMode,
> ControlInfo(controls::draft::NoiseReductionModeValues) },
>  };
>
> diff --git a/src/android/camera_device.cpp b/src/android/camera_device.cpp
> index dc0c8f5f..9e267b5f 100644
> --- a/src/android/camera_device.cpp
> +++ b/src/android/camera_device.cpp
> @@ -864,7 +864,7 @@ const camera_metadata_t
> *CameraDevice::getStaticMetadata()
>
>         int64_t minFrameDurationNsec = -1;
>         int64_t maxFrameDurationNsec = -1;
> -       const auto frameDurationsInfo =
> controlsInfo.find(&controls::FrameDurations);
> +       const auto frameDurationsInfo =
> controlsInfo.find(&controls::FrameDurationLimits);
>         if (frameDurationsInfo != controlsInfo.end()) {
>                 minFrameDurationNsec =
> frameDurationsInfo->second.min().get<int64_t>() * 1000;
>                 maxFrameDurationNsec =
> frameDurationsInfo->second.max().get<int64_t>() * 1000;
> diff --git a/src/ipa/raspberrypi/raspberrypi.cpp
> b/src/ipa/raspberrypi/raspberrypi.cpp
> index e5bb8159..0c4752ec 100644
> --- a/src/ipa/raspberrypi/raspberrypi.cpp
> +++ b/src/ipa/raspberrypi/raspberrypi.cpp
> @@ -859,7 +859,7 @@ void IPARPi::queueRequest(const ControlList &controls)
>                         break;
>                 }
>
> -               case controls::FRAME_DURATIONS: {
> +               case controls::FRAME_DURATION_LIMITS: {
>                         auto frameDurations = ctrl.second.get<Span<const
> int64_t>>();
>                         applyFrameDurations(frameDurations[0],
> frameDurations[1]);
>                         break;
> @@ -1074,7 +1074,7 @@ void IPARPi::applyFrameDurations(double
> minFrameDuration, double maxFrameDuratio
>         maxFrameDuration_ = std::max(maxFrameDuration_, minFrameDuration_);
>
>         /* Return the validated limits via metadata. */
> -       libcameraMetadata_.set(controls::FrameDurations,
> +       libcameraMetadata_.set(controls::FrameDurationLimits,
>                                { static_cast<int64_t>(minFrameDuration_),
>                                  static_cast<int64_t>(maxFrameDuration_)
> });
>
> diff --git a/src/libcamera/control_ids.yaml
> b/src/libcamera/control_ids.yaml
> index 88d81ac4..f62ade48 100644
> --- a/src/libcamera/control_ids.yaml
> +++ b/src/libcamera/control_ids.yaml
> @@ -323,7 +323,14 @@ controls:
>          step to respect the received gain factor and shall report
>          their total value in the request metadata.
>
> -  - FrameDurations:
> +  - FrameDuration:
> +      type: int64_t
> +      description: |
> +        The instantaneous frame duration from start of frame exposure to
> start
> +        of next exposure, expressed in microseconds. This control is
> meant to
> +        be returned in metadata.
> +
> +  - FrameDurationLimits:
>        type: int64_t
>        description: |
>          The minimum and maximum (in that order) frame duration,
> diff --git a/src/libcamera/pipeline/ipu3/ipu3.cpp
> b/src/libcamera/pipeline/ipu3/ipu3.cpp
> index 25203256..2d15d488 100644
> --- a/src/libcamera/pipeline/ipu3/ipu3.cpp
> +++ b/src/libcamera/pipeline/ipu3/ipu3.cpp
> @@ -983,9 +983,10 @@ int PipelineHandlerIPU3::initControls(IPU3CameraData
> *data)
>                 frameDurations[i] = frameSize / (sensorInfo.pixelRate /
> 1000000U);
>         }
>
> -       controls[&controls::FrameDurations] =
> ControlInfo(frameDurations[0],
> -
>  frameDurations[1],
> -
>  frameDurations[2]);
> +       controls[&controls::FrameDurationLimits] =
> +               ControlInfo(frameDurations[0],
> +                           frameDurations[1],
> +                           frameDurations[2]);
>
>         /*
>          * Compute the scaler crop limits.
> --
> 2.27.0
>
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <https://lists.libcamera.org/pipermail/libcamera-devel/attachments/20210526/a155741b/attachment.htm>