[v2,6/9] android: Cleanup CAMERA3_MSG_ERROR_REQUEST
diff mbox series

Message ID 20241127092632.3145984-7-chenghaoyang@chromium.org
State New
Headers show
Series
  • Signal metadataAvailable and Android partial result
Related show

Commit Message

Cheng-Hao Yang Nov. 27, 2024, 9:25 a.m. UTC 
When a request is completed with failure, CAMERA3_MSG_ERROR_RESULT
should be used instead.

This patch also cleans up aborting when flushing with
sendCaptureResult().

Signed-off-by: Han-Lin Chen <hanlinchen@chromium.org>
Co-developed-by: Harvey Yang <chenghaoyang@chromium.org>
Signed-off-by: Harvey Yang <chenghaoyang@chromium.org>
---
 src/android/camera_device.cpp | 28 +++++++++++++++-------------
 1 file changed, 15 insertions(+), 13 deletions(-)

Comments

Jacopo Mondi Nov. 28, 2024, 3:02 p.m. UTC | #1 
Hi Harvey

On Wed, Nov 27, 2024 at 09:25:56AM +0000, Harvey Yang wrote:
> When a request is completed with failure, CAMERA3_MSG_ERROR_RESULT
> should be used instead.
>
> This patch also cleans up aborting when flushing with
> sendCaptureResult().
>
> Signed-off-by: Han-Lin Chen <hanlinchen@chromium.org>
> Co-developed-by: Harvey Yang <chenghaoyang@chromium.org>
> Signed-off-by: Harvey Yang <chenghaoyang@chromium.org>
> ---
>  src/android/camera_device.cpp | 28 +++++++++++++++-------------
>  1 file changed, 15 insertions(+), 13 deletions(-)
>
> diff --git a/src/android/camera_device.cpp b/src/android/camera_device.cpp
> index 0377cf215..3fb92268e 100644
> --- a/src/android/camera_device.cpp
> +++ b/src/android/camera_device.cpp
> @@ -868,6 +868,8 @@ void CameraDevice::abortRequest(Camera3RequestDescriptor *descriptor) const
>  		buffer.status = StreamBuffer::Status::Error;
>
>  	descriptor->status_ = Camera3RequestDescriptor::Status::Error;
> +
> +	sendCaptureResult(descriptor);
>  }
>
>  bool CameraDevice::isValidRequest(camera3_capture_request_t *camera3Request) const
> @@ -1136,14 +1138,7 @@ int CameraDevice::processCaptureRequest(camera3_capture_request_t *camera3Reques
>  	MutexLocker stateLock(stateMutex_);
>
>  	if (state_ == State::Flushing) {
> -		Camera3RequestDescriptor *rawDescriptor = descriptor.get();
> -		{
> -			MutexLocker descriptorsLock(descriptorsMutex_);
> -			descriptors_.push(std::move(descriptor));
> -		}
> -		abortRequest(rawDescriptor);
> -		completeDescriptor(rawDescriptor);
> -
> +		abortRequest(descriptor.get());

I'm not sure I get this change ?

Specifically why we don't complete the descriptor anymore..


>  		return 0;
>  	}
>
> @@ -1211,10 +1206,7 @@ void CameraDevice::requestComplete(Request *request)
>  				<< " not successfully completed: "
>  				<< request->status();
>
> -		abortRequest(descriptor);
> -		completeDescriptor(descriptor);
> -
> -		return;
> +		descriptor->status_ = Camera3RequestDescriptor::Status::Error;
>  	}
>
>  	/*
> @@ -1239,7 +1231,7 @@ void CameraDevice::requestComplete(Request *request)
>  	 */
>  	descriptor->resultMetadata_ = getResultMetadata(*descriptor);
>  	if (!descriptor->resultMetadata_) {
> -		notifyError(descriptor->frameNumber_, nullptr, CAMERA3_MSG_ERROR_RESULT);
> +		descriptor->status_ = Camera3RequestDescriptor::Status::Error;
>
>  		/*
>  		 * The camera framework expects an empty metadata pack on error.
> @@ -1325,6 +1317,16 @@ void CameraDevice::sendCaptureResults()
>  		descriptors_.pop();
>
>  		sendCaptureResult(descriptor.get());
> +
> +		/*
> +		 * Call notify with CAMERA3_MSG_ERROR_RESULT to indicate some
> +		 * of the expected result metadata might not be available
> +		 * because the capture is cancelled by the camera. Only notify
> +		 * it when the final result is sent, since Android will ignore
> +		 * the following metadata.
> +		 */
> +		if (descriptor->status_ == Camera3RequestDescriptor::Status::Error)
> +			notifyError(descriptor->frameNumber_, nullptr, CAMERA3_MSG_ERROR_RESULT);
>  	}
>  }
>
> --
> 2.47.0.338.g60cca15819-goog
>
Cheng-Hao Yang Nov. 28, 2024, 3:30 p.m. UTC | #2 
Hi Jacopo,

