[libcamera-devel,v6,3/7] android: camera_device: Refactor descriptor status and sendCaptureResults()
diff mbox series

Message ID 20211023103302.152769-4-umang.jain@ideasonboard.com
State Superseded
Headers show
Series
  • Async Post Processor
Related show

Commit Message

Umang Jain Oct. 23, 2021, 10:32 a.m. UTC 
Currently, we use Camera3RequestDescriptor::Status to determine:
- When the descriptor has been completely processed by HAL
- Whether any errors were encountered, during its processing

Both of these are essential to know whether the descriptor is eligible
to call process_capture_results() through sendCaptureResults().
When a status(Success/Error) is set on the descriptor, it is ready to
be sent back via sendCaptureResults(). However, this might lead to
undesired results especially when sendCaptureResults() runs in a
different thread (for e.g. stream's post-processor async completion
slot).

This patch decouples the descriptor status (Success/Error) from the
descriptor's completion status (pending or complete). The advantage
of this is we can set the completion status when the descriptor has
been processed fully by the layer and we can set the error status on
the descriptor wherever an error is encountered, throughout the
lifetime descriptor in the HAL layer.

While at it, introduce a wrapper completeDescriptor() around
sendCaptureResults(). completeDescriptor() as the name suggests will
mark the descriptor as complete, so it is ready to be sent back.
The locking mechanism is moved from sendCaptureResults() to this wrapper
since the intention is to use completeDescriptor() in place of existing
sendCaptureResults() calls.

Signed-off-by: Umang Jain <umang.jain@ideasonboard.com>
---
 src/android/camera_device.cpp  | 33 ++++++++++++++++++---------------
 src/android/camera_device.h    |  1 +
 src/android/camera_request.cpp |  2 +-
 src/android/camera_request.h   |  7 ++++---
 4 files changed, 24 insertions(+), 19 deletions(-)

Comments

Laurent Pinchart Oct. 25, 2021, 5:07 a.m. UTC | #1 
Hi Umang,

Thank you for the patch.

On Sat, Oct 23, 2021 at 04:02:58PM +0530, Umang Jain wrote:
> Currently, we use Camera3RequestDescriptor::Status to determine:
> - When the descriptor has been completely processed by HAL
> - Whether any errors were encountered, during its processing
> 
> Both of these are essential to know whether the descriptor is eligible
> to call process_capture_results() through sendCaptureResults().
> When a status(Success/Error) is set on the descriptor, it is ready to
> be sent back via sendCaptureResults(). However, this might lead to
> undesired results especially when sendCaptureResults() runs in a
> different thread (for e.g. stream's post-processor async completion
> slot).
> 
> This patch decouples the descriptor status (Success/Error) from the
> descriptor's completion status (pending or complete). The advantage
> of this is we can set the completion status when the descriptor has
> been processed fully by the layer and we can set the error status on
> the descriptor wherever an error is encountered, throughout the
> lifetime descriptor in the HAL layer.
> 
> While at it, introduce a wrapper completeDescriptor() around
> sendCaptureResults(). completeDescriptor() as the name suggests will
> mark the descriptor as complete, so it is ready to be sent back.
> The locking mechanism is moved from sendCaptureResults() to this wrapper
> since the intention is to use completeDescriptor() in place of existing
> sendCaptureResults() calls.
> 
> Signed-off-by: Umang Jain <umang.jain@ideasonboard.com>
> ---
>  src/android/camera_device.cpp  | 33 ++++++++++++++++++---------------
>  src/android/camera_device.h    |  1 +
>  src/android/camera_request.cpp |  2 +-
>  src/android/camera_request.h   |  7 ++++---
>  4 files changed, 24 insertions(+), 19 deletions(-)
> 
> diff --git a/src/android/camera_device.cpp b/src/android/camera_device.cpp
> index 806b4090..f4d4fb09 100644
> --- a/src/android/camera_device.cpp
> +++ b/src/android/camera_device.cpp
> @@ -982,13 +982,13 @@ int CameraDevice::processCaptureRequest(camera3_capture_request_t *camera3Reques
>  	MutexLocker stateLock(stateMutex_);
>  
>  	if (state_ == State::Flushing) {
> -		abortRequest(descriptor.get());
> +		Camera3RequestDescriptor *rawDescriptor = descriptor.get();
> +		abortRequest(rawDescriptor);

Could we move the abortRequest() call just before completeDescriptor(),
to always operate with the descriptor already added to the queue ? It
should make no difference today, but consistent call patterns will make
it easier to understand the code and the possible race conditions.

>  		{
>  			MutexLocker descriptorsLock(descriptorsMutex_);
>  			descriptors_.push(std::move(descriptor));
>  		}
> -
> -		sendCaptureResults();
> +		completeDescriptor(rawDescriptor);
>  
>  		return 0;
>  	}
> @@ -1079,7 +1079,7 @@ void CameraDevice::requestComplete(Request *request)
>  				<< request->status();
>  
>  		abortRequest(descriptor);
> -		sendCaptureResults();
> +		completeDescriptor(descriptor);
>  
>  		return;
>  	}
> @@ -1117,6 +1117,7 @@ void CameraDevice::requestComplete(Request *request)
>  	}
>  
>  	/* Handle post-processing. */
> +	bool hasPostProcessingErrors = false;
>  	for (auto &buffer : descriptor->buffers_) {
>  		CameraStream *stream = buffer.stream;
>  
> @@ -1129,6 +1130,7 @@ void CameraDevice::requestComplete(Request *request)
>  			buffer.status = Camera3RequestDescriptor::Status::Error;
>  			notifyError(descriptor->frameNumber_, stream->camera3Stream(),
>  				    CAMERA3_MSG_ERROR_BUFFER);
> +			hasPostProcessingErrors = true;

You can set

			descriptor->status_ = Camera3RequestDescriptor::Status::Error;

here.

>  			continue;
>  		}
>  
> @@ -1143,29 +1145,32 @@ void CameraDevice::requestComplete(Request *request)
>  
>  		if (ret) {
>  			buffer.status = Camera3RequestDescriptor::Status::Error;
> +			hasPostProcessingErrors = true;

And here too.

>  			notifyError(descriptor->frameNumber_, stream->camera3Stream(),
>  				    CAMERA3_MSG_ERROR_BUFFER);
>  		}
>  	}
>  
> -	descriptor->status_ = Camera3RequestDescriptor::Status::Success;
> +	if (hasPostProcessingErrors)
> +		descriptor->status_ = Camera3RequestDescriptor::Status::Error;

