[libcamera-devel] [PATCH] libcamera: pipeline: simple: Fix crash when storing timestamp in metadata

[Niklas Söderlund]

Hi Laurent,

Thanks for your work.

On 2021-06-17 02:44:22 +0300, Laurent Pinchart wrote:
> Commit 922833f774f6 ("libcamera: simple: Report sensor timestamp")
> unconditionally tries to access the request through the capture buffer
> to store the capture timestamp in the metadata. This causes a null
> pointer dereference when using a converter, as the capture buffers are
> free-wheeling in that case, and not associated with a request.
> 
> Fix this by getting the request from the user-facing buffer, which can
> be the capture buffer when no converter is used.
> 
> Fixes: 922833f774f6 ("libcamera: simple: Report sensor timestamp")
> Signed-off-by: Laurent Pinchart <laurent.pinchart at ideasonboard.com>

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

> ---
>  src/libcamera/pipeline/simple/simple.cpp | 20 +++++++++++++++++---
>  1 file changed, 17 insertions(+), 3 deletions(-)
> 
> diff --git a/src/libcamera/pipeline/simple/simple.cpp b/src/libcamera/pipeline/simple/simple.cpp
> index 36fd9b852b33..e63d0bfd2fcb 100644
> --- a/src/libcamera/pipeline/simple/simple.cpp
> +++ b/src/libcamera/pipeline/simple/simple.cpp
> @@ -1127,14 +1127,28 @@ void SimplePipelineHandler::bufferReady(FrameBuffer *buffer)
>  	}
>  
>  	/*
> -	 * Record the sensor's timestamp in the request metadata.
> +	 * Record the sensor's timestamp in the request metadata. The request
> +	 * needs to be obtained from the user-facing buffer, as internal
> +	 * buffers are free-wheeling and have no request associated with them.
>  	 *
>  	 * \todo The sensor timestamp should be better estimated by connecting
>  	 * to the V4L2Device::frameStart signal if the platform provides it.
>  	 */
>  	Request *request = buffer->request();
> -	request->metadata().set(controls::SensorTimestamp,
> -				buffer->metadata().timestamp);
> +
> +	if (data->useConverter_ && !data->converterQueue_.empty()) {
> +		const std::map<unsigned int, FrameBuffer *> &outputs =
> +			data->converterQueue_.front();
> +		if (!outputs.empty()) {
> +			FrameBuffer *outputBuffer = outputs.begin()->second;
> +			if (outputBuffer)
> +				request = outputBuffer->request();
> +		}
> +	}
> +
> +	if (request)
> +		request->metadata().set(controls::SensorTimestamp,
> +					buffer->metadata().timestamp);
>  
>  	/*
>  	 * Queue the captured and the request buffer to the converter if format
> -- 
> Regards,
> 
> Laurent Pinchart
> 

-- 
Regards,
Niklas Söderlund