Message ID | 20210910070638.467294-10-umang.jain@ideasonboard.com |
---|---|
State | Superseded |
Delegated to: | Umang Jain |
Headers | show |
Series |
|
Related | show |
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@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,
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 + * 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,
Stopping the post-processor thread from CameraDevice is not ideal. But it temporarily avoids the crash on stopping the camera. Signed-off-by: Umang Jain <umang.jain@ideasonboard.com> --- src/android/camera_device.cpp | 8 ++++++++ src/android/camera_stream.h | 7 +++++++ 2 files changed, 15 insertions(+)