[libcamera-devel] [PATCH v2 3/8] libcamera: buffer: Initialise status

[Jacopo Mondi]

Hi Kieran,

On Fri, Mar 12, 2021 at 05:47:22AM +0000, Kieran Bingham wrote:
> Buffers queued to a pipeline handler may not be yet queued to a device
> when the request is cancelled.
>
> This can lead to the FrameMetadata having never been explicitly set by
> an underlying V4L2 device.
>
> The status field on this is used to check the state of the buffer to
> determine if it was correctly filled, or if it was cancelled.
>
> In the event that the buffer is not used, it must be marked as Error as
> the metadata associated with that frame will not be valid.
>
> Initialise the FrameMetadata to FrameError to prevent uninitialised access.
> Furthermore, swap the Enum values of the Status such that the first
> state represents the initial Error state.
>
> Signed-off-by: Kieran Bingham <kieran.bingham at ideasonboard.com>

Probably a test from users of RkISP1 and simple pipeline would solve
Niklas' concerns which I do share as some code paths might bet of the
fact that the status is set to FrameSuccess on construction.

> ---
>  include/libcamera/buffer.h | 4 ++--
>  1 file changed, 2 insertions(+), 2 deletions(-)
>
> diff --git a/include/libcamera/buffer.h b/include/libcamera/buffer.h
> index 302fe3d3e86b..3f5d0f1b6363 100644
> --- a/include/libcamera/buffer.h
> +++ b/include/libcamera/buffer.h
> @@ -19,8 +19,8 @@ class Request;
>
>  struct FrameMetadata {
>  	enum Status {
> -		FrameSuccess,
>  		FrameError,
> +		FrameSuccess,
>  		FrameCancelled,
>  	};
>
> @@ -28,7 +28,7 @@ struct FrameMetadata {
>  		unsigned int bytesused;
>  	};
>
> -	Status status;
> +	Status status = FrameError;
>  	unsigned int sequence;
>  	uint64_t timestamp;
>  	std::vector<Plane> planes;
> --
> 2.25.1
>
> _______________________________________________
> libcamera-devel mailing list
> libcamera-devel at lists.libcamera.org
> https://lists.libcamera.org/listinfo/libcamera-devel