[{"id":16942,"web_url":"https://patchwork.libcamera.org/comment/16942/","msgid":"","date":"2021-05-14T09:32:11","subject":"Re: [libcamera-devel] [PATCH] android: camera_metadata: Add\n\tfunctions for instrumenting resizing","submitter":{"id":4,"url":"https://patchwork.libcamera.org/api/people/4/","name":"Kieran Bingham","email":"kieran.bingham@ideasonboard.com"},"content":"Hi Paul,\n\nOn 14/05/2021 10:27, Paul Elder wrote:\n> Add utility functions to CameraMetadata to check if it has been resized,\n> and for outputting the actual entry and data count. This is meant to be\n> used to output information on resizing, to assist developers in\n> choosing proper initial sizes to avoid resizing. Also make CameraDevice\n> use these functions for static and result metadata.\n\nGreat, that's exactly what I had envisioned, and allows developers to\ninitialise the static metadata reservations with exact sizes when they\nget adjusted, while still allowing the system to perform correctly if\nthe initial reservations were too low.\n\nFor metadata which is used more dynamically, it will be harder to get an\nexact size - but that's up to the developers to identify and size\ncorrectly to balance the initial reservation against expected usages.\n\nThanks!\n\nReviewed-by: Kieran Bingham \n\n\n> Signed-off-by: Paul Elder \n> ---\n> src/android/camera_device.cpp | 16 ++++++++++++++++\n> src/android/camera_metadata.cpp | 14 +++++++++++++-\n> src/android/camera_metadata.h | 4 ++++\n> 3 files changed, 33 insertions(+), 1 deletion(-)\n> \n> diff --git a/src/android/camera_device.cpp b/src/android/camera_device.cpp\n> index b32e8be5..15f81b04 100644\n> --- a/src/android/camera_device.cpp\n> +++ b/src/android/camera_device.cpp\n> @@ -1391,6 +1391,14 @@ const camera_metadata_t *CameraDevice::getStaticMetadata()\n> \t\treturn nullptr;\n> \t}\n> \n> +\tif (staticMetadata_->resized()) {\n> +\t\tsize_t entryCount, dataCount;\n> +\t\tstd::tie(entryCount, dataCount) = staticMetadata_->usage();\n> +\t\tLOG(HAL, Info)\n> +\t\t\t<< \"Static metadata resized: \" << entryCount\n> +\t\t\t<< \" entries and \" << dataCount << \" bytes used\";\n> +\t}\n> +\n> \treturn staticMetadata_->get();\n> }\n> \n> @@ -2269,5 +2277,13 @@ CameraDevice::getResultMetadata(const Camera3RequestDescriptor &descriptor) cons\n> \t\tLOG(HAL, Error) << \"Failed to construct result metadata\";\n> \t}\n> \n> +\tif (resultMetadata->resized()) {\n> +\t\tsize_t entryCount, dataCount;\n> +\t\tstd::tie(entryCount, dataCount) = resultMetadata->usage();\n> +\t\tLOG(HAL, Info)\n> +\t\t\t<< \"Result metadata resized: \" << entryCount\n> +\t\t\t<< \" entries and \" << dataCount << \" bytes used\";\n> +\t}\n> +\n> \treturn resultMetadata;\n> }\n> diff --git a/src/android/camera_metadata.cpp b/src/android/camera_metadata.cpp\n> index bf8d2781..0588ea4e 100644\n> --- a/src/android/camera_metadata.cpp\n> +++ b/src/android/camera_metadata.cpp\n> @@ -14,17 +14,19 @@ using namespace libcamera;\n> LOG_DEFINE_CATEGORY(CameraMetadata)\n> \n> CameraMetadata::CameraMetadata()\n> -\t: metadata_(nullptr), valid_(false)\n> +\t: metadata_(nullptr), valid_(false), resized_(false)\n> {\n> }\n> \n> CameraMetadata::CameraMetadata(size_t entryCapacity, size_t dataCapacity)\n> +\t: resized_(false)\n> {\n> \tmetadata_ = allocate_camera_metadata(entryCapacity, dataCapacity);\n> \tvalid_ = metadata_ != nullptr;\n> }\n> \n> CameraMetadata::CameraMetadata(const camera_metadata_t *metadata)\n> +\t: resized_(false)\n> {\n> \tmetadata_ = clone_camera_metadata(metadata);\n> \tvalid_ = metadata_ != nullptr;\n> @@ -55,6 +57,14 @@ CameraMetadata &CameraMetadata::operator=(const CameraMetadata &other)\n> \treturn *this;\n> }\n> \n> +std::tuple CameraMetadata::usage() const\n> +{\n> +\tsize_t currentEntryCount = get_camera_metadata_entry_count(metadata_);\n> +\tsize_t currentDataCount = get_camera_metadata_data_count(metadata_);\n> +\n> +\treturn { currentEntryCount, currentDataCount };\n> +}\n> +\n> bool CameraMetadata::getEntry(uint32_t tag, camera_metadata_ro_entry_t *entry) const\n> {\n> \tif (find_camera_metadata_ro_entry(metadata_, tag, entry))\n> @@ -104,6 +114,8 @@ bool CameraMetadata::resize(size_t count, size_t size)\n> \n> \t\tappend_camera_metadata(metadata_, oldMetadata);\n> \t\tfree_camera_metadata(oldMetadata);\n> +\n> +\t\tresized_ = true;\n> \t}\n> \n> \treturn true;\n> diff --git a/src/android/camera_metadata.h b/src/android/camera_metadata.h\n> index 07afd4b2..b291fbf9 100644\n> --- a/src/android/camera_metadata.h\n> +++ b/src/android/camera_metadata.h\n> @@ -23,6 +23,9 @@ public:\n> \n> \tCameraMetadata &operator=(const CameraMetadata &other);\n> \n> +\tstd::tuple usage() const;\n> +\tbool resized() const { return resized_; }\n> +\n> \tbool isValid() const { return valid_; }\n> \tbool resize(size_t count, size_t size);\n> \tbool getEntry(uint32_t tag, camera_metadata_ro_entry_t *entry) const;\n> @@ -81,6 +84,7 @@ public:\n> private:\n> \tcamera_metadata_t *metadata_;\n> \tbool valid_;\n> +\tbool resized_;\n> };\n> \n> #endif /* __ANDROID_CAMERA_METADATA_H__ */\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 00230C31F8\n\tfor ;\n\tFri, 14 May 2021 09:32:16 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id 5F43D6891C;\n\tFri, 14 May 2021 11:32:16 +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 C460C68911\n\tfor ;\n\tFri, 14 May 2021 11:32:14 +0200 (CEST)","from [192.168.0.20]\n\t(cpc89244-aztw30-2-0-cust3082.18-1.cable.virginm.net [86.31.172.11])\n\tby perceval.ideasonboard.com (Postfix) with ESMTPSA id 416B99F0;\n\tFri, 14 May 2021 11:32:14 +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=\"GCcNeizZ\"; dkim-atps=neutral","DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com;\n\ts=mail; t=1620984734;\n\tbh=0Jg+sN23b5fNxTeus2tXJuAMnF8fB1RHhNS4oW130Ik=;\n\th=Reply-To:Subject:To:References:From:Date:In-Reply-To:From;\n\tb=GCcNeizZH/Kaj10OyHbW13sYBUlYkb8sNhAMrmW/FHyGQ6PsPrDZ6OH1OXa2i/J0u\n\t1cLawsUWJeNiiGrO0+Xgm7++7Rxw7Fq45fMYPLZFxKseaTz3tsMbBjDxiD7wKUeHlo\n\tZrTB1AXPjyOOFEIzfEs7gp9V8Rz5nOPQNMLKIRyw=","To":"Paul Elder ,\n\tlibcamera-devel@lists.libcamera.org","References":"<20210514092745.814353-1-paul.elder@ideasonboard.com>","From":"Kieran Bingham ","Organization":"Ideas on Board","Message-ID":"","Date":"Fri, 14 May 2021 10:32:11 +0100","User-Agent":"Mozilla/5.0 (X11; Linux x86_64; rv:78.0) Gecko/20100101\n\tThunderbird/78.7.1","MIME-Version":"1.0","In-Reply-To":"<20210514092745.814353-1-paul.elder@ideasonboard.com>","Content-Language":"en-GB","Subject":"Re: [libcamera-devel] [PATCH] android: camera_metadata: Add\n\tfunctions for instrumenting resizing","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","Reply-To":"kieran.bingham@ideasonboard.com","Content-Type":"text/plain; charset=\"us-ascii\"","Content-Transfer-Encoding":"7bit","Errors-To":"libcamera-devel-bounces@lists.libcamera.org","Sender":"\"libcamera-devel\" "}},{"id":16964,"web_url":"https://patchwork.libcamera.org/comment/16964/","msgid":"","date":"2021-05-17T03:31:46","subject":"Re: [libcamera-devel] [PATCH] android: camera_metadata: Add\n\tfunctions for instrumenting resizing","submitter":{"id":63,"url":"https://patchwork.libcamera.org/api/people/63/","name":"Hirokazu Honda","email":"hiroh@chromium.org"},"content":"Hi Paul,\n\nOn Fri, May 14, 2021 at 6:32 PM Kieran Bingham <\nkieran.bingham@ideasonboard.com> wrote:\n\n> Hi Paul,\n>\n> On 14/05/2021 10:27, Paul Elder wrote:\n> > Add utility functions to CameraMetadata to check if it has been resized,\n> > and for outputting the actual entry and data count. This is meant to be\n> > used to output information on resizing, to assist developers in\n> > choosing proper initial sizes to avoid resizing. Also make CameraDevice\n> > use these functions for static and result metadata.\n>\n> Great, that's exactly what I had envisioned, and allows developers to\n> initialise the static metadata reservations with exact sizes when they\n> get adjusted, while still allowing the system to perform correctly if\n> the initial reservations were too low.\n>\n> For metadata which is used more dynamically, it will be harder to get an\n> exact size - but that's up to the developers to identify and size\n> correctly to balance the initial reservation against expected usages.\n>\n> Thanks!\n>\n> Reviewed-by: Kieran Bingham \n>\n>\n> > Signed-off-by: Paul Elder \n> > ---\n> > src/android/camera_device.cpp | 16 ++++++++++++++++\n> > src/android/camera_metadata.cpp | 14 +++++++++++++-\n> > src/android/camera_metadata.h | 4 ++++\n> > 3 files changed, 33 insertions(+), 1 deletion(-)\n> >\n> > diff --git a/src/android/camera_device.cpp\n> b/src/android/camera_device.cpp\n> > index b32e8be5..15f81b04 100644\n> > --- a/src/android/camera_device.cpp\n> > +++ b/src/android/camera_device.cpp\n> > @@ -1391,6 +1391,14 @@ const camera_metadata_t\n> *CameraDevice::getStaticMetadata()\n> > return nullptr;\n> > }\n> >\n> > + if (staticMetadata_->resized()) {\n> > + size_t entryCount, dataCount;\n> > + std::tie(entryCount, dataCount) = staticMetadata_->usage();\n>\n\nnit: You can write these two lines as auto [entryCount, dataCount] =\nstaticMetadata->usage() since C++17.\n\nReviewed-by: Hirokazu Honda \n\n> + LOG(HAL, Info)\n> > + << \"Static metadata resized: \" << entryCount\n> > + << \" entries and \" << dataCount << \" bytes used\";\n> > + }\n> > +\n> > return staticMetadata_->get();\n> > }\n> >\n> > @@ -2269,5 +2277,13 @@ CameraDevice::getResultMetadata(const\n> Camera3RequestDescriptor &descriptor) cons\n> > LOG(HAL, Error) << \"Failed to construct result metadata\";\n> > }\n> >\n> > + if (resultMetadata->resized()) {\n> > + size_t entryCount, dataCount;\n> > + std::tie(entryCount, dataCount) = resultMetadata->usage();\n> > + LOG(HAL, Info)\n> > + << \"Result metadata resized: \" << entryCount\n> > + << \" entries and \" << dataCount << \" bytes used\";\n> > + }\n> > +\n> > return resultMetadata;\n> > }\n> > diff --git a/src/android/camera_metadata.cpp\n> b/src/android/camera_metadata.cpp\n> > index bf8d2781..0588ea4e 100644\n> > --- a/src/android/camera_metadata.cpp\n> > +++ b/src/android/camera_metadata.cpp\n> > @@ -14,17 +14,19 @@ using namespace libcamera;\n> > LOG_DEFINE_CATEGORY(CameraMetadata)\n> >\n> > CameraMetadata::CameraMetadata()\n> > - : metadata_(nullptr), valid_(false)\n> > + : metadata_(nullptr), valid_(false), resized_(false)\n> > {\n> > }\n> >\n> > CameraMetadata::CameraMetadata(size_t entryCapacity, size_t\n> dataCapacity)\n> > + : resized_(false)\n> > {\n> > metadata_ = allocate_camera_metadata(entryCapacity, dataCapacity);\n> > valid_ = metadata_ != nullptr;\n> > }\n> >\n> > CameraMetadata::CameraMetadata(const camera_metadata_t *metadata)\n> > + : resized_(false)\n> > {\n> > metadata_ = clone_camera_metadata(metadata);\n> > valid_ = metadata_ != nullptr;\n> > @@ -55,6 +57,14 @@ CameraMetadata &CameraMetadata::operator=(const\n> CameraMetadata &other)\n> > return *this;\n> > }\n> >\n> > +std::tuple CameraMetadata::usage() const\n> > +{\n> > + size_t currentEntryCount =\n> get_camera_metadata_entry_count(metadata_);\n> > + size_t currentDataCount =\n> get_camera_metadata_data_count(metadata_);\n> > +\n> > + return { currentEntryCount, currentDataCount };\n> > +}\n> > +\n> > bool CameraMetadata::getEntry(uint32_t tag, camera_metadata_ro_entry_t\n> *entry) const\n> > {\n> > if (find_camera_metadata_ro_entry(metadata_, tag, entry))\n> > @@ -104,6 +114,8 @@ bool CameraMetadata::resize(size_t count, size_t\n> size)\n> >\n> > append_camera_metadata(metadata_, oldMetadata);\n> > free_camera_metadata(oldMetadata);\n> > +\n> > + resized_ = true;\n> > }\n> >\n> > return true;\n> > diff --git a/src/android/camera_metadata.h\n> b/src/android/camera_metadata.h\n> > index 07afd4b2..b291fbf9 100644\n> > --- a/src/android/camera_metadata.h\n> > +++ b/src/android/camera_metadata.h\n> > @@ -23,6 +23,9 @@ public:\n> >\n> > CameraMetadata &operator=(const CameraMetadata &other);\n> >\n> > + std::tuple usage() const;\n> > + bool resized() const { return resized_; }\n> > +\n> > bool isValid() const { return valid_; }\n> > bool resize(size_t count, size_t size);\n> > bool getEntry(uint32_t tag, camera_metadata_ro_entry_t *entry)\n> const;\n> > @@ -81,6 +84,7 @@ public:\n> > private:\n> > camera_metadata_t *metadata_;\n> > bool valid_;\n> > + bool resized_;\n> > };\n> >\n> > #endif /* __ANDROID_CAMERA_METADATA_H__ */\n> >\n>\n> --\n> Regards\n> --\n> Kieran\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 4EB8EC31FC\n\tfor ;\n\tMon, 17 May 2021 03:32:00 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id B63346891E;\n\tMon, 17 May 2021 05:31:59 +0200 (CEST)","from mail-ed1-x52f.google.com (mail-ed1-x52f.google.com\n\t[IPv6:2a00:1450:4864:20::52f])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTPS id 909E7602B1\n\tfor ;\n\tMon, 17 May 2021 05:31:57 +0200 (CEST)","by mail-ed1-x52f.google.com with SMTP id t3so5101965edc.7\n\tfor ;\n\tSun, 16 May 2021 20:31:57 -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=\"EVIFwJrz\"; 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=toQQBV+ZffWKp5SxkY1LonVaeDAu1vi5lyzlS79Qf+k=;\n\tb=EVIFwJrzOyKMt80qHqmpBK6AOPMVUMwqLu+338pcs/O5BcIA9lBTRJMHZuM/Eictlz\n\tci/RGPKzwNCPE9OSeLZ0QDPGCYUs0MgDCLyYqlp53gx42ifwJkrB+sNsQhsH2uANYa9c\n\tU5FSrqtasjUSkl+NxVEm0lCr25r7s7xwJwTCg=","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=toQQBV+ZffWKp5SxkY1LonVaeDAu1vi5lyzlS79Qf+k=;\n\tb=seiK7gxaKm+6pq179V4LNBwzu8PMm3CZjXVn1nrgJuwE3VyZ2TY30LJU+6gbpqHwoK\n\tDLAtMuMmSzdev7dmzHKye605n7fNelzMITwtuZHK21GLPqR1FHuId4Ntr6bx7Zz0cW65\n\tsLxRjXi4nsSEgSv8Fk7zBsK/6pEtSx8kDR/q4QoPtmwWC+NsC8u4I6U7n4yBTbfrc/ne\n\tgZlCx008gkGObvzSWDidQn6bxlzHibRVos68pRsOb96xSxgNnf7rc9hma/RsuSeKPAVg\n\tKMM2bPcG2Y7Pb8irucHss4VK6NU7NB6G7dIt0j+AtFkWVn+f3F41C/nVQLY/u+lMUyht\n\ttWjA==","X-Gm-Message-State":"AOAM530ySn5ez+lQ64EaoW+L+aGBGpcUejMH2uhvwl32/65LWW1S93jA\n\tp79AhMyPW8y3ESAUfr3SfjYAUUSd3zlAivuzEY0YcEavTQc=","X-Google-Smtp-Source":"ABdhPJyjj0375NI4pj5RwTvNYPSSgixVF/qyYtHB+LqFpEYJAU4le2vGtO1yaFyqoeLeqDHX1ox7hKBc8yfqgE6k3Zo=","X-Received":"by 2002:a05:6402:cb8:: with SMTP id\n\tcn24mr3116232edb.325.1621222317015; \n\tSun, 16 May 2021 20:31:57 -0700 (PDT)","MIME-Version":"1.0","References":"<20210514092745.814353-1-paul.elder@ideasonboard.com>\n\t","In-Reply-To":"","From":"Hirokazu Honda ","Date":"Mon, 17 May 2021 12:31:46 +0900","Message-ID":"","To":"Kieran Bingham ","Content-Type":"multipart/alternative; boundary=\"0000000000004ed78f05c27e3cc4\"","Subject":"Re: [libcamera-devel] [PATCH] android: camera_metadata: Add\n\tfunctions for instrumenting resizing","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\" "}}]