[PATCH v1 1/6] libcamera: framebuffer: Add FrameMetadata::Status::FrameStartup

[Jacopo Mondi]

Hi Naush

On Mon, May 19, 2025 at 10:20:49AM +0100, Naushir Patuck wrote:
> Add a new status enum, FrameStartup, used to denote that even though
> the frame has been successfully captured, the IQ parameters set by the
> IPA will cause the frame to be unusable and applications are advised to
> not consume this frame. An example of this would be on a cold-start of
> the 3A algorithms, and there will be large oscillations to converge to
> a stable state quickly.
>
> Additional, update the definition of the FrameError state to include its
> usage when the sensor is known to produce a number of invalid/error
> frames after stream-on.
>
> Signed-off-by: Naushir Patuck <naush at raspberrypi.com>
> ---
>  include/libcamera/framebuffer.h |  1 +
>  src/libcamera/framebuffer.cpp   | 11 +++++++++--
>  2 files changed, 10 insertions(+), 2 deletions(-)
>
> diff --git a/include/libcamera/framebuffer.h b/include/libcamera/framebuffer.h
> index ff83924300ac..e83825b466aa 100644
> --- a/include/libcamera/framebuffer.h
> +++ b/include/libcamera/framebuffer.h
> @@ -26,6 +26,7 @@ struct FrameMetadata {
>  		FrameSuccess,
>  		FrameError,
>  		FrameCancelled,
> +		FrameStartup,
>  	};
>
>  	struct Plane {
> diff --git a/src/libcamera/framebuffer.cpp b/src/libcamera/framebuffer.cpp
> index 826848f75a56..36e56d593bc3 100644
> --- a/src/libcamera/framebuffer.cpp
> +++ b/src/libcamera/framebuffer.cpp
> @@ -44,11 +44,18 @@ LOG_DEFINE_CATEGORY(Buffer)
>   * of the FrameMetadata structure are valid.
>   * \var FrameMetadata::FrameError
>   * An error occurred during capture of the frame. The frame data may be partly
> - * or fully invalid. The sequence and timestamp fields of the FrameMetadata
> - * structure is valid, the other fields may be invalid.
> + * or fully invalid. This status may also indicate an invalid frame produced by
> + * the sensor during its startup or restart phase. The sequence and timestamp
> + * fields of the FrameMetadata structure is valid, the other fields may be
> + * invalid.

This suggests to me that either you expect pipelines to set

        FrameError | FrameStartup

to indicate a 'sensor startup' frame, which I don't think it's the
intention here.

I wonder if we actually add anything here.

Or maybe just change the definition to:

 * The frame data may be partly or fully invalid. The sequence and
 * timestamp fields of the FrameMetadata structure is valid, the other
 * fields may be invalid.

to remove the "An error occurred" part, as "sensor startup" frames are
not errors, but contain known-to-be-invalid data.

What matters for applications, is, after all, knowing if frames can be
consumed or not, so I think we can drop mentioning "errors" or more
precise failure conditions.



>   * \var FrameMetadata::FrameCancelled
>   * Capture stopped before the frame completed. The frame data is not valid. All
>   * fields of the FrameMetadata structure but the status field are invalid.
> + * \var FrameMetadata::FrameStartup The frame has been successfully captured.

Following the above I would say that

      "The frame data is valid, however the...

> + * However, the IPA is in a cold-start or reset phase and will result in image

Do we need to mention IPA ?

      ".. however the capture pipeline is in a cold-start or reset
       phase"


> + * quality parameters producing unusable images. Applications are recommended to
> + * not consume these frames. All fields of the FrameMetadata structure are

All other fields ? Or are you ok with how it is ?

Thanks
  j

> + * valid.
>   */
>
>  /**
> --
> 2.43.0
>