[{"id":19802,"web_url":"https://patchwork.libcamera.org/comment/19802/","msgid":"<20210923073326.GO4382@pyrite.rasen.tech>","date":"2021-09-23T07:33:26","subject":"Re: [libcamera-devel] [PATCH v3 1/2] android: camera_capabilities:\n\tClarify CameraMetadata allocation","submitter":{"id":17,"url":"https://patchwork.libcamera.org/api/people/17/","name":"Paul Elder","email":"paul.elder@ideasonboard.com"},"content":"Hi Umang,\n\nOn Thu, Sep 23, 2021 at 12:54:52PM +0530, Umang Jain wrote:\n> CameraMetadata's constructor take in number of entries and number of\n> bytes to be allocated for those entries. However, CameraMetadata is\n> already capable of resizing its container on the fly, in case more\n> entries are added to it. Hence, the numbers passed in during the\n> construction acts as hint values for initialization.\n> \n> Clarify this in CameraCapabilities::requestTemplatePreview() and\n> remove the \\todo, as the arguments and the \\todo gives the perspective\n> that we need to be quite accurate with the numbers of entries / bytes,\n> which is not the case.\n> \n> Signed-off-by: Umang Jain <umang.jain@ideasonboard.com>\n> ---\n>  src/android/camera_capabilities.cpp | 8 ++++++--\n>  1 file changed, 6 insertions(+), 2 deletions(-)\n> \n> diff --git a/src/android/camera_capabilities.cpp b/src/android/camera_capabilities.cpp\n> index e92bca42..238b44db 100644\n> --- a/src/android/camera_capabilities.cpp\n> +++ b/src/android/camera_capabilities.cpp\n> @@ -1340,8 +1340,12 @@ std::unique_ptr<CameraMetadata> CameraCapabilities::requestTemplateManual() cons\n>  std::unique_ptr<CameraMetadata> CameraCapabilities::requestTemplatePreview() const\n>  {\n>  \t/*\n> -\t * \\todo Keep this in sync with the actual number of entries.\n> -\t * Currently: 20 entries, 35 bytes\n> +\t * Give initial hint of entries and number of bytes to be allocated.\n> +\t * It is deliberate that the hint is slightly larger than required, to\n> +\t * avoid resizing the container.\n> +\t *\n> +\t * CameraMetadata is capable to resize the container on the fly, if the\n\ns/to resize/of resizing/\n\ns/the$/adding a new entry will exceed its capacity./\n\n\nReviewed-by: Paul Elder <paul.elder@ideasonboard.com>\n\n> +\t * number of entries get exceeded.\n>  \t */\n>  \tauto requestTemplate = std::make_unique<CameraMetadata>(21, 36);\n>  \tif (!requestTemplate->isValid()) {\n> -- \n> 2.31.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 0599ABDC71\n\tfor <parsemail@patchwork.libcamera.org>;\n\tThu, 23 Sep 2021 07:33:36 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id 56ABE6918C;\n\tThu, 23 Sep 2021 09:33:35 +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 0DA25687DC\n\tfor <libcamera-devel@lists.libcamera.org>;\n\tThu, 23 Sep 2021 09:33:34 +0200 (CEST)","from pyrite.rasen.tech (unknown\n\t[IPv6:2400:4051:61:600:2c71:1b79:d06d:5032])\n\tby perceval.ideasonboard.com (Postfix) with ESMTPSA id 7052E45E;\n\tThu, 23 Sep 2021 09:33:32 +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=\"Abus6KrL\"; dkim-atps=neutral","DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com;\n\ts=mail; t=1632382413;\n\tbh=6A/VFgtnitSV6AoaEuYY5ME5NXQTSuZy2pYNLzMVXdM=;\n\th=Date:From:To:Cc:Subject:References:In-Reply-To:From;\n\tb=Abus6KrL4b4C78bxAt+daEIo4WfFhyn0g0t/Px7sLV+D3DWAP3cn8AIa+LdFlQO/R\n\tXvRTsVZkDiq9Ajw+DheEZzXJELPEZG+bwzhAxkK8OpZ4mX/gd2ysOaW8rglnNSK9rp\n\tF7GSF8Zora116CLrHPtwX6fgfw1U0Tp5udu7l2rU=","Date":"Thu, 23 Sep 2021 16:33:26 +0900","From":"paul.elder@ideasonboard.com","To":"Umang Jain <umang.jain@ideasonboard.com>","Message-ID":"<20210923073326.GO4382@pyrite.rasen.tech>","References":"<20210923072453.130346-1-umang.jain@ideasonboard.com>\n\t<20210923072453.130346-2-umang.jain@ideasonboard.com>","MIME-Version":"1.0","Content-Type":"text/plain; charset=us-ascii","Content-Disposition":"inline","In-Reply-To":"<20210923072453.130346-2-umang.jain@ideasonboard.com>","Subject":"Re: [libcamera-devel] [PATCH v3 1/2] android: camera_capabilities:\n\tClarify CameraMetadata allocation","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":19807,"web_url":"https://patchwork.libcamera.org/comment/19807/","msgid":"<YUxS6m6/k24KuJOM@pendragon.ideasonboard.com>","date":"2021-09-23T10:11:54","subject":"Re: [libcamera-devel] [PATCH v3 1/2] android: camera_capabilities:\n\tClarify CameraMetadata allocation","submitter":{"id":2,"url":"https://patchwork.libcamera.org/api/people/2/","name":"Laurent Pinchart","email":"laurent.pinchart@ideasonboard.com"},"content":"On Thu, Sep 23, 2021 at 04:33:26PM +0900, paul.elder@ideasonboard.com wrote:\n> On Thu, Sep 23, 2021 at 12:54:52PM +0530, Umang Jain wrote:\n> > CameraMetadata's constructor take in number of entries and number of\n\ns/take/takes/\n\n> > bytes to be allocated for those entries. However, CameraMetadata is\n> > already capable of resizing its container on the fly, in case more\n> > entries are added to it. Hence, the numbers passed in during the\n> > construction acts as hint values for initialization.\n\ns/acts/act/\n\n> > Clarify this in CameraCapabilities::requestTemplatePreview() and\n> > remove the \\todo, as the arguments and the \\todo gives the perspective\n> > that we need to be quite accurate with the numbers of entries / bytes,\n> > which is not the case.\n> > \n> > Signed-off-by: Umang Jain <umang.jain@ideasonboard.com>\n> > ---\n> >  src/android/camera_capabilities.cpp | 8 ++++++--\n> >  1 file changed, 6 insertions(+), 2 deletions(-)\n> > \n> > diff --git a/src/android/camera_capabilities.cpp b/src/android/camera_capabilities.cpp\n> > index e92bca42..238b44db 100644\n> > --- a/src/android/camera_capabilities.cpp\n> > +++ b/src/android/camera_capabilities.cpp\n> > @@ -1340,8 +1340,12 @@ std::unique_ptr<CameraMetadata> CameraCapabilities::requestTemplateManual() cons\n> >  std::unique_ptr<CameraMetadata> CameraCapabilities::requestTemplatePreview() const\n> >  {\n> >  \t/*\n> > -\t * \\todo Keep this in sync with the actual number of entries.\n> > -\t * Currently: 20 entries, 35 bytes\n> > +\t * Give initial hint of entries and number of bytes to be allocated.\n> > +\t * It is deliberate that the hint is slightly larger than required, to\n> > +\t * avoid resizing the container.\n> > +\t *\n> > +\t * CameraMetadata is capable to resize the container on the fly, if the\n> \n> s/to resize/of resizing/\n> \n> s/the$/adding a new entry will exceed its capacity./\n> \n> Reviewed-by: Paul Elder <paul.elder@ideasonboard.com>\n\nWith this,\n\nReviewed-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>\n\n> > +\t * number of entries get exceeded.\n> >  \t */\n> >  \tauto requestTemplate = std::make_unique<CameraMetadata>(21, 36);\n> >  \tif (!requestTemplate->isValid()) {","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 6F3DBBDC71\n\tfor <parsemail@patchwork.libcamera.org>;\n\tThu, 23 Sep 2021 10:11:59 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id B74B26918C;\n\tThu, 23 Sep 2021 12:11:58 +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 5334F69189\n\tfor <libcamera-devel@lists.libcamera.org>;\n\tThu, 23 Sep 2021 12:11:57 +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 B825945E;\n\tThu, 23 Sep 2021 12:11:56 +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=\"QjjCIYTi\"; dkim-atps=neutral","DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com;\n\ts=mail; t=1632391916;\n\tbh=1L4kZKsctH8sd6+Z6z2LZH/bFlX13ywRc9nGVF39KNQ=;\n\th=Date:From:To:Cc:Subject:References:In-Reply-To:From;\n\tb=QjjCIYTiCT5JanjvZVxaNJnguSf+0OGqx9bv6bGrieDYwkFCMG9vfUt/iKq7XXiys\n\tnucbYUGgaS+w+J8nQ6Z+YCWywyWsUvrU6+bOsjttbjoNAN9uve0vS9uQFy4u3hjPkC\n\tQU1w8kh617mEnWrSc9kBpkwFhFnH8HBpcNSmP640=","Date":"Thu, 23 Sep 2021 13:11:54 +0300","From":"Laurent Pinchart <laurent.pinchart@ideasonboard.com>","To":"paul.elder@ideasonboard.com","Message-ID":"<YUxS6m6/k24KuJOM@pendragon.ideasonboard.com>","References":"<20210923072453.130346-1-umang.jain@ideasonboard.com>\n\t<20210923072453.130346-2-umang.jain@ideasonboard.com>\n\t<20210923073326.GO4382@pyrite.rasen.tech>","MIME-Version":"1.0","Content-Type":"text/plain; charset=utf-8","Content-Disposition":"inline","In-Reply-To":"<20210923073326.GO4382@pyrite.rasen.tech>","Subject":"Re: [libcamera-devel] [PATCH v3 1/2] android: camera_capabilities:\n\tClarify CameraMetadata allocation","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>"}}]