On Thu, Nov 28, 2024 at 11:02 PM Jacopo Mondi
<jacopo.mondi@ideasonboard.com> wrote:
>
> Hi Harvey
>
> On Wed, Nov 27, 2024 at 09:25:56AM +0000, Harvey Yang wrote:
> > When a request is completed with failure, CAMERA3_MSG_ERROR_RESULT
> > should be used instead.
> >
> > This patch also cleans up aborting when flushing with
> > sendCaptureResult().
> >
> > Signed-off-by: Han-Lin Chen <hanlinchen@chromium.org>
> > Co-developed-by: Harvey Yang <chenghaoyang@chromium.org>
> > Signed-off-by: Harvey Yang <chenghaoyang@chromium.org>
> > ---
> >  src/android/camera_device.cpp | 28 +++++++++++++++-------------
> >  1 file changed, 15 insertions(+), 13 deletions(-)
> >
> > diff --git a/src/android/camera_device.cpp b/src/android/camera_device.cpp
> > index 0377cf215..3fb92268e 100644
> > --- a/src/android/camera_device.cpp
> > +++ b/src/android/camera_device.cpp
> > @@ -868,6 +868,8 @@ void CameraDevice::abortRequest(Camera3RequestDescriptor *descriptor) const
> >               buffer.status = StreamBuffer::Status::Error;
> >
> >       descriptor->status_ = Camera3RequestDescriptor::Status::Error;
> > +
> > +     sendCaptureResult(descriptor);
> >  }
> >
> >  bool CameraDevice::isValidRequest(camera3_capture_request_t *camera3Request) const
> > @@ -1136,14 +1138,7 @@ int CameraDevice::processCaptureRequest(camera3_capture_request_t *camera3Reques
> >       MutexLocker stateLock(stateMutex_);
> >
> >       if (state_ == State::Flushing) {
> > -             Camera3RequestDescriptor *rawDescriptor = descriptor.get();
> > -             {
> > -                     MutexLocker descriptorsLock(descriptorsMutex_);
> > -                     descriptors_.push(std::move(descriptor));
> > -             }
> > -             abortRequest(rawDescriptor);
> > -             completeDescriptor(rawDescriptor);
> > -
> > +             abortRequest(descriptor.get());
>
> I'm not sure I get this change ?
>
> Specifically why we don't complete the descriptor anymore..

Okay, apart from the fix of CAMERA3_MSG_ERROR_RESULT
instead of CAMERA3_MSG_ERROR_REQUEST, this patch
contains one more thing that is only briefly mentioned in the
commit message:

When flushing and aborting new requests, we should notify
CAMERA3_MSG_ERROR_REQUEST, and call
`process_capture_result` earlier, instead of waiting for the
previous requests being completed first.

`completeDescriptor` would trigger the in-order completion
of requests only, while in this patch, we call
`sendCaptureResult` directly, without the necessity to
set `complete_` anymore.

Hope it helps explain.

BR,
Harvey

>
>
> >               return 0;
> >       }
> >
> > @@ -1211,10 +1206,7 @@ void CameraDevice::requestComplete(Request *request)
> >                               << " not successfully completed: "
> >                               << request->status();
> >
> > -             abortRequest(descriptor);
> > -             completeDescriptor(descriptor);
> > -
> > -             return;
> > +             descriptor->status_ = Camera3RequestDescriptor::Status::Error;
> >       }
> >
> >       /*
> > @@ -1239,7 +1231,7 @@ void CameraDevice::requestComplete(Request *request)
> >        */
> >       descriptor->resultMetadata_ = getResultMetadata(*descriptor);
> >       if (!descriptor->resultMetadata_) {
> > -             notifyError(descriptor->frameNumber_, nullptr, CAMERA3_MSG_ERROR_RESULT);
> > +             descriptor->status_ = Camera3RequestDescriptor::Status::Error;
> >
> >               /*
> >                * The camera framework expects an empty metadata pack on error.
> > @@ -1325,6 +1317,16 @@ void CameraDevice::sendCaptureResults()
> >               descriptors_.pop();
> >
> >               sendCaptureResult(descriptor.get());
> > +
> > +             /*
> > +              * Call notify with CAMERA3_MSG_ERROR_RESULT to indicate some
> > +              * of the expected result metadata might not be available
> > +              * because the capture is cancelled by the camera. Only notify
> > +              * it when the final result is sent, since Android will ignore
> > +              * the following metadata.
> > +              */
> > +             if (descriptor->status_ == Camera3RequestDescriptor::Status::Error)
> > +                     notifyError(descriptor->frameNumber_, nullptr, CAMERA3_MSG_ERROR_RESULT);
> >       }
> >  }
> >
> > --
> > 2.47.0.338.g60cca15819-goog
> >
Jacopo Mondi Dec. 2, 2024, 4:17 p.m. UTC | #3 
Hi Harvey

