[{"id":19467,"web_url":"https://patchwork.libcamera.org/comment/19467/","msgid":"<CAO5uPHO7QxrFSxLSc0CvrNqqF5SPKW9sACwmn2utoB36Q5ngHg@mail.gmail.com>","date":"2021-09-06T14:26:48","subject":"Re: [libcamera-devel] [PATCH 2/5] android: camera_device: Clear\n\tstreams_ in stop()","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\nOn Mon, Sep 6, 2021 at 11:01 PM Jacopo Mondi <jacopo@jmondi.org> wrote:\n>\n> The CameraDevice::streams_ class member is populated at\n> configureStreams() time with one item per each stream requested to the\n> Camera device.\n>\n> When a new configuration is applied to the Camera HAL the list of\n> requested streams has to be re-initialized and the streams_ vector needs\n> to be manually cleared in order to be populated from scratch.\n>\n> As configureStreams() class CameraDevice::stop() at the very beginning,\n> centralize clearing the streams_ vector there even in the case the\n> camera has been stopped by a previous call to flush().\n>\n> Suggested-by: Hirokazu Honda <hiroh@chromium.org>\n> Signed-off-by: Jacopo Mondi <jacopo@jmondi.org>\n\nReviewed-by: Hirokazu Honda <hiroh@chromium.org>\n\n> ---\n>  src/android/camera_device.cpp | 21 ++++++++++++---------\n>  1 file changed, 12 insertions(+), 9 deletions(-)\n>\n> diff --git a/src/android/camera_device.cpp b/src/android/camera_device.cpp\n> index fda77db4540c..30c173a69720 100644\n> --- a/src/android/camera_device.cpp\n> +++ b/src/android/camera_device.cpp\n> @@ -448,8 +448,18 @@ void CameraDevice::flush()\n>  void CameraDevice::stop()\n>  {\n>         MutexLocker stateLock(stateMutex_);\n> -       if (state_ == State::Stopped)\n> +       if (state_ == State::Stopped) {\n> +               /*\n> +                * We get here if the list of streams requested by the camera\n> +                * service has to be updated but the camera has been stopped\n> +                * already, which happens if configureStreams() gets called\n> +                * after a flush(). Clear the list of stream configurations,\n> +                * no need to clear descriptors as stopping the camera completes\n> +                * all the pending requests.\n> +                */\n> +               streams_.clear();\n>                 return;\n> +       }\n>\n>         worker_.stop();\n>         camera_->stop();\n> @@ -544,6 +554,7 @@ int CameraDevice::configureStreams(camera3_stream_configuration_t *stream_list)\n>                 LOG(HAL, Error) << \"No streams in configuration\";\n>                 return -EINVAL;\n>         }\n> +       streams_.reserve(stream_list->num_streams);\n>\n>  #if defined(OS_CHROMEOS)\n>         if (!validateCropRotate(*stream_list))\n> @@ -560,14 +571,6 @@ int CameraDevice::configureStreams(camera3_stream_configuration_t *stream_list)\n>                 return -EINVAL;\n>         }\n>\n> -       /*\n> -        * Clear and remove any existing configuration from previous calls, and\n> -        * ensure the required entries are available without further\n> -        * reallocation.\n> -        */\n> -       streams_.clear();\n> -       streams_.reserve(stream_list->num_streams);\n> -\n>         std::vector<Camera3StreamConfig> streamConfigs;\n>         streamConfigs.reserve(stream_list->num_streams);\n>\n> --\n> 2.32.0\n>","headers":{"Return-Path":"<libcamera-devel-bounces@lists.libcamera.org>","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 EE51FBDC71\n\tfor <parsemail@patchwork.libcamera.org>;\n\tMon,  6 Sep 2021 14:27:02 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id 462BE6916C;\n\tMon,  6 Sep 2021 16:27:02 +0200 (CEST)","from mail-ej1-x62f.google.com (mail-ej1-x62f.google.com\n\t[IPv6:2a00:1450:4864:20::62f])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTPS id 28E8360137\n\tfor <libcamera-devel@lists.libcamera.org>;\n\tMon,  6 Sep 2021 16:27:00 +0200 (CEST)","by mail-ej1-x62f.google.com with SMTP id n27so13859204eja.5\n\tfor <libcamera-devel@lists.libcamera.org>;\n\tMon, 06 Sep 2021 07:27:00 -0700 (PDT)"],"Authentication-Results":"lancelot.ideasonboard.com;\n\tdkim=fail reason=\"signature verification failed\" (1024-bit key;\n\tunprotected) header.d=chromium.org header.i=@chromium.org\n\theader.b=\"QCKnAynk\"; 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=GmvMubrX0qUD16/FA1Pm6ydGd3ECA4hBodeq3yHP3UA=;\n\tb=QCKnAynk7NltuqytApzd9EvXZbIB4AmTd8Zf6s3JordDgxTaf3/mANTsPQbCxIwyGq\n\tfPubdZyYgqXWao1sdBEcpTINwOXftmL9zbHfoHcytkwjOzlzK3BK5Y1RK1pbJ6f7bbKv\n\tN2HNs/pGcISNQz4LV94vBefJ/WmTKfjta+t/4=","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=GmvMubrX0qUD16/FA1Pm6ydGd3ECA4hBodeq3yHP3UA=;\n\tb=WRlWjD2VpBsBvqWHDX+wC+C1QBVkvA8zjJa9PENm1St1CulwTVzG5jr5xmGSYG6vQu\n\t5KlRXul5WzAjNtHxeYrpGF2H9TUdK6IK7/fbzuc2yp5LdSxjwxNhlfE10GP15Gl3VpLX\n\tXAIfBDL83noQnMGFEB+53NqB0nP8JUC4D7WBvrWik8CjCOy/qqDz7V+ZuW3zv1WrbFlk\n\tNzz1A50zG4NpP7lCFF6h1u96rzSlp1lo5lvO2EFssB6tk5kzjIt5Cs4D9jPwCErW6MdY\n\tlikKcR8bh2mhQTmrm5XoWjYsMHN/dzS0Syyio0oW/HPzLbHJelaX09NMz3Ol0/F4QwEB\n\tJV+g==","X-Gm-Message-State":"AOAM530PZwzjqSNBafYjS8Rz3gb/QYoGoHUZdLHGkGJ0xbFcrtXwqmhl\n\tJxskjnFkz7oSr/NKApcP/EgeB26on+VtZRl//2wsV2VF4nQ=","X-Google-Smtp-Source":"ABdhPJyXTL9w/anBQE9ruT5avZ8HvF1hXJLv/2xKZ+NNZdMoKCFGshDVVnJRrOmTy6pzPO+tLrCfC/k3qUaU8vWGBcE=","X-Received":"by 2002:a17:906:4fd6:: with SMTP id\n\ti22mr14005202ejw.92.1630938419736; \n\tMon, 06 Sep 2021 07:26:59 -0700 (PDT)","MIME-Version":"1.0","References":"<20210906140152.636883-1-jacopo@jmondi.org>\n\t<20210906140152.636883-3-jacopo@jmondi.org>","In-Reply-To":"<20210906140152.636883-3-jacopo@jmondi.org>","From":"Hirokazu Honda <hiroh@chromium.org>","Date":"Mon, 6 Sep 2021 23:26:48 +0900","Message-ID":"<CAO5uPHO7QxrFSxLSc0CvrNqqF5SPKW9sACwmn2utoB36Q5ngHg@mail.gmail.com>","To":"Jacopo Mondi <jacopo@jmondi.org>","Content-Type":"text/plain; charset=\"UTF-8\"","Subject":"Re: [libcamera-devel] [PATCH 2/5] android: camera_device: Clear\n\tstreams_ in stop()","X-BeenThere":"libcamera-devel@lists.libcamera.org","X-Mailman-Version":"2.1.29","Precedence":"list","List-Id":"<libcamera-devel.lists.libcamera.org>","List-Unsubscribe":"<https://lists.libcamera.org/options/libcamera-devel>,\n\t<mailto:libcamera-devel-request@lists.libcamera.org?subject=unsubscribe>","List-Archive":"<https://lists.libcamera.org/pipermail/libcamera-devel/>","List-Post":"<mailto:libcamera-devel@lists.libcamera.org>","List-Help":"<mailto:libcamera-devel-request@lists.libcamera.org?subject=help>","List-Subscribe":"<https://lists.libcamera.org/listinfo/libcamera-devel>,\n\t<mailto:libcamera-devel-request@lists.libcamera.org?subject=subscribe>","Cc":"libcamera devel <libcamera-devel@lists.libcamera.org>","Errors-To":"libcamera-devel-bounces@lists.libcamera.org","Sender":"\"libcamera-devel\" <libcamera-devel-bounces@lists.libcamera.org>"}},{"id":19468,"web_url":"https://patchwork.libcamera.org/comment/19468/","msgid":"<CAO5uPHP9ML=ErXc9HS+_B8U2OPB0eP2Rb+PH98RLAGFu-CE0ag@mail.gmail.com>","date":"2021-09-06T14:27:58","subject":"Re: [libcamera-devel] [PATCH 3/5] android: camera_device: Make\n\tabortRequest() take a const","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\nOn Mon, Sep 6, 2021 at 11:01 PM Jacopo Mondi <jacopo@jmondi.org> wrote:\n>\n> The CameraDevice::abortRequest() function should operate on const\n> pointers.\n>\n> Fix that.\n>\n> Signed-off-by: Jacopo Mondi <jacopo@jmondi.org>\n> ---\n>  src/android/camera_device.cpp | 2 +-\n>  src/android/camera_device.h   | 2 +-\n>  2 files changed, 2 insertions(+), 2 deletions(-)\n>\n> diff --git a/src/android/camera_device.cpp b/src/android/camera_device.cpp\n> index 30c173a69720..a0ea138d9499 100644\n> --- a/src/android/camera_device.cpp\n> +++ b/src/android/camera_device.cpp\n> @@ -845,7 +845,7 @@ int CameraDevice::processControls(Camera3RequestDescriptor *descriptor)\n>         return 0;\n>  }\n>\n> -void CameraDevice::abortRequest(camera3_capture_request_t *request)\n> +void CameraDevice::abortRequest(const camera3_capture_request_t *request)\n>  {\n>         notifyError(request->frame_number, nullptr, CAMERA3_MSG_ERROR_REQUEST);\n\nIs it possible to make this const reference?\n\n-Hiro\n>\n> diff --git a/src/android/camera_device.h b/src/android/camera_device.h\n> index a55769272651..54c4cb9ab499 100644\n> --- a/src/android/camera_device.h\n> +++ b/src/android/camera_device.h\n> @@ -96,7 +96,7 @@ private:\n>         libcamera::FrameBuffer *createFrameBuffer(const buffer_handle_t camera3buffer,\n>                                                   libcamera::PixelFormat pixelFormat,\n>                                                   const libcamera::Size &size);\n> -       void abortRequest(camera3_capture_request_t *request);\n> +       void abortRequest(const camera3_capture_request_t *request);\n>         bool isValidRequest(camera3_capture_request_t *request) const;\n>         void notifyShutter(uint32_t frameNumber, uint64_t timestamp);\n>         void notifyError(uint32_t frameNumber, camera3_stream_t *stream,\n> --\n> 2.32.0\n>","headers":{"Return-Path":"<libcamera-devel-bounces@lists.libcamera.org>","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 AF60EBDC71\n\tfor <parsemail@patchwork.libcamera.org>;\n\tMon,  6 Sep 2021 14:28:11 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id 55B9469167;\n\tMon,  6 Sep 2021 16:28:11 +0200 (CEST)","from mail-ed1-x533.google.com (mail-ed1-x533.google.com\n\t[IPv6:2a00:1450:4864:20::533])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTPS id E2EDA60137\n\tfor <libcamera-devel@lists.libcamera.org>;\n\tMon,  6 Sep 2021 16:28:09 +0200 (CEST)","by mail-ed1-x533.google.com with SMTP id n11so9747890edv.11\n\tfor <libcamera-devel@lists.libcamera.org>;\n\tMon, 06 Sep 2021 07:28:09 -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=\"hG9PgQ3B\"; 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=4cZ5KJo4WMrTxJhN92M8JgcSDopyYLrNnpd/v02YlCo=;\n\tb=hG9PgQ3Baji+XNkMnZd2kpsbaS3XMUeI9ldu/gPFdox6ZPU4eh4rPenXeUrDNz5WDE\n\tQ4XEwe6E30lKcVycyHSlksWjrlLADfPwCxabPw5rNAMAg0iJDVyKEsPiitIsaAr8UjDG\n\tOvCEV8x8SXUNhpcNKY2Pl71g9LwEfwyVGrjEw=","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=4cZ5KJo4WMrTxJhN92M8JgcSDopyYLrNnpd/v02YlCo=;\n\tb=dUrHn/p048BapekmA4OMA1G5qZNH1Abt11Z25kgxXtnlAr2gvZXAVCOZ8jkXbuhX83\n\tJjgWs3VAnL/8D/DAjahyl9lpBCLQmLFRFyhC6lyVf1SKx+Y/IoPrjot7LL+E1iVbJRUx\n\t8pKwPUL0lFKjjr6l9NdWi2mTvosM5RHjbgzKksXDd+rJc78zz+5eQRT2yl+lykdbVq7B\n\toeQgxq6VHCk+MWebiqT/WBIsiCbf301T7gDZ+RsQWpFsaxcEz4fs4EfJCIcvVUH3fT24\n\tVqMjHRYHIc43wi5R2hwYqmqGbFqPX8/31opFK8fo1WstPzPimZ6yQmyEwyahDZJgPGoS\n\t5bJA==","X-Gm-Message-State":"AOAM532/F/rt6wNjWnSQEJhz+f8YmAMokCldF7s/ggIT4vfG+KHSOxtw\n\t7poqMvRLZuBcolHgQ5wsIJ4JUW0YhK8WMTEjsh1CAm6huP4=","X-Google-Smtp-Source":"ABdhPJxYwnXLA3WaDYC99b+6zMZPtvsDE4v3wxTFDYxBZ/+gT/cqgqqn1/JKFKWnlTq/BrnkwcCDzeVB8T9sCzwZ8Fg=","X-Received":"by 2002:a05:6402:2931:: with SMTP id\n\tee49mr13789370edb.220.1630938489362; \n\tMon, 06 Sep 2021 07:28:09 -0700 (PDT)","MIME-Version":"1.0","References":"<20210906140152.636883-1-jacopo@jmondi.org>\n\t<20210906140152.636883-4-jacopo@jmondi.org>","In-Reply-To":"<20210906140152.636883-4-jacopo@jmondi.org>","From":"Hirokazu Honda <hiroh@chromium.org>","Date":"Mon, 6 Sep 2021 23:27:58 +0900","Message-ID":"<CAO5uPHP9ML=ErXc9HS+_B8U2OPB0eP2Rb+PH98RLAGFu-CE0ag@mail.gmail.com>","To":"Jacopo Mondi <jacopo@jmondi.org>","Content-Type":"text/plain; charset=\"UTF-8\"","Subject":"Re: [libcamera-devel] [PATCH 3/5] android: camera_device: Make\n\tabortRequest() take a const","X-BeenThere":"libcamera-devel@lists.libcamera.org","X-Mailman-Version":"2.1.29","Precedence":"list","List-Id":"<libcamera-devel.lists.libcamera.org>","List-Unsubscribe":"<https://lists.libcamera.org/options/libcamera-devel>,\n\t<mailto:libcamera-devel-request@lists.libcamera.org?subject=unsubscribe>","List-Archive":"<https://lists.libcamera.org/pipermail/libcamera-devel/>","List-Post":"<mailto:libcamera-devel@lists.libcamera.org>","List-Help":"<mailto:libcamera-devel-request@lists.libcamera.org?subject=help>","List-Subscribe":"<https://lists.libcamera.org/listinfo/libcamera-devel>,\n\t<mailto:libcamera-devel-request@lists.libcamera.org?subject=subscribe>","Cc":"libcamera devel <libcamera-devel@lists.libcamera.org>","Errors-To":"libcamera-devel-bounces@lists.libcamera.org","Sender":"\"libcamera-devel\" <libcamera-devel-bounces@lists.libcamera.org>"}},{"id":19470,"web_url":"https://patchwork.libcamera.org/comment/19470/","msgid":"<CAO5uPHPneVPOK8jsdoXVuHZYdGmnTyVBoZYksFT5_UDURTbMJw@mail.gmail.com>","date":"2021-09-06T14:56:50","subject":"Re: [libcamera-devel] [PATCH 5/5] android: camera_device: Assert\n\tdescriptors_ is empty","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\nOn Mon, Sep 6, 2021 at 11:01 PM Jacopo Mondi <jacopo@jmondi.org> wrote:\n>\n> The CameraDevice::descriptors_ class member holds one item per each\n> in-flight request. A new descriptor is added to the map each time a new\n> request is queued to the camera and it gets then popped out when the\n> request completes or when an error happens before queueing it.\n>\n> As stopping the camera guarantees that all in-flight requests are\n> completed synchronously to the Camera::stop() call, there is no need to\n> manually clear the descriptors_ map, on the contrary, it might hides\n> issues in the handling of in-flight requests.\n>\n> Remove it and replace it with an assertion to make sure the underlying\n> camera behaves as intended and all Request are completed when the camera\n> is stopped.\n>\n> Signed-off-by: Jacopo Mondi <jacopo@jmondi.org>\n> ---\n>  src/android/camera_device.cpp | 11 +++++++----\n>  1 file changed, 7 insertions(+), 4 deletions(-)\n>\n> diff --git a/src/android/camera_device.cpp b/src/android/camera_device.cpp\n> index 0ce9b699bfae..1591ad98c7d5 100644\n> --- a/src/android/camera_device.cpp\n> +++ b/src/android/camera_device.cpp\n> @@ -442,6 +442,9 @@ void CameraDevice::flush()\n>         worker_.stop();\n>         camera_->stop();\n>\n> +       /* Make sure all in-flight requests have been completed. */\n\nShall we say descriptors are flushed in worker.stop()?\n\nReviewed-by: Hirokazu Honda <hiroh@chromium.org>\n\n-Hiro\n> +       ASSERT(descriptors_.empty());\n> +\n>         MutexLocker stateLock(stateMutex_);\n>         state_ = State::Stopped;\n>  }\n> @@ -454,9 +457,7 @@ void CameraDevice::stop()\n>                  * We get here if the list of streams requested by the camera\n>                  * service has to be updated but the camera has been stopped\n>                  * already, which happens if configureStreams() gets called\n> -                * after a flush(). Clear the list of stream configurations,\n> -                * no need to clear descriptors as stopping the camera completes\n> -                * all the pending requests.\n> +                * after a flush().\n>                  */\n>                 streams_.clear();\n>                 return;\n> @@ -465,7 +466,9 @@ void CameraDevice::stop()\n>         worker_.stop();\n>         camera_->stop();\n>\n> -       descriptors_.clear();\n> +       /* Make sure all in-flight requests have been completed. */\n> +       ASSERT(descriptors_.empty());\n> +\n>         streams_.clear();\n>\n>         state_ = State::Stopped;\n> --\n> 2.32.0\n>","headers":{"Return-Path":"<libcamera-devel-bounces@lists.libcamera.org>","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 C7B01BD87D\n\tfor <parsemail@patchwork.libcamera.org>;\n\tMon,  6 Sep 2021 14:57:02 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id 70E336916A;\n\tMon,  6 Sep 2021 16:57:02 +0200 (CEST)","from mail-ed1-x52a.google.com (mail-ed1-x52a.google.com\n\t[IPv6:2a00:1450:4864:20::52a])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTPS id 3B6DC60137\n\tfor <libcamera-devel@lists.libcamera.org>;\n\tMon,  6 Sep 2021 16:57:01 +0200 (CEST)","by mail-ed1-x52a.google.com with SMTP id u19so9889703edb.3\n\tfor <libcamera-devel@lists.libcamera.org>;\n\tMon, 06 Sep 2021 07:57:01 -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=\"JPvTdVBT\"; 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=Be/CNqCeOdXya91O16dwd1EGLyIYUtYjIJBFNXELqO4=;\n\tb=JPvTdVBTm5atGoSx5ZiABQY98ePHUsgZ+gQW/iSaY+Ly10fpmbNzh4086kY+j3BqUI\n\tuDZtuaT+1/DACLSlReEhdhhkL7fam6xGSjApOcpzFtWlvLRPs1sPMegAeD4sfl7aPwLN\n\tAKjG9Rb4+VDODFOIwSCIMmjaWfd3RA5qKEFTU=","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=Be/CNqCeOdXya91O16dwd1EGLyIYUtYjIJBFNXELqO4=;\n\tb=VEhsBhJ695h/KaA7yj0zWtXdUFvZM+To9iKeW7LGbZFFSevUo8TT9v5wIehUK1ESmn\n\tDgSiS1cx0qLJzD3EKbK6NlMAXBCoCwK5TzgPRq5Y+ymktrEs7YuOgqQ+EaXhWkLulEu/\n\tCuvxUzel6VMJOAeP1Vhb6nbTj8s+jOl28/j5IRExDGmU1Ull2NdLedh4tN3LoTo9kZ7C\n\t9aEvf9Dp7HkEyFD1fC1R8RgJ4vvCtsV7L6mE6/9xHfc+jmk9BY2lsfMZItJ0SmklLDo2\n\tj6lITuBMpdPe85/Guq1fPN4SzvlCWiYfCMknmImwWWIfodQVE30H3EJQZEcxjCMClCvM\n\tG3/w==","X-Gm-Message-State":"AOAM533kTJmU2aPqgOlCtTFYDsUWvXY/Yk5XNwlR3JJWCNPOiTBCojbO\n\tYeq2g5O9GSt7flO4LSF8K5NH6Q3GTNXakKj9gHweva/G+6U=","X-Google-Smtp-Source":"ABdhPJxUR7O326WndNnKvZKwh4H4JzymMstBNA8+KYvp8NQYhRtRdz79R2K+ii7ejn4b+2X1jHVbRi2oF/jcCtXQ/eA=","X-Received":"by 2002:a05:6402:2931:: with SMTP id\n\tee49mr13904406edb.220.1630940220845; \n\tMon, 06 Sep 2021 07:57:00 -0700 (PDT)","MIME-Version":"1.0","References":"<20210906140152.636883-1-jacopo@jmondi.org>\n\t<20210906140152.636883-6-jacopo@jmondi.org>","In-Reply-To":"<20210906140152.636883-6-jacopo@jmondi.org>","From":"Hirokazu Honda <hiroh@chromium.org>","Date":"Mon, 6 Sep 2021 23:56:50 +0900","Message-ID":"<CAO5uPHPneVPOK8jsdoXVuHZYdGmnTyVBoZYksFT5_UDURTbMJw@mail.gmail.com>","To":"Jacopo Mondi <jacopo@jmondi.org>","Content-Type":"text/plain; charset=\"UTF-8\"","Subject":"Re: [libcamera-devel] [PATCH 5/5] android: camera_device: Assert\n\tdescriptors_ is empty","X-BeenThere":"libcamera-devel@lists.libcamera.org","X-Mailman-Version":"2.1.29","Precedence":"list","List-Id":"<libcamera-devel.lists.libcamera.org>","List-Unsubscribe":"<https://lists.libcamera.org/options/libcamera-devel>,\n\t<mailto:libcamera-devel-request@lists.libcamera.org?subject=unsubscribe>","List-Archive":"<https://lists.libcamera.org/pipermail/libcamera-devel/>","List-Post":"<mailto:libcamera-devel@lists.libcamera.org>","List-Help":"<mailto:libcamera-devel-request@lists.libcamera.org?subject=help>","List-Subscribe":"<https://lists.libcamera.org/listinfo/libcamera-devel>,\n\t<mailto:libcamera-devel-request@lists.libcamera.org?subject=subscribe>","Cc":"libcamera devel <libcamera-devel@lists.libcamera.org>","Errors-To":"libcamera-devel-bounces@lists.libcamera.org","Sender":"\"libcamera-devel\" <libcamera-devel-bounces@lists.libcamera.org>"}},{"id":19471,"web_url":"https://patchwork.libcamera.org/comment/19471/","msgid":"<20210906160759.x75c6p5cr6k2abfj@uno.localdomain>","date":"2021-09-06T16:07:59","subject":"Re: [libcamera-devel] [PATCH 3/5] android: camera_device: Make\n\tabortRequest() take a const","submitter":{"id":3,"url":"https://patchwork.libcamera.org/api/people/3/","name":"Jacopo Mondi","email":"jacopo@jmondi.org"},"content":"Hi Hiro,\n\nOn Mon, Sep 06, 2021 at 11:27:58PM +0900, Hirokazu Honda wrote:\n> Hi Jacopo, thank you for the patch.\n>\n> On Mon, Sep 6, 2021 at 11:01 PM Jacopo Mondi <jacopo@jmondi.org> wrote:\n> >\n> > The CameraDevice::abortRequest() function should operate on const\n> > pointers.\n> >\n> > Fix that.\n> >\n> > Signed-off-by: Jacopo Mondi <jacopo@jmondi.org>\n> > ---\n> >  src/android/camera_device.cpp | 2 +-\n> >  src/android/camera_device.h   | 2 +-\n> >  2 files changed, 2 insertions(+), 2 deletions(-)\n> >\n> > diff --git a/src/android/camera_device.cpp b/src/android/camera_device.cpp\n> > index 30c173a69720..a0ea138d9499 100644\n> > --- a/src/android/camera_device.cpp\n> > +++ b/src/android/camera_device.cpp\n> > @@ -845,7 +845,7 @@ int CameraDevice::processControls(Camera3RequestDescriptor *descriptor)\n> >         return 0;\n> >  }\n> >\n> > -void CameraDevice::abortRequest(camera3_capture_request_t *request)\n> > +void CameraDevice::abortRequest(const camera3_capture_request_t *request)\n> >  {\n> >         notifyError(request->frame_number, nullptr, CAMERA3_MSG_ERROR_REQUEST);\n>\n> Is it possible to make this const reference?\n\nAll the HAL entry points receive parameters by pointer from the\nframework. If we want to use references we would have to dereference\nthem in the HAL code when using them as parameter. I don't think we\nwould gain much to be honest.\n\nThanks\n   j\n\n>\n> -Hiro\n> >\n> > diff --git a/src/android/camera_device.h b/src/android/camera_device.h\n> > index a55769272651..54c4cb9ab499 100644\n> > --- a/src/android/camera_device.h\n> > +++ b/src/android/camera_device.h\n> > @@ -96,7 +96,7 @@ private:\n> >         libcamera::FrameBuffer *createFrameBuffer(const buffer_handle_t camera3buffer,\n> >                                                   libcamera::PixelFormat pixelFormat,\n> >                                                   const libcamera::Size &size);\n> > -       void abortRequest(camera3_capture_request_t *request);\n> > +       void abortRequest(const camera3_capture_request_t *request);\n> >         bool isValidRequest(camera3_capture_request_t *request) const;\n> >         void notifyShutter(uint32_t frameNumber, uint64_t timestamp);\n> >         void notifyError(uint32_t frameNumber, camera3_stream_t *stream,\n> > --\n> > 2.32.0\n> >","headers":{"Return-Path":"<libcamera-devel-bounces@lists.libcamera.org>","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 99D6EBDC71\n\tfor <parsemail@patchwork.libcamera.org>;\n\tMon,  6 Sep 2021 16:07:13 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id 0E43F6916A;\n\tMon,  6 Sep 2021 18:07:13 +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 45F2F60137\n\tfor <libcamera-devel@lists.libcamera.org>;\n\tMon,  6 Sep 2021 18:07:12 +0200 (CEST)","(Authenticated sender: jacopo@jmondi.org)\n\tby relay8-d.mail.gandi.net (Postfix) with ESMTPSA id BF58F1BF209;\n\tMon,  6 Sep 2021 16:07:11 +0000 (UTC)"],"Date":"Mon, 6 Sep 2021 18:07:59 +0200","From":"Jacopo Mondi <jacopo@jmondi.org>","To":"Hirokazu Honda <hiroh@chromium.org>","Message-ID":"<20210906160759.x75c6p5cr6k2abfj@uno.localdomain>","References":"<20210906140152.636883-1-jacopo@jmondi.org>\n\t<20210906140152.636883-4-jacopo@jmondi.org>\n\t<CAO5uPHP9ML=ErXc9HS+_B8U2OPB0eP2Rb+PH98RLAGFu-CE0ag@mail.gmail.com>","MIME-Version":"1.0","Content-Type":"text/plain; charset=utf-8","Content-Disposition":"inline","In-Reply-To":"<CAO5uPHP9ML=ErXc9HS+_B8U2OPB0eP2Rb+PH98RLAGFu-CE0ag@mail.gmail.com>","Subject":"Re: [libcamera-devel] [PATCH 3/5] android: camera_device: Make\n\tabortRequest() take a const","X-BeenThere":"libcamera-devel@lists.libcamera.org","X-Mailman-Version":"2.1.29","Precedence":"list","List-Id":"<libcamera-devel.lists.libcamera.org>","List-Unsubscribe":"<https://lists.libcamera.org/options/libcamera-devel>,\n\t<mailto:libcamera-devel-request@lists.libcamera.org?subject=unsubscribe>","List-Archive":"<https://lists.libcamera.org/pipermail/libcamera-devel/>","List-Post":"<mailto:libcamera-devel@lists.libcamera.org>","List-Help":"<mailto:libcamera-devel-request@lists.libcamera.org?subject=help>","List-Subscribe":"<https://lists.libcamera.org/listinfo/libcamera-devel>,\n\t<mailto:libcamera-devel-request@lists.libcamera.org?subject=subscribe>","Cc":"libcamera devel <libcamera-devel@lists.libcamera.org>","Errors-To":"libcamera-devel-bounces@lists.libcamera.org","Sender":"\"libcamera-devel\" <libcamera-devel-bounces@lists.libcamera.org>"}},{"id":19473,"web_url":"https://patchwork.libcamera.org/comment/19473/","msgid":"<20210906162251.m6jvcxwmevnywmnw@uno.localdomain>","date":"2021-09-06T16:22:51","subject":"Re: [libcamera-devel] [PATCH 5/5] android: camera_device: Assert\n\tdescriptors_ is empty","submitter":{"id":3,"url":"https://patchwork.libcamera.org/api/people/3/","name":"Jacopo Mondi","email":"jacopo@jmondi.org"},"content":"Hi Hiro\n\nOn Mon, Sep 06, 2021 at 11:56:50PM +0900, Hirokazu Honda wrote:\n> Hi Jacopo, thank you for the patch.\n>\n> On Mon, Sep 6, 2021 at 11:01 PM Jacopo Mondi <jacopo@jmondi.org> wrote:\n> >\n> > The CameraDevice::descriptors_ class member holds one item per each\n> > in-flight request. A new descriptor is added to the map each time a new\n> > request is queued to the camera and it gets then popped out when the\n> > request completes or when an error happens before queueing it.\n> >\n> > As stopping the camera guarantees that all in-flight requests are\n> > completed synchronously to the Camera::stop() call, there is no need to\n> > manually clear the descriptors_ map, on the contrary, it might hides\n> > issues in the handling of in-flight requests.\n> >\n> > Remove it and replace it with an assertion to make sure the underlying\n> > camera behaves as intended and all Request are completed when the camera\n> > is stopped.\n> >\n> > Signed-off-by: Jacopo Mondi <jacopo@jmondi.org>\n> > ---\n> >  src/android/camera_device.cpp | 11 +++++++----\n> >  1 file changed, 7 insertions(+), 4 deletions(-)\n> >\n> > diff --git a/src/android/camera_device.cpp b/src/android/camera_device.cpp\n> > index 0ce9b699bfae..1591ad98c7d5 100644\n> > --- a/src/android/camera_device.cpp\n> > +++ b/src/android/camera_device.cpp\n> > @@ -442,6 +442,9 @@ void CameraDevice::flush()\n> >         worker_.stop();\n> >         camera_->stop();\n> >\n> > +       /* Make sure all in-flight requests have been completed. */\n>\n> Shall we say descriptors are flushed in worker.stop()?\n\ndescriptors, in my understanding, as removed from the descriptors_ map\none by one as requests get completed, as a consequence of the regular\nflow of operations or because of a force completion caused by\nCamera::stop().\n\nHave I missed your comment ?\n\nThanks\n  j\n\n>\n> Reviewed-by: Hirokazu Honda <hiroh@chromium.org>\n>\n> -Hiro\n> > +       ASSERT(descriptors_.empty());\n> > +\n> >         MutexLocker stateLock(stateMutex_);\n> >         state_ = State::Stopped;\n> >  }\n> > @@ -454,9 +457,7 @@ void CameraDevice::stop()\n> >                  * We get here if the list of streams requested by the camera\n> >                  * service has to be updated but the camera has been stopped\n> >                  * already, which happens if configureStreams() gets called\n> > -                * after a flush(). Clear the list of stream configurations,\n> > -                * no need to clear descriptors as stopping the camera completes\n> > -                * all the pending requests.\n> > +                * after a flush().\n> >                  */\n> >                 streams_.clear();\n> >                 return;\n> > @@ -465,7 +466,9 @@ void CameraDevice::stop()\n> >         worker_.stop();\n> >         camera_->stop();\n> >\n> > -       descriptors_.clear();\n> > +       /* Make sure all in-flight requests have been completed. */\n> > +       ASSERT(descriptors_.empty());\n> > +\n> >         streams_.clear();\n> >\n> >         state_ = State::Stopped;\n> > --\n> > 2.32.0\n> >","headers":{"Return-Path":"<libcamera-devel-bounces@lists.libcamera.org>","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 C975CBD87D\n\tfor <parsemail@patchwork.libcamera.org>;\n\tMon,  6 Sep 2021 16:22:05 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id 484F46916A;\n\tMon,  6 Sep 2021 18:22:05 +0200 (CEST)","from relay6-d.mail.gandi.net (relay6-d.mail.gandi.net\n\t[217.70.183.198])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTPS id 49AC160137\n\tfor <libcamera-devel@lists.libcamera.org>;\n\tMon,  6 Sep 2021 18:22:04 +0200 (CEST)","(Authenticated sender: jacopo@jmondi.org)\n\tby relay6-d.mail.gandi.net (Postfix) with ESMTPSA id C9862C0006;\n\tMon,  6 Sep 2021 16:22:03 +0000 (UTC)"],"Date":"Mon, 6 Sep 2021 18:22:51 +0200","From":"Jacopo Mondi <jacopo@jmondi.org>","To":"Hirokazu Honda <hiroh@chromium.org>","Message-ID":"<20210906162251.m6jvcxwmevnywmnw@uno.localdomain>","References":"<20210906140152.636883-1-jacopo@jmondi.org>\n\t<20210906140152.636883-6-jacopo@jmondi.org>\n\t<CAO5uPHPneVPOK8jsdoXVuHZYdGmnTyVBoZYksFT5_UDURTbMJw@mail.gmail.com>","MIME-Version":"1.0","Content-Type":"text/plain; charset=utf-8","Content-Disposition":"inline","In-Reply-To":"<CAO5uPHPneVPOK8jsdoXVuHZYdGmnTyVBoZYksFT5_UDURTbMJw@mail.gmail.com>","Subject":"Re: [libcamera-devel] [PATCH 5/5] android: camera_device: Assert\n\tdescriptors_ is empty","X-BeenThere":"libcamera-devel@lists.libcamera.org","X-Mailman-Version":"2.1.29","Precedence":"list","List-Id":"<libcamera-devel.lists.libcamera.org>","List-Unsubscribe":"<https://lists.libcamera.org/options/libcamera-devel>,\n\t<mailto:libcamera-devel-request@lists.libcamera.org?subject=unsubscribe>","List-Archive":"<https://lists.libcamera.org/pipermail/libcamera-devel/>","List-Post":"<mailto:libcamera-devel@lists.libcamera.org>","List-Help":"<mailto:libcamera-devel-request@lists.libcamera.org?subject=help>","List-Subscribe":"<https://lists.libcamera.org/listinfo/libcamera-devel>,\n\t<mailto:libcamera-devel-request@lists.libcamera.org?subject=subscribe>","Cc":"libcamera devel <libcamera-devel@lists.libcamera.org>","Errors-To":"libcamera-devel-bounces@lists.libcamera.org","Sender":"\"libcamera-devel\" <libcamera-devel-bounces@lists.libcamera.org>"}},{"id":19599,"web_url":"https://patchwork.libcamera.org/comment/19599/","msgid":"<bfe91bb3-281e-83fb-9389-3021b4ff2574@ideasonboard.com>","date":"2021-09-10T12:34:15","subject":"Re: [libcamera-devel] [PATCH 2/5] android: camera_device: Clear\n\tstreams_ in stop()","submitter":{"id":86,"url":"https://patchwork.libcamera.org/api/people/86/","name":"Umang Jain","email":"umang.jain@ideasonboard.com"},"content":"Hi Jacopo,\n\nOn 9/6/21 7:31 PM, Jacopo Mondi wrote:\n> The CameraDevice::streams_ class member is populated at\n> configureStreams() time with one item per each stream requested to the\n> Camera device.\n>\n> When a new configuration is applied to the Camera HAL the list of\n> requested streams has to be re-initialized and the streams_ vector needs\n> to be manually cleared in order to be populated from scratch.\n>\n> As configureStreams() class CameraDevice::stop() at the very beginning,\n> centralize clearing the streams_ vector there even in the case the\n> camera has been stopped by a previous call to flush().\n>\n> Suggested-by: Hirokazu Honda <hiroh@chromium.org>\n> Signed-off-by: Jacopo Mondi <jacopo@jmondi.org>\n> ---\n>   src/android/camera_device.cpp | 21 ++++++++++++---------\n>   1 file changed, 12 insertions(+), 9 deletions(-)\n>\n> diff --git a/src/android/camera_device.cpp b/src/android/camera_device.cpp\n> index fda77db4540c..30c173a69720 100644\n> --- a/src/android/camera_device.cpp\n> +++ b/src/android/camera_device.cpp\n> @@ -448,8 +448,18 @@ void CameraDevice::flush()\n>   void CameraDevice::stop()\n>   {\n>   \tMutexLocker stateLock(stateMutex_);\n> -\tif (state_ == State::Stopped)\n> +\tif (state_ == State::Stopped) {\n> +\t\t/*\n> +\t\t * We get here if the list of streams requested by the camera\n> +\t\t * service has to be updated but the camera has been stopped\n> +\t\t * already, which happens if configureStreams() gets called\n> +\t\t * after a flush(). Clear the list of stream configurations,\n> +\t\t * no need to clear descriptors as stopping the camera completes\n> +\t\t * all the pending requests.\n> +\t\t */\n> +\t\tstreams_.clear();\n>   \t\treturn;\n> +\t}\n\n\nThis makes sense to me, but I facing a more fundamental question that \nwhy does flush() tries to stop the libcamera::Camera instance on it's \nown and setting state_ = State::Stopped; manually. Ideally it should \nhappen by calling CameraDevice::stop() inside flush(). Seems I am \nmissing something.. need to look at flush() documentation in depth.\n\nAs far as I have looked the current configureStreams() behaviour, the \nchange and reasons makes sense to me, so\n\nReviewed-by: Umang Jain <umang.jain@ideasonboard.com>\n\n>   \n>   \tworker_.stop();\n>   \tcamera_->stop();\n> @@ -544,6 +554,7 @@ int CameraDevice::configureStreams(camera3_stream_configuration_t *stream_list)\n>   \t\tLOG(HAL, Error) << \"No streams in configuration\";\n>   \t\treturn -EINVAL;\n>   \t}\n> +\tstreams_.reserve(stream_list->num_streams);\n>   \n>   #if defined(OS_CHROMEOS)\n>   \tif (!validateCropRotate(*stream_list))\n> @@ -560,14 +571,6 @@ int CameraDevice::configureStreams(camera3_stream_configuration_t *stream_list)\n>   \t\treturn -EINVAL;\n>   \t}\n>   \n> -\t/*\n> -\t * Clear and remove any existing configuration from previous calls, and\n> -\t * ensure the required entries are available without further\n> -\t * reallocation.\n> -\t */\n> -\tstreams_.clear();\n> -\tstreams_.reserve(stream_list->num_streams);\n> -\n>   \tstd::vector<Camera3StreamConfig> streamConfigs;\n>   \tstreamConfigs.reserve(stream_list->num_streams);\n>","headers":{"Return-Path":"<libcamera-devel-bounces@lists.libcamera.org>","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 CCA8BBDC71\n\tfor <parsemail@patchwork.libcamera.org>;\n\tFri, 10 Sep 2021 12:34:23 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id E850769170;\n\tFri, 10 Sep 2021 14:34:22 +0200 (CEST)","from perceval.ideasonboard.com (perceval.ideasonboard.com\n\t[IPv6:2001:4b98:dc2:55:216:3eff:fef7:d647])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTPS id EA4C469169\n\tfor <libcamera-devel@lists.libcamera.org>;\n\tFri, 10 Sep 2021 14:34:20 +0200 (CEST)","from [192.168.1.104] (unknown [103.251.226.149])\n\tby perceval.ideasonboard.com (Postfix) with ESMTPSA id 0E95D883;\n\tFri, 10 Sep 2021 14:34:19 +0200 (CEST)"],"Authentication-Results":"lancelot.ideasonboard.com; dkim=pass (1024-bit key;\n\tunprotected) header.d=ideasonboard.com header.i=@ideasonboard.com\n\theader.b=\"Oob7RHgl\"; dkim-atps=neutral","DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com;\n\ts=mail; t=1631277260;\n\tbh=sM3VFgqO6u6wJ5vgBKcMUriOZ6u4q+Z7IPckZz9p0EQ=;\n\th=Subject:To:References:From:Date:In-Reply-To:From;\n\tb=Oob7RHglJJ9M1MuPgeW1jR/WCkK6Vt4wmyaTrz8ml5G1vanZDv2D7y0QvtA1ktnsO\n\tjLZXlf6BXGAuBeOc8oY2vorWPlEZVBQJJ9d//bSA5Iw+ww0ZrRIvncuPm6eZ2kUvE2\n\tqsLs/i/7LouO1AmPTi28z0K7Mj8bM0BQ+4zpgGWg=","To":"Jacopo Mondi <jacopo@jmondi.org>, libcamera-devel@lists.libcamera.org","References":"<20210906140152.636883-1-jacopo@jmondi.org>\n\t<20210906140152.636883-3-jacopo@jmondi.org>","From":"Umang Jain <umang.jain@ideasonboard.com>","Message-ID":"<bfe91bb3-281e-83fb-9389-3021b4ff2574@ideasonboard.com>","Date":"Fri, 10 Sep 2021 18:04:15 +0530","User-Agent":"Mozilla/5.0 (X11; Linux x86_64; rv:78.0) Gecko/20100101\n\tThunderbird/78.10.2","MIME-Version":"1.0","In-Reply-To":"<20210906140152.636883-3-jacopo@jmondi.org>","Content-Type":"text/plain; charset=utf-8; format=flowed","Content-Transfer-Encoding":"7bit","Content-Language":"en-US","Subject":"Re: [libcamera-devel] [PATCH 2/5] android: camera_device: Clear\n\tstreams_ in stop()","X-BeenThere":"libcamera-devel@lists.libcamera.org","X-Mailman-Version":"2.1.29","Precedence":"list","List-Id":"<libcamera-devel.lists.libcamera.org>","List-Unsubscribe":"<https://lists.libcamera.org/options/libcamera-devel>,\n\t<mailto:libcamera-devel-request@lists.libcamera.org?subject=unsubscribe>","List-Archive":"<https://lists.libcamera.org/pipermail/libcamera-devel/>","List-Post":"<mailto:libcamera-devel@lists.libcamera.org>","List-Help":"<mailto:libcamera-devel-request@lists.libcamera.org?subject=help>","List-Subscribe":"<https://lists.libcamera.org/listinfo/libcamera-devel>,\n\t<mailto:libcamera-devel-request@lists.libcamera.org?subject=subscribe>","Errors-To":"libcamera-devel-bounces@lists.libcamera.org","Sender":"\"libcamera-devel\" <libcamera-devel-bounces@lists.libcamera.org>"}},{"id":19600,"web_url":"https://patchwork.libcamera.org/comment/19600/","msgid":"<5d9f690c-433f-58c4-4e28-da17db00ea9b@ideasonboard.com>","date":"2021-09-10T12:34:46","subject":"Re: [libcamera-devel] [PATCH 3/5] android: camera_device: Make\n\tabortRequest() take a const","submitter":{"id":86,"url":"https://patchwork.libcamera.org/api/people/86/","name":"Umang Jain","email":"umang.jain@ideasonboard.com"},"content":"Hi Jacopo,\n\nOn 9/6/21 7:31 PM, Jacopo Mondi wrote:\n> The CameraDevice::abortRequest() function should operate on const\n> pointers.\n>\n> Fix that.\n>\n> Signed-off-by: Jacopo Mondi <jacopo@jmondi.org>\nReviewed-by: Umang Jain <umang.jain@ideasonboard.com>\n> ---\n>   src/android/camera_device.cpp | 2 +-\n>   src/android/camera_device.h   | 2 +-\n>   2 files changed, 2 insertions(+), 2 deletions(-)\n>\n> diff --git a/src/android/camera_device.cpp b/src/android/camera_device.cpp\n> index 30c173a69720..a0ea138d9499 100644\n> --- a/src/android/camera_device.cpp\n> +++ b/src/android/camera_device.cpp\n> @@ -845,7 +845,7 @@ int CameraDevice::processControls(Camera3RequestDescriptor *descriptor)\n>   \treturn 0;\n>   }\n>   \n> -void CameraDevice::abortRequest(camera3_capture_request_t *request)\n> +void CameraDevice::abortRequest(const camera3_capture_request_t *request)\n>   {\n>   \tnotifyError(request->frame_number, nullptr, CAMERA3_MSG_ERROR_REQUEST);\n>   \n> diff --git a/src/android/camera_device.h b/src/android/camera_device.h\n> index a55769272651..54c4cb9ab499 100644\n> --- a/src/android/camera_device.h\n> +++ b/src/android/camera_device.h\n> @@ -96,7 +96,7 @@ private:\n>   \tlibcamera::FrameBuffer *createFrameBuffer(const buffer_handle_t camera3buffer,\n>   \t\t\t\t\t\t  libcamera::PixelFormat pixelFormat,\n>   \t\t\t\t\t\t  const libcamera::Size &size);\n> -\tvoid abortRequest(camera3_capture_request_t *request);\n> +\tvoid abortRequest(const camera3_capture_request_t *request);\n>   \tbool isValidRequest(camera3_capture_request_t *request) const;\n>   \tvoid notifyShutter(uint32_t frameNumber, uint64_t timestamp);\n>   \tvoid notifyError(uint32_t frameNumber, camera3_stream_t *stream,","headers":{"Return-Path":"<libcamera-devel-bounces@lists.libcamera.org>","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 1A61ABDC71\n\tfor <parsemail@patchwork.libcamera.org>;\n\tFri, 10 Sep 2021 12:34:53 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id CC72069170;\n\tFri, 10 Sep 2021 14:34:52 +0200 (CEST)","from perceval.ideasonboard.com (perceval.ideasonboard.com\n\t[213.167.242.64])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTPS id BBCCC69169\n\tfor <libcamera-devel@lists.libcamera.org>;\n\tFri, 10 Sep 2021 14:34:50 +0200 (CEST)","from [192.168.1.104] (unknown [103.251.226.149])\n\tby perceval.ideasonboard.com (Postfix) with ESMTPSA id B5EAC883;\n\tFri, 10 Sep 2021 14:34:49 +0200 (CEST)"],"Authentication-Results":"lancelot.ideasonboard.com; dkim=pass (1024-bit key;\n\tunprotected) header.d=ideasonboard.com header.i=@ideasonboard.com\n\theader.b=\"P7jvtxuH\"; dkim-atps=neutral","DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com;\n\ts=mail; t=1631277290;\n\tbh=Z/3lDk+jV+mfmLPJvYAmyQKfJRK/UWWjoQVcuLkrxS4=;\n\th=Subject:To:References:From:Date:In-Reply-To:From;\n\tb=P7jvtxuHrmwdxPgIhPv0U2j6LwOxHQd29KZOH7PqqizpGRSvcZ9DCRDo+/VqB0mOz\n\te/ZeJ2O1FEjPIvZ1qeHufkhIxLdNXN73dn5hpCIVfepToHykJe2jIrEK3Ge+ReKkpo\n\tWxaRjko83UXzXA+gJUz1DJecihJouEMP0yPGiWxc=","To":"Jacopo Mondi <jacopo@jmondi.org>, libcamera-devel@lists.libcamera.org","References":"<20210906140152.636883-1-jacopo@jmondi.org>\n\t<20210906140152.636883-4-jacopo@jmondi.org>","From":"Umang Jain <umang.jain@ideasonboard.com>","Message-ID":"<5d9f690c-433f-58c4-4e28-da17db00ea9b@ideasonboard.com>","Date":"Fri, 10 Sep 2021 18:04:46 +0530","User-Agent":"Mozilla/5.0 (X11; Linux x86_64; rv:78.0) Gecko/20100101\n\tThunderbird/78.10.2","MIME-Version":"1.0","In-Reply-To":"<20210906140152.636883-4-jacopo@jmondi.org>","Content-Type":"text/plain; charset=utf-8; format=flowed","Content-Transfer-Encoding":"7bit","Content-Language":"en-US","Subject":"Re: [libcamera-devel] [PATCH 3/5] android: camera_device: Make\n\tabortRequest() take a const","X-BeenThere":"libcamera-devel@lists.libcamera.org","X-Mailman-Version":"2.1.29","Precedence":"list","List-Id":"<libcamera-devel.lists.libcamera.org>","List-Unsubscribe":"<https://lists.libcamera.org/options/libcamera-devel>,\n\t<mailto:libcamera-devel-request@lists.libcamera.org?subject=unsubscribe>","List-Archive":"<https://lists.libcamera.org/pipermail/libcamera-devel/>","List-Post":"<mailto:libcamera-devel@lists.libcamera.org>","List-Help":"<mailto:libcamera-devel-request@lists.libcamera.org?subject=help>","List-Subscribe":"<https://lists.libcamera.org/listinfo/libcamera-devel>,\n\t<mailto:libcamera-devel-request@lists.libcamera.org?subject=subscribe>","Errors-To":"libcamera-devel-bounces@lists.libcamera.org","Sender":"\"libcamera-devel\" <libcamera-devel-bounces@lists.libcamera.org>"}},{"id":19601,"web_url":"https://patchwork.libcamera.org/comment/19601/","msgid":"<193ef895-6c15-6cbc-f7ca-95f89bda3fee@ideasonboard.com>","date":"2021-09-10T12:40:21","subject":"Re: [libcamera-devel] [PATCH 5/5] android: camera_device: Assert\n\tdescriptors_ is empty","submitter":{"id":86,"url":"https://patchwork.libcamera.org/api/people/86/","name":"Umang Jain","email":"umang.jain@ideasonboard.com"},"content":"Hi Jacopo,\n\nOn 9/6/21 7:31 PM, Jacopo Mondi wrote:\n> The CameraDevice::descriptors_ class member holds one item per each\n> in-flight request. A new descriptor is added to the map each time a new\n> request is queued to the camera and it gets then popped out when the\n> request completes or when an error happens before queueing it.\n>\n> As stopping the camera guarantees that all in-flight requests are\n> completed synchronously to the Camera::stop() call, there is no need to\n> manually clear the descriptors_ map, on the contrary, it might hides\n> issues in the handling of in-flight requests.\n>\n> Remove it and replace it with an assertion to make sure the underlying\n> camera behaves as intended and all Request are completed when the camera\n> is stopped.\n>\n> Signed-off-by: Jacopo Mondi <jacopo@jmondi.org>\n> ---\n>   src/android/camera_device.cpp | 11 +++++++----\n>   1 file changed, 7 insertions(+), 4 deletions(-)\n>\n> diff --git a/src/android/camera_device.cpp b/src/android/camera_device.cpp\n> index 0ce9b699bfae..1591ad98c7d5 100644\n> --- a/src/android/camera_device.cpp\n> +++ b/src/android/camera_device.cpp\n> @@ -442,6 +442,9 @@ void CameraDevice::flush()\n>   \tworker_.stop();\n>   \tcamera_->stop();\n>   \n> +\t/* Make sure all in-flight requests have been completed. */\n> +\tASSERT(descriptors_.empty());\n> +\n>   \tMutexLocker stateLock(stateMutex_);\n>   \tstate_ = State::Stopped;\n>   }\n> @@ -454,9 +457,7 @@ void CameraDevice::stop()\n>   \t\t * We get here if the list of streams requested by the camera\n>   \t\t * service has to be updated but the camera has been stopped\n>   \t\t * already, which happens if configureStreams() gets called\n> -\t\t * after a flush(). Clear the list of stream configurations,\n> -\t\t * no need to clear descriptors as stopping the camera completes\n> -\t\t * all the pending requests.\n> +\t\t * after a flush().\n>   \t\t */\n>   \t\tstreams_.clear();\n>   \t\treturn;\n> @@ -465,7 +466,9 @@ void CameraDevice::stop()\n>   \tworker_.stop();\n>   \tcamera_->stop();\n>   \n> -\tdescriptors_.clear();\n> +\t/* Make sure all in-flight requests have been completed. */\n> +\tASSERT(descriptors_.empty());\n> +\n\nWell, the descriptors_ mapped might seem empty but some descriptors  can \nbe queued somewhere else for async stuff in future ;-)\n\nanyways looks good to me as per master so,\n\nReviewed-by: Umang Jain <umang.jain@ideasonboard.com>\n\n>   \tstreams_.clear();\n>   \n>   \tstate_ = State::Stopped;","headers":{"Return-Path":"<libcamera-devel-bounces@lists.libcamera.org>","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 50027BDB1D\n\tfor <parsemail@patchwork.libcamera.org>;\n\tFri, 10 Sep 2021 12:40:29 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id A7DFD6917C;\n\tFri, 10 Sep 2021 14:40:28 +0200 (CEST)","from perceval.ideasonboard.com (perceval.ideasonboard.com\n\t[IPv6:2001:4b98:dc2:55:216:3eff:fef7:d647])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTPS id 37DEC69169\n\tfor <libcamera-devel@lists.libcamera.org>;\n\tFri, 10 Sep 2021 14:40:27 +0200 (CEST)","from [192.168.1.104] (unknown [103.251.226.149])\n\tby perceval.ideasonboard.com (Postfix) with ESMTPSA id 53C24883;\n\tFri, 10 Sep 2021 14:40:26 +0200 (CEST)"],"Authentication-Results":"lancelot.ideasonboard.com; dkim=pass (1024-bit key;\n\tunprotected) header.d=ideasonboard.com header.i=@ideasonboard.com\n\theader.b=\"DAFGGDY3\"; dkim-atps=neutral","DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com;\n\ts=mail; t=1631277626;\n\tbh=3ox2T9Qa4rlMQ9ynUDUq3a8ysW2sgfB+V2PEQTAi3yY=;\n\th=Subject:To:References:From:Date:In-Reply-To:From;\n\tb=DAFGGDY3p1aiDRYO9kffqaUOmcbUttbVg+hjW7K26nCsJv0LW2LgfEWU8vSr7VF5s\n\tj0I34jeICnU/nHJfa4IUPCuPr3YQaBFoIQ9knm4lwv4v/KvB90U7j8W3mdtHx0hQru\n\t9+5mOHa/ocubAF8Nzed2V+P5sTriIpZ6oIRMtMjQ=","To":"Jacopo Mondi <jacopo@jmondi.org>, libcamera-devel@lists.libcamera.org","References":"<20210906140152.636883-1-jacopo@jmondi.org>\n\t<20210906140152.636883-6-jacopo@jmondi.org>","From":"Umang Jain <umang.jain@ideasonboard.com>","Message-ID":"<193ef895-6c15-6cbc-f7ca-95f89bda3fee@ideasonboard.com>","Date":"Fri, 10 Sep 2021 18:10:21 +0530","User-Agent":"Mozilla/5.0 (X11; Linux x86_64; rv:78.0) Gecko/20100101\n\tThunderbird/78.10.2","MIME-Version":"1.0","In-Reply-To":"<20210906140152.636883-6-jacopo@jmondi.org>","Content-Type":"text/plain; charset=utf-8; format=flowed","Content-Transfer-Encoding":"8bit","Content-Language":"en-US","Subject":"Re: [libcamera-devel] [PATCH 5/5] android: camera_device: Assert\n\tdescriptors_ is empty","X-BeenThere":"libcamera-devel@lists.libcamera.org","X-Mailman-Version":"2.1.29","Precedence":"list","List-Id":"<libcamera-devel.lists.libcamera.org>","List-Unsubscribe":"<https://lists.libcamera.org/options/libcamera-devel>,\n\t<mailto:libcamera-devel-request@lists.libcamera.org?subject=unsubscribe>","List-Archive":"<https://lists.libcamera.org/pipermail/libcamera-devel/>","List-Post":"<mailto:libcamera-devel@lists.libcamera.org>","List-Help":"<mailto:libcamera-devel-request@lists.libcamera.org?subject=help>","List-Subscribe":"<https://lists.libcamera.org/listinfo/libcamera-devel>,\n\t<mailto:libcamera-devel-request@lists.libcamera.org?subject=subscribe>","Errors-To":"libcamera-devel-bounces@lists.libcamera.org","Sender":"\"libcamera-devel\" <libcamera-devel-bounces@lists.libcamera.org>"}},{"id":19604,"web_url":"https://patchwork.libcamera.org/comment/19604/","msgid":"<CAO5uPHOjGGBRt4YB9GSh_knQZg46XgaWiohEVwsR_8LwPZEmhg@mail.gmail.com>","date":"2021-09-10T12:54:45","subject":"Re: [libcamera-devel] [PATCH 3/5] android: camera_device: Make\n\tabortRequest() take a const","submitter":{"id":63,"url":"https://patchwork.libcamera.org/api/people/63/","name":"Hirokazu Honda","email":"hiroh@chromium.org"},"content":"Hi Jacopo,\n\nOn Fri, Sep 10, 2021 at 9:34 PM Umang Jain <umang.jain@ideasonboard.com> wrote:\n>\n> Hi Jacopo,\n>\n> On 9/6/21 7:31 PM, Jacopo Mondi wrote:\n> > The CameraDevice::abortRequest() function should operate on const\n> > pointers.\n> >\n> > Fix that.\n> >\n> > Signed-off-by: Jacopo Mondi <jacopo@jmondi.org>\n> Reviewed-by: Umang Jain <umang.jain@ideasonboard.com>\n\nReviewed-by: Hirokazu Honda <hiroh@chromium.org>\n> > ---\n> >   src/android/camera_device.cpp | 2 +-\n> >   src/android/camera_device.h   | 2 +-\n> >   2 files changed, 2 insertions(+), 2 deletions(-)\n> >\n> > diff --git a/src/android/camera_device.cpp b/src/android/camera_device.cpp\n> > index 30c173a69720..a0ea138d9499 100644\n> > --- a/src/android/camera_device.cpp\n> > +++ b/src/android/camera_device.cpp\n> > @@ -845,7 +845,7 @@ int CameraDevice::processControls(Camera3RequestDescriptor *descriptor)\n> >       return 0;\n> >   }\n> >\n> > -void CameraDevice::abortRequest(camera3_capture_request_t *request)\n> > +void CameraDevice::abortRequest(const camera3_capture_request_t *request)\n> >   {\n> >       notifyError(request->frame_number, nullptr, CAMERA3_MSG_ERROR_REQUEST);\n> >\n> > diff --git a/src/android/camera_device.h b/src/android/camera_device.h\n> > index a55769272651..54c4cb9ab499 100644\n> > --- a/src/android/camera_device.h\n> > +++ b/src/android/camera_device.h\n> > @@ -96,7 +96,7 @@ private:\n> >       libcamera::FrameBuffer *createFrameBuffer(const buffer_handle_t camera3buffer,\n> >                                                 libcamera::PixelFormat pixelFormat,\n> >                                                 const libcamera::Size &size);\n> > -     void abortRequest(camera3_capture_request_t *request);\n> > +     void abortRequest(const camera3_capture_request_t *request);\n> >       bool isValidRequest(camera3_capture_request_t *request) const;\n> >       void notifyShutter(uint32_t frameNumber, uint64_t timestamp);\n> >       void notifyError(uint32_t frameNumber, camera3_stream_t *stream,","headers":{"Return-Path":"<libcamera-devel-bounces@lists.libcamera.org>","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 DFC33BDB1D\n\tfor <parsemail@patchwork.libcamera.org>;\n\tFri, 10 Sep 2021 12:54:57 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id AE7126917C;\n\tFri, 10 Sep 2021 14:54:57 +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 E703369169\n\tfor <libcamera-devel@lists.libcamera.org>;\n\tFri, 10 Sep 2021 14:54:56 +0200 (CEST)","by mail-ej1-x62c.google.com with SMTP id i21so4099452ejd.2\n\tfor <libcamera-devel@lists.libcamera.org>;\n\tFri, 10 Sep 2021 05:54:56 -0700 (PDT)"],"Authentication-Results":"lancelot.ideasonboard.com;\n\tdkim=fail reason=\"signature verification failed\" (1024-bit key;\n\tunprotected) header.d=chromium.org header.i=@chromium.org\n\theader.b=\"lNj03uqJ\"; 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=Wy8vbgkw8zbTddlEYDyd02n0U56QWSwfsJHLwIwch/Y=;\n\tb=lNj03uqJAN/2FRxI4Z+Z1m3AWwW9vhKF7HNUtAr2Q8lcde+Pvn75Za9qlzEGlvONjw\n\tssiekY5geNBNPqKuPwq/KYJCLk8960Ii6V+lJfemUXQB6Ljk/KFnXVBbAK2oZ5nAH+Wz\n\tjszWYD4Z2wi9rsCLtDEhBUZkmA/obYujD2PEI=","X-Google-DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/relaxed;\n\td=1e100.net; s=20210112;\n\th=x-gm-message-state:mime-version:references:in-reply-to:from:date\n\t:message-id:subject:to:cc;\n\tbh=Wy8vbgkw8zbTddlEYDyd02n0U56QWSwfsJHLwIwch/Y=;\n\tb=zc5ZU/PM1UxU307JUlOXWgz+a36g8IVs7qLZjyhULsQkYr52JzH4e042Okvn4/ZJ4u\n\thjMM4Q584E1Ct1UniZdugAS3fU0OTCKyBOGJVNdI4qLohvwhfkayLvkZxxVEe5t21CKX\n\t0oGSYh2IWxROgGJPpZo7gR826w6IlsrIolr3K2+ckWXhKbNMq3yt5RCgJkyEi42MhJnp\n\tTGjJVsdJ3/bflx7kQhLiWCfvIHkg9uH4a4kSs87rZSNMKVsPfPQCQe7xMCcJwZflQFnw\n\tDUxt60ePfOCr2CCraovMtUKf9Q+87RFDiXdQSTyGIqGQPnhC/FhBGaP5JopuqwXBjiAk\n\thF9Q==","X-Gm-Message-State":"AOAM5331urNM7A3KeioZE2Nc6tfl977y+UzFn4XYZDG8z4A0nPNM/zLf\n\thJSUHnGjs62fFrTDOyvZyiwUdZXQcHPgqbLQrCSbWXIve3g=","X-Google-Smtp-Source":"ABdhPJx9s56SLUa54sVyx5281j1/jpaA+1gpTjT2TCmPwNOUWPJKKodP/yoj1wygJNGUSO3yRx4NQZX7FxkTgvuokMU=","X-Received":"by 2002:a17:906:2b0d:: with SMTP id\n\ta13mr4759336ejg.150.1631278496628; \n\tFri, 10 Sep 2021 05:54:56 -0700 (PDT)","MIME-Version":"1.0","References":"<20210906140152.636883-1-jacopo@jmondi.org>\n\t<20210906140152.636883-4-jacopo@jmondi.org>\n\t<5d9f690c-433f-58c4-4e28-da17db00ea9b@ideasonboard.com>","In-Reply-To":"<5d9f690c-433f-58c4-4e28-da17db00ea9b@ideasonboard.com>","From":"Hirokazu Honda <hiroh@chromium.org>","Date":"Fri, 10 Sep 2021 21:54:45 +0900","Message-ID":"<CAO5uPHOjGGBRt4YB9GSh_knQZg46XgaWiohEVwsR_8LwPZEmhg@mail.gmail.com>","To":"Umang Jain <umang.jain@ideasonboard.com>","Content-Type":"text/plain; charset=\"UTF-8\"","Subject":"Re: [libcamera-devel] [PATCH 3/5] android: camera_device: Make\n\tabortRequest() take a const","X-BeenThere":"libcamera-devel@lists.libcamera.org","X-Mailman-Version":"2.1.29","Precedence":"list","List-Id":"<libcamera-devel.lists.libcamera.org>","List-Unsubscribe":"<https://lists.libcamera.org/options/libcamera-devel>,\n\t<mailto:libcamera-devel-request@lists.libcamera.org?subject=unsubscribe>","List-Archive":"<https://lists.libcamera.org/pipermail/libcamera-devel/>","List-Post":"<mailto:libcamera-devel@lists.libcamera.org>","List-Help":"<mailto:libcamera-devel-request@lists.libcamera.org?subject=help>","List-Subscribe":"<https://lists.libcamera.org/listinfo/libcamera-devel>,\n\t<mailto:libcamera-devel-request@lists.libcamera.org?subject=subscribe>","Cc":"libcamera devel <libcamera-devel@lists.libcamera.org>","Errors-To":"libcamera-devel-bounces@lists.libcamera.org","Sender":"\"libcamera-devel\" <libcamera-devel-bounces@lists.libcamera.org>"}},{"id":19697,"web_url":"https://patchwork.libcamera.org/comment/19697/","msgid":"<YUFiow6rkD2YwWn6@pendragon.ideasonboard.com>","date":"2021-09-15T03:04:03","subject":"Re: [libcamera-devel] [PATCH 2/5] android: camera_device: Clear\n\tstreams_ in stop()","submitter":{"id":2,"url":"https://patchwork.libcamera.org/api/people/2/","name":"Laurent Pinchart","email":"laurent.pinchart@ideasonboard.com"},"content":"Hello,\n\nOn Fri, Sep 10, 2021 at 06:04:15PM +0530, Umang Jain wrote:\n> On 9/6/21 7:31 PM, Jacopo Mondi wrote:\n> > The CameraDevice::streams_ class member is populated at\n> > configureStreams() time with one item per each stream requested to the\n> > Camera device.\n> >\n> > When a new configuration is applied to the Camera HAL the list of\n> > requested streams has to be re-initialized and the streams_ vector needs\n> > to be manually cleared in order to be populated from scratch.\n> >\n> > As configureStreams() class CameraDevice::stop() at the very beginning,\n\ns/class/calls/\n\n> > centralize clearing the streams_ vector there even in the case the\n> > camera has been stopped by a previous call to flush().\n> >\n> > Suggested-by: Hirokazu Honda <hiroh@chromium.org>\n> > Signed-off-by: Jacopo Mondi <jacopo@jmondi.org>\n> > ---\n> >   src/android/camera_device.cpp | 21 ++++++++++++---------\n> >   1 file changed, 12 insertions(+), 9 deletions(-)\n> >\n> > diff --git a/src/android/camera_device.cpp b/src/android/camera_device.cpp\n> > index fda77db4540c..30c173a69720 100644\n> > --- a/src/android/camera_device.cpp\n> > +++ b/src/android/camera_device.cpp\n> > @@ -448,8 +448,18 @@ void CameraDevice::flush()\n> >   void CameraDevice::stop()\n> >   {\n> >   \tMutexLocker stateLock(stateMutex_);\n> > -\tif (state_ == State::Stopped)\n> > +\tif (state_ == State::Stopped) {\n> > +\t\t/*\n> > +\t\t * We get here if the list of streams requested by the camera\n> > +\t\t * service has to be updated but the camera has been stopped\n> > +\t\t * already, which happens if configureStreams() gets called\n> > +\t\t * after a flush(). Clear the list of stream configurations,\n> > +\t\t * no need to clear descriptors as stopping the camera completes\n> > +\t\t * all the pending requests.\n> > +\t\t */\n> > +\t\tstreams_.clear();\n\nstop() is called in configureStreams() and in close(). close() already\nhas a streams_.clear() call, which can't be removed (yet ?) as there's\nno guarantee close() will be called with state_ == State::Stopped. I'm\nthus not sure what this patch brings, given that the streams_.clear()\ncall here is not \"centralized\", it's only meant for the flush() case.\nMaybe I'll have a better understanding when reading the next patches.\n\n> >   \t\treturn;\n> > +\t}\n> \n> This makes sense to me, but I facing a more fundamental question that \n> why does flush() tries to stop the libcamera::Camera instance on it's \n> own and setting state_ = State::Stopped; manually. Ideally it should \n> happen by calling CameraDevice::stop() inside flush(). Seems I am \n> missing something.. need to look at flush() documentation in depth.\n> \n> As far as I have looked the current configureStreams() behaviour, the \n> change and reasons makes sense to me, so\n> \n> Reviewed-by: Umang Jain <umang.jain@ideasonboard.com>\n> \n> >   \n> >   \tworker_.stop();\n> >   \tcamera_->stop();\n> > @@ -544,6 +554,7 @@ int CameraDevice::configureStreams(camera3_stream_configuration_t *stream_list)\n> >   \t\tLOG(HAL, Error) << \"No streams in configuration\";\n> >   \t\treturn -EINVAL;\n> >   \t}\n\nBlank line.\n\n> > +\tstreams_.reserve(stream_list->num_streams);\n> >   \n> >   #if defined(OS_CHROMEOS)\n> >   \tif (!validateCropRotate(*stream_list))\n> > @@ -560,14 +571,6 @@ int CameraDevice::configureStreams(camera3_stream_configuration_t *stream_list)\n> >   \t\treturn -EINVAL;\n> >   \t}\n> >   \n> > -\t/*\n> > -\t * Clear and remove any existing configuration from previous calls, and\n> > -\t * ensure the required entries are available without further\n> > -\t * reallocation.\n> > -\t */\n> > -\tstreams_.clear();\n> > -\tstreams_.reserve(stream_list->num_streams);\n> > -\n> >   \tstd::vector<Camera3StreamConfig> streamConfigs;\n> >   \tstreamConfigs.reserve(stream_list->num_streams);\n> >","headers":{"Return-Path":"<libcamera-devel-bounces@lists.libcamera.org>","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 91400BDB1D\n\tfor <parsemail@patchwork.libcamera.org>;\n\tWed, 15 Sep 2021 03:04:31 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id EF01869189;\n\tWed, 15 Sep 2021 05:04:30 +0200 (CEST)","from perceval.ideasonboard.com (perceval.ideasonboard.com\n\t[IPv6:2001:4b98:dc2:55:216:3eff:fef7:d647])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTPS id A482F60247\n\tfor <libcamera-devel@lists.libcamera.org>;\n\tWed, 15 Sep 2021 05:04:28 +0200 (CEST)","from pendragon.ideasonboard.com (62-78-145-57.bb.dnainternet.fi\n\t[62.78.145.57])\n\tby perceval.ideasonboard.com (Postfix) with ESMTPSA id 1060E24F;\n\tWed, 15 Sep 2021 05:04:27 +0200 (CEST)"],"Authentication-Results":"lancelot.ideasonboard.com;\n\tdkim=fail reason=\"signature verification failed\" (1024-bit key;\n\tunprotected) header.d=ideasonboard.com header.i=@ideasonboard.com\n\theader.b=\"hAKejNXU\"; dkim-atps=neutral","DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com;\n\ts=mail; t=1631675068;\n\tbh=0634IA4bi2nbkNY7SkOf/GPKGMsoCphhiJik/g3Ypos=;\n\th=Date:From:To:Cc:Subject:References:In-Reply-To:From;\n\tb=hAKejNXU8yZsZHzflygb27/hOULLX65cwkipkCvS8tGDCfRb3J927MSzRW8BzWyoM\n\tSsJzYtS+HcFCpeGLsFJaBf9YIWSDGRa7V1mpfLhDILsyyTOEQfw05lK/Qu1ps9uOCK\n\t2w8Tu9ecY9+ibv5ws0PNvymrjUJ8LGaBLVRkLSgI=","Date":"Wed, 15 Sep 2021 06:04:03 +0300","From":"Laurent Pinchart <laurent.pinchart@ideasonboard.com>","To":"Umang Jain <umang.jain@ideasonboard.com>","Message-ID":"<YUFiow6rkD2YwWn6@pendragon.ideasonboard.com>","References":"<20210906140152.636883-1-jacopo@jmondi.org>\n\t<20210906140152.636883-3-jacopo@jmondi.org>\n\t<bfe91bb3-281e-83fb-9389-3021b4ff2574@ideasonboard.com>","MIME-Version":"1.0","Content-Type":"text/plain; charset=utf-8","Content-Disposition":"inline","In-Reply-To":"<bfe91bb3-281e-83fb-9389-3021b4ff2574@ideasonboard.com>","Subject":"Re: [libcamera-devel] [PATCH 2/5] android: camera_device: Clear\n\tstreams_ in stop()","X-BeenThere":"libcamera-devel@lists.libcamera.org","X-Mailman-Version":"2.1.29","Precedence":"list","List-Id":"<libcamera-devel.lists.libcamera.org>","List-Unsubscribe":"<https://lists.libcamera.org/options/libcamera-devel>,\n\t<mailto:libcamera-devel-request@lists.libcamera.org?subject=unsubscribe>","List-Archive":"<https://lists.libcamera.org/pipermail/libcamera-devel/>","List-Post":"<mailto:libcamera-devel@lists.libcamera.org>","List-Help":"<mailto:libcamera-devel-request@lists.libcamera.org?subject=help>","List-Subscribe":"<https://lists.libcamera.org/listinfo/libcamera-devel>,\n\t<mailto:libcamera-devel-request@lists.libcamera.org?subject=subscribe>","Cc":"libcamera-devel@lists.libcamera.org","Errors-To":"libcamera-devel-bounces@lists.libcamera.org","Sender":"\"libcamera-devel\" <libcamera-devel-bounces@lists.libcamera.org>"}},{"id":19698,"web_url":"https://patchwork.libcamera.org/comment/19698/","msgid":"<YUFji0MSMp6A84lB@pendragon.ideasonboard.com>","date":"2021-09-15T03:07:55","subject":"Re: [libcamera-devel] [PATCH 3/5] android: camera_device: Make\n\tabortRequest() take a const","submitter":{"id":2,"url":"https://patchwork.libcamera.org/api/people/2/","name":"Laurent Pinchart","email":"laurent.pinchart@ideasonboard.com"},"content":"Hi Jacopo,\n\nThank you for the patch.\n\nOn Mon, Sep 06, 2021 at 04:01:50PM +0200, Jacopo Mondi wrote:\n> The CameraDevice::abortRequest() function should operate on const\n> pointers.\n> \n> Fix that.\n> \n> Signed-off-by: Jacopo Mondi <jacopo@jmondi.org>\n\nReviewed-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>\n\n> ---\n>  src/android/camera_device.cpp | 2 +-\n>  src/android/camera_device.h   | 2 +-\n>  2 files changed, 2 insertions(+), 2 deletions(-)\n> \n> diff --git a/src/android/camera_device.cpp b/src/android/camera_device.cpp\n> index 30c173a69720..a0ea138d9499 100644\n> --- a/src/android/camera_device.cpp\n> +++ b/src/android/camera_device.cpp\n> @@ -845,7 +845,7 @@ int CameraDevice::processControls(Camera3RequestDescriptor *descriptor)\n>  \treturn 0;\n>  }\n>  \n> -void CameraDevice::abortRequest(camera3_capture_request_t *request)\n> +void CameraDevice::abortRequest(const camera3_capture_request_t *request)\n>  {\n>  \tnotifyError(request->frame_number, nullptr, CAMERA3_MSG_ERROR_REQUEST);\n>  \n> diff --git a/src/android/camera_device.h b/src/android/camera_device.h\n> index a55769272651..54c4cb9ab499 100644\n> --- a/src/android/camera_device.h\n> +++ b/src/android/camera_device.h\n> @@ -96,7 +96,7 @@ private:\n>  \tlibcamera::FrameBuffer *createFrameBuffer(const buffer_handle_t camera3buffer,\n>  \t\t\t\t\t\t  libcamera::PixelFormat pixelFormat,\n>  \t\t\t\t\t\t  const libcamera::Size &size);\n> -\tvoid abortRequest(camera3_capture_request_t *request);\n> +\tvoid abortRequest(const camera3_capture_request_t *request);\n>  \tbool isValidRequest(camera3_capture_request_t *request) const;\n>  \tvoid notifyShutter(uint32_t frameNumber, uint64_t timestamp);\n>  \tvoid notifyError(uint32_t frameNumber, camera3_stream_t *stream,","headers":{"Return-Path":"<libcamera-devel-bounces@lists.libcamera.org>","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 A29D5BDC71\n\tfor <parsemail@patchwork.libcamera.org>;\n\tWed, 15 Sep 2021 03:08:23 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id 060D069189;\n\tWed, 15 Sep 2021 05:08:23 +0200 (CEST)","from perceval.ideasonboard.com (perceval.ideasonboard.com\n\t[IPv6:2001:4b98:dc2:55:216:3eff:fef7:d647])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTPS id EB1AE60247\n\tfor <libcamera-devel@lists.libcamera.org>;\n\tWed, 15 Sep 2021 05:08:20 +0200 (CEST)","from pendragon.ideasonboard.com (62-78-145-57.bb.dnainternet.fi\n\t[62.78.145.57])\n\tby perceval.ideasonboard.com (Postfix) with ESMTPSA id 5C45A24F;\n\tWed, 15 Sep 2021 05:08:20 +0200 (CEST)"],"Authentication-Results":"lancelot.ideasonboard.com; dkim=pass (1024-bit key;\n\tunprotected) header.d=ideasonboard.com header.i=@ideasonboard.com\n\theader.b=\"cqSOlV2Q\"; dkim-atps=neutral","DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com;\n\ts=mail; t=1631675300;\n\tbh=wjINNwnjaRnc0kIHCN4gt93in0+g6I1sZPy95ENRF74=;\n\th=Date:From:To:Cc:Subject:References:In-Reply-To:From;\n\tb=cqSOlV2Q46RfXXvkSZC20E4moNyUyXqBEmMlI1njtRT85B6nlvXBiCYQxDF17S8m1\n\tMLyQihAw4HxUNU8+92ZcbMF6WmxINIOntkarm3jrWP/Cok2hkmGtkOQy4i0JNY8Mdq\n\tKFcORzBPePrwLf18XSlUUxCbETzfHysW5TgrNFSA=","Date":"Wed, 15 Sep 2021 06:07:55 +0300","From":"Laurent Pinchart <laurent.pinchart@ideasonboard.com>","To":"Jacopo Mondi <jacopo@jmondi.org>","Message-ID":"<YUFji0MSMp6A84lB@pendragon.ideasonboard.com>","References":"<20210906140152.636883-1-jacopo@jmondi.org>\n\t<20210906140152.636883-4-jacopo@jmondi.org>","MIME-Version":"1.0","Content-Type":"text/plain; charset=utf-8","Content-Disposition":"inline","In-Reply-To":"<20210906140152.636883-4-jacopo@jmondi.org>","Subject":"Re: [libcamera-devel] [PATCH 3/5] android: camera_device: Make\n\tabortRequest() take a const","X-BeenThere":"libcamera-devel@lists.libcamera.org","X-Mailman-Version":"2.1.29","Precedence":"list","List-Id":"<libcamera-devel.lists.libcamera.org>","List-Unsubscribe":"<https://lists.libcamera.org/options/libcamera-devel>,\n\t<mailto:libcamera-devel-request@lists.libcamera.org?subject=unsubscribe>","List-Archive":"<https://lists.libcamera.org/pipermail/libcamera-devel/>","List-Post":"<mailto:libcamera-devel@lists.libcamera.org>","List-Help":"<mailto:libcamera-devel-request@lists.libcamera.org?subject=help>","List-Subscribe":"<https://lists.libcamera.org/listinfo/libcamera-devel>,\n\t<mailto:libcamera-devel-request@lists.libcamera.org?subject=subscribe>","Cc":"libcamera-devel@lists.libcamera.org","Errors-To":"libcamera-devel-bounces@lists.libcamera.org","Sender":"\"libcamera-devel\" <libcamera-devel-bounces@lists.libcamera.org>"}},{"id":19830,"web_url":"https://patchwork.libcamera.org/comment/19830/","msgid":"<CAO5uPHNwVxQ6-0-k=TnN=6vx4W42RP1u_Rsy815osUap8cCDqw@mail.gmail.com>","date":"2021-09-24T08:00:28","subject":"Re: [libcamera-devel] [PATCH 0/5] android: Fix descriptors_ clean up","submitter":{"id":63,"url":"https://patchwork.libcamera.org/api/people/63/","name":"Hirokazu Honda","email":"hiroh@chromium.org"},"content":"Hi Jacopo, can you merge this?\n\nOn Mon, Sep 6, 2021 at 11:01 PM Jacopo Mondi <jacopo@jmondi.org> wrote:\n>\n> While testing Hiro's patch, which I'm re-sending here, I tried as well to\n> centralize the streams_ and descriptors_ cleanup.\n>\n> This lead to the discovery of a potentially missed frame error in CameraWorker.\n>\n> On top of that, I have added two assertions to enforce the correct Camera\n> behavior after a stop() call.\n>\n> Thanks\n>    j\n>\n> Hirokazu Honda (1):\n>   android: camera_device: Fix crash in calling CameraDevice::close()\n>\n> Jacopo Mondi (4):\n>   android: camera_device: Clear streams_ in stop()\n>   android: camera_device: Make abortRequest() take a const\n>   android: camera_worker: Notify fence wait failures\n>   android: camera_device: Assert descriptors_ is empty\n>\n>  src/android/camera_device.cpp | 54 ++++++++++++++++++++++++++---------\n>  src/android/camera_device.h   |  3 +-\n>  src/android/camera_worker.cpp | 10 ++++---\n>  src/android/camera_worker.h   |  9 ++++--\n>  4 files changed, 55 insertions(+), 21 deletions(-)\n>\n> --\n> 2.32.0\n>","headers":{"Return-Path":"<libcamera-devel-bounces@lists.libcamera.org>","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 CED93BDC71\n\tfor <parsemail@patchwork.libcamera.org>;\n\tFri, 24 Sep 2021 08:00:42 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id 442696918C;\n\tFri, 24 Sep 2021 10:00:41 +0200 (CEST)","from mail-ed1-x52d.google.com (mail-ed1-x52d.google.com\n\t[IPv6:2a00:1450:4864:20::52d])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTPS id E683869186\n\tfor <libcamera-devel@lists.libcamera.org>;\n\tFri, 24 Sep 2021 10:00:39 +0200 (CEST)","by mail-ed1-x52d.google.com with SMTP id g8so32838201edt.7\n\tfor <libcamera-devel@lists.libcamera.org>;\n\tFri, 24 Sep 2021 01:00:39 -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=\"bN0DV8ny\"; 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=ZBHW0qeSVJr36RluDJZIK/QShzwhhkZbqPFIhoYi0sg=;\n\tb=bN0DV8ny90YxS5rXBumLagLYLhdI5MSlsGWNR0J4a/wMJ6m0xoJQD2+TS9B7ajsIBv\n\t3v2K6ZdUP9WxtYGJncYTjJFUVoZzzy+AZAYrb0ZUXWULaSw/y8LdnPAwQRQff814LutU\n\tgQqnWGErzdtLPFhapJ59YoxjEqyR1d72arop8=","X-Google-DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/relaxed;\n\td=1e100.net; s=20210112;\n\th=x-gm-message-state:mime-version:references:in-reply-to:from:date\n\t:message-id:subject:to:cc;\n\tbh=ZBHW0qeSVJr36RluDJZIK/QShzwhhkZbqPFIhoYi0sg=;\n\tb=2+OY8jVlqTFpyJbwn2HjT+6U53a+araUNR17lSLaPmW/GnoZnAXHDxJzm+3pDJm5cn\n\t3HZJjkw2gXTt3EagvsLBLTfQndggBb1eXh/ZrdEgrDztV7rAfoH3cXHGEtsr7Fo29GIm\n\tyuaNJxNcZ8ZTs02t2wxmZi/C/WrekYtgsvORshQjPQK+z+yp7clxg+DSSdGCkVrmP97j\n\tufGHUI9JcScjEi2N9pasYbcB29UF48IexFkkE9WBiGXXr2Q7K2Ebz6blb4PBhwEEhNa+\n\tT1XC8WQFub2NdQkRDKj+tkWF8FadqOHVB7w405gxpzTaO61Z2oFfHOfYP+S/XNrw510u\n\tN3cQ==","X-Gm-Message-State":"AOAM530/w3TkaeV1RPXk2wQ1d5i3W6LAFffu6VTsJOapB6y7/BzJNT9j\n\tam8FTnSn/KPNrUO3Y82Fnhfu/GA9Oup2bCTbMHrINeuvf9w=","X-Google-Smtp-Source":"ABdhPJxuJv1dk+/gj+CIKVmVPaxqudZ+0UP0c2TQlGvS4OVitNJLEfMgz2ksTPvKZ5Tu+ndcB/AbPgSMq+kETQmvqoQ=","X-Received":"by 2002:a17:906:4fd6:: with SMTP id\n\ti22mr9612751ejw.92.1632470439489; \n\tFri, 24 Sep 2021 01:00:39 -0700 (PDT)","MIME-Version":"1.0","References":"<20210906140152.636883-1-jacopo@jmondi.org>","In-Reply-To":"<20210906140152.636883-1-jacopo@jmondi.org>","From":"Hirokazu Honda <hiroh@chromium.org>","Date":"Fri, 24 Sep 2021 17:00:28 +0900","Message-ID":"<CAO5uPHNwVxQ6-0-k=TnN=6vx4W42RP1u_Rsy815osUap8cCDqw@mail.gmail.com>","To":"Jacopo Mondi <jacopo@jmondi.org>","Content-Type":"text/plain; charset=\"UTF-8\"","Subject":"Re: [libcamera-devel] [PATCH 0/5] android: Fix descriptors_ clean up","X-BeenThere":"libcamera-devel@lists.libcamera.org","X-Mailman-Version":"2.1.29","Precedence":"list","List-Id":"<libcamera-devel.lists.libcamera.org>","List-Unsubscribe":"<https://lists.libcamera.org/options/libcamera-devel>,\n\t<mailto:libcamera-devel-request@lists.libcamera.org?subject=unsubscribe>","List-Archive":"<https://lists.libcamera.org/pipermail/libcamera-devel/>","List-Post":"<mailto:libcamera-devel@lists.libcamera.org>","List-Help":"<mailto:libcamera-devel-request@lists.libcamera.org?subject=help>","List-Subscribe":"<https://lists.libcamera.org/listinfo/libcamera-devel>,\n\t<mailto:libcamera-devel-request@lists.libcamera.org?subject=subscribe>","Cc":"libcamera devel <libcamera-devel@lists.libcamera.org>","Errors-To":"libcamera-devel-bounces@lists.libcamera.org","Sender":"\"libcamera-devel\" <libcamera-devel-bounces@lists.libcamera.org>"}}]