[libcamera-devel] [PATCH] android: camera_device: Fix crash in calling CameraDevice::close()
Umang Jain
umang.jain at ideasonboard.com
Tue Aug 31 14:02:18 CEST 2021
Hi Hiro,
The commit message can be somewhat more contextual of the problem.
On 8/31/21 1:35 PM, Hirokazu Honda wrote:
> This fixes the crash when post processing is needed and some capture
> request is being processed by PipelineHandler upon calling
> CameraDevice::close().
> The crash happens in Request::completeBuffer(), which is invoked by
> Camera::stop(). It uses FrameBuffer after free. The FrameBuffer is
> allocated and owned by FrameBufferAllocator, and FrameBufferAllocator
> is owned by CamerStream. So Camera::stop() should be executed before
> destroying CameraStream.
One question: I can see the problem but what's exactly is crashing in
Request::completeBuffer() is something we should know? Is it the ASSERT?
That might help us to define the problem statement better.
The problem is happening because we seem to add a CameraStream
associated buffer(depending on the CameraStream::Type) to the Request,
in CameraDevice::processCaptureRequest().
However, when the camera stops, all the current buffers are marked with
FrameMetadata::FrameCancelled and proceed to completion. But the buffer
associated with the CameraStream (that was previously added to the
request) has now been cleared out with a part of streams_.clear(), even
before the camera stop() has been invoked. Any access to those request
buffers after they have been cleared, shall result in a crash.
>
> Signed-off-by: Hirokazu Honda <hiroh at chromium.org>
> ---
> src/android/camera_device.cpp | 4 ++--
> 1 file changed, 2 insertions(+), 2 deletions(-)
>
> diff --git a/src/android/camera_device.cpp b/src/android/camera_device.cpp
> index 8ca76719..74a95a2a 100644
> --- a/src/android/camera_device.cpp
> +++ b/src/android/camera_device.cpp
> @@ -423,10 +423,10 @@ int CameraDevice::open(const hw_module_t *hardwareModule)
>
> void CameraDevice::close()
> {
> - streams_.clear();
> -
> stop();
>
> + streams_.clear();
How about this being a part of a CameraDevice::stop() instead ?
> +
> camera_->release();
> }
>
More information about the libcamera-devel
mailing list