And then drop this.

> +
> +	completeDescriptor(descriptor);
> +}
> +
> +void CameraDevice::completeDescriptor(Camera3RequestDescriptor *descriptor)
> +{
> +	MutexLocker lock(descriptorsMutex_);
> +	descriptor->completeDescriptor();
> +
>  	sendCaptureResults();
>  }
>  
>  void CameraDevice::sendCaptureResults()
>  {
> -	MutexLocker lock(descriptorsMutex_);
>  	while (!descriptors_.empty() && !descriptors_.front()->isPending()) {
>  		auto descriptor = std::move(descriptors_.front());
>  		descriptors_.pop();
>  
> -		/*
> -		 * \todo Releasing and re-acquiring the critical section for
> -		 * every request completion (grain-locking) might have an
> -		 * impact on performance which should be measured.
> -		 */
> -		lock.unlock();
> -
>  		camera3_capture_result_t captureResult = {};
>  
>  		captureResult.frame_number = descriptor->frameNumber_;
> @@ -1201,8 +1206,6 @@ void CameraDevice::sendCaptureResults()
>  			captureResult.partial_result = 1;
>  
>  		callbacks_->process_capture_result(callbacks_, &captureResult);
> -
> -		lock.lock();
>  	}
>  }
>  
> diff --git a/src/android/camera_device.h b/src/android/camera_device.h
> index 863cf414..e544f2bd 100644
> --- a/src/android/camera_device.h
> +++ b/src/android/camera_device.h
> @@ -93,6 +93,7 @@ private:
>  	void notifyError(uint32_t frameNumber, camera3_stream_t *stream,
>  			 camera3_error_msg_code code) const;
>  	int processControls(Camera3RequestDescriptor *descriptor);
> +	void completeDescriptor(Camera3RequestDescriptor *descriptor);
>  	void sendCaptureResults();
>  	std::unique_ptr<CameraMetadata> getResultMetadata(
>  		const Camera3RequestDescriptor &descriptor) const;
> diff --git a/src/android/camera_request.cpp b/src/android/camera_request.cpp
> index faa85ada..16cf266f 100644
> --- a/src/android/camera_request.cpp
> +++ b/src/android/camera_request.cpp
> @@ -36,7 +36,7 @@ Camera3RequestDescriptor::Camera3RequestDescriptor(
>  			static_cast<CameraStream *>(buffer.stream->priv);
>  
>  		buffers_.push_back({ stream, buffer.buffer, nullptr,
> -				     buffer.acquire_fence, Status::Pending });
> +				     buffer.acquire_fence, Status::Success });
>  	}
>  
>  	/* Clone the controls associated with the camera3 request. */
> diff --git a/src/android/camera_request.h b/src/android/camera_request.h
> index 05dabf89..904886da 100644
> --- a/src/android/camera_request.h
> +++ b/src/android/camera_request.h
> @@ -26,7 +26,6 @@ class Camera3RequestDescriptor
>  {
>  public:
>  	enum class Status {
> -		Pending,
>  		Success,
>  		Error,
>  	};
> @@ -43,7 +42,8 @@ public:
>  				 const camera3_capture_request_t *camera3Request);
>  	~Camera3RequestDescriptor();
>  
> -	bool isPending() const { return status_ == Status::Pending; }
> +	bool isPending() const { return !complete_; }
> +	void completeDescriptor() { complete_ = true; }