On Thu, Nov 28, 2024 at 11:30:15PM +0800, Cheng-Hao Yang wrote:
> Hi Jacopo,
>
> On Thu, Nov 28, 2024 at 11:02 PM Jacopo Mondi
> <jacopo.mondi@ideasonboard.com> wrote:
> >
> > Hi Harvey
> >
> > On Wed, Nov 27, 2024 at 09:25:56AM +0000, Harvey Yang wrote:
> > > When a request is completed with failure, CAMERA3_MSG_ERROR_RESULT
> > > should be used instead.
> > >
> > > This patch also cleans up aborting when flushing with
> > > sendCaptureResult().
> > >
> > > Signed-off-by: Han-Lin Chen <hanlinchen@chromium.org>
> > > Co-developed-by: Harvey Yang <chenghaoyang@chromium.org>
> > > Signed-off-by: Harvey Yang <chenghaoyang@chromium.org>
> > > ---
> > >  src/android/camera_device.cpp | 28 +++++++++++++++-------------
> > >  1 file changed, 15 insertions(+), 13 deletions(-)
> > >
> > > diff --git a/src/android/camera_device.cpp b/src/android/camera_device.cpp
> > > index 0377cf215..3fb92268e 100644
> > > --- a/src/android/camera_device.cpp
> > > +++ b/src/android/camera_device.cpp
> > > @@ -868,6 +868,8 @@ void CameraDevice::abortRequest(Camera3RequestDescriptor *descriptor) const
> > >               buffer.status = StreamBuffer::Status::Error;
> > >
> > >       descriptor->status_ = Camera3RequestDescriptor::Status::Error;
> > > +
> > > +     sendCaptureResult(descriptor);
> > >  }
> > >
> > >  bool CameraDevice::isValidRequest(camera3_capture_request_t *camera3Request) const
> > > @@ -1136,14 +1138,7 @@ int CameraDevice::processCaptureRequest(camera3_capture_request_t *camera3Reques
> > >       MutexLocker stateLock(stateMutex_);
> > >
> > >       if (state_ == State::Flushing) {
> > > -             Camera3RequestDescriptor *rawDescriptor = descriptor.get();
> > > -             {
> > > -                     MutexLocker descriptorsLock(descriptorsMutex_);
> > > -                     descriptors_.push(std::move(descriptor));
> > > -             }
> > > -             abortRequest(rawDescriptor);
> > > -             completeDescriptor(rawDescriptor);
> > > -
> > > +             abortRequest(descriptor.get());
> >
> > I'm not sure I get this change ?
> >
> > Specifically why we don't complete the descriptor anymore..
>
> Okay, apart from the fix of CAMERA3_MSG_ERROR_RESULT
> instead of CAMERA3_MSG_ERROR_REQUEST, this patch

Where did this happen ? In flush ? I read it used to call
abortRequest() that

	notifyError(descriptor->frameNumber_, nullptr, CAMERA3_MSG_ERROR_REQUEST);

and used to call

void CameraDevice::completeDescriptor(Camera3RequestDescriptor *descriptor)
{
	MutexLocker lock(descriptorsMutex_);
	descriptor->complete_ = true;

	sendCaptureResults();
}

So I might have missed what do we gain with this change (as
abortRequest() has a single call place if I'm not mistaken.
Also, this new version won't set

	descriptor->complete_ = true;

as the previous call to completeDescriptor() did.

> contains one more thing that is only briefly mentioned in the
> commit message:
>
> When flushing and aborting new requests, we should notify
> CAMERA3_MSG_ERROR_REQUEST, and call
> `process_capture_result` earlier, instead of waiting for the
> previous requests being completed first.
>
> `completeDescriptor` would trigger the in-order completion
> of requests only, while in this patch, we call
> `sendCaptureResult` directly, without the necessity to
> set `complete_` anymore.
>
> Hope it helps explain.

Ah yes, thanks, so we can't go through completeDescriptor() because it
completes in-order but you need to to go through a direct call to
sendCaptureResult().

Thanks, it helps. I would have helped even more if you have added it
to the commit message, because this

   When a request is completed with failure, CAMERA3_MSG_ERROR_RESULT
    should be used instead.

    This patch also cleans up aborting when flushing with
    sendCaptureResult().

doesn't explain me anything and we had to go through three emails and
me trying to understand what you have done. Spending 5 more minutes to
write a proper commit message would have avoided that.

>
> BR,
> Harvey
>
> >
> >
> > >               return 0;
> > >       }
> > >
> > > @@ -1211,10 +1206,7 @@ void CameraDevice::requestComplete(Request *request)
> > >                               << " not successfully completed: "
> > >                               << request->status();
> > >
> > > -             abortRequest(descriptor);
> > > -             completeDescriptor(descriptor);
> > > -
> > > -             return;
> > > +             descriptor->status_ = Camera3RequestDescriptor::Status::Error;
> > >       }
> > >
> > >       /*
> > > @@ -1239,7 +1231,7 @@ void CameraDevice::requestComplete(Request *request)
> > >        */
> > >       descriptor->resultMetadata_ = getResultMetadata(*descriptor);
> > >       if (!descriptor->resultMetadata_) {
> > > -             notifyError(descriptor->frameNumber_, nullptr, CAMERA3_MSG_ERROR_RESULT);
> > > +             descriptor->status_ = Camera3RequestDescriptor::Status::Error;
> > >
> > >               /*
> > >                * The camera framework expects an empty metadata pack on error.
> > > @@ -1325,6 +1317,16 @@ void CameraDevice::sendCaptureResults()
> > >               descriptors_.pop();
> > >
> > >               sendCaptureResult(descriptor.get());
> > > +
> > > +             /*
> > > +              * Call notify with CAMERA3_MSG_ERROR_RESULT to indicate some
> > > +              * of the expected result metadata might not be available
> > > +              * because the capture is cancelled by the camera. Only notify
> > > +              * it when the final result is sent, since Android will ignore
> > > +              * the following metadata.
> > > +              */
> > > +             if (descriptor->status_ == Camera3RequestDescriptor::Status::Error)
> > > +                     notifyError(descriptor->frameNumber_, nullptr, CAMERA3_MSG_ERROR_RESULT);

Is this part of the patch related to the above ? What happens here ?
Why do you delay the call to notifyError() ?

> > >       }
> > >  }
> > >
> > > --
> > > 2.47.0.338.g60cca15819-goog
> > >
Cheng-Hao Yang Dec. 3, 2024, 10:21 a.m. UTC | #4 
Hi Jacopo,

