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

[Niklas Söderlund]

Hi Kieran,

On 2021-03-12 05:47:22 +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>

I like this patch and I fear it could expose issues, have it been tested 
with all supported pipelines? In either case I think it's the right 
thing.

Reviewed-by: Niklas Söderlund <niklas.soderlund at ragnatech.se>

> ---
>  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

-- 
Regards,
Niklas Söderlund