[PATCH 1/4] libcamera: controls: Populate direction field in control definitions

[Jacopo Mondi]

Hi Paul

On Tue, Nov 26, 2024 at 12:30:00AM +0900, Paul Elder wrote:
> In preparation for adding support for querying direction information
> from controls, populate the corresponding field in the control ID
> defintions.

I wish I could save you the usual bikeshedding on names, but...

Are 'out' and 'in' the best terms ? Android divides 'controls' (from
app to camera) from 'metadata' (from camera to app) and I quite like
it (and I think most people in the industry is also accustomed to
them, but I agree it makes hard to express 'both' easily, so yeah 'in'
'out' and 'inout' makes sense. How do we document that ?

Also, the absence of the field indicates 'inout'. This should be
documented, and I wonder if 'inout' is valid as a value if it's
implicitly the default.

>
> Signed-off-by: Paul Elder <paul.elder at ideasonboard.com>
> ---
>  src/libcamera/control_ids_core.yaml  | 12 ++++++++++++
>  src/libcamera/control_ids_draft.yaml |  7 +++++++
>  2 files changed, 19 insertions(+)
>
> diff --git a/src/libcamera/control_ids_core.yaml b/src/libcamera/control_ids_core.yaml
> index d34a2d068b60..c0e91d1852cd 100644
> --- a/src/libcamera/control_ids_core.yaml
> +++ b/src/libcamera/control_ids_core.yaml

At the beginning of the file we have

# Unless otherwise stated, all controls are bi-directional, i.e. they can be
# set through Request::controls() and returned out through Request::metadata().

Could you expand it by mentioning 'direction' ?

> @@ -17,6 +17,7 @@ controls:
>
>    - AeLocked:
>        type: bool
> +      direction: out
>        description: |
>          Report the lock status of a running AE algorithm.
>
> @@ -236,6 +237,7 @@ controls:
>
>    - AeFlickerDetected:
>        type: int32_t
> +      direction: out
>        description: |
>          Flicker period detected in microseconds.
>
> @@ -274,6 +276,7 @@ controls:
>
>    - Lux:
>        type: float
> +      direction: out
>        description: |
>          Report an estimate of the current illuminance level in lux.
>
> @@ -324,6 +327,7 @@ controls:
>
>    - AwbLocked:
>        type: bool
> +      direction: out
>        description: |
>          Report the lock status of a running AWB algorithm.
>

ColourTemperature is missing

        Report the estimate of the colour temperature for the frame, in kelvin.

        The ColourTemperature control can only be returned in metadata.


> @@ -361,6 +365,7 @@ controls:
>
>    - SensorBlackLevels:
>        type: int32_t
> +      direction: out
>        description: |
>          Reports the sensor black levels used for processing a frame.
>
> @@ -385,6 +390,7 @@ controls:
>
>    - FocusFoM:
>        type: int32_t
> +      direction: out
>        description: |
>          Reports a Figure of Merit (FoM) to indicate how in-focus the frame is.
>
> @@ -442,6 +448,7 @@ controls:
>
>    - FrameDuration:
>        type: int64_t
> +      direction: out
>        description: |
>          The instantaneous frame duration from start of frame exposure to start
>          of next exposure, expressed in microseconds.
> @@ -450,6 +457,7 @@ controls:
>
>    - FrameDurationLimits:
>        type: int64_t
> +      direction: in

I read

       When reported in
        metadata, the control expresses the minimum and maximum frame
        durations used after being clipped to the sensor provided frame
        duration limits.

>        description: |
>          The minimum and maximum (in that order) frame duration, expressed in
>          microseconds.
> @@ -487,6 +495,7 @@ controls:
>
>    - SensorTemperature:
>        type: float
> +      direction: out
>        description: |
>          Temperature measure from the camera sensor in Celsius.
>
> @@ -499,6 +508,7 @@ controls:
>
>    - SensorTimestamp:
>        type: int64_t
> +      direction: out
>        description: |
>          The time when the first row of the image sensor active array is exposed.
>
> @@ -667,6 +677,7 @@ controls:
>
>    - AfTrigger:
>        type: int32_t
> +      direction: in
>        description: |
>          Start an autofocus scan.
>
> @@ -692,6 +703,7 @@ controls:
>
>    - AfPause:
>        type: int32_t
> +      direction: in
>        description: |
>          Pause lens movements when in continuous autofocus mode.

Should AfState be out only ?
Also AfPauseState seems to be a metadata only.

HdrChannel I read

        This value is reported back to the application so that it can discover
        whether this capture corresponds to the short or long exposure image
        (or any other image used by the HDR procedure). An application can
        monitor the HDR channel to discover when the differently exposed images
        have arrived.

which suggests me it's a metadata only

and DebugMetadataEnable is possibily input only ?

>
> diff --git a/src/libcamera/control_ids_draft.yaml b/src/libcamera/control_ids_draft.yaml
> index 1b284257f601..59aa6141c25c 100644
> --- a/src/libcamera/control_ids_draft.yaml
> +++ b/src/libcamera/control_ids_draft.yaml
> @@ -79,6 +79,7 @@ controls:
>
>    - AeState:
>        type: int32_t
> +      direction: out
>        description: |
>         Control to report the current AE algorithm state. Currently identical to
>         ANDROID_CONTROL_AE_STATE.
> @@ -108,6 +109,7 @@ controls:
>
>    - AwbState:
>        type: int32_t
> +      direction: out
>        description: |
>         Control to report the current AWB algorithm state. Currently identical
>         to ANDROID_CONTROL_AWB_STATE.
> @@ -129,6 +131,7 @@ controls:
>
>    - SensorRollingShutterSkew:
>        type: int64_t
> +      direction: out
>        description: |
>         Control to report the time between the start of exposure of the first
>         row and the start of exposure of the last row. Currently identical to
> @@ -262,6 +265,7 @@ controls:
>
>    - FaceDetectFaceRectangles:
>        type: Rectangle
> +      direction: out
>        description: |
>          Boundary rectangles of the detected faces. The number of values is
>          the number of detected faces.
> @@ -273,6 +277,7 @@ controls:
>
>    - FaceDetectFaceScores:
>        type: uint8_t
> +      direction: out
>        description: |
>          Confidence score of each of the detected faces. The range of score is
>          [0, 100]. The number of values should be the number of faces reported
> @@ -285,6 +290,7 @@ controls:
>
>    - FaceDetectFaceLandmarks:
>        type: Point
> +      direction: out
>        description: |
>          Array of human face landmark coordinates in format [..., left_eye_i,
>          right_eye_i, mouth_i, left_eye_i+1, ...], with i = index of face. The
> @@ -298,6 +304,7 @@ controls:
>
>    - FaceDetectFaceIds:
>        type: int32_t
> +      direction: out
>        description: |
>          Each detected face is given a unique ID that is valid for as long as the
>          face is visible to the camera device. A face that leaves the field of

Have you gone through control_ids_rpi.yaml and control_ids_debug.yaml
as well ?

> --
> 2.39.2
>