[{"id":15963,"web_url":"https://patchwork.libcamera.org/comment/15963/","msgid":"<YF/eZSUAsTMrb9MX@pendragon.ideasonboard.com>","date":"2021-03-28T01:39:49","subject":"Re: [libcamera-devel] [PATCH v2] android: camera_device: Add null\n\tcheck for ScalerCrop control","submitter":{"id":2,"url":"https://patchwork.libcamera.org/api/people/2/","name":"Laurent Pinchart","email":"laurent.pinchart@ideasonboard.com"},"content":"Hi Phi-Bang,\n\nThank you for the patch.\n\nOn Fri, Mar 26, 2021 at 11:04:10PM +0100, Phi-Bang Nguyen wrote:\n> The ScalerCrop control does not contain the null check which can\n> cause the camera HAL crash at boot. Fix it.\n\nThe ScalerCrop control is indeed not mandatory. I'll add\n\nFixes: 31a1a628cd0e (\"android: camera_device: Register MAX_DIGITAL_ZOOM\")\n\nReviewed-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>\n\nJacopo, could you confirm that I'm not missing anything and that this is\ncorrect ?\n\n> Signed-off-by: Phi-Bang Nguyen <pnguyen@baylibre.com>\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 ae693664..a8108e3a 100644\n> --- a/src/android/camera_device.cpp\n> +++ b/src/android/camera_device.cpp\n> @@ -1107,11 +1107,14 @@ const camera_metadata_t *CameraDevice::getStaticMetadata()\n>  \t\t * use the maximum and minimum crop rectangles to calculate the\n>  \t\t * digital zoom factor.\n>  \t\t */\n> +\t\tfloat maxZoom = { 1 };\n>  \t\tconst auto info = controlsInfo.find(&controls::ScalerCrop);\n> -\t\tRectangle min = info->second.min().get<Rectangle>();\n> -\t\tRectangle max = info->second.max().get<Rectangle>();\n> -\t\tfloat maxZoom = std::min(1.0f * max.width / min.width,\n> -\t\t\t\t\t 1.0f * max.height / min.height);\n> +\t\tif (info != controlsInfo.end()) {\n> +\t\t\tRectangle min = info->second.min().get<Rectangle>();\n> +\t\t\tRectangle max = info->second.max().get<Rectangle>();\n> +\t\t\tmaxZoom = std::min(1.0f * max.width / min.width,\n> +\t\t\t\t\t   1.0f * max.height / min.height);\n> +\t\t}\n>  \t\tstaticMetadata_->addEntry(ANDROID_SCALER_AVAILABLE_MAX_DIGITAL_ZOOM,\n>  \t\t\t\t\t  &maxZoom, 1);\n>  \t}","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 0DE34C32EA\n\tfor <parsemail@patchwork.libcamera.org>;\n\tSun, 28 Mar 2021 01:40:36 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id 40C826877F;\n\tSun, 28 Mar 2021 03:40:35 +0200 (CEST)","from perceval.ideasonboard.com (perceval.ideasonboard.com\n\t[213.167.242.64])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTPS id 7989B6877C\n\tfor <libcamera-devel@lists.libcamera.org>;\n\tSun, 28 Mar 2021 03:40:33 +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 C0BA6102;\n\tSun, 28 Mar 2021 03:40:32 +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=\"kgChqVpA\"; dkim-atps=neutral","DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com;\n\ts=mail; t=1616895633;\n\tbh=HYieIfnVpAft8XJxtQf2gLaipcu3Xd51uRxhSL18yGY=;\n\th=Date:From:To:Cc:Subject:References:In-Reply-To:From;\n\tb=kgChqVpAZEu/IMpFQ1WKhVqcOijfwZ9i/S+JX8xC+6+9aORLFbLl1IoXHinJBFTPS\n\tKbtJdC9A2l12smG/A2wKlESFjN72YeaSMN6kNhET/uI/hAFJVTAPZk/zvyxvZAl92h\n\tiuNxvXMbHKwrffjGVV7675LG0G4as3E8vWsmhpV8=","Date":"Sun, 28 Mar 2021 04:39:49 +0300","From":"Laurent Pinchart <laurent.pinchart@ideasonboard.com>","To":"Phi-Bang Nguyen <pnguyen@baylibre.com>","Message-ID":"<YF/eZSUAsTMrb9MX@pendragon.ideasonboard.com>","References":"<YF49KirQp2QUQ1LA@pendragon.ideasonboard.com>\n\t<20210326220410.329299-1-pnguyen@baylibre.com>","MIME-Version":"1.0","Content-Disposition":"inline","In-Reply-To":"<20210326220410.329299-1-pnguyen@baylibre.com>","Subject":"Re: [libcamera-devel] [PATCH v2] android: camera_device: Add null\n\tcheck for ScalerCrop control","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","Content-Type":"text/plain; charset=\"us-ascii\"","Content-Transfer-Encoding":"7bit","Errors-To":"libcamera-devel-bounces@lists.libcamera.org","Sender":"\"libcamera-devel\" <libcamera-devel-bounces@lists.libcamera.org>"}},{"id":16002,"web_url":"https://patchwork.libcamera.org/comment/16002/","msgid":"<20210329075617.rxxmwu6idghbping@uno.localdomain>","date":"2021-03-29T07:56:17","subject":"Re: [libcamera-devel] [PATCH v2] android: camera_device: Add null\n\tcheck for ScalerCrop control","submitter":{"id":3,"url":"https://patchwork.libcamera.org/api/people/3/","name":"Jacopo Mondi","email":"jacopo@jmondi.org"},"content":"Hi Phi-Bang,\n\nOn Fri, Mar 26, 2021 at 11:04:10PM +0100, Phi-Bang Nguyen wrote:\n> The ScalerCrop control does not contain the null check which can\n> cause the camera HAL crash at boot. Fix it.\n>\n> Signed-off-by: Phi-Bang Nguyen <pnguyen@baylibre.com>\n\nI'm a bit debated as I feel the control should be reported by the\nCamera (aka registered by the pipeline handler). True that if you have\nno use for that, it should be safely defaulted to 1, as you do here\nbelow.\n\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 ae693664..a8108e3a 100644\n> --- a/src/android/camera_device.cpp\n> +++ b/src/android/camera_device.cpp\n> @@ -1107,11 +1107,14 @@ const camera_metadata_t *CameraDevice::getStaticMetadata()\n>  \t\t * use the maximum and minimum crop rectangles to calculate the\n>  \t\t * digital zoom factor.\n>  \t\t */\n> +\t\tfloat maxZoom = { 1 };\n\nWhy a braced list initializer for a variable of type float ? It\ncompiles as C++ allows it, but it's a bit unusual. Is this me ?\n\n>  \t\tconst auto info = controlsInfo.find(&controls::ScalerCrop);\n> -\t\tRectangle min = info->second.min().get<Rectangle>();\n> -\t\tRectangle max = info->second.max().get<Rectangle>();\n> -\t\tfloat maxZoom = std::min(1.0f * max.width / min.width,\n> -\t\t\t\t\t 1.0f * max.height / min.height);\n> +\t\tif (info != controlsInfo.end()) {\n> +\t\t\tRectangle min = info->second.min().get<Rectangle>();\n> +\t\t\tRectangle max = info->second.max().get<Rectangle>();\n> +\t\t\tmaxZoom = std::min(1.0f * max.width / min.width,\n> +\t\t\t\t\t   1.0f * max.height / min.height);\n> +\t\t}\n\nOne minor nit. As you can see this code now lives in its own scope\n\n        {\n                ...\n                init max digital zoom\n                ...\n\n        }\n\nThat was because I wanted to use 'min', 'max', 'info' and not care\nabout name clashes with other variables in the function. Now that you\nhave made the control registration conditional, I think you can remove\nthe additional scope and make this looks like:\n\n        float maxZoom = 1.0;\n        const auto scalerCrop = controlsInfo.find(&controls::ScalerCrop);\n        if (scalerCrop != controlsInfo.end()) {\n\n                ....\n        }\n\nWhat do you think ?\n\nThe patch is good for the rest\nReviewed-by: Jacopo Mondi <jacopo@jmondi.org>\n\nThanks\n  j\n\n>  \t\tstaticMetadata_->addEntry(ANDROID_SCALER_AVAILABLE_MAX_DIGITAL_ZOOM,\n>  \t\t\t\t\t  &maxZoom, 1);\n>  \t}\n> --\n> 2.25.1\n>\n> _______________________________________________\n> libcamera-devel mailing list\n> libcamera-devel@lists.libcamera.org\n> https://lists.libcamera.org/listinfo/libcamera-devel","headers":{"Return-Path":"<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 30C48C32EF\n\tfor <parsemail@patchwork.libcamera.org>;\n\tMon, 29 Mar 2021 07:55:45 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id E157868783;\n\tMon, 29 Mar 2021 09:55:44 +0200 (CEST)","from relay11.mail.gandi.net (relay11.mail.gandi.net\n\t[217.70.178.231])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTPS id 4BF206877E\n\tfor <libcamera-devel@lists.libcamera.org>;\n\tMon, 29 Mar 2021 09:55:43 +0200 (CEST)","from uno.localdomain (93-34-118-233.ip49.fastwebnet.it\n\t[93.34.118.233]) (Authenticated sender: jacopo@jmondi.org)\n\tby relay11.mail.gandi.net (Postfix) with ESMTPSA id 7867A10000D;\n\tMon, 29 Mar 2021 07:55:42 +0000 (UTC)"],"Date":"Mon, 29 Mar 2021 09:56:17 +0200","From":"Jacopo Mondi <jacopo@jmondi.org>","To":"Phi-Bang Nguyen <pnguyen@baylibre.com>","Message-ID":"<20210329075617.rxxmwu6idghbping@uno.localdomain>","References":"<YF49KirQp2QUQ1LA@pendragon.ideasonboard.com>\n\t<20210326220410.329299-1-pnguyen@baylibre.com>","MIME-Version":"1.0","Content-Disposition":"inline","In-Reply-To":"<20210326220410.329299-1-pnguyen@baylibre.com>","Subject":"Re: [libcamera-devel] [PATCH v2] android: camera_device: Add null\n\tcheck for ScalerCrop control","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","Content-Type":"text/plain; charset=\"us-ascii\"","Content-Transfer-Encoding":"7bit","Errors-To":"libcamera-devel-bounces@lists.libcamera.org","Sender":"\"libcamera-devel\" <libcamera-devel-bounces@lists.libcamera.org>"}},{"id":16006,"web_url":"https://patchwork.libcamera.org/comment/16006/","msgid":"<YGGNCu7el9F0isDL@pendragon.ideasonboard.com>","date":"2021-03-29T08:17:14","subject":"Re: [libcamera-devel] [PATCH v2] android: camera_device: Add null\n\tcheck for ScalerCrop control","submitter":{"id":2,"url":"https://patchwork.libcamera.org/api/people/2/","name":"Laurent Pinchart","email":"laurent.pinchart@ideasonboard.com"},"content":"On Mon, Mar 29, 2021 at 09:56:17AM +0200, Jacopo Mondi wrote:\n> Hi Phi-Bang,\n> \n> On Fri, Mar 26, 2021 at 11:04:10PM +0100, Phi-Bang Nguyen wrote:\n> > The ScalerCrop control does not contain the null check which can\n> > cause the camera HAL crash at boot. Fix it.\n> >\n> > Signed-off-by: Phi-Bang Nguyen <pnguyen@baylibre.com>\n> \n> I'm a bit debated as I feel the control should be reported by the\n> Camera (aka registered by the pipeline handler). True that if you have\n> no use for that, it should be safely defaulted to 1, as you do here\n> below.\n\nWe should really add a mandatory flag in controls_ids.yaml at some\npoint. I'm sure we'll then spend quite a bit of time discussing which\ncontrols should be mandatory :-) For ScalerCrop, my initial opinion is\nthat it should be optional, as not all platforms can scale.\n\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 ae693664..a8108e3a 100644\n> > --- a/src/android/camera_device.cpp\n> > +++ b/src/android/camera_device.cpp\n> > @@ -1107,11 +1107,14 @@ const camera_metadata_t *CameraDevice::getStaticMetadata()\n> >  \t\t * use the maximum and minimum crop rectangles to calculate the\n> >  \t\t * digital zoom factor.\n> >  \t\t */\n> > +\t\tfloat maxZoom = { 1 };\n> \n> Why a braced list initializer for a variable of type float ? It\n> compiles as C++ allows it, but it's a bit unusual. Is this me ?\n\nIndeed, just = 1.0f would be better.\n\n> >  \t\tconst auto info = controlsInfo.find(&controls::ScalerCrop);\n> > -\t\tRectangle min = info->second.min().get<Rectangle>();\n> > -\t\tRectangle max = info->second.max().get<Rectangle>();\n> > -\t\tfloat maxZoom = std::min(1.0f * max.width / min.width,\n> > -\t\t\t\t\t 1.0f * max.height / min.height);\n> > +\t\tif (info != controlsInfo.end()) {\n> > +\t\t\tRectangle min = info->second.min().get<Rectangle>();\n> > +\t\t\tRectangle max = info->second.max().get<Rectangle>();\n> > +\t\t\tmaxZoom = std::min(1.0f * max.width / min.width,\n> > +\t\t\t\t\t   1.0f * max.height / min.height);\n> > +\t\t}\n> \n> One minor nit. As you can see this code now lives in its own scope\n> \n>         {\n>                 ...\n>                 init max digital zoom\n>                 ...\n> \n>         }\n> \n> That was because I wanted to use 'min', 'max', 'info' and not care\n> about name clashes with other variables in the function. Now that you\n> have made the control registration conditional, I think you can remove\n> the additional scope and make this looks like:\n> \n>         float maxZoom = 1.0;\n>         const auto scalerCrop = controlsInfo.find(&controls::ScalerCrop);\n>         if (scalerCrop != controlsInfo.end()) {\n> \n>                 ....\n>         }\n> \n> What do you think ?\n\nLooks good to me.\n\n> The patch is good for the rest\n> Reviewed-by: Jacopo Mondi <jacopo@jmondi.org>\n> \n> >  \t\tstaticMetadata_->addEntry(ANDROID_SCALER_AVAILABLE_MAX_DIGITAL_ZOOM,\n> >  \t\t\t\t\t  &maxZoom, 1);\n> >  \t}","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 3CDE4C32EF\n\tfor <parsemail@patchwork.libcamera.org>;\n\tMon, 29 Mar 2021 08:18:00 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id AB14C68787;\n\tMon, 29 Mar 2021 10:17:59 +0200 (CEST)","from perceval.ideasonboard.com (perceval.ideasonboard.com\n\t[213.167.242.64])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTPS id 558EB6877E\n\tfor <libcamera-devel@lists.libcamera.org>;\n\tMon, 29 Mar 2021 10:17:58 +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 B44A231A;\n\tMon, 29 Mar 2021 10:17:57 +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=\"d+JSllqr\"; dkim-atps=neutral","DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com;\n\ts=mail; t=1617005877;\n\tbh=RbsmR5lAd5uSU511wkHRt3RJz6s+TTTm7tewSTIud44=;\n\th=Date:From:To:Cc:Subject:References:In-Reply-To:From;\n\tb=d+JSllqrAjnJQR/NWN+94EQG0jqGnuqbPIpL62SowaQIZ2XEo9cEI+d9PfGa6hNcy\n\tuX6RZxllEGfn682BSKK9jJy8TD7fXFuCw+cMpydsYzLl6y9SGE1abjRQPQMcBCWjY6\n\t2SPX4T/Brx7WPZBf6vRJaHglaVoATSRuwn0/o9oM=","Date":"Mon, 29 Mar 2021 11:17:14 +0300","From":"Laurent Pinchart <laurent.pinchart@ideasonboard.com>","To":"Jacopo Mondi <jacopo@jmondi.org>","Message-ID":"<YGGNCu7el9F0isDL@pendragon.ideasonboard.com>","References":"<YF49KirQp2QUQ1LA@pendragon.ideasonboard.com>\n\t<20210326220410.329299-1-pnguyen@baylibre.com>\n\t<20210329075617.rxxmwu6idghbping@uno.localdomain>","MIME-Version":"1.0","Content-Disposition":"inline","In-Reply-To":"<20210329075617.rxxmwu6idghbping@uno.localdomain>","Subject":"Re: [libcamera-devel] [PATCH v2] android: camera_device: Add null\n\tcheck for ScalerCrop control","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","Content-Type":"text/plain; charset=\"us-ascii\"","Content-Transfer-Encoding":"7bit","Errors-To":"libcamera-devel-bounces@lists.libcamera.org","Sender":"\"libcamera-devel\" <libcamera-devel-bounces@lists.libcamera.org>"}},{"id":16025,"web_url":"https://patchwork.libcamera.org/comment/16025/","msgid":"<CAO4hSrh4hYxUF5prpycGN__YSdW5jDKzAL6LtaoHXVNkt5zwzA@mail.gmail.com>","date":"2021-03-29T17:36:58","subject":"Re: [libcamera-devel] [PATCH v2] android: camera_device: Add null\n\tcheck for ScalerCrop control","submitter":{"id":79,"url":"https://patchwork.libcamera.org/api/people/79/","name":"Phi-bang Nguyen","email":"pnguyen@baylibre.com"},"content":"Hi Jacopo & Laurent,\n\nThank you for your reviews !\nI take your comments in v3.\n\nOn Mon, Mar 29, 2021 at 10:17 AM Laurent Pinchart <\nlaurent.pinchart@ideasonboard.com> wrote:\n\n> On Mon, Mar 29, 2021 at 09:56:17AM +0200, Jacopo Mondi wrote:\n> > Hi Phi-Bang,\n> >\n> > On Fri, Mar 26, 2021 at 11:04:10PM +0100, Phi-Bang Nguyen wrote:\n> > > The ScalerCrop control does not contain the null check which can\n> > > cause the camera HAL crash at boot. Fix it.\n> > >\n> > > Signed-off-by: Phi-Bang Nguyen <pnguyen@baylibre.com>\n> >\n> > I'm a bit debated as I feel the control should be reported by the\n> > Camera (aka registered by the pipeline handler). True that if you have\n> > no use for that, it should be safely defaulted to 1, as you do here\n> > below.\n>\n> We should really add a mandatory flag in controls_ids.yaml at some\n> point. I'm sure we'll then spend quite a bit of time discussing which\n> controls should be mandatory :-) For ScalerCrop, my initial opinion is\n> that it should be optional, as not all platforms can scale.\n>\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\n> b/src/android/camera_device.cpp\n> > > index ae693664..a8108e3a 100644\n> > > --- a/src/android/camera_device.cpp\n> > > +++ b/src/android/camera_device.cpp\n> > > @@ -1107,11 +1107,14 @@ const camera_metadata_t\n> *CameraDevice::getStaticMetadata()\n> > >              * use the maximum and minimum crop rectangles to\n> calculate the\n> > >              * digital zoom factor.\n> > >              */\n> > > +           float maxZoom = { 1 };\n> >\n> > Why a braced list initializer for a variable of type float ? It\n> > compiles as C++ allows it, but it's a bit unusual. Is this me ?\n>\n> Indeed, just = 1.0f would be better.\n>\n> > >             const auto info = controlsInfo.find(&controls::ScalerCrop);\n> > > -           Rectangle min = info->second.min().get<Rectangle>();\n> > > -           Rectangle max = info->second.max().get<Rectangle>();\n> > > -           float maxZoom = std::min(1.0f * max.width / min.width,\n> > > -                                    1.0f * max.height / min.height);\n> > > +           if (info != controlsInfo.end()) {\n> > > +                   Rectangle min =\n> info->second.min().get<Rectangle>();\n> > > +                   Rectangle max =\n> info->second.max().get<Rectangle>();\n> > > +                   maxZoom = std::min(1.0f * max.width / min.width,\n> > > +                                      1.0f * max.height / min.height);\n> > > +           }\n> >\n> > One minor nit. As you can see this code now lives in its own scope\n> >\n> >         {\n> >                 ...\n> >                 init max digital zoom\n> >                 ...\n> >\n> >         }\n> >\n> > That was because I wanted to use 'min', 'max', 'info' and not care\n> > about name clashes with other variables in the function. Now that you\n> > have made the control registration conditional, I think you can remove\n> > the additional scope and make this looks like:\n> >\n> >         float maxZoom = 1.0;\n> >         const auto scalerCrop = controlsInfo.find(&controls::ScalerCrop);\n> >         if (scalerCrop != controlsInfo.end()) {\n> >\n> >                 ....\n> >         }\n> >\n> > What do you think ?\n>\n> Looks good to me.\n>\n> > The patch is good for the rest\n> > Reviewed-by: Jacopo Mondi <jacopo@jmondi.org>\n> >\n> > >\n>  staticMetadata_->addEntry(ANDROID_SCALER_AVAILABLE_MAX_DIGITAL_ZOOM,\n> > >                                       &maxZoom, 1);\n> > >     }\n>\n> --\n> Regards,\n>\n> Laurent Pinchart\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 0AD98C32EA\n\tfor <parsemail@patchwork.libcamera.org>;\n\tMon, 29 Mar 2021 17:37:13 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id 5D1EB68784;\n\tMon, 29 Mar 2021 19:37:12 +0200 (CEST)","from mail-il1-x12f.google.com (mail-il1-x12f.google.com\n\t[IPv6:2607:f8b0:4864:20::12f])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTPS id A82566877D\n\tfor <libcamera-devel@lists.libcamera.org>;\n\tMon, 29 Mar 2021 19:37:10 +0200 (CEST)","by mail-il1-x12f.google.com with SMTP id t6so11862972ilp.11\n\tfor <libcamera-devel@lists.libcamera.org>;\n\tMon, 29 Mar 2021 10:37:10 -0700 (PDT)"],"Authentication-Results":"lancelot.ideasonboard.com;\n\tdkim=fail reason=\"signature verification failed\" (2048-bit key;\n\tunprotected) header.d=baylibre-com.20150623.gappssmtp.com\n\theader.i=@baylibre-com.20150623.gappssmtp.com\n\theader.b=\"YnZhIkJ7\"; dkim-atps=neutral","DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/relaxed;\n\td=baylibre-com.20150623.gappssmtp.com; s=20150623;\n\th=mime-version:references:in-reply-to:from:date:message-id:subject:to\n\t:cc; bh=I0qO74G0yrp9+nJIwkb7NhtEDOW39whdy+a9iTwPnX4=;\n\tb=YnZhIkJ7YBjS8lp74UB3v+hDkAJZDkzrDOE27hIYiv7NQka/TRvu4bR4H35hkSwIUb\n\tPsYySF+bDwagXxmmRzvt78WbbaoA8WdDP44v3suLw4oLt8zU1Hic0Fl/whWTGBjR3ZTC\n\tk/Y3H0btqNbfl3XCHyzxIeqzAnFilUDWjMYFflYEPF2LDx/wKAXV0XKHDUGDHBM/nu79\n\tDlrUMptT8f9YxFUfgTm38DNOyPUMuhogJp8ADseoxtylToObxwhWqbrcCMmc68wLCncF\n\tLSNYw0DS9lqC10WxIkXCVYQ3gIN8dU26wuscMO/8RdVGkSAPKrenGBP1EJx/S3kHly2A\n\tP7jg==","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=I0qO74G0yrp9+nJIwkb7NhtEDOW39whdy+a9iTwPnX4=;\n\tb=CGVZGhzhY3jq2zOCz54/K/7FThJcfJl+y6irb6gqkt+rTqdemApg/TQguFu4hVHxDe\n\t8G6COSO75Z6cpe6pcF64dIz/WE+tsueu+Qi8vgmLrRQ4fatNpemWV2mrsn1v/pczFf5e\n\tsofVpckNb8RQtNac2yYE50Tm+m/PVZCnUubMMihoNofJRbY7UYHfweDiiIVjybtzrP8V\n\t0opJoJWeVZf3GgTL9dHrz4rmvzTS+1HvFSFmmPexLEFBy3xh4EaePkrIvRliBpoTl7C9\n\tf6gdsHOmajKoXUZcn1jUb66cVCemWvV2OMHFWxo79CPkaY68pYXZklserxlIlxD78+Ii\n\tSGPg==","X-Gm-Message-State":"AOAM5336J3cHt3c7XzT6wTs9skTfadegU8Z9mVvvFuyCr9r/evC/sVcX\n\tXqY31vJTyAYp6PxMKo/P8DJI1/YiatZ5YENsdnX6ZMH+E+0=","X-Google-Smtp-Source":"ABdhPJznNonJwD7bD7x+hGD1Klr5hGW8p9h17JglRvwCuvrQdQslBXhocN/1IfaV9ZM4cuGDBCP1hknny5ydoLZogv0=","X-Received":"by 2002:a92:db43:: with SMTP id\n\tw3mr16733387ilq.150.1617039429238; \n\tMon, 29 Mar 2021 10:37:09 -0700 (PDT)","MIME-Version":"1.0","References":"<YF49KirQp2QUQ1LA@pendragon.ideasonboard.com>\n\t<20210326220410.329299-1-pnguyen@baylibre.com>\n\t<20210329075617.rxxmwu6idghbping@uno.localdomain>\n\t<YGGNCu7el9F0isDL@pendragon.ideasonboard.com>","In-Reply-To":"<YGGNCu7el9F0isDL@pendragon.ideasonboard.com>","From":"Phi-bang Nguyen <pnguyen@baylibre.com>","Date":"Mon, 29 Mar 2021 19:36:58 +0200","Message-ID":"<CAO4hSrh4hYxUF5prpycGN__YSdW5jDKzAL6LtaoHXVNkt5zwzA@mail.gmail.com>","To":"Laurent Pinchart <laurent.pinchart@ideasonboard.com>","Subject":"Re: [libcamera-devel] [PATCH v2] android: camera_device: Add null\n\tcheck for ScalerCrop control","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","Content-Type":"multipart/mixed;\n\tboundary=\"===============1322286008055595877==\"","Errors-To":"libcamera-devel-bounces@lists.libcamera.org","Sender":"\"libcamera-devel\" <libcamera-devel-bounces@lists.libcamera.org>"}}]