As the function is a member of the Camera3RequestDescriptor class, I
wouldn't repeat "descriptor" in the name. Given that the complete_
member is public, I'd actually drop the function and access complete_
directly, the same way we access status_. If you prefer an accessor, I'd
make the complete_ member private, but that's probably overkill for now
given that all other members are public.

Reviewed-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>

>  	uint32_t frameNumber_ = 0;
>  
> @@ -53,7 +53,8 @@ public:
>  	std::unique_ptr<CaptureRequest> request_;
>  	std::unique_ptr<CameraMetadata> resultMetadata_;
>  
> -	Status status_ = Status::Pending;
> +	bool complete_ = false;
> +	Status status_ = Status::Success;
>  
>  private:
>  	LIBCAMERA_DISABLE_COPY(Camera3RequestDescriptor)
Hirokazu Honda Oct. 25, 2021, 5:38 a.m. UTC | #2 
Hi Umang,

On Mon, Oct 25, 2021 at 2:07 PM Laurent Pinchart
<laurent.pinchart@ideasonboard.com> wrote:
>
> Hi Umang,
>
> Thank you for the patch.
>
> On Sat, Oct 23, 2021 at 04:02:58PM +0530, Umang Jain wrote:
> > Currently, we use Camera3RequestDescriptor::Status to determine:
> > - When the descriptor has been completely processed by HAL
> > - Whether any errors were encountered, during its processing
> >
> > Both of these are essential to know whether the descriptor is eligible
> > to call process_capture_results() through sendCaptureResults().
> > When a status(Success/Error) is set on the descriptor, it is ready to
> > be sent back via sendCaptureResults(). However, this might lead to
> > undesired results especially when sendCaptureResults() runs in a
> > different thread (for e.g. stream's post-processor async completion
> > slot).
> >
> > This patch decouples the descriptor status (Success/Error) from the
> > descriptor's completion status (pending or complete). The advantage
> > of this is we can set the completion status when the descriptor has
> > been processed fully by the layer and we can set the error status on
> > the descriptor wherever an error is encountered, throughout the
> > lifetime descriptor in the HAL layer.
> >
> > While at it, introduce a wrapper completeDescriptor() around
> > sendCaptureResults(). completeDescriptor() as the name suggests will
> > mark the descriptor as complete, so it is ready to be sent back.
> > The locking mechanism is moved from sendCaptureResults() to this wrapper
> > since the intention is to use completeDescriptor() in place of existing
> > sendCaptureResults() calls.
> >
> > Signed-off-by: Umang Jain <umang.jain@ideasonboard.com>
> > ---
> >  src/android/camera_device.cpp  | 33 ++++++++++++++++++---------------
> >  src/android/camera_device.h    |  1 +
> >  src/android/camera_request.cpp |  2 +-
> >  src/android/camera_request.h   |  7 ++++---
> >  4 files changed, 24 insertions(+), 19 deletions(-)
> >
> > diff --git a/src/android/camera_device.cpp b/src/android/camera_device.cpp
> > index 806b4090..f4d4fb09 100644
> > --- a/src/android/camera_device.cpp
> > +++ b/src/android/camera_device.cpp
> > @@ -982,13 +982,13 @@ int CameraDevice::processCaptureRequest(camera3_capture_request_t *camera3Reques
> >       MutexLocker stateLock(stateMutex_);
> >
> >       if (state_ == State::Flushing) {
> > -             abortRequest(descriptor.get());
> > +             Camera3RequestDescriptor *rawDescriptor = descriptor.get();
> > +             abortRequest(rawDescriptor);
>
> Could we move the abortRequest() call just before completeDescriptor(),
> to always operate with the descriptor already added to the queue ? It
> should make no difference today, but consistent call patterns will make
> it easier to understand the code and the possible race conditions.
>
> >               {
> >                       MutexLocker descriptorsLock(descriptorsMutex_);
> >                       descriptors_.push(std::move(descriptor));
> >               }
> > -
> > -             sendCaptureResults();
> > +             completeDescriptor(rawDescriptor);
> >
> >               return 0;
> >       }
> > @@ -1079,7 +1079,7 @@ void CameraDevice::requestComplete(Request *request)
> >                               << request->status();
> >
> >               abortRequest(descriptor);
> > -             sendCaptureResults();
> > +             completeDescriptor(descriptor);
> >
> >               return;
> >       }
> > @@ -1117,6 +1117,7 @@ void CameraDevice::requestComplete(Request *request)
> >       }
> >
> >       /* Handle post-processing. */
> > +     bool hasPostProcessingErrors = false;
> >       for (auto &buffer : descriptor->buffers_) {
> >               CameraStream *stream = buffer.stream;
> >
> > @@ -1129,6 +1130,7 @@ void CameraDevice::requestComplete(Request *request)
> >                       buffer.status = Camera3RequestDescriptor::Status::Error;
> >                       notifyError(descriptor->frameNumber_, stream->camera3Stream(),
> >                                   CAMERA3_MSG_ERROR_BUFFER);
> > +                     hasPostProcessingErrors = true;
>
> You can set
>
>                         descriptor->status_ = Camera3RequestDescriptor::Status::Error;
>
> here.
>
> >                       continue;
> >               }
> >
> > @@ -1143,29 +1145,32 @@ void CameraDevice::requestComplete(Request *request)
> >
> >               if (ret) {
> >                       buffer.status = Camera3RequestDescriptor::Status::Error;
> > +                     hasPostProcessingErrors = true;
>
> And here too.
>
> >                       notifyError(descriptor->frameNumber_, stream->camera3Stream(),
> >                                   CAMERA3_MSG_ERROR_BUFFER);
> >               }
> >       }
> >
> > -     descriptor->status_ = Camera3RequestDescriptor::Status::Success;
> > +     if (hasPostProcessingErrors)
> > +             descriptor->status_ = Camera3RequestDescriptor::Status::Error;
>
> And then drop this.
>
> > +
> > +     completeDescriptor(descriptor);
> > +}
> > +
> > +void CameraDevice::completeDescriptor(Camera3RequestDescriptor *descriptor)
> > +{
> > +     MutexLocker lock(descriptorsMutex_);
> > +     descriptor->completeDescriptor();
> > +
> >       sendCaptureResults();
> >  }
> >
> >  void CameraDevice::sendCaptureResults()
> >  {
> > -     MutexLocker lock(descriptorsMutex_);

Just note:
We may want to have an assertion to acquire lock.
Looks like there is no way of doing this in std::mutex only.
The only way is to pass MutexLocker reference to here and
ASSERT(lock.owns_lock()).

Another brilliant way is to use thread annotation and annotate this
function by REQUIRES().
https://bugs.libcamera.org/show_bug.cgi?id=23

Change looks good.
Reviewed-by: Hirokazu Honda <hiroh@chromium.org>

> >       while (!descriptors_.empty() && !descriptors_.front()->isPending()) {
> >               auto descriptor = std::move(descriptors_.front());
> >               descriptors_.pop();
> >
> > -             /*
> > -              * \todo Releasing and re-acquiring the critical section for
> > -              * every request completion (grain-locking) might have an
> > -              * impact on performance which should be measured.
> > -              */
> > -             lock.unlock();
> > -
> >               camera3_capture_result_t captureResult = {};
> >
> >               captureResult.frame_number = descriptor->frameNumber_;
> > @@ -1201,8 +1206,6 @@ void CameraDevice::sendCaptureResults()
> >                       captureResult.partial_result = 1;
> >
> >               callbacks_->process_capture_result(callbacks_, &captureResult);
> > -
> > -             lock.lock();
> >       }
> >  }
> >
> > diff --git a/src/android/camera_device.h b/src/android/camera_device.h
> > index 863cf414..e544f2bd 100644
> > --- a/src/android/camera_device.h
> > +++ b/src/android/camera_device.h
> > @@ -93,6 +93,7 @@ private:
> >       void notifyError(uint32_t frameNumber, camera3_stream_t *stream,
> >                        camera3_error_msg_code code) const;
> >       int processControls(Camera3RequestDescriptor *descriptor);
> > +     void completeDescriptor(Camera3RequestDescriptor *descriptor);
> >       void sendCaptureResults();
> >       std::unique_ptr<CameraMetadata> getResultMetadata(
> >               const Camera3RequestDescriptor &descriptor) const;
> > diff --git a/src/android/camera_request.cpp b/src/android/camera_request.cpp
> > index faa85ada..16cf266f 100644
> > --- a/src/android/camera_request.cpp
> > +++ b/src/android/camera_request.cpp
> > @@ -36,7 +36,7 @@ Camera3RequestDescriptor::Camera3RequestDescriptor(
> >                       static_cast<CameraStream *>(buffer.stream->priv);
> >
> >               buffers_.push_back({ stream, buffer.buffer, nullptr,
> > -                                  buffer.acquire_fence, Status::Pending });
> > +                                  buffer.acquire_fence, Status::Success });
> >       }
> >
> >       /* Clone the controls associated with the camera3 request. */
> > diff --git a/src/android/camera_request.h b/src/android/camera_request.h
> > index 05dabf89..904886da 100644
> > --- a/src/android/camera_request.h
> > +++ b/src/android/camera_request.h
> > @@ -26,7 +26,6 @@ class Camera3RequestDescriptor
> >  {
> >  public:
> >       enum class Status {
> > -             Pending,
> >               Success,
> >               Error,
> >       };
> > @@ -43,7 +42,8 @@ public:
> >                                const camera3_capture_request_t *camera3Request);
> >       ~Camera3RequestDescriptor();
> >
> > -     bool isPending() const { return status_ == Status::Pending; }
> > +     bool isPending() const { return !complete_; }
> > +     void completeDescriptor() { complete_ = true; }
>
> As the function is a member of the Camera3RequestDescriptor class, I
> wouldn't repeat "descriptor" in the name. Given that the complete_
> member is public, I'd actually drop the function and access complete_
> directly, the same way we access status_. If you prefer an accessor, I'd
> make the complete_ member private, but that's probably overkill for now
> given that all other members are public.
>
> Reviewed-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
>
> >       uint32_t frameNumber_ = 0;
> >
> > @@ -53,7 +53,8 @@ public:
> >       std::unique_ptr<CaptureRequest> request_;
> >       std::unique_ptr<CameraMetadata> resultMetadata_;
> >
> > -     Status status_ = Status::Pending;
> > +     bool complete_ = false;
> > +     Status status_ = Status::Success;
> >
> >  private:
> >       LIBCAMERA_DISABLE_COPY(Camera3RequestDescriptor)
>
> --
> Regards,
>
> Laurent Pinchart

