[libcamera-devel] [PATCH] pipeline: vimc: Force complete of request on cancelled buffers
Kieran Bingham
kieran.bingham at ideasonboard.com
Mon Aug 16 16:09:31 CEST 2021
Hi Umang,
On 16/08/2021 14:03, Umang Jain wrote:
> When the stream is stopped, the V4L2VideoDevice sends back all
> the queued buffers with FrameMetadata::FrameCancelled status.
> It is the responsibility of the pipeline handler to handle
> these buffers with FrameMetadata::FrameCancelled. VIMC is
> currently missing this handling path.
>
> As the FrameMetadata::FrameCancelled is set when the stream is
> stopped, we can be sure that no more queued and re-use of request
> shall happen. Hence, cancel all the requests' buffers force a
> complete with completeBuffer().
>
> The issue is caught by the gstreamer_single_stream_test.cpp running
> with vimc. During the check with meson built-in option
> '-Db_sanitize=address,undefined'
> it was observed:
>
> ==118003==ERROR: AddressSanitizer: heap-use-after-free on address 0x60e000037108 at pc 0x7f225160c9ac bp 0x7f224a47b620 sp 0x7f224a47b618
> READ of size 4 at 0x60e000037108 thread T1
> #0 0x7f225160c9ab in libcamera::Request::sequence() const ../include/libcamera/request.h:55
> #1 0x7f22518297aa in libcamera::VimcCameraData::bufferReady(libcamera::FrameBuffer*) ../src/libcamera/pipeline/vimc/vimc.cpp:577
> #2 0x7f225183b1ef in libcamera::BoundMethodMember<libcamera::VimcCameraData, void, libcamera::FrameBuffer*>::activate(libcamera::FrameBuffer*, bool) ../include/libcamera/base/bound_method.h:194
> #3 0x7f22515cc91f in libcamera::Signal<libcamera::FrameBuffer*>::emit(libcamera::FrameBuffer*) ../include/libcamera/base/signal.h:126
> #4 0x7f22515c3305 in libcamera::V4L2VideoDevice::streamOff() ../src/libcamera/v4l2_videodevice.cpp:1605
> #5 0x7f225181f345 in libcamera::PipelineHandlerVimc::stop(libcamera::Camera*) ../src/libcamera/pipeline/vimc/vimc.cpp:365
>
> The VimcCameraData::bufferReady seems to emit even after the stream
> is stopped. It's primarily due to vimc's lack of handling
> FrameMetadata::FrameCancelled in its pipeline handler.
>
> Signed-off-by: Umang Jain <umang.jain at ideasonboard.com>
> ---
> src/libcamera/pipeline/vimc/vimc.cpp | 12 ++++++++++++
> 1 file changed, 12 insertions(+)
>
> diff --git a/src/libcamera/pipeline/vimc/vimc.cpp b/src/libcamera/pipeline/vimc/vimc.cpp
> index 92b30f2e..e98adab6 100644
> --- a/src/libcamera/pipeline/vimc/vimc.cpp
> +++ b/src/libcamera/pipeline/vimc/vimc.cpp
> @@ -571,6 +571,18 @@ void VimcCameraData::bufferReady(FrameBuffer *buffer)
> request->metadata().set(controls::SensorTimestamp,
> buffer->metadata().timestamp);
>
> + /* If the buffer is cancelled force a complete of the whole request. */
> + if (buffer->metadata().status == FrameMetadata::FrameCancelled) {
> + for (auto it : request->buffers()) {
> + FrameBuffer *b = it.second;
> + b->cancel();
> + pipe_->completeBuffer(request, b);
> + }
> +
> + pipe_->completeRequest(request);
> + return;
> + }
> +
Reviewed-by: Kieran Bingham <kieran.bingham at ideasonboard.com>
I see this is the same location as in the IPU3, so I don't mind if it's
here or above the timestamp setting, as Laurent suggested.
If you make a v2 of this with the FrameCancelled check first, you might
also want to make a patch to move the handling in the
IPU3CameraData::cio2BufferReady() to make it consistent, and check the
others too.
--
Kieran
> pipe_->completeBuffer(request, buffer);
> pipe_->completeRequest(request);
>
>
More information about the libcamera-devel
mailing list