| Message ID | 20210510105235.28319-7-jacopo@jmondi.org |
|---|---|
| State | Superseded |
| Delegated to: | Jacopo Mondi |
| Headers | show |
| Series |
|
| Related | show |
Hi Jacopo, Thanks for your work. On 2021-05-10 12:52:33 +0200, Jacopo Mondi wrote: > Guard access to the camera state and the start/stop sequences > with a mutex. > > Currently only stop() and the first call to processCaptureRequest() > start and stop the camera, and they're not meant to race with each > other. With the introduction of flush() the camera can be stopped > concurrently to a processCaptureRequest() call, hence access to the > camera state will need to be protected. > > Prepare for that by guarding the existing paths with a mutex. > > Signed-off-by: Jacopo Mondi <jacopo@jmondi.org> Reviewed-by: Niklas Söderlund <niklas.soderlund@ragnatech.se> > --- > src/android/camera_device.cpp | 23 ++++++++++++++--------- > src/android/camera_device.h | 1 + > 2 files changed, 15 insertions(+), 9 deletions(-) > > diff --git a/src/android/camera_device.cpp b/src/android/camera_device.cpp > index ff965a1c8c86..14a3f79a7402 100644 > --- a/src/android/camera_device.cpp > +++ b/src/android/camera_device.cpp > @@ -758,6 +758,7 @@ void CameraDevice::close() > > void CameraDevice::stop() > { > + MutexLocker cameraLock(cameraMutex_); > if (state_ == CameraStopped) > return; > > @@ -1913,17 +1914,21 @@ int CameraDevice::processCaptureRequest(camera3_capture_request_t *camera3Reques > if (!isValidRequest(camera3Request)) > return -EINVAL; > > - /* Start the camera if that's the first request we handle. */ > - if (state_ == CameraStopped) { > - worker_.start(); > + { > + MutexLocker cameraLock(cameraMutex_); > > - int ret = camera_->start(); > - if (ret) { > - LOG(HAL, Error) << "Failed to start camera"; > - return ret; > - } > + /* Start the camera if that's the first request we handle. */ > + if (state_ == CameraStopped) { > + worker_.start(); > > - state_ = CameraRunning; > + int ret = camera_->start(); > + if (ret) { > + LOG(HAL, Error) << "Failed to start camera"; > + return ret; > + } > + > + state_ = CameraRunning; > + } > } > > /* > diff --git a/src/android/camera_device.h b/src/android/camera_device.h > index 30b364decaab..f263fdae472a 100644 > --- a/src/android/camera_device.h > +++ b/src/android/camera_device.h > @@ -120,6 +120,7 @@ private: > > CameraWorker worker_; > > + libcamera::Mutex cameraMutex_; /* Protects access to the camera state. */ > State state_; > > std::shared_ptr<libcamera::Camera> camera_; > -- > 2.31.1 > > _______________________________________________ > libcamera-devel mailing list > libcamera-devel@lists.libcamera.org > https://lists.libcamera.org/listinfo/libcamera-devel
diff --git a/src/android/camera_device.cpp b/src/android/camera_device.cpp index ff965a1c8c86..14a3f79a7402 100644 --- a/src/android/camera_device.cpp +++ b/src/android/camera_device.cpp @@ -758,6 +758,7 @@ void CameraDevice::close() void CameraDevice::stop() { + MutexLocker cameraLock(cameraMutex_); if (state_ == CameraStopped) return; @@ -1913,17 +1914,21 @@ int CameraDevice::processCaptureRequest(camera3_capture_request_t *camera3Reques if (!isValidRequest(camera3Request)) return -EINVAL; - /* Start the camera if that's the first request we handle. */ - if (state_ == CameraStopped) { - worker_.start(); + { + MutexLocker cameraLock(cameraMutex_); - int ret = camera_->start(); - if (ret) { - LOG(HAL, Error) << "Failed to start camera"; - return ret; - } + /* Start the camera if that's the first request we handle. */ + if (state_ == CameraStopped) { + worker_.start(); - state_ = CameraRunning; + int ret = camera_->start(); + if (ret) { + LOG(HAL, Error) << "Failed to start camera"; + return ret; + } + + state_ = CameraRunning; + } } /* diff --git a/src/android/camera_device.h b/src/android/camera_device.h index 30b364decaab..f263fdae472a 100644 --- a/src/android/camera_device.h +++ b/src/android/camera_device.h @@ -120,6 +120,7 @@ private: CameraWorker worker_; + libcamera::Mutex cameraMutex_; /* Protects access to the camera state. */ State state_; std::shared_ptr<libcamera::Camera> camera_;
Guard access to the camera state and the start/stop sequences with a mutex. Currently only stop() and the first call to processCaptureRequest() start and stop the camera, and they're not meant to race with each other. With the introduction of flush() the camera can be stopped concurrently to a processCaptureRequest() call, hence access to the camera state will need to be protected. Prepare for that by guarding the existing paths with a mutex. Signed-off-by: Jacopo Mondi <jacopo@jmondi.org> --- src/android/camera_device.cpp | 23 ++++++++++++++--------- src/android/camera_device.h | 1 + 2 files changed, 15 insertions(+), 9 deletions(-)