[{"id":13345,"web_url":"https://patchwork.libcamera.org/comment/13345/","msgid":"<35960608-c8a7-f0b1-24ee-9dfe8c40e25e@ideasonboard.com>","date":"2020-10-21T13:17:52","subject":"Re: [libcamera-devel] [PATCH v2 11/13] android: camera_device:\n\tHandle NOISE_REDUCTION_MODES","submitter":{"id":4,"url":"https://patchwork.libcamera.org/api/people/4/","name":"Kieran Bingham","email":"kieran.bingham@ideasonboard.com"},"content":"Hi Jacopo,\n\nOn 20/10/2020 19:05, Jacopo Mondi wrote:\n> Register the ANDROID_NOISE_REDUCTION_AVAILABLE_NOISE_REDUCTION_MODES\n> static metadata property inspecting the values retuned by the pipeline\n> handler.\n> \n> Reserve in the static metadata pack enough space to support all the 5\n> available noise reduction modes Android defines.\n> \n> Signed-off-by: Jacopo Mondi \n> ---\n> src/android/camera_device.cpp | 17 +++++++++++++----\n> 1 file changed, 13 insertions(+), 4 deletions(-)\n> \n> diff --git a/src/android/camera_device.cpp b/src/android/camera_device.cpp\n> index 8166b09bb69a..a4d9e6ddc519 100644\n> --- a/src/android/camera_device.cpp\n> +++ b/src/android/camera_device.cpp\n> @@ -553,7 +553,7 @@ std::tuple CameraDevice::calculateStaticMetadataSize()\n> \t * Currently: 51 entries, 687 bytes of static metadata\n> \t */\n> \tuint32_t numEntries = 51;\n> -\tuint32_t byteSize = 687;\n> +\tuint32_t byteSize = 691;\n> \n> \t/*\n> \t * Calculate space occupation in bytes for dynamically built metadata\n> @@ -830,9 +830,18 @@ const camera_metadata_t *CameraDevice::getStaticMetadata()\n> \t\t\t\t &minFocusDistance, 1);\n> \n> \t/* Noise reduction modes. */\n> -\tuint8_t noiseReductionModes = ANDROID_NOISE_REDUCTION_MODE_OFF;\n> -\tstaticMetadata_->addEntry(ANDROID_NOISE_REDUCTION_AVAILABLE_NOISE_REDUCTION_MODES,\n> -\t\t\t\t &noiseReductionModes, 1);\n> +\t{\n> +\t\tstd::vector data(5);\n> +\t\tconst auto &infoMap = controlsInfo.find(&controls::draft::NoiseReductionMode);\n> +\t\tif (infoMap != controlsInfo.end()) {\n> +\t\t\tfor (const auto &value : infoMap->second.values())\n> +\t\t\t\tdata.push_back(value.get());\n\nI'm a bit concerned by this, as it has the hidden assumption that the\ncontrol values are the same value as the ANDROID enum values.\n\nWhile they're in draft that's true, but it might not be when we remove\n::draft:: ... I suspect the best option here is to add a \\todo: or\nsomething to highlight that fact so it's not a hidden assumption.\n\nAnyway, I won't block on that, as this is clearly still in ::draft::.\n\nReviewed-by: Kieran Bingham \n\n> +\t\t} else {\n> +\t\t\tdata.push_back(ANDROID_NOISE_REDUCTION_MODE_OFF);\n> +\t\t}\n> +\t\tstaticMetadata_->addEntry(ANDROID_NOISE_REDUCTION_AVAILABLE_NOISE_REDUCTION_MODES,\n> +\t\t\t\t\t data.data(), data.size());\n> +\t}\n> \n> \t/* Scaler static metadata. */\n> \tfloat maxDigitalZoom = 1;\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 573F4C3D3C\n\tfor ;\n\tWed, 21 Oct 2020 13:17:57 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id 2437961DC5;\n\tWed, 21 Oct 2020 15:17:57 +0200 (CEST)","from perceval.ideasonboard.com (perceval.ideasonboard.com\n\t[213.167.242.64])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTPS id 8CD4D61D7F\n\tfor ;\n\tWed, 21 Oct 2020 15:17:56 +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 981D192;\n\tWed, 21 Oct 2020 15:17:55 +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=\"wESOtdDj\"; dkim-atps=neutral","DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com;\n\ts=mail; t=1603286276;\n\tbh=6sEHVWJioBHbQmaBs6+Cr95Q+JYB1eAjHGBrgH9OVp0=;\n\th=Reply-To:Subject:To:References:From:Date:In-Reply-To:From;\n\tb=wESOtdDjJRxOeske6QIrCXzutydLRM8ijGPZr9nb+j0Uf4l+wkG9QiHzUUUcIgS6C\n\tGFGW07obN3lFKzBQ/9xnSd3cBaMSwT23r0mFzHl1eSYsl/op3/4653+by1x+rZT6rU\n\t586VsYlm4RB5OWXYz2L7EmJ172Osvjf/qxs8b7oc=","To":"Jacopo Mondi , libcamera-devel@lists.libcamera.org","References":"<20201020180534.36855-1-jacopo@jmondi.org>\n\t<20201020180534.36855-12-jacopo@jmondi.org>","From":"Kieran Bingham ","Autocrypt":"addr=kieran.bingham@ideasonboard.com; keydata=\n\tmQINBFYE/WYBEACs1PwjMD9rgCu1hlIiUA1AXR4rv2v+BCLUq//vrX5S5bjzxKAryRf0uHat\n\tV/zwz6hiDrZuHUACDB7X8OaQcwhLaVlq6byfoBr25+hbZG7G3+5EUl9cQ7dQEdvNj6V6y/SC\n\trRanWfelwQThCHckbobWiQJfK9n7rYNcPMq9B8e9F020LFH7Kj6YmO95ewJGgLm+idg1Kb3C\n\tpotzWkXc1xmPzcQ1fvQMOfMwdS+4SNw4rY9f07Xb2K99rjMwZVDgESKIzhsDB5GY465sCsiQ\n\tcSAZRxqE49RTBq2+EQsbrQpIc8XiffAB8qexh5/QPzCmR4kJgCGeHIXBtgRj+nIkCJPZvZtf\n\tKr2EAbc6tgg6DkAEHJb+1okosV09+0+TXywYvtEop/WUOWQ+zo+Y/OBd+8Ptgt1pDRyOBzL8\n\tRXa8ZqRf0Mwg75D+dKntZeJHzPRJyrlfQokngAAs4PaFt6UfS+ypMAF37T6CeDArQC41V3ko\n\tlPn1yMsVD0p+6i3DPvA/GPIksDC4owjnzVX9kM8Zc5Cx+XoAN0w5Eqo4t6qEVbuettxx55gq\n\t8K8FieAjgjMSxngo/HST8TpFeqI5nVeq0/lqtBRQKumuIqDg+Bkr4L1V/PSB6XgQcOdhtd36\n\tOe9X9dXB8YSNt7VjOcO7BTmFn/Z8r92mSAfHXpb07YJWJosQOQARAQABtDBLaWVyYW4gQmlu\n\tZ2hhbSA8a2llcmFuLmJpbmdoYW1AaWRlYXNvbmJvYXJkLmNvbT6JAlcEEwEKAEECGwMFCwkI\n\tBwIGFQgJCgsCBBYCAwECHgECF4ACGQEWIQSQLdeYP70o/eNy1HqhHkZyEKRh/QUCXWTtygUJ\n\tCyJXZAAKCRChHkZyEKRh/f8dEACTDsbLN2nioNZMwyLuQRUAFcXNolDX48xcUXsWS2QjxaPm\n\tVsJx8Uy8aYkS85mdPBh0C83OovQR/OVbr8AxhGvYqBs3nQvbWuTl/+4od7DfK2VZOoKBAu5S\n\tQK2FYuUcikDqYcFWJ8DQnubxfE8dvzojHEkXw0sA4igINHDDFX3HJGZtLio+WpEFQtCbfTAG\n\tYZslasz1YZRbwEdSsmO3/kqy5eMnczlm8a21A3fKUo3g8oAZEFM+f4DUNzqIltg31OAB/kZS\n\tenKZQ/SWC8PmLg/ZXBrReYakxXtkP6w3FwMlzOlhGxqhIRNiAJfXJBaRhuUWzPOpEDE9q5YJ\n\tBmqQL2WJm1VSNNVxbXJHpaWMH1sA2R00vmvRrPXGwyIO0IPYeUYQa3gsy6k+En/aMQJd27dp\n\taScf9am9PFICPY5T4ppneeJLif2lyLojo0mcHOV+uyrds9XkLpp14GfTkeKPdPMrLLTsHRfH\n\tfA4I4OBpRrEPiGIZB/0im98MkGY/Mu6qxeZmYLCcgD6qz4idOvfgVOrNh+aA8HzIVR+RMW8H\n\tQGBN9f0E3kfwxuhl3omo6V7lDw8XOdmuWZNC9zPq1UfryVHANYbLGz9KJ4Aw6M+OgBC2JpkD\n\thXMdHUkC+d20dwXrwHTlrJi1YNp6rBc+xald3wsUPOZ5z8moTHUX/uPA/qhGsbkCDQRWBP1m\n\tARAAzijkb+Sau4hAncr1JjOY+KyFEdUNxRy+hqTJdJfaYihxyaj0Ee0P0zEi35CbE6lgU0Uz\n\ttih9fiUbSV3wfsWqg1Ut3/5rTKu7kLFp15kF7eqvV4uezXRD3Qu4yjv/rMmEJbbD4cTvGCYI\n\td6MDC417f7vK3hCbCVIZSp3GXxyC1LU+UQr3fFcOyCwmP9vDUR9JV0BSqHHxRDdpUXE26Dk6\n\tmhf0V1YkspE5St814ETXpEus2urZE5yJIUROlWPIL+hm3NEWfAP06vsQUyLvr/GtbOT79vXl\n\tEn1aulcYyu20dRRxhkQ6iILaURcxIAVJJKPi8dsoMnS8pB0QW12AHWuirPF0g6DiuUfPmrA5\n\tPKe56IGlpkjc8cO51lIxHkWTpCMWigRdPDexKX+Sb+W9QWK/0JjIc4t3KBaiG8O4yRX8ml2R\n\t+rxfAVKM6V769P/hWoRGdgUMgYHFpHGSgEt80OKK5HeUPy2cngDUXzwrqiM5Sz6Od0qw5pCk\n\tNlXqI0W/who0iSVM+8+RmyY0OEkxEcci7rRLsGnM15B5PjLJjh1f2ULYkv8s4SnDwMZ/kE04\n\t/UqCMK/KnX8pwXEMCjz0h6qWNpGwJ0/tYIgQJZh6bqkvBrDogAvuhf60Sogw+mH8b+PBlx1L\n\toeTK396wc+4c3BfiC6pNtUS5GpsPMMjYMk7kVvEAEQEAAYkCPAQYAQoAJgIbDBYhBJAt15g/\n\tvSj943LUeqEeRnIQpGH9BQJdizzIBQkLSKZiAAoJEKEeRnIQpGH9eYgQAJpjaWNgqNOnMTmD\n\tMJggbwjIotypzIXfhHNCeTkG7+qCDlSaBPclcPGYrTwCt0YWPU2TgGgJrVhYT20ierN8LUvj\n\t6qOPTd+Uk7NFzL65qkh80ZKNBFddx1AabQpSVQKbdcLb8OFs85kuSvFdgqZwgxA1vl4TFhNz\n\tPZ79NAmXLackAx3sOVFhk4WQaKRshCB7cSl+RIng5S/ThOBlwNlcKG7j7W2MC06BlTbdEkUp\n\tECzuuRBv8wX4OQl+hbWbB/VKIx5HKlLu1eypen/5lNVzSqMMIYkkZcjV2SWQyUGxSwq0O/sx\n\tS0A8/atCHUXOboUsn54qdxrVDaK+6jIAuo8JiRWctP16KjzUM7MO0/+4zllM8EY57rXrj48j\n\tsbEYX0YQnzaj+jO6kJtoZsIaYR7rMMq9aUAjyiaEZpmP1qF/2sYenDx0Fg2BSlLvLvXM0vU8\n\tpQk3kgDu7kb/7PRYrZvBsr21EIQoIjXbZxDz/o7z95frkP71EaICttZ6k9q5oxxA5WC6sTXc\n\tMW8zs8avFNuA9VpXt0YupJd2ijtZy2mpZNG02fFVXhIn4G807G7+9mhuC4XG5rKlBBUXTvPU\n\tAfYnB4JBDLmLzBFavQfvonSfbitgXwCG3vS+9HEwAjU30Bar1PEOmIbiAoMzuKeRm2LVpmq4\n\tWZw01QYHU/GUV/zHJSFk","Organization":"Ideas on Board","Message-ID":"<35960608-c8a7-f0b1-24ee-9dfe8c40e25e@ideasonboard.com>","Date":"Wed, 21 Oct 2020 14:17:52 +0100","User-Agent":"Mozilla/5.0 (X11; Linux x86_64; rv:68.0) Gecko/20100101\n\tThunderbird/68.10.0","MIME-Version":"1.0","In-Reply-To":"<20201020180534.36855-12-jacopo@jmondi.org>","Content-Language":"en-GB","Subject":"Re: [libcamera-devel] [PATCH v2 11/13] android: camera_device:\n\tHandle NOISE_REDUCTION_MODES","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":13357,"web_url":"https://patchwork.libcamera.org/comment/13357/","msgid":"<20201021135740.2c66bg2xudeegdac@uno.localdomain>","date":"2020-10-21T13:57:40","subject":"Re: [libcamera-devel] [PATCH v2 11/13] android: camera_device:\n\tHandle NOISE_REDUCTION_MODES","submitter":{"id":3,"url":"https://patchwork.libcamera.org/api/people/3/","name":"Jacopo Mondi","email":"jacopo@jmondi.org"},"content":"Hi Kieran,\n\nOn Wed, Oct 21, 2020 at 02:17:52PM +0100, Kieran Bingham wrote:\n> Hi Jacopo,\n>\n> On 20/10/2020 19:05, Jacopo Mondi wrote:\n> > Register the ANDROID_NOISE_REDUCTION_AVAILABLE_NOISE_REDUCTION_MODES\n> > static metadata property inspecting the values retuned by the pipeline\n> > handler.\n> >\n> > Reserve in the static metadata pack enough space to support all the 5\n> > available noise reduction modes Android defines.\n> >\n> > Signed-off-by: Jacopo Mondi \n> > ---\n> > src/android/camera_device.cpp | 17 +++++++++++++----\n> > 1 file changed, 13 insertions(+), 4 deletions(-)\n> >\n> > diff --git a/src/android/camera_device.cpp b/src/android/camera_device.cpp\n> > index 8166b09bb69a..a4d9e6ddc519 100644\n> > --- a/src/android/camera_device.cpp\n> > +++ b/src/android/camera_device.cpp\n> > @@ -553,7 +553,7 @@ std::tuple CameraDevice::calculateStaticMetadataSize()\n> > \t * Currently: 51 entries, 687 bytes of static metadata\n> > \t */\n> > \tuint32_t numEntries = 51;\n> > -\tuint32_t byteSize = 687;\n> > +\tuint32_t byteSize = 691;\n> >\n> > \t/*\n> > \t * Calculate space occupation in bytes for dynamically built metadata\n> > @@ -830,9 +830,18 @@ const camera_metadata_t *CameraDevice::getStaticMetadata()\n> > \t\t\t\t &minFocusDistance, 1);\n> >\n> > \t/* Noise reduction modes. */\n> > -\tuint8_t noiseReductionModes = ANDROID_NOISE_REDUCTION_MODE_OFF;\n> > -\tstaticMetadata_->addEntry(ANDROID_NOISE_REDUCTION_AVAILABLE_NOISE_REDUCTION_MODES,\n> > -\t\t\t\t &noiseReductionModes, 1);\n> > +\t{\n> > +\t\tstd::vector data(5);\n> > +\t\tconst auto &infoMap = controlsInfo.find(&controls::draft::NoiseReductionMode);\n> > +\t\tif (infoMap != controlsInfo.end()) {\n> > +\t\t\tfor (const auto &value : infoMap->second.values())\n> > +\t\t\t\tdata.push_back(value.get());\n>\n> I'm a bit concerned by this, as it has the hidden assumption that the\n> control values are the same value as the ANDROID enum values.\n>\n> While they're in draft that's true, but it might not be when we remove\n> ::draft:: ... I suspect the best option here is to add a \\todo: or\n> something to highlight that fact so it's not a hidden assumption.\n>\n> Anyway, I won't block on that, as this is clearly still in ::draft::.\n\nWhenerver the draft control will be moved outside of the draft::\nnamespace this will anyway be reworked\n\n>\n> Reviewed-by: Kieran Bingham \n>\n> > +\t\t} else {\n> > +\t\t\tdata.push_back(ANDROID_NOISE_REDUCTION_MODE_OFF);\n> > +\t\t}\n> > +\t\tstaticMetadata_->addEntry(ANDROID_NOISE_REDUCTION_AVAILABLE_NOISE_REDUCTION_MODES,\n> > +\t\t\t\t\t data.data(), data.size());\n> > +\t}\n> >\n> > \t/* Scaler static metadata. */\n> > \tfloat maxDigitalZoom = 1;\n> >\n>\n> --\n> Regards\n> --\n> Kieran","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 C44DFBDB13\n\tfor ;\n\tWed, 21 Oct 2020 13:57:43 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id A367561E10;\n\tWed, 21 Oct 2020 15:57:43 +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 5100261DDB\n\tfor ;\n\tWed, 21 Oct 2020 15:57:42 +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 relay8-d.mail.gandi.net (Postfix) with ESMTPSA id DCED61BF20A;\n\tWed, 21 Oct 2020 13:57:41 +0000 (UTC)"],"X-Originating-IP":"93.34.118.233","Date":"Wed, 21 Oct 2020 15:57:40 +0200","From":"Jacopo Mondi ","To":"Kieran Bingham ","Message-ID":"<20201021135740.2c66bg2xudeegdac@uno.localdomain>","References":"<20201020180534.36855-1-jacopo@jmondi.org>\n\t<20201020180534.36855-12-jacopo@jmondi.org>\n\t<35960608-c8a7-f0b1-24ee-9dfe8c40e25e@ideasonboard.com>","MIME-Version":"1.0","Content-Disposition":"inline","In-Reply-To":"<35960608-c8a7-f0b1-24ee-9dfe8c40e25e@ideasonboard.com>","Subject":"Re: [libcamera-devel] [PATCH v2 11/13] android: camera_device:\n\tHandle NOISE_REDUCTION_MODES","X-BeenThere":"libcamera-devel@lists.libcamera.org","X-Mailman-Version":"2.1.29","Precedence":"list","List-Id":"","List-Unsubscribe":",\n\t","List-Archive":"","List-Post":"","List-Help":"","List-Subscribe":",\n\t","Cc":"libcamera-devel@lists.libcamera.org","Content-Type":"text/plain; charset=\"us-ascii\"","Content-Transfer-Encoding":"7bit","Errors-To":"libcamera-devel-bounces@lists.libcamera.org","Sender":"\"libcamera-devel\" "}}]