[libcamera-devel] [PATCH v2 9/9] RFC: Stop PostProcessor when camera is stopped

[Laurent Pinchart]

Hi Umang,

Thank you for the patch.

On Fri, Sep 10, 2021 at 12:36:38PM +0530, Umang Jain wrote:
> Stopping the post-processor thread from CameraDevice is not ideal.

Why is so ? I think that explicit start() and stop() functions for
CameraStream() could make sense.

> But it temporarily avoids the crash on stopping the camera.

What crash ?

> Signed-off-by: Umang Jain <umang.jain at ideasonboard.com>
> ---
>  src/android/camera_device.cpp | 8 ++++++++
>  src/android/camera_stream.h   | 7 +++++++
>  2 files changed, 15 insertions(+)
> 
> diff --git a/src/android/camera_device.cpp b/src/android/camera_device.cpp
> index 73eb5758..fa2db72f 100644
> --- a/src/android/camera_device.cpp
> +++ b/src/android/camera_device.cpp
> @@ -425,6 +425,14 @@ int CameraDevice::open(const hw_module_t *hardwareModule)
>  
>  void CameraDevice::close()
>  {
> +	for (CameraStream &cameraStream : streams_) {
> +		/* CameraStream doesn't have user defined destructor as
> +		 * it is MoveConstructible. Stop the post-processor thread

Why does being move-constructible prevent having a user-defined
destructor ? Adding a user-defined destructor will prevent the compiler
from adding an implicitly-declared move constructor, but you could add
an explicit

	CameraStream(CameraStream &&other) = default;

Or better,

	CameraStream(CameraStream &&other);

in camera_device.h, and

CameraStream::CameraStream(CameraStream &&other) = default;

in camera_device.cpp to avoid making it inline.

> +		 * from here before clearing streams_.
> +		 */
> +		cameraStream.stopPostProcessorThread();
> +	}
> +
>  	streams_.clear();
>  
>  	stop();
> diff --git a/src/android/camera_stream.h b/src/android/camera_stream.h
> index dbb7eee3..bd37473e 100644
> --- a/src/android/camera_stream.h
> +++ b/src/android/camera_stream.h
> @@ -133,6 +133,13 @@ public:
>  		    const Camera3RequestDescriptor *context);
>  	libcamera::FrameBuffer *getBuffer();
>  	void putBuffer(libcamera::FrameBuffer *buffer);
> +	void stopPostProcessorThread()
> +	{
> +		if (thread_) {
> +			thread_->exit();
> +			thread_->wait();
> +		}
> +	}
>  
>  	enum ProcessStatus {
>  		Succeeded,

-- 
Regards,

Laurent Pinchart