Patch
diff mbox series 

diff --git a/src/android/camera_device.cpp b/src/android/camera_device.cpp
index 806b4090..f4d4fb09 100644
--- a/src/android/camera_device.cpp
+++ b/src/android/camera_device.cpp
@@ -982,13 +982,13 @@  int CameraDevice::processCaptureRequest(camera3_capture_request_t *camera3Reques
 	MutexLocker stateLock(stateMutex_);
 
 	if (state_ == State::Flushing) {
-		abortRequest(descriptor.get());
+		Camera3RequestDescriptor *rawDescriptor = descriptor.get();
+		abortRequest(rawDescriptor);
 		{
 			MutexLocker descriptorsLock(descriptorsMutex_);
 			descriptors_.push(std::move(descriptor));
 		}
-
-		sendCaptureResults();
+		completeDescriptor(rawDescriptor);
 
 		return 0;
 	}
@@ -1079,7 +1079,7 @@  void CameraDevice::requestComplete(Request *request)
 				<< request->status();
 
 		abortRequest(descriptor);
-		sendCaptureResults();
+		completeDescriptor(descriptor);
 
 		return;
 	}
@@ -1117,6 +1117,7 @@  void CameraDevice::requestComplete(Request *request)
 	}
 
 	/* Handle post-processing. */
