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

[Kieran Bingham]

Hi Niklas,

On 12/03/2021 23:23, Niklas Söderlund wrote:
> 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.
> 

This series has been tested on Raspberry Pi - but not Rockchip.
I don't have an easy way to test that currently. Would you be able to
run this series on your test platform please?



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