android: camera_device: Always clear descriptors_ in stop()
diff mbox series

Message ID 20240404101818.21484-1-jacopo.mondi@ideasonboard.com
State Accepted
Headers show
Series
  • android: camera_device: Always clear descriptors_ in stop()
Related show

Commit Message

Jacopo Mondi April 4, 2024, 10:18 a.m. UTC
From: Anle Pan <anle.pan@nxp.com>

When flush() is called and then a new stream configuration is set, the
descriptors_ queue might have a chance to be not cleared in stop(), as
the Camera is already in Stopped state.

This will prevent further requests from being completed in
sendCaptureResults() as the descriptors_ queue is not empty.

To fix the issue, clear the descriptors_ even if the Camera State is
Stopped. As a drawback the libcamera::Camera::stop() function might be
called twice, but this is allowed by the Camera state machine, and the
second call is guaranteed to be a nop.

Signed-off-by: Anle Pan <anle.pan@nxp.com>
Signed-off-by: Fang Hui <hui.fang@nxp.com>
Tested-by: Jacopo Mondi <jacopo.mondi@ideasonboard.com>
Reviewed-by: Jacopo Mondi <jacopo.mondi@ideasonboard.com>
---

CTS shows no regressions compared to v0.2.0

---
 src/android/camera_device.cpp | 2 --
 1 file changed, 2 deletions(-)

--
2.44.0

Comments

Umang Jain April 4, 2024, 10:31 a.m. UTC | #1
Hi Jacopo,

Thanks for the patch and testing

On 04/04/24 3:48 pm, Jacopo Mondi wrote:
> From: Anle Pan <anle.pan@nxp.com>
>
> When flush() is called and then a new stream configuration is set, the
> descriptors_ queue might have a chance to be not cleared in stop(), as
> the Camera is already in Stopped state.
>
> This will prevent further requests from being completed in
> sendCaptureResults() as the descriptors_ queue is not empty.
>
> To fix the issue, clear the descriptors_ even if the Camera State is
> Stopped. As a drawback the libcamera::Camera::stop() function might be
> called twice, but this is allowed by the Camera state machine, and the
> second call is guaranteed to be a nop.
>
> Signed-off-by: Anle Pan <anle.pan@nxp.com>
> Signed-off-by: Fang Hui <hui.fang@nxp.com>
> Tested-by: Jacopo Mondi <jacopo.mondi@ideasonboard.com>
> Reviewed-by: Jacopo Mondi <jacopo.mondi@ideasonboard.com>
> ---
>
> CTS shows no regressions compared to v0.2.0

I like this!

Reviewed-by: Umang Jain <umang.jain@ideasonboard.com>

>
> ---
>   src/android/camera_device.cpp | 2 --
>   1 file changed, 2 deletions(-)
>
> diff --git a/src/android/camera_device.cpp b/src/android/camera_device.cpp
> index d2679a9722e2..1b6f3f3a2b29 100644
> --- a/src/android/camera_device.cpp
> +++ b/src/android/camera_device.cpp
> @@ -433,8 +433,6 @@ void CameraDevice::flush()
>   void CameraDevice::stop()
>   {
>   	MutexLocker stateLock(stateMutex_);
> -	if (state_ == State::Stopped)
> -		return;
>
>   	camera_->stop();
>
> --
> 2.44.0
>

Patch
diff mbox series

diff --git a/src/android/camera_device.cpp b/src/android/camera_device.cpp
index d2679a9722e2..1b6f3f3a2b29 100644
--- a/src/android/camera_device.cpp
+++ b/src/android/camera_device.cpp
@@ -433,8 +433,6 @@  void CameraDevice::flush()
 void CameraDevice::stop()
 {
 	MutexLocker stateLock(stateMutex_);
-	if (state_ == State::Stopped)
-		return;

 	camera_->stop();