+	bool hasPostProcessingErrors = false;
 	for (auto &buffer : descriptor->buffers_) {
 		CameraStream *stream = buffer.stream;
 
@@ -1129,6 +1130,7 @@  void CameraDevice::requestComplete(Request *request)
 			buffer.status = Camera3RequestDescriptor::Status::Error;
 			notifyError(descriptor->frameNumber_, stream->camera3Stream(),
 				    CAMERA3_MSG_ERROR_BUFFER);
+			hasPostProcessingErrors = true;
 			continue;
 		}
 
@@ -1143,29 +1145,32 @@  void CameraDevice::requestComplete(Request *request)
 
 		if (ret) {
 			buffer.status = Camera3RequestDescriptor::Status::Error;
+			hasPostProcessingErrors = true;
 			notifyError(descriptor->frameNumber_, stream->camera3Stream(),
 				    CAMERA3_MSG_ERROR_BUFFER);
 		}
 	}
 
-	descriptor->status_ = Camera3RequestDescriptor::Status::Success;
+	if (hasPostProcessingErrors)
+		descriptor->status_ = Camera3RequestDescriptor::Status::Error;
+
+	completeDescriptor(descriptor);
+}
+
+void CameraDevice::completeDescriptor(Camera3RequestDescriptor *descriptor)
+{
+	MutexLocker lock(descriptorsMutex_);
+	descriptor->completeDescriptor();
+
 	sendCaptureResults();
 }
 
 void CameraDevice::sendCaptureResults()
 {
-	MutexLocker lock(descriptorsMutex_);
 	while (!descriptors_.empty() && !descriptors_.front()->isPending()) {
 		auto descriptor = std::move(descriptors_.front());
 		descriptors_.pop();
 
-		/*
-		 * \todo Releasing and re-acquiring the critical section for
-		 * every request completion (grain-locking) might have an
-		 * impact on performance which should be measured.
-		 */
-		lock.unlock();
-
 		camera3_capture_result_t captureResult = {};
 
 		captureResult.frame_number = descriptor->frameNumber_;
@@ -1201,8 +1206,6 @@  void CameraDevice::sendCaptureResults()
 			captureResult.partial_result = 1;
 
 		callbacks_->process_capture_result(callbacks_, &captureResult);
-
-		lock.lock();
 	}
 }
 
