<div dir="ltr"><div dir="ltr">Hi Jacopo, thank you for the work.</div><br><div class="gmail_quote"><div dir="ltr" class="gmail_attr">On Fri, May 28, 2021 at 4:33 PM Niklas Söderlund <<a href="mailto:niklas.soderlund@ragnatech.se">niklas.soderlund@ragnatech.se</a>> wrote:<br></div><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left:1px solid rgb(204,204,204);padding-left:1ex">Hi Jacopo,<br>
<br>
Thanks for your work.<br>
<br>
On 2021-05-28 00:03:59 +0200, Jacopo Mondi wrote:<br>
> Implement the flush() camera operation in the CameraDevice class<br>
> and make it available to the camera framework by implementing the<br>
> operation wrapper in camera_ops.cpp.<br>
> <br>
> Introduce a new camera state State::Flushing to handle concurrent<br>
> flush() and process_capture_request() calls.<br>
> <br>
> As flush() can race with processCaptureRequest() protect it<br>
> by introducing a new State::Flushing state that<br>
> processCaptureRequest() inspects before queuing the Request to the<br>
> Camera. If flush() is in progress while processCaptureRequest() is<br>
> called, return the current Request immediately in error state. If<br>
> flush() has completed and a new call to processCaptureRequest() is<br>
> made just after, start the camera again before queuing the request.<br>
> <br>
> Signed-off-by: Jacopo Mondi <<a href="mailto:jacopo@jmondi.org" target="_blank">jacopo@jmondi.org</a>><br>
<br>
Reviewed-by: Niklas Söderlund <<a href="mailto:niklas.soderlund@ragnatech.se" target="_blank">niklas.soderlund@ragnatech.se</a>><br>
<br></blockquote><div><br></div><div>Reviewed-by: Hirokazu Honda <<a href="mailto:hiroh@chromium.org">hiroh@chromium.org</a>></div><div> </div><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left:1px solid rgb(204,204,204);padding-left:1ex">
> ---<br>
> src/android/camera_device.cpp | 74 ++++++++++++++++++++++++++++++++++-<br>
> src/android/camera_device.h | 3 ++<br>
> src/android/camera_ops.cpp | 8 +++-<br>
> 3 files changed, 83 insertions(+), 2 deletions(-)<br>
> <br>
> diff --git a/src/android/camera_device.cpp b/src/android/camera_device.cpp<br>
> index a20c3eaa0ff6..6a8d4d4d5f76 100644<br>
> --- a/src/android/camera_device.cpp<br>
> +++ b/src/android/camera_device.cpp<br>
> @@ -797,6 +797,23 @@ void CameraDevice::close()<br>
> camera_->release();<br>
> }<br>
> <br>
> +void CameraDevice::flush()<br>
> +{<br>
> + {<br>
> + MutexLocker stateLock(stateMutex_);<br>
> + if (state_ != State::Running)<br>
> + return;<br>
> +<br>
> + state_ = State::Flushing;<br>
> + }<br>
> +<br>
> + worker_.stop();<br>
> + camera_->stop();<br>
> +<br>
> + MutexLocker stateLock(stateMutex_);<br>
> + state_ = State::Stopped;<br>
> +}<br>
> +<br>
> void CameraDevice::stop()<br>
> {<br>
> MutexLocker stateLock(stateMutex_);<br>
> @@ -1894,15 +1911,46 @@ int CameraDevice::processControls(Camera3RequestDescriptor *descriptor)<br>
> return 0;<br>
> }<br>
> <br>
> +void CameraDevice::abortRequest(camera3_capture_request_t *request)<br>
> +{<br>
> + notifyError(request->frame_number, nullptr, CAMERA3_MSG_ERROR_REQUEST);<br>
> +<br>
> + camera3_capture_result_t result = {};<br>
> + result.num_output_buffers = request->num_output_buffers;<br>
> + result.frame_number = request->frame_number;<br>
> + result.partial_result = 0;<br>
> +<br>
> + std::vector<camera3_stream_buffer_t> resultBuffers(result.num_output_buffers);<br>
> + for (auto [i, buffer] : utils::enumerate(resultBuffers)) {<br>
> + buffer = request->output_buffers[i];<br>
> + buffer.release_fence = request->output_buffers[i].acquire_fence;<br>
> + buffer.acquire_fence = -1;<br>
> + buffer.status = CAMERA3_BUFFER_STATUS_ERROR;<br>
> + }<br>
> + result.output_buffers = resultBuffers.data();<br>
> +<br>
> + callbacks_->process_capture_result(callbacks_, &result);<br>
> +}<br>
> +<br>
> int CameraDevice::processCaptureRequest(camera3_capture_request_t *camera3Request)<br>
> {<br>
> if (!isValidRequest(camera3Request))<br>
> return -EINVAL;<br>
> <br>
> {<br>
> + /*<br>
> + * Start the camera if that's the first request we handle after<br>
> + * a configuration or after a flush.<br>
> + *<br>
> + * If flush is in progress, return the pending request<br>
> + * immediately in error state.<br>
> + */<br>
> MutexLocker stateLock(stateMutex_);<br>
> + if (state_ == State::Flushing) {<br>
> + abortRequest(camera3Request);<br>
> + return 0;<br>
> + }<br>
> <br>
> - /* Start the camera if that's the first request we handle. */<br>
> if (state_ == State::Stopped) {<br>
> worker_.start();<br>
> <br>
> @@ -2004,6 +2052,30 @@ int CameraDevice::processCaptureRequest(camera3_capture_request_t *camera3Reques<br>
> if (ret)<br>
> return ret;<br>
> <br>
> + /*<br>
> + * Just before queuing the request, make sure flush() has not<br>
> + * been called while this function was running. If flush is in progress<br>
> + * abort the request. If flush has completed and has stopped the camera<br>
> + * we have to re-start it to be able to process the request.<br>
> + */<br>
> + MutexLocker stateLock(stateMutex_);<br>
> + if (state_ == State::Flushing) {<br>
> + abortRequest(camera3Request);<br>
> + return 0;<br>
> + }<br>
> +<br>
> + if (state_ == State::Stopped) {<br>
> + worker_.start();<br>
> +<br>
> + ret = camera_->start();<br>
> + if (ret) {<br>
> + LOG(HAL, Error) << "Failed to start camera";<br>
> + return ret;<br>
> + }<br>
> +<br>
> + state_ = State::Running;<br>
> + }<br>
> +<br>
> worker_.queueRequest(descriptor.request_.get());<br>
> <br>
> {<br>
> diff --git a/src/android/camera_device.h b/src/android/camera_device.h<br>
> index c949fa509ca4..4aadb27c562c 100644<br>
> --- a/src/android/camera_device.h<br>
> +++ b/src/android/camera_device.h<br>
> @@ -43,6 +43,7 @@ public:<br>
> <br>
> int open(const hw_module_t *hardwareModule);<br>
> void close();<br>
> + void flush();<br>
> <br>
> unsigned int id() const { return id_; }<br>
> camera3_device_t *camera3Device() { return &camera3Device_; }<br>
> @@ -92,6 +93,7 @@ private:<br>
> <br>
> enum class State {<br>
> Stopped,<br>
> + Flushing,<br>
> Running,<br>
> };<br>
> <br>
> @@ -106,6 +108,7 @@ private:<br>
> getRawResolutions(const libcamera::PixelFormat &pixelFormat);<br>
> <br>
> libcamera::FrameBuffer *createFrameBuffer(const buffer_handle_t camera3buffer);<br>
> + void abortRequest(camera3_capture_request_t *request);<br>
> void notifyShutter(uint32_t frameNumber, uint64_t timestamp);<br>
> void notifyError(uint32_t frameNumber, camera3_stream_t *stream,<br>
> camera3_error_msg_code code);<br>
> diff --git a/src/android/camera_ops.cpp b/src/android/camera_ops.cpp<br>
> index 696e80436821..8a3cfa175ff5 100644<br>
> --- a/src/android/camera_ops.cpp<br>
> +++ b/src/android/camera_ops.cpp<br>
> @@ -66,8 +66,14 @@ static void hal_dev_dump([[maybe_unused]] const struct camera3_device *dev,<br>
> {<br>
> }<br>
> <br>
> -static int hal_dev_flush([[maybe_unused]] const struct camera3_device *dev)<br>
> +static int hal_dev_flush(const struct camera3_device *dev)<br>
> {<br>
> + if (!dev)<br>
> + return -EINVAL;<br>
> +<br>
> + CameraDevice *camera = reinterpret_cast<CameraDevice *>(dev->priv);<br>
> + camera->flush();<br>
> +<br>
> return 0;<br>
> }<br>
> <br>
> -- <br>
> 2.31.1<br>
> <br>
<br>
-- <br>
Regards,<br>
Niklas Söderlund<br>
</blockquote></div></div>