On Tue, Dec 3, 2024 at 12:17 AM Jacopo Mondi
<jacopo.mondi@ideasonboard.com> wrote:
>
> Hi Harvey
>
> On Thu, Nov 28, 2024 at 11:30:15PM +0800, Cheng-Hao Yang wrote:
> > Hi Jacopo,
> >
> > On Thu, Nov 28, 2024 at 11:02 PM Jacopo Mondi
> > <jacopo.mondi@ideasonboard.com> wrote:
> > >
> > > Hi Harvey
> > >
> > > On Wed, Nov 27, 2024 at 09:25:56AM +0000, Harvey Yang wrote:
> > > > When a request is completed with failure, CAMERA3_MSG_ERROR_RESULT
> > > > should be used instead.
> > > >
> > > > This patch also cleans up aborting when flushing with
> > > > sendCaptureResult().
> > > >
> > > > Signed-off-by: Han-Lin Chen <hanlinchen@chromium.org>
> > > > Co-developed-by: Harvey Yang <chenghaoyang@chromium.org>
> > > > Signed-off-by: Harvey Yang <chenghaoyang@chromium.org>
> > > > ---
> > > >  src/android/camera_device.cpp | 28 +++++++++++++++-------------
> > > >  1 file changed, 15 insertions(+), 13 deletions(-)
> > > >
> > > > diff --git a/src/android/camera_device.cpp b/src/android/camera_device.cpp
> > > > index 0377cf215..3fb92268e 100644
> > > > --- a/src/android/camera_device.cpp
> > > > +++ b/src/android/camera_device.cpp
> > > > @@ -868,6 +868,8 @@ void CameraDevice::abortRequest(Camera3RequestDescriptor *descriptor) const
> > > >               buffer.status = StreamBuffer::Status::Error;
> > > >
> > > >       descriptor->status_ = Camera3RequestDescriptor::Status::Error;
> > > > +
> > > > +     sendCaptureResult(descriptor);
> > > >  }
> > > >
> > > >  bool CameraDevice::isValidRequest(camera3_capture_request_t *camera3Request) const
> > > > @@ -1136,14 +1138,7 @@ int CameraDevice::processCaptureRequest(camera3_capture_request_t *camera3Reques
> > > >       MutexLocker stateLock(stateMutex_);
> > > >
> > > >       if (state_ == State::Flushing) {
> > > > -             Camera3RequestDescriptor *rawDescriptor = descriptor.get();
> > > > -             {
> > > > -                     MutexLocker descriptorsLock(descriptorsMutex_);
> > > > -                     descriptors_.push(std::move(descriptor));
> > > > -             }
> > > > -             abortRequest(rawDescriptor);
> > > > -             completeDescriptor(rawDescriptor);
> > > > -
> > > > +             abortRequest(descriptor.get());
> > >
> > > I'm not sure I get this change ?
> > >
> > > Specifically why we don't complete the descriptor anymore..
> >
> > Okay, apart from the fix of CAMERA3_MSG_ERROR_RESULT
> > instead of CAMERA3_MSG_ERROR_REQUEST, this patch
>
> Where did this happen ? In flush ? I read it used to call
> abortRequest() that
>
>         notifyError(descriptor->frameNumber_, nullptr, CAMERA3_MSG_ERROR_REQUEST);
>
> and used to call
>
> void CameraDevice::completeDescriptor(Camera3RequestDescriptor *descriptor)
> {
>         MutexLocker lock(descriptorsMutex_);
>         descriptor->complete_ = true;
>
>         sendCaptureResults();
> }
>
> So I might have missed what do we gain with this change (as
> abortRequest() has a single call place if I'm not mistaken.
> Also, this new version won't set
>
>         descriptor->complete_ = true;
>
> as the previous call to completeDescriptor() did.

Seems that you got most of the ideas, regarding your following
comments? Let me know if you still have questions :)

>
> > contains one more thing that is only briefly mentioned in the
> > commit message:
> >
> > When flushing and aborting new requests, we should notify
> > CAMERA3_MSG_ERROR_REQUEST, and call
> > `process_capture_result` earlier, instead of waiting for the
> > previous requests being completed first.
> >
> > `completeDescriptor` would trigger the in-order completion
> > of requests only, while in this patch, we call
> > `sendCaptureResult` directly, without the necessity to
> > set `complete_` anymore.
> >
> > Hope it helps explain.
>
> Ah yes, thanks, so we can't go through completeDescriptor() because it
> completes in-order but you need to to go through a direct call to
> sendCaptureResult().
>
> Thanks, it helps. I would have helped even more if you have added it
> to the commit message, because this
>
>    When a request is completed with failure, CAMERA3_MSG_ERROR_RESULT
>     should be used instead.
>
>     This patch also cleans up aborting when flushing with
>     sendCaptureResult().
>
> doesn't explain me anything and we had to go through three emails and
> me trying to understand what you have done. Spending 5 more minutes to
> write a proper commit message would have avoided that.

Yeah sorry, I'll do better next time...

>
> >
> > BR,
> > Harvey
> >
> > >
> > >
> > > >               return 0;
> > > >       }
> > > >
> > > > @@ -1211,10 +1206,7 @@ void CameraDevice::requestComplete(Request *request)
> > > >                               << " not successfully completed: "
> > > >                               << request->status();
> > > >
> > > > -             abortRequest(descriptor);
> > > > -             completeDescriptor(descriptor);
> > > > -
> > > > -             return;
> > > > +             descriptor->status_ = Camera3RequestDescriptor::Status::Error;
> > > >       }
> > > >
> > > >       /*
> > > > @@ -1239,7 +1231,7 @@ void CameraDevice::requestComplete(Request *request)
> > > >        */
> > > >       descriptor->resultMetadata_ = getResultMetadata(*descriptor);
> > > >       if (!descriptor->resultMetadata_) {
> > > > -             notifyError(descriptor->frameNumber_, nullptr, CAMERA3_MSG_ERROR_RESULT);
> > > > +             descriptor->status_ = Camera3RequestDescriptor::Status::Error;
> > > >
> > > >               /*
> > > >                * The camera framework expects an empty metadata pack on error.
> > > > @@ -1325,6 +1317,16 @@ void CameraDevice::sendCaptureResults()
> > > >               descriptors_.pop();
> > > >
> > > >               sendCaptureResult(descriptor.get());
> > > > +
> > > > +             /*
> > > > +              * Call notify with CAMERA3_MSG_ERROR_RESULT to indicate some
> > > > +              * of the expected result metadata might not be available
> > > > +              * because the capture is cancelled by the camera. Only notify
> > > > +              * it when the final result is sent, since Android will ignore
> > > > +              * the following metadata.
> > > > +              */
> > > > +             if (descriptor->status_ == Camera3RequestDescriptor::Status::Error)
> > > > +                     notifyError(descriptor->frameNumber_, nullptr, CAMERA3_MSG_ERROR_RESULT);
>
> Is this part of the patch related to the above ? What happens here ?
> Why do you delay the call to notifyError() ?

Actually there are quite some things to discuss here: We don't think
the way tot is implemented that notifying CAMERA3_MSG_ERROR_RESULT
when `getResultMetadata()` fails make sense, as `getResultMetadata()`
only fails when we fail to allocate memory for CameraMetadata IIUC. In
the last patch, you can find that this logic is removed. We may also
consider removing this logic with an extra patch.

This patch calls CAMERA3_MSG_ERROR_RESULT when the libcamera::Request
was cancelled, which will happen when any buffer was cancelled [1].
It's quite different from CAMERA3_MSG_ERROR_RESULT's description [2]
though, which indicates that there is some/all metadata missing.
Currently we don't check the absence of metadata and report an error,
yet.
My question is: Do you think that simplifying and taking
RequestCancelled as CAMERA3_MSG_ERROR_RESULT makes sense?

We can also not keep it as CAMERA3_MSG_ERROR_REQUEST, because we're
trying to support partial results, and CAMERA3_MSG_ERROR_REQUEST is
currently triggered in signal requestCompleted, and it's for "pending
requests that have not done any processing". Having completed partial
results and CAMERA3_MSG_ERROR_REQUEST doesn't make much sense to me
(although according to the description, it's not restricted as well?
I'm not sure...)

WDYT?

BR,
Harvey

[1]: https://git.libcamera.org/libcamera/libcamera.git/tree/src/libcamera/request.cpp#n108
[2]: https://source.android.com/reference/hal/structcamera3__device__ops

>
> > > >       }
> > > >  }
> > > >
> > > > --
> > > > 2.47.0.338.g60cca15819-goog
> > > >

Patch
diff mbox series 

diff --git a/src/android/camera_device.cpp b/src/android/camera_device.cpp
index 0377cf215..3fb92268e 100644
--- a/src/android/camera_device.cpp
+++ b/src/android/camera_device.cpp
@@ -868,6 +868,8 @@  void CameraDevice::abortRequest(Camera3RequestDescriptor *descriptor) const
 		buffer.status = StreamBuffer::Status::Error;
 
 	descriptor->status_ = Camera3RequestDescriptor::Status::Error;
+
+	sendCaptureResult(descriptor);
 }
 
 bool CameraDevice::isValidRequest(camera3_capture_request_t *camera3Request) const
@@ -1136,14 +1138,7 @@  int CameraDevice::processCaptureRequest(camera3_capture_request_t *camera3Reques
 	MutexLocker stateLock(stateMutex_);
 
 	if (state_ == State::Flushing) {
-		Camera3RequestDescriptor *rawDescriptor = descriptor.get();
-		{
-			MutexLocker descriptorsLock(descriptorsMutex_);
-			descriptors_.push(std::move(descriptor));
-		}
-		abortRequest(rawDescriptor);
-		completeDescriptor(rawDescriptor);
-
+		abortRequest(descriptor.get());
 		return 0;
 	}
 
@@ -1211,10 +1206,7 @@  void CameraDevice::requestComplete(Request *request)
 				<< " not successfully completed: "
 				<< request->status();
 
-		abortRequest(descriptor);
-		completeDescriptor(descriptor);
-
-		return;
+		descriptor->status_ = Camera3RequestDescriptor::Status::Error;
 	}
 
 	/*
@@ -1239,7 +1231,7 @@  void CameraDevice::requestComplete(Request *request)
 	 */
 	descriptor->resultMetadata_ = getResultMetadata(*descriptor);
 	if (!descriptor->resultMetadata_) {
-		notifyError(descriptor->frameNumber_, nullptr, CAMERA3_MSG_ERROR_RESULT);
+		descriptor->status_ = Camera3RequestDescriptor::Status::Error;
 
 		/*
 		 * The camera framework expects an empty metadata pack on error.
@@ -1325,6 +1317,16 @@  void CameraDevice::sendCaptureResults()
 		descriptors_.pop();
 
 		sendCaptureResult(descriptor.get());
+
+		/*
+		 * Call notify with CAMERA3_MSG_ERROR_RESULT to indicate some
+		 * of the expected result metadata might not be available
+		 * because the capture is cancelled by the camera. Only notify
+		 * it when the final result is sent, since Android will ignore
+		 * the following metadata.
+		 */
+		if (descriptor->status_ == Camera3RequestDescriptor::Status::Error)
+			notifyError(descriptor->frameNumber_, nullptr, CAMERA3_MSG_ERROR_RESULT);
 	}
 }