[PATCH v3 6/7] android: notify CAMERA3_MSG_ERROR_REQUEST out of order

[Jacopo Mondi]

On Wed, Dec 04, 2024 at 04:36:31PM +0000, Harvey Yang wrote:
> When a request hasn't done any processing, CAMERA3_MSG_ERROR_REQUEST and
> the following process_capture_result don't need to wait for the previous
> requests' completion. This patch avoids pushing the aborted requests
> into the request queue.
>
> Signed-off-by: Han-Lin Chen <hanlinchen at chromium.org>
> Co-developed-by: Harvey Yang <chenghaoyang at chromium.org>
> Signed-off-by: Harvey Yang <chenghaoyang at chromium.org>
> ---
>  src/android/camera_device.cpp | 11 +++--------
>  1 file changed, 3 insertions(+), 8 deletions(-)
>
> diff --git a/src/android/camera_device.cpp b/src/android/camera_device.cpp
> index dd2c603e0..18e974f56 100644
> --- a/src/android/camera_device.cpp
> +++ b/src/android/camera_device.cpp
> @@ -867,6 +867,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());

Right, if we don't have to complete in order this indeed seems
correct.

However, we fall in this case, right ?

     * 2. For pending requests that have not done any processing, the HAL must call notify
     *    CAMERA3_MSG_ERROR_REQUEST, and return all the output buffers with
     *    process_capture_result in the error state (CAMERA3_BUFFER_STATUS_ERROR).
     *    The HAL must not place the release fence into an error state, instead,
     *    the release fences must be set to the acquire fences passed by the framework,
     *    or -1 if they have been waited on by the HAL already. This is also the path
     *    to follow for any captures for which the HAL already called notify() with
     *    CAMERA3_MSG_SHUTTER but won't be producing any metadata/valid buffers for.
     *    After CAMERA3_MSG_ERROR_REQUEST, for a given frame, only process_capture_results with
     *    buffers in CAMERA3_BUFFER_STATUS_ERROR are allowed. No further notifys or
     *    process_capture_result with non-null metadata is allowed.

Apparently we're not handling fences here. Not your fault, seems like
it wasn't there already. While at it could you add a \todo or, if you
feel like it's worth fixing it, pile up a patch ?

The patch itself seems good to me
Reviewed-by: Jacopo Mondi <jacopo.mondi at ideasonboard.com>

>  		return 0;
>  	}
>
> --
> 2.47.0.338.g60cca15819-goog
>