[libcamera-devel] [PATCH 02/13] libcamera: ipu3: Report sensor timestamp

[Kieran Bingham]

On 19/04/2021 14:35, Kieran Bingham wrote:
> Hi Jacopo,
> 
> On 19/04/2021 14:14, Jacopo Mondi wrote:
>> Report the sensor's timestamp in the Request metadata by using the
>> CIO2 buffer timestamp as an initial approximation.
>>
>> The buffer's timestamp is recorded at DMA-transfer time, and it does not
>> theoretically matches the 'start of exposure' definition, but when used
>> to compare two consecutive frames it gives an acceptable estimation of
>> the sensor frame period duration.
>>
>> Reviewed-by: Hirokazu Honda <hiroh at chromium.org>
>> Reviewed-by: Laurent Pinchart <laurent.pinchart at ideasonboard.com>
>> Signed-off-by: Jacopo Mondi <jacopo at jmondi.org>
>> ---
>>  src/libcamera/pipeline/ipu3/ipu3.cpp | 9 +++++++++
>>  1 file changed, 9 insertions(+)
>>
>> diff --git a/src/libcamera/pipeline/ipu3/ipu3.cpp b/src/libcamera/pipeline/ipu3/ipu3.cpp
>> index 51446fcf5bc1..28e849a43a3e 100644
>> --- a/src/libcamera/pipeline/ipu3/ipu3.cpp
>> +++ b/src/libcamera/pipeline/ipu3/ipu3.cpp
>> @@ -1255,6 +1255,15 @@ void IPU3CameraData::cio2BufferReady(FrameBuffer *buffer)
>>  
>>  	Request *request = info->request;
>>  
>> +	/*
>> +	 * Record the sensor's timestamp in the request metadata.
>> +	 *
>> +	 * \todo The sensor timestamp should be better estimated by connecting
>> +	 * to the V4L2Device::frameStart signal.
> 
> Shouldn't that be handled by the sensor driver itself then?
> 
> What defines the temporal location/point of the timestamp that is
> returned currently?
> 
> I would have thought getting the time at the frameStart() signal would
> be susceptible to more latency due to the signal handling and calling up
> to userspace?
> 
> This must surely be something better handled by the kernel drivers right?
> 
> 
>> +	 */
>> +	request->metadata().set(controls::SensorTimestamp,
>> +				buffer->metadata().timestamp);

But for this....

Reviewed-by: Kieran Bingham <kieran.bingham at ideasonboard.com>

if the todo is correctly applicable then of course it can stay ;)



>> +
>>  	/* If the buffer is cancelled force a complete of the whole request. */
>>  	if (buffer->metadata().status == FrameMetadata::FrameCancelled) {
>>  		for (auto it : request->buffers())
>>
> 

-- 
Regards
--
Kieran