diff --git a/src/android/camera_device.h b/src/android/camera_device.h
index 863cf414..e544f2bd 100644
--- a/src/android/camera_device.h
+++ b/src/android/camera_device.h
@@ -93,6 +93,7 @@  private:
 	void notifyError(uint32_t frameNumber, camera3_stream_t *stream,
 			 camera3_error_msg_code code) const;
 	int processControls(Camera3RequestDescriptor *descriptor);
+	void completeDescriptor(Camera3RequestDescriptor *descriptor);
 	void sendCaptureResults();
 	std::unique_ptr<CameraMetadata> getResultMetadata(
 		const Camera3RequestDescriptor &descriptor) const;
diff --git a/src/android/camera_request.cpp b/src/android/camera_request.cpp
index faa85ada..16cf266f 100644
--- a/src/android/camera_request.cpp
+++ b/src/android/camera_request.cpp
@@ -36,7 +36,7 @@  Camera3RequestDescriptor::Camera3RequestDescriptor(
 			static_cast<CameraStream *>(buffer.stream->priv);
 
 		buffers_.push_back({ stream, buffer.buffer, nullptr,
-				     buffer.acquire_fence, Status::Pending });
+				     buffer.acquire_fence, Status::Success });
 	}
 
 	/* Clone the controls associated with the camera3 request. */
diff --git a/src/android/camera_request.h b/src/android/camera_request.h
index 05dabf89..904886da 100644
--- a/src/android/camera_request.h
+++ b/src/android/camera_request.h
@@ -26,7 +26,6 @@  class Camera3RequestDescriptor
 {
 public:
 	enum class Status {
-		Pending,
 		Success,
 		Error,
 	};
@@ -43,7 +42,8 @@  public:
 				 const camera3_capture_request_t *camera3Request);
 	~Camera3RequestDescriptor();
 
-	bool isPending() const { return status_ == Status::Pending; }
+	bool isPending() const { return !complete_; }
+	void completeDescriptor() { complete_ = true; }
 
 	uint32_t frameNumber_ = 0;
 
@@ -53,7 +53,8 @@  public:
 	std::unique_ptr<CaptureRequest> request_;
 	std::unique_ptr<CameraMetadata> resultMetadata_;
 
-	Status status_ = Status::Pending;
+	bool complete_ = false;
+	Status status_ = Status::Success;
 
 private:
 	LIBCAMERA_DISABLE_COPY(Camera3RequestDescriptor)