[{"id":16978,"web_url":"https://patchwork.libcamera.org/comment/16978/","msgid":"","date":"2021-05-17T05:51:45","subject":"Re: [libcamera-devel] [PATCH v2 8/8] android: Implement flush()\n\tcamera operation","submitter":{"id":63,"url":"https://patchwork.libcamera.org/api/people/63/","name":"Hirokazu Honda","email":"hiroh@chromium.org"},"content":"Hi Jacopo, thank you for the patch.\n\n\n\nOn Thu, May 13, 2021 at 6:22 PM Jacopo Mondi wrote:\n>\n> Implement the flush() camera operation in the CameraDevice class\n> and make it available to the camera framework by implementing the\n> operation wrapper in camera_ops.cpp.\n>\n> The flush() implementation stops the Camera and the worker thread and\n> waits for all in-flight requests to be returned. Stopping the Camera\n> forces all Requests already queued to be returned immediately in error\n> state. As flush() has to wait until all of them have been returned, make\nit\n> wait on a newly introduced condition variable which is notified by the\n> request completion handler when the queue of pending requests has been\n> exhausted.\n>\n> As flush() can race with processCaptureRequest() protect the requests\n> queueing by introducing a new CameraState::CameraFlushing state that\n> processCaptureRequest() inspects before queuing the Request to the\n> Camera. If flush() has been called while processCaptureRequest() was\n> executing, return the current Request immediately in error state.\n>\n> Signed-off-by: Jacopo Mondi \n> ---\n> src/android/camera_device.cpp | 91 +++++++++++++++++++++++++++++++++--\n> src/android/camera_device.h | 9 +++-\n> src/android/camera_ops.cpp | 8 ++-\n> 3 files changed, 102 insertions(+), 6 deletions(-)\n>\n> diff --git a/src/android/camera_device.cpp b/src/android/camera_device.cpp\n> index 7f8c9bd7832d..6d6730a50ec7 100644\n> --- a/src/android/camera_device.cpp\n> +++ b/src/android/camera_device.cpp\n> @@ -750,16 +750,64 @@ int CameraDevice::open(const hw_module_t\n*hardwareModule)\n>\n> void CameraDevice::close()\n> {\n> - streams_.clear();\n> + MutexLocker cameraLock(cameraMutex_);\n> + if (state_ == CameraFlushing) {\n> + MutexLocker flushLock(flushMutex_);\n> + flushed_.wait(flushLock, [&] { return state_ !=\nCameraStopped; });\n> + camera_->release();\n>\n> + return;\n> + }\n> +\n> + streams_.clear();\n> stop();\n>\n> camera_->release();\n> }\n>\n> +/*\n> + * Flush is similar to stop() but sets the camera state to 'flushing'\nand wait\n> + * until all the in-flight requests have been returned before setting the\n> + * camera state to stopped.\n> + *\n> + * Once flushing is done it unlocks concurrent camera close() calls or\nnew\n> + * camera configurations.\n> + */\n> +void CameraDevice::flush()\n> +{\n> + {\n> + MutexLocker cameraLock(cameraMutex_);\n> +\n> + if (state_ != CameraRunning)\n> + return;\n> +\n> + worker_.stop();\n> + camera_->stop();\n> + state_ = CameraFlushing;\n> + }\n> +\n> + /*\n> + * Now wait for all the in-flight requests to be completed before\n> + * continuing. Stopping the Camera guarantees that all in-flight\n> + * requests are completed in error state.\n> + */\n> + {\n> + MutexLocker requestsLock(requestsMutex_);\n> + flushing_.wait(requestsLock, [&] { return\ndescriptors_.empty(); });\n> + }\n> +\n> + /*\n> + * Unlock close() or configureStreams() that might be waiting for\n> + * flush to be completed.\n> + */\n> + MutexLocker flushLocker(flushMutex_);\n\n\nI found state_ is touched without cameraMutex_ here and the condition\nvariable of flushed_.wait().\nI suspect that flushMutex_ is unnecessary and should be replaced with\ncameraMutex_.\n\n\n>\n> + state_ = CameraStopped;\n> + flushed_.notify_one();\n> +}\n> +\n> +/* Calls to stop() must be protected by cameraMutex_ being held by the\ncaller. */\n> void CameraDevice::stop()\n> {\n> - MutexLocker cameraLock(cameraMutex_);\n> if (state_ == CameraStopped)\n> return;\n>\n> @@ -1652,8 +1700,20 @@ PixelFormat CameraDevice::toPixelFormat(int\nformat) const\n> */\n> int CameraDevice::configureStreams(camera3_stream_configuration_t\n*stream_list)\n> {\n> - /* Before any configuration attempt, stop the camera. */\n> - stop();\n> + {\n> + /*\n> + * If a flush is in progress, wait for it to complete and\nto\n> + * stop the camera, otherwise before any new configuration\n> + * attempt we have to stop the camera explictely.\n> + */\n> + MutexLocker cameraLock(cameraMutex_);\n> + if (state_ == CameraFlushing) {\n> + MutexLocker flushLock(flushMutex_);\n> + flushed_.wait(flushLock, [&] { return state_ !=\nCameraStopped; });\n> + } else {\n> + stop();\n> + }\n> + }\n\n\nI wonder if we should hold cameraMutex_ entirely in configureStreams()\nbecause we are touching streams_ and camera_, which are also accessed by\nflush() and stop().\n\n>\n>\n> if (stream_list->num_streams == 0) {\n> LOG(HAL, Error) << \"No streams in configuration\";\n\n\nCaptureRequest, which is constructed through Camera3RequestDescriptor.\nCaptureRequest::queue() calls camera_->queueRequest(). It is necessary to\nmake sure camera_ is valid at that time.\nIt seems to be difficult because it is a different thread and therefore we\ndon't have any idea when it is called.\nAlthough I think https://patchwork.libcamera.org/patch/12089 helps our\nsituation.\n\n>\n> @@ -2021,6 +2081,25 @@ int\nCameraDevice::processCaptureRequest(camera3_capture_request_t *camera3Reques\n> if (ret)\n> return ret;\n>\n> + /*\n> + * Just before queuing the request, make sure flush() has not\n> + * been called after this function has been executed. In that\n> + * case, immediately return the request with errors.\n> + */\n> + MutexLocker cameraLock(cameraMutex_);\n> + if (state_ == CameraFlushing || state_ == CameraStopped) {\n> + for (camera3_stream_buffer_t &buffer :\ndescriptor.buffers_) {\n> + buffer.status = CAMERA3_BUFFER_STATUS_ERROR;\n> + buffer.release_fence = buffer.acquire_fence;\n> + }\n> +\n> + notifyError(descriptor.frameNumber_,\n> + descriptor.buffers_[0].stream,\n> + CAMERA3_MSG_ERROR_REQUEST);\n> +\n> + return 0;\n> + }\n> +\n> worker_.queueRequest(descriptor.request_.get());\n>\n> {\n> @@ -2050,6 +2129,10 @@ void CameraDevice::requestComplete(Request\n*request)\n> return;\n> }\n>\n> + /* Release flush if all the pending requests have been\ncompleted. */\n> + if (descriptors_.empty())\n> + flushing_.notify_one();\n> +\n\nThe flush() document states\n\nflush() should only return when there are no more outstanding buffers or\nrequests left in the HAL.\n\n\nIs it okay that flush() returns before notifyError() and\nprocess_capture_result() is called?\n\n-Hiro\n\n>\n> node = descriptors_.extract(it);\n> }\n> Camera3RequestDescriptor &descriptor = node.mapped();\n> diff --git a/src/android/camera_device.h b/src/android/camera_device.h\n> index ed992ae56d5d..9c55cc2674b7 100644\n> --- a/src/android/camera_device.h\n> +++ b/src/android/camera_device.h\n> @@ -7,6 +7,7 @@\n> #ifndef __ANDROID_CAMERA_DEVICE_H__\n> #define __ANDROID_CAMERA_DEVICE_H__\n>\n> +#include \n> #include \n> #include \n> #include \n> @@ -42,6 +43,7 @@ public:\n>\n> int open(const hw_module_t *hardwareModule);\n> void close();\n> + void flush();\n>\n> unsigned int id() const { return id_; }\n> camera3_device_t *camera3Device() { return &camera3Device_; }\n> @@ -92,6 +94,7 @@ private:\n> enum State {\n> CameraStopped,\n> CameraRunning,\n> + CameraFlushing,\n> };\n>\n> void stop();\n> @@ -124,6 +127,9 @@ private:\n> libcamera::Mutex cameraMutex_; /* Protects access to the camera\nstate. */\n> State state_;\n>\n> + libcamera::Mutex flushMutex_; /* Protects the flushed_ condition\nvariable. */\n> + std::condition_variable flushed_;\n> +\n> std::shared_ptr camera_;\n> std::unique_ptr config_;\n>\n> @@ -135,8 +141,9 @@ private:\n> std::map formatsMap_;\n> std::vector streams_;\n>\n> - libcamera::Mutex requestsMutex_; /* Protects descriptors_. */\n> + libcamera::Mutex requestsMutex_; /* Protects descriptors_ and\nflushing_. */\n> std::map descriptors_;\n> + std::condition_variable flushing_;\n>\n> std::string maker_;\n> std::string model_;\n> diff --git a/src/android/camera_ops.cpp b/src/android/camera_ops.cpp\n> index 696e80436821..8a3cfa175ff5 100644\n> --- a/src/android/camera_ops.cpp\n> +++ b/src/android/camera_ops.cpp\n> @@ -66,8 +66,14 @@ static void hal_dev_dump([[maybe_unused]] const struct\ncamera3_device *dev,\n> {\n> }\n>\n> -static int hal_dev_flush([[maybe_unused]] const struct camera3_device\n*dev)\n> +static int hal_dev_flush(const struct camera3_device *dev)\n> {\n> + if (!dev)\n> + return -EINVAL;\n> +\n> + CameraDevice *camera = reinterpret_cast(dev->priv);\n> + camera->flush();\n> +\n> return 0;\n> }\n>\n> --\n> 2.31.1\n>\n> _______________________________________________\n> libcamera-devel mailing list\n> libcamera-devel@lists.libcamera.org\n> https://lists.libcamera.org/listinfo/libcamera-devel","headers":{"Return-Path":"","X-Original-To":"parsemail@patchwork.libcamera.org","Delivered-To":"parsemail@patchwork.libcamera.org","Received":["from lancelot.ideasonboard.com (lancelot.ideasonboard.com\n\t[92.243.16.209])\n\tby patchwork.libcamera.org (Postfix) with ESMTPS id 48F28C31FC\n\tfor ;\n\tMon, 17 May 2021 05:52:01 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id B521668920;\n\tMon, 17 May 2021 07:52:00 +0200 (CEST)","from mail-ej1-x62c.google.com (mail-ej1-x62c.google.com\n\t[IPv6:2a00:1450:4864:20::62c])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTPS id 2F81A602B1\n\tfor ;\n\tMon, 17 May 2021 07:51:59 +0200 (CEST)","by mail-ej1-x62c.google.com with SMTP id c20so7328743ejm.3\n\tfor ;\n\tSun, 16 May 2021 22:51:59 -0700 (PDT)"],"Authentication-Results":"lancelot.ideasonboard.com; dkim=pass (1024-bit key;\n\tunprotected) header.d=chromium.org header.i=@chromium.org\n\theader.b=\"G5De9fAv\"; dkim-atps=neutral","DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/relaxed; d=chromium.org;\n\ts=google; \n\th=mime-version:references:in-reply-to:from:date:message-id:subject:to\n\t:cc; bh=xAT9yZX/q5yeD9TYwbhzK4183Roe66lGq1inRr2t0bQ=;\n\tb=G5De9fAvUh1eM7LBLxTbFEYfVFWyK6Feuu5bxb2Gw+s1/1neEWzoOCSD/kGaetJdvi\n\tcQQXFOk99da8K29wQ3RLYO4XVo7LXNRVA9JVFfS5M2+vhdMdClPI7FvBGsofw2tU5jds\n\tTEov9Ck3V8b2lc8BifAjy+rmFvCEsEj1IH/rg=","X-Google-DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/relaxed;\n\td=1e100.net; s=20161025;\n\th=x-gm-message-state:mime-version:references:in-reply-to:from:date\n\t:message-id:subject:to:cc;\n\tbh=xAT9yZX/q5yeD9TYwbhzK4183Roe66lGq1inRr2t0bQ=;\n\tb=XEDD+70HEkbg/uUBrwX1EP4WmsD1AwgaJUHmjpuNMlZgA1DRdY2ka8/RSuGjomm9bP\n\tEc2UCyE9E2dKtCLIYFL4EmUMshZWaySroNSk4e6imcgDjeKB/7Joj+C8ptdl/lwJ6P79\n\t72CoWYYun6DnII3cb/Jxcr7UNHjc0VwBl3wu6OMohRHXI3Jhc+CxOKrTxISnjlK5M9pX\n\tvRonV77x62SKWG98Sz/XhWTbiEpxRBUQZMkGjeVQrknnotATQf75RVY3symDCnh/VdS0\n\tSuGdWqMdl1jkt1wA9xr9UdvoJsFCPtBca8CO/HpaaBnlrTEf6/BLxAOPnrjJp9LyhvV5\n\tSfJg==","X-Gm-Message-State":"AOAM530RE32TYbROKI/dwhsCAcDUbMYNUgWPwYYlZK/q7EJz4lSNL1pz\n\tKfYe28A3uZx9nmVHXnV9HBXGjnsOd2QgWTnF3WFWmoLvaYzL/w==","X-Google-Smtp-Source":"ABdhPJyaDlhWYEVs8ERLE7GG7q25V3s6dXdYIEfkjZjIWk8B6coM4LDDBS6w2SNLJAT8d5BNp223lbPnvJReWbvnAPQ=","X-Received":"by 2002:a17:907:1b06:: with SMTP id\n\tmp6mr61189961ejc.292.1621230718811; \n\tSun, 16 May 2021 22:51:58 -0700 (PDT)","MIME-Version":"1.0","References":"<20210513092246.42847-1-jacopo@jmondi.org>\n\t<20210513092246.42847-9-jacopo@jmondi.org>","In-Reply-To":"<20210513092246.42847-9-jacopo@jmondi.org>","From":"Hirokazu Honda ","Date":"Mon, 17 May 2021 14:51:45 +0900","Message-ID":"","To":"Jacopo Mondi ","Content-Type":"multipart/alternative; boundary=\"0000000000001813a205c28031b1\"","Subject":"Re: [libcamera-devel] [PATCH v2 8/8] android: Implement flush()\n\tcamera operation","X-BeenThere":"libcamera-devel@lists.libcamera.org","X-Mailman-Version":"2.1.29","Precedence":"list","List-Id":"","List-Unsubscribe":",\n\t","List-Archive":"","List-Post":"","List-Help":"","List-Subscribe":",\n\t","Cc":"libcamera-devel@lists.libcamera.org","Errors-To":"libcamera-devel-bounces@lists.libcamera.org","Sender":"\"libcamera-devel\" "}},{"id":17041,"web_url":"https://patchwork.libcamera.org/comment/17041/","msgid":"<20210520083019.fejqotf2ct6qmi7k@uno.localdomain>","date":"2021-05-20T08:30:19","subject":"Re: [libcamera-devel] [PATCH v2 8/8] android: Implement flush()\n\tcamera operation","submitter":{"id":3,"url":"https://patchwork.libcamera.org/api/people/3/","name":"Jacopo Mondi","email":"jacopo@jmondi.org"},"content":"Hi Hiro,\n\nOn Mon, May 17, 2021 at 02:51:45PM +0900, Hirokazu Honda wrote:\n> Hi Jacopo, thank you for the patch.\n>\n>\n>\n> On Thu, May 13, 2021 at 6:22 PM Jacopo Mondi wrote:\n> >\n> > Implement the flush() camera operation in the CameraDevice class\n> > and make it available to the camera framework by implementing the\n> > operation wrapper in camera_ops.cpp.\n> >\n> > The flush() implementation stops the Camera and the worker thread and\n> > waits for all in-flight requests to be returned. Stopping the Camera\n> > forces all Requests already queued to be returned immediately in error\n> > state. As flush() has to wait until all of them have been returned, make\n> it\n> > wait on a newly introduced condition variable which is notified by the\n> > request completion handler when the queue of pending requests has been\n> > exhausted.\n> >\n> > As flush() can race with processCaptureRequest() protect the requests\n> > queueing by introducing a new CameraState::CameraFlushing state that\n> > processCaptureRequest() inspects before queuing the Request to the\n> > Camera. If flush() has been called while processCaptureRequest() was\n> > executing, return the current Request immediately in error state.\n> >\n> > Signed-off-by: Jacopo Mondi \n> > ---\n> > src/android/camera_device.cpp | 91 +++++++++++++++++++++++++++++++++--\n> > src/android/camera_device.h | 9 +++-\n> > src/android/camera_ops.cpp | 8 ++-\n> > 3 files changed, 102 insertions(+), 6 deletions(-)\n> >\n> > diff --git a/src/android/camera_device.cpp b/src/android/camera_device.cpp\n> > index 7f8c9bd7832d..6d6730a50ec7 100644\n> > --- a/src/android/camera_device.cpp\n> > +++ b/src/android/camera_device.cpp\n> > @@ -750,16 +750,64 @@ int CameraDevice::open(const hw_module_t\n> *hardwareModule)\n> >\n> > void CameraDevice::close()\n> > {\n> > - streams_.clear();\n> > + MutexLocker cameraLock(cameraMutex_);\n> > + if (state_ == CameraFlushing) {\n> > + MutexLocker flushLock(flushMutex_);\n> > + flushed_.wait(flushLock, [&] { return state_ !=\n> CameraStopped; });\n> > + camera_->release();\n> >\n> > + return;\n> > + }\n> > +\n> > + streams_.clear();\n> > stop();\n> >\n> > camera_->release();\n> > }\n> >\n> > +/*\n> > + * Flush is similar to stop() but sets the camera state to 'flushing'\n> and wait\n> > + * until all the in-flight requests have been returned before setting the\n> > + * camera state to stopped.\n> > + *\n> > + * Once flushing is done it unlocks concurrent camera close() calls or\n> new\n> > + * camera configurations.\n> > + */\n> > +void CameraDevice::flush()\n> > +{\n> > + {\n> > + MutexLocker cameraLock(cameraMutex_);\n> > +\n> > + if (state_ != CameraRunning)\n> > + return;\n> > +\n> > + worker_.stop();\n> > + camera_->stop();\n> > + state_ = CameraFlushing;\n> > + }\n> > +\n> > + /*\n> > + * Now wait for all the in-flight requests to be completed before\n> > + * continuing. Stopping the Camera guarantees that all in-flight\n> > + * requests are completed in error state.\n> > + */\n> > + {\n> > + MutexLocker requestsLock(requestsMutex_);\n> > + flushing_.wait(requestsLock, [&] { return\n> descriptors_.empty(); });\n> > + }\n> > +\n> > + /*\n> > + * Unlock close() or configureStreams() that might be waiting for\n> > + * flush to be completed.\n> > + */\n> > + MutexLocker flushLocker(flushMutex_);\n>\n>\n> I found state_ is touched without cameraMutex_ here and the condition\n> variable of flushed_.wait().\n\nI didn't get the \"and the condition variable...\" part.\n\nHowever, on the first part, I don't think it's a problem, and that's\nmy reasoning: if we get here the camera state is set to flushing\nalready.\n\nWhatever other thread that calls a concurrent close(),\nconfigureStreams() or processCaptureRequest() will find the\nCameraState set to flushing, if flush() gets the CameraMutex_ critical\nsection first. Those threads will see the CameraState as\nCameraFlushing and will wait on the flushed_ condition variable, until\nflush() notifies them here below, after having changed the camera state.\n\nif close() or configureStreams() gets the CameraMutex_ critical\nsection first they will just stop the camera. When mutex gets in the\ncritical section, it will find the camera is not running anymore, and\nwill simply return.\n\nif processCaptureRequest() gets the critical section first, it will\nqueue the request normally. Once flush() enters the critical section\nit will stop the camera and the request will be completed with errors\nas usual.\n\n> I suspect that flushMutex_ is unnecessary and should be replaced with\n> cameraMutex_.\n\nYou might be right.. flushMutex_ usage in close() and\nconfigureStreams() can be replaced with cameraMutex_ easily.\nI'll try this, thanks for the suggestion.\n\n>\n>\n> >\n> > + state_ = CameraStopped;\n> > + flushed_.notify_one();\n> > +}\n> > +\n> > +/* Calls to stop() must be protected by cameraMutex_ being held by the\n> caller. */\n> > void CameraDevice::stop()\n> > {\n> > - MutexLocker cameraLock(cameraMutex_);\n> > if (state_ == CameraStopped)\n> > return;\n> >\n> > @@ -1652,8 +1700,20 @@ PixelFormat CameraDevice::toPixelFormat(int\n> format) const\n> > */\n> > int CameraDevice::configureStreams(camera3_stream_configuration_t\n> *stream_list)\n> > {\n> > - /* Before any configuration attempt, stop the camera. */\n> > - stop();\n> > + {\n> > + /*\n> > + * If a flush is in progress, wait for it to complete and\n> to\n> > + * stop the camera, otherwise before any new configuration\n> > + * attempt we have to stop the camera explictely.\n> > + */\n> > + MutexLocker cameraLock(cameraMutex_);\n> > + if (state_ == CameraFlushing) {\n> > + MutexLocker flushLock(flushMutex_);\n> > + flushed_.wait(flushLock, [&] { return state_ !=\n> CameraStopped; });\n> > + } else {\n> > + stop();\n> > + }\n> > + }\n>\n>\n> I wonder if we should hold cameraMutex_ entirely in configureStreams()\n> because we are touching streams_ and camera_, which are also accessed by\n> flush() and stop().\n>\n\nI refrained from accessing streams_ in flush to avoid locking the\nwhole function, which is quite a poor solution as we're back to\nbasically serialize function calls and that's it. Regarding the camera\nstate, the critical section at the beginning makes sure\nconfigureStreams() runs with the camera in CameraStopped state, in\nwhich case a flush() called during configureStreams() execution will\nbail out immediately, as there are no requests to flush. OTOH if\nconfigureStreams() is called while flush() is executing, it will wait\non the flushed_ condition untile flush() has completed and the camera\nis again in stopped state.\n\n\n> >\n> >\n> > if (stream_list->num_streams == 0) {\n> > LOG(HAL, Error) << \"No streams in configuration\";\n>\n>\n> CaptureRequest, which is constructed through Camera3RequestDescriptor.\n> CaptureRequest::queue() calls camera_->queueRequest(). It is necessary to\n> make sure camera_ is valid at that time.\n\nThe request queueing in processCaptureRequest() is performed while\nhelding the CameraMutex_ to prevent a flush to interrupt it. Did I get\nyour comment wrongly ?\n\n> It seems to be difficult because it is a different thread and therefore we\n> don't have any idea when it is called.\n> Although I think https://patchwork.libcamera.org/patch/12089 helps our\n> situation.\n\nAh you meant what happens to requests that have not been yet queued on\nthe Camera as they worker is waiting on their fences ? yes, I think\nstopping the worker_ should return the request immediately. I missed\nthat patch, I'll give it a go.\n\nOverall, my understanding is that trying to remove flushMutex_ and use\ncameraMutex_ in its place should be enough to solve your concerns ?\n\nThanks\n j\n\n>\n> >\n> > @@ -2021,6 +2081,25 @@ int\n> CameraDevice::processCaptureRequest(camera3_capture_request_t *camera3Reques\n> > if (ret)\n> > return ret;\n> >\n> > + /*\n> > + * Just before queuing the request, make sure flush() has not\n> > + * been called after this function has been executed. In that\n> > + * case, immediately return the request with errors.\n> > + */\n> > + MutexLocker cameraLock(cameraMutex_);\n> > + if (state_ == CameraFlushing || state_ == CameraStopped) {\n> > + for (camera3_stream_buffer_t &buffer :\n> descriptor.buffers_) {\n> > + buffer.status = CAMERA3_BUFFER_STATUS_ERROR;\n> > + buffer.release_fence = buffer.acquire_fence;\n> > + }\n> > +\n> > + notifyError(descriptor.frameNumber_,\n> > + descriptor.buffers_[0].stream,\n> > + CAMERA3_MSG_ERROR_REQUEST);\n> > +\n> > + return 0;\n> > + }\n> > +\n> > worker_.queueRequest(descriptor.request_.get());\n> >\n> > {\n> > @@ -2050,6 +2129,10 @@ void CameraDevice::requestComplete(Request\n> *request)\n> > return;\n> > }\n> >\n> > + /* Release flush if all the pending requests have been\n> completed. */\n> > + if (descriptors_.empty())\n> > + flushing_.notify_one();\n> > +\n>\n> The flush() document states\n>\n> flush() should only return when there are no more outstanding buffers or\n> requests left in the HAL.\n>\n>\n> Is it okay that flush() returns before notifyError() and\n> process_capture_result() is called?\n>\n> -Hiro\n>\n> >\n> > node = descriptors_.extract(it);\n> > }\n> > Camera3RequestDescriptor &descriptor = node.mapped();\n> > diff --git a/src/android/camera_device.h b/src/android/camera_device.h\n> > index ed992ae56d5d..9c55cc2674b7 100644\n> > --- a/src/android/camera_device.h\n> > +++ b/src/android/camera_device.h\n> > @@ -7,6 +7,7 @@\n> > #ifndef __ANDROID_CAMERA_DEVICE_H__\n> > #define __ANDROID_CAMERA_DEVICE_H__\n> >\n> > +#include \n> > #include \n> > #include \n> > #include \n> > @@ -42,6 +43,7 @@ public:\n> >\n> > int open(const hw_module_t *hardwareModule);\n> > void close();\n> > + void flush();\n> >\n> > unsigned int id() const { return id_; }\n> > camera3_device_t *camera3Device() { return &camera3Device_; }\n> > @@ -92,6 +94,7 @@ private:\n> > enum State {\n> > CameraStopped,\n> > CameraRunning,\n> > + CameraFlushing,\n> > };\n> >\n> > void stop();\n> > @@ -124,6 +127,9 @@ private:\n> > libcamera::Mutex cameraMutex_; /* Protects access to the camera\n> state. */\n> > State state_;\n> >\n> > + libcamera::Mutex flushMutex_; /* Protects the flushed_ condition\n> variable. */\n> > + std::condition_variable flushed_;\n> > +\n> > std::shared_ptr camera_;\n> > std::unique_ptr config_;\n> >\n> > @@ -135,8 +141,9 @@ private:\n> > std::map formatsMap_;\n> > std::vector streams_;\n> >\n> > - libcamera::Mutex requestsMutex_; /* Protects descriptors_. */\n> > + libcamera::Mutex requestsMutex_; /* Protects descriptors_ and\n> flushing_. */\n> > std::map descriptors_;\n> > + std::condition_variable flushing_;\n> >\n> > std::string maker_;\n> > std::string model_;\n> > diff --git a/src/android/camera_ops.cpp b/src/android/camera_ops.cpp\n> > index 696e80436821..8a3cfa175ff5 100644\n> > --- a/src/android/camera_ops.cpp\n> > +++ b/src/android/camera_ops.cpp\n> > @@ -66,8 +66,14 @@ static void hal_dev_dump([[maybe_unused]] const struct\n> camera3_device *dev,\n> > {\n> > }\n> >\n> > -static int hal_dev_flush([[maybe_unused]] const struct camera3_device\n> *dev)\n> > +static int hal_dev_flush(const struct camera3_device *dev)\n> > {\n> > + if (!dev)\n> > + return -EINVAL;\n> > +\n> > + CameraDevice *camera = reinterpret_cast *>(dev->priv);\n> > + camera->flush();\n> > +\n> > return 0;\n> > }\n> >\n> > --\n> > 2.31.1\n> >\n> > _______________________________________________\n> > libcamera-devel mailing list\n> > libcamera-devel@lists.libcamera.org\n> > https://lists.libcamera.org/listinfo/libcamera-devel","headers":{"Return-Path":"","X-Original-To":"parsemail@patchwork.libcamera.org","Delivered-To":"parsemail@patchwork.libcamera.org","Received":["from lancelot.ideasonboard.com (lancelot.ideasonboard.com\n\t[92.243.16.209])\n\tby patchwork.libcamera.org (Postfix) with ESMTPS id 99A7EC31FF\n\tfor ;\n\tThu, 20 May 2021 08:29:36 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id D45FE6891E;\n\tThu, 20 May 2021 10:29:35 +0200 (CEST)","from relay8-d.mail.gandi.net (relay8-d.mail.gandi.net\n\t[217.70.183.201])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTPS id 1363768919\n\tfor ;\n\tThu, 20 May 2021 10:29:34 +0200 (CEST)","(Authenticated sender: jacopo@jmondi.org)\n\tby relay8-d.mail.gandi.net (Postfix) with ESMTPSA id 73B1E1BF205;\n\tThu, 20 May 2021 08:29:33 +0000 (UTC)"],"Date":"Thu, 20 May 2021 10:30:19 +0200","From":"Jacopo Mondi ","To":"Hirokazu Honda ","Message-ID":"<20210520083019.fejqotf2ct6qmi7k@uno.localdomain>","References":"<20210513092246.42847-1-jacopo@jmondi.org>\n\t<20210513092246.42847-9-jacopo@jmondi.org>\n\t","MIME-Version":"1.0","Content-Type":"text/plain; charset=utf-8","Content-Disposition":"inline","In-Reply-To":"","Subject":"Re: [libcamera-devel] [PATCH v2 8/8] android: Implement flush()\n\tcamera operation","X-BeenThere":"libcamera-devel@lists.libcamera.org","X-Mailman-Version":"2.1.29","Precedence":"list","List-Id":"","List-Unsubscribe":",\n\t","List-Archive":"","List-Post":"","List-Help":"","List-Subscribe":",\n\t","Cc":"libcamera-devel@lists.libcamera.org","Errors-To":"libcamera-devel-bounces@lists.libcamera.org","Sender":"\"libcamera-devel\" "}},{"id":17051,"web_url":"https://patchwork.libcamera.org/comment/17051/","msgid":"","date":"2021-05-21T04:07:26","subject":"Re: [libcamera-devel] [PATCH v2 8/8] android: Implement flush()\n\tcamera operation","submitter":{"id":63,"url":"https://patchwork.libcamera.org/api/people/63/","name":"Hirokazu Honda","email":"hiroh@chromium.org"},"content":"Hi Jacopo,\n\nOn Thu, May 20, 2021 at 5:29 PM Jacopo Mondi wrote:\n\n> Hi Hiro,\n>\n> On Mon, May 17, 2021 at 02:51:45PM +0900, Hirokazu Honda wrote:\n> > Hi Jacopo, thank you for the patch.\n> >\n> >\n> >\n> > On Thu, May 13, 2021 at 6:22 PM Jacopo Mondi wrote:\n> > >\n> > > Implement the flush() camera operation in the CameraDevice class\n> > > and make it available to the camera framework by implementing the\n> > > operation wrapper in camera_ops.cpp.\n> > >\n> > > The flush() implementation stops the Camera and the worker thread and\n> > > waits for all in-flight requests to be returned. Stopping the Camera\n> > > forces all Requests already queued to be returned immediately in error\n> > > state. As flush() has to wait until all of them have been returned,\n> make\n> > it\n> > > wait on a newly introduced condition variable which is notified by the\n> > > request completion handler when the queue of pending requests has been\n> > > exhausted.\n> > >\n> > > As flush() can race with processCaptureRequest() protect the requests\n> > > queueing by introducing a new CameraState::CameraFlushing state that\n> > > processCaptureRequest() inspects before queuing the Request to the\n> > > Camera. If flush() has been called while processCaptureRequest() was\n> > > executing, return the current Request immediately in error state.\n> > >\n> > > Signed-off-by: Jacopo Mondi \n> > > ---\n> > > src/android/camera_device.cpp | 91 +++++++++++++++++++++++++++++++++--\n> > > src/android/camera_device.h | 9 +++-\n> > > src/android/camera_ops.cpp | 8 ++-\n> > > 3 files changed, 102 insertions(+), 6 deletions(-)\n> > >\n> > > diff --git a/src/android/camera_device.cpp\n> b/src/android/camera_device.cpp\n> > > index 7f8c9bd7832d..6d6730a50ec7 100644\n> > > --- a/src/android/camera_device.cpp\n> > > +++ b/src/android/camera_device.cpp\n> > > @@ -750,16 +750,64 @@ int CameraDevice::open(const hw_module_t\n> > *hardwareModule)\n> > >\n> > > void CameraDevice::close()\n> > > {\n> > > - streams_.clear();\n> > > + MutexLocker cameraLock(cameraMutex_);\n> > > + if (state_ == CameraFlushing) {\n> > > + MutexLocker flushLock(flushMutex_);\n> > > + flushed_.wait(flushLock, [&] { return state_ !=\n> > CameraStopped; });\n> > > + camera_->release();\n> > >\n> > > + return;\n> > > + }\n> > > +\n> > > + streams_.clear();\n> > > stop();\n> > >\n> > > camera_->release();\n> > > }\n> > >\n> > > +/*\n> > > + * Flush is similar to stop() but sets the camera state to 'flushing'\n> > and wait\n> > > + * until all the in-flight requests have been returned before setting\n> the\n> > > + * camera state to stopped.\n> > > + *\n> > > + * Once flushing is done it unlocks concurrent camera close() calls or\n> > new\n> > > + * camera configurations.\n> > > + */\n> > > +void CameraDevice::flush()\n> > > +{\n> > > + {\n> > > + MutexLocker cameraLock(cameraMutex_);\n> > > +\n> > > + if (state_ != CameraRunning)\n> > > + return;\n> > > +\n> > > + worker_.stop();\n> > > + camera_->stop();\n> > > + state_ = CameraFlushing;\n> > > + }\n> > > +\n> > > + /*\n> > > + * Now wait for all the in-flight requests to be completed\n> before\n> > > + * continuing. Stopping the Camera guarantees that all\n> in-flight\n> > > + * requests are completed in error state.\n> > > + */\n> > > + {\n> > > + MutexLocker requestsLock(requestsMutex_);\n> > > + flushing_.wait(requestsLock, [&] { return\n> > descriptors_.empty(); });\n> > > + }\n> > > +\n> > > + /*\n> > > + * Unlock close() or configureStreams() that might be waiting\n> for\n> > > + * flush to be completed.\n> > > + */\n> > > + MutexLocker flushLocker(flushMutex_);\n> >\n> >\n> > I found state_ is touched without cameraMutex_ here and the condition\n> > variable of flushed_.wait().\n>\n> I didn't get the \"and the condition variable...\" part.\n>\n>\nProbably I should have said condition block.\nI meant flushed_.wait(..., { HERE }) in configureStreams() by \"the\ncondition variable\" .\n + MutexLocker flushLock(flushMutex_);\n + flushed_.wait(flushLock, [&] { return state_ !=\nCameraStopped; });\n\nThe part is protected by cameraMutex_ but I consider more, by that,\nflushMutex_ is useless.\n\n\n> However, on the first part, I don't think it's a problem, and that's\n> my reasoning: if we get here the camera state is set to flushing\n> already.\n>\n> Whatever other thread that calls a concurrent close(),\n> configureStreams() or processCaptureRequest() will find the\n> CameraState set to flushing, if flush() gets the CameraMutex_ critical\n> section first. Those threads will see the CameraState as\n> CameraFlushing and will wait on the flushed_ condition variable, until\n> flush() notifies them here below, after having changed the camera state.\n>\n> if close() or configureStreams() gets the CameraMutex_ critical\n> section first they will just stop the camera. When mutex gets in the\n> critical section, it will find the camera is not running anymore, and\n> will simply return.\n>\n> if processCaptureRequest() gets the critical section first, it will\n> queue the request normally. Once flush() enters the critical section\n> it will stop the camera and the request will be completed with errors\n> as usual.\n>\n>\nI got it. You're right. But I would not do so as it breaks the rule that\nstate_ is protected by cameraMutex_.\n\n\n> > I suspect that flushMutex_ is unnecessary and should be replaced with\n> > cameraMutex_.\n>\n> You might be right.. flushMutex_ usage in close() and\n> configureStreams() can be replaced with cameraMutex_ easily.\n> I'll try this, thanks for the suggestion.\n>\n> >\n> >\n> > >\n> > > + state_ = CameraStopped;\n> > > + flushed_.notify_one();\n> > > +}\n> > > +\n> > > +/* Calls to stop() must be protected by cameraMutex_ being held by the\n> > caller. */\n> > > void CameraDevice::stop()\n> > > {\n> > > - MutexLocker cameraLock(cameraMutex_);\n>\n\n\nIf I looked the flow correctly, the state is never Flushing here, so let's\nadd\nASSERT(state_ != CameraFlushing);\n\n\n> > > if (state_ == CameraStopped)\n> > > return;\n> > >\n> > > @@ -1652,8 +1700,20 @@ PixelFormat CameraDevice::toPixelFormat(int\n> > format) const\n> > > */\n> > > int CameraDevice::configureStreams(camera3_stream_configuration_t\n> > *stream_list)\n> > > {\n> > > - /* Before any configuration attempt, stop the camera. */\n> > > - stop();\n> > > + {\n> > > + /*\n> > > + * If a flush is in progress, wait for it to complete\n> and\n> > to\n> > > + * stop the camera, otherwise before any new\n> configuration\n> > > + * attempt we have to stop the camera explictely.\n> > > + */\n> > > + MutexLocker cameraLock(cameraMutex_);\n> > > + if (state_ == CameraFlushing) {\n> > > + MutexLocker flushLock(flushMutex_);\n> > > + flushed_.wait(flushLock, [&] { return state_ !=\n> > CameraStopped; });\n> > > + } else {\n> > > + stop();\n> > > + }\n> > > + }\n> >\n> >\n> > I wonder if we should hold cameraMutex_ entirely in configureStreams()\n> > because we are touching streams_ and camera_, which are also accessed by\n> > flush() and stop().\n> >\n>\n> I refrained from accessing streams_ in flush to avoid locking the\n> whole function, which is quite a poor solution as we're back to\n> basically serialize function calls and that's it. Regarding the camera\n> state, the critical section at the beginning makes sure\n> configureStreams() runs with the camera in CameraStopped state, in\n> which case a flush() called during configureStreams() execution will\n> bail out immediately, as there are no requests to flush. OTOH if\n> configureStreams() is called while flush() is executing, it will wait\n> on the flushed_ condition untile flush() has completed and the camera\n> is again in stopped state.\n>\n>\n>\nThis sounds correct.\n\n\n> > >\n> > >\n> > > if (stream_list->num_streams == 0) {\n> > > LOG(HAL, Error) << \"No streams in configuration\";\n> >\n> >\n> > CaptureRequest, which is constructed through Camera3RequestDescriptor.\n> > CaptureRequest::queue() calls camera_->queueRequest(). It is necessary to\n> > make sure camera_ is valid at that time.\n>\n> The request queueing in processCaptureRequest() is performed while\n> helding the CameraMutex_ to prevent a flush to interrupt it. Did I get\n> your comment wrongly ?\n>\n> It seems to be difficult because it is a different thread and therefore we\n> > don't have any idea when it is called.\n> > Although I think https://patchwork.libcamera.org/patch/12089 helps our\n> > situation.\n>\n> Ah you meant what happens to requests that have not been yet queued on\n> the Camera as they worker is waiting on their fences ? yes, I think\n> stopping the worker_ should return the request immediately. I missed\n> that patch, I'll give it a go.\n>\n>\nYes.\nCameraWorker::queueRequest() is\nworker_.InvokeMethod(&processRequest, ConnectionTypeQueued, request);\nIt is not guaranteed that Camera::queueRequest() is called by the end of\nprocessCaptureRequest().\n\n\n> Overall, my understanding is that trying to remove flushMutex_ and use\n> cameraMutex_ in its place should be enough to solve your concerns ?\n>\n>\nYeah, I think the action item is only that.\n\nThanks,\n-Hiro\n\n\n> Thanks\n> j\n>\n> >\n> > >\n> > > @@ -2021,6 +2081,25 @@ int\n> > CameraDevice::processCaptureRequest(camera3_capture_request_t\n> *camera3Reques\n> > > if (ret)\n> > > return ret;\n> > >\n> > > + /*\n> > > + * Just before queuing the request, make sure flush() has not\n> > > + * been called after this function has been executed. In that\n> > > + * case, immediately return the request with errors.\n> > > + */\n> > > + MutexLocker cameraLock(cameraMutex_);\n> > > + if (state_ == CameraFlushing || state_ == CameraStopped) {\n> > > + for (camera3_stream_buffer_t &buffer :\n> > descriptor.buffers_) {\n> > > + buffer.status = CAMERA3_BUFFER_STATUS_ERROR;\n> > > + buffer.release_fence = buffer.acquire_fence;\n> > > + }\n> > > +\n> > > + notifyError(descriptor.frameNumber_,\n> > > + descriptor.buffers_[0].stream,\n> > > + CAMERA3_MSG_ERROR_REQUEST);\n> > > +\n> > > + return 0;\n> > > + }\n> > > +\n> > > worker_.queueRequest(descriptor.request_.get());\n> > >\n> > > {\n> > > @@ -2050,6 +2129,10 @@ void CameraDevice::requestComplete(Request\n> > *request)\n> > > return;\n> > > }\n> > >\n> > > + /* Release flush if all the pending requests have been\n> > completed. */\n> > > + if (descriptors_.empty())\n> > > + flushing_.notify_one();\n> > > +\n> >\n> > The flush() document states\n> >\n> > flush() should only return when there are no more outstanding buffers or\n> > requests left in the HAL.\n> >\n> >\n> > Is it okay that flush() returns before notifyError() and\n> > process_capture_result() is called?\n> >\n> > -Hiro\n> >\n> > >\n> > > node = descriptors_.extract(it);\n> > > }\n> > > Camera3RequestDescriptor &descriptor = node.mapped();\n> > > diff --git a/src/android/camera_device.h b/src/android/camera_device.h\n> > > index ed992ae56d5d..9c55cc2674b7 100644\n> > > --- a/src/android/camera_device.h\n> > > +++ b/src/android/camera_device.h\n> > > @@ -7,6 +7,7 @@\n> > > #ifndef __ANDROID_CAMERA_DEVICE_H__\n> > > #define __ANDROID_CAMERA_DEVICE_H__\n> > >\n> > > +#include \n> > > #include \n> > > #include \n> > > #include \n> > > @@ -42,6 +43,7 @@ public:\n> > >\n> > > int open(const hw_module_t *hardwareModule);\n> > > void close();\n> > > + void flush();\n> > >\n> > > unsigned int id() const { return id_; }\n> > > camera3_device_t *camera3Device() { return &camera3Device_; }\n> > > @@ -92,6 +94,7 @@ private:\n> > > enum State {\n> > > CameraStopped,\n> > > CameraRunning,\n> > > + CameraFlushing,\n> > > };\n> > >\n> > > void stop();\n> > > @@ -124,6 +127,9 @@ private:\n> > > libcamera::Mutex cameraMutex_; /* Protects access to the camera\n> > state. */\n> > > State state_;\n> > >\n> > > + libcamera::Mutex flushMutex_; /* Protects the flushed_\n> condition\n> > variable. */\n> > > + std::condition_variable flushed_;\n> > > +\n> > > std::shared_ptr camera_;\n> > > std::unique_ptr config_;\n> > >\n> > > @@ -135,8 +141,9 @@ private:\n> > > std::map formatsMap_;\n> > > std::vector streams_;\n> > >\n> > > - libcamera::Mutex requestsMutex_; /* Protects descriptors_. */\n> > > + libcamera::Mutex requestsMutex_; /* Protects descriptors_ and\n> > flushing_. */\n> > > std::map descriptors_;\n> > > + std::condition_variable flushing_;\n> > >\n> > > std::string maker_;\n> > > std::string model_;\n> > > diff --git a/src/android/camera_ops.cpp b/src/android/camera_ops.cpp\n> > > index 696e80436821..8a3cfa175ff5 100644\n> > > --- a/src/android/camera_ops.cpp\n> > > +++ b/src/android/camera_ops.cpp\n> > > @@ -66,8 +66,14 @@ static void hal_dev_dump([[maybe_unused]] const\n> struct\n> > camera3_device *dev,\n> > > {\n> > > }\n> > >\n> > > -static int hal_dev_flush([[maybe_unused]] const struct camera3_device\n> > *dev)\n> > > +static int hal_dev_flush(const struct camera3_device *dev)\n> > > {\n> > > + if (!dev)\n> > > + return -EINVAL;\n> > > +\n> > > + CameraDevice *camera = reinterpret_cast > *>(dev->priv);\n> > > + camera->flush();\n> > > +\n> > > return 0;\n> > > }\n> > >\n> > > --\n> > > 2.31.1\n> > >\n> > > _______________________________________________\n> > > libcamera-devel mailing list\n> > > libcamera-devel@lists.libcamera.org\n> > > https://lists.libcamera.org/listinfo/libcamera-devel\n>","headers":{"Return-Path":"","X-Original-To":"parsemail@patchwork.libcamera.org","Delivered-To":"parsemail@patchwork.libcamera.org","Received":["from lancelot.ideasonboard.com (lancelot.ideasonboard.com\n\t[92.243.16.209])\n\tby patchwork.libcamera.org (Postfix) with ESMTPS id 97FE6C31FF\n\tfor ;\n\tFri, 21 May 2021 04:07:40 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id EC2576891B;\n\tFri, 21 May 2021 06:07:39 +0200 (CEST)","from mail-ej1-x632.google.com (mail-ej1-x632.google.com\n\t[IPv6:2a00:1450:4864:20::632])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTPS id 5A3F56050E\n\tfor ;\n\tFri, 21 May 2021 06:07:38 +0200 (CEST)","by mail-ej1-x632.google.com with SMTP id n2so28467226ejy.7\n\tfor ;\n\tThu, 20 May 2021 21:07:38 -0700 (PDT)"],"Authentication-Results":"lancelot.ideasonboard.com; dkim=pass (1024-bit key;\n\tunprotected) header.d=chromium.org header.i=@chromium.org\n\theader.b=\"oar6OSa2\"; dkim-atps=neutral","DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/relaxed; d=chromium.org;\n\ts=google; \n\th=mime-version:references:in-reply-to:from:date:message-id:subject:to\n\t:cc; bh=6HikuAqZNI2vvvJgwAnMHwhs5d5vgWl2wz6NcpFv2FY=;\n\tb=oar6OSa2HVWgLhbsfHfWQWDR7PQZLEBqLn67FfBzixUl36WHcJrRRcKSPUllbdM7KB\n\tVFChtghR9fMyJ0RONlSt8ltyjT2ihiD+MNVYa+Hsb0a5jwzExB/t4IeDdrCjl7Z39A/d\n\tRtXb1sQA91st0GwNffMYcvbgtImtg+0gW2/mI=","X-Google-DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/relaxed;\n\td=1e100.net; s=20161025;\n\th=x-gm-message-state:mime-version:references:in-reply-to:from:date\n\t:message-id:subject:to:cc;\n\tbh=6HikuAqZNI2vvvJgwAnMHwhs5d5vgWl2wz6NcpFv2FY=;\n\tb=q+GN4qgW9UMBz5oin0YL4bce05V9+bbAyuo19Wz/tKlVSolT2uTN970Yf9aqayqd7G\n\tW/AWaVqC/M24XqlT1uaB8JyRSBorH0VJXVI4dls9SA6EV7rc8zC7E55Ite4cVhs7ZPKI\n\tpKd+8yohU6JYLnMixyhY+J7rRgEy4miU99y/2fMCTzkw5xDb9gbuMUjyU7CykLpkVl2U\n\tJEMbS/lC6PI/Fr9yay2h5i6Nt5hI8cRE3ruPE9b+2ooCVqj1DhSKIwO046+bFW6T7PGk\n\t7ZVxaBYzHgZr2xgj9YnOg6mBH6BHkzLveY7SV9+26ZyRTOQO8yCtzsCkVNorT+Q15H27\n\tqiVA==","X-Gm-Message-State":"AOAM530PNo5DCxrf6tnAJNEuor2RWk/1QJGKUnDyEQjeGxvz0cW6Aziq\n\txAC0bJEXjdp6Wcz8UlNMrdkmbb85rXhsMcBo2J/HcQ==","X-Google-Smtp-Source":"ABdhPJyys18pC2k2+ATGh5f8D3Mw2c7zCOsEksE7wpCeogvtzxb4A5MjsPx7vwthbkMQ0oA7PYnU6YFxACvlWzJT4EM=","X-Received":"by 2002:a17:907:1b06:: with SMTP id\n\tmp6mr8283523ejc.292.1621570057903; \n\tThu, 20 May 2021 21:07:37 -0700 (PDT)","MIME-Version":"1.0","References":"<20210513092246.42847-1-jacopo@jmondi.org>\n\t<20210513092246.42847-9-jacopo@jmondi.org>\n\t\n\t<20210520083019.fejqotf2ct6qmi7k@uno.localdomain>","In-Reply-To":"<20210520083019.fejqotf2ct6qmi7k@uno.localdomain>","From":"Hirokazu Honda ","Date":"Fri, 21 May 2021 13:07:26 +0900","Message-ID":"","To":"Jacopo Mondi ","Content-Type":"multipart/alternative; boundary=\"00000000000047b3c005c2cf33be\"","Subject":"Re: [libcamera-devel] [PATCH v2 8/8] android: Implement flush()\n\tcamera operation","X-BeenThere":"libcamera-devel@lists.libcamera.org","X-Mailman-Version":"2.1.29","Precedence":"list","List-Id":"","List-Unsubscribe":",\n\t","List-Archive":"","List-Post":"","List-Help":"","List-Subscribe":",\n\t","Cc":"libcamera devel ","Errors-To":"libcamera-devel-bounces@lists.libcamera.org","Sender":"\"libcamera-devel\" "}}]