[{"id":19696,"web_url":"https://patchwork.libcamera.org/comment/19696/","msgid":"","date":"2021-09-15T02:13:49","subject":"Re: [libcamera-devel] [PATCH 3/3] android: Plumb noise reduction","submitter":{"id":2,"url":"https://patchwork.libcamera.org/api/people/2/","name":"Laurent Pinchart","email":"laurent.pinchart@ideasonboard.com"},"content":"Hi Paul,\n\nThank you for the patch.\n\nOn Mon, Sep 13, 2021 at 07:20:07PM +0900, Paul Elder wrote:\n> Add:\n> - hardware level detection based on available noise reduction values\n> (no sepcific capabilities seem to require it; only FULL in general)\n\ns/sepcific/specific/\n\n> - Conversion from android to libcamera noise reduction modes for request\n> (use switch-case instead of direct copy just in case)\n> - Conversion from libcamera to android noise reduction modes for result\n> \n> We already have the mechanism to report the available noise reduction\n> modes, so that is not added.\n> \n> Signed-off-by: Paul Elder \n> ---\n> src/android/camera_capabilities.cpp | 19 +++++++++++++\n> src/android/camera_device.cpp | 41 +++++++++++++++++++++++++++++\n> 2 files changed, 60 insertions(+)\n> \n> diff --git a/src/android/camera_capabilities.cpp b/src/android/camera_capabilities.cpp\n> index 08e44a1a..be6687d9 100644\n> --- a/src/android/camera_capabilities.cpp\n> +++ b/src/android/camera_capabilities.cpp\n> @@ -375,6 +375,25 @@ void CameraCapabilities::computeHwLevel(\n> \tif (!found || *entry.data.i32 != 0)\n> \t\thwLevel = ANDROID_INFO_SUPPORTED_HARDWARE_LEVEL_LIMITED;\n> \n> +\tfound = staticMetadata_->entryContains(\n> +\t\tANDROID_NOISE_REDUCTION_AVAILABLE_NOISE_REDUCTION_MODES,\n> +\t\tANDROID_NOISE_REDUCTION_MODE_OFF);\n> +\tif (!found)\n> +\t\thwLevel = ANDROID_INFO_SUPPORTED_HARDWARE_LEVEL_LIMITED;\n> +\n> +\tfound = staticMetadata_->entryContains(\n> +\t\tANDROID_NOISE_REDUCTION_AVAILABLE_NOISE_REDUCTION_MODES,\n> +\t\tANDROID_NOISE_REDUCTION_MODE_FAST);\n> +\tif (!found)\n> +\t\thwLevel = ANDROID_INFO_SUPPORTED_HARDWARE_LEVEL_LIMITED;\n> +\n> +\t/* Android docs don't say this is required, but CTS does. */\n> +\tfound = staticMetadata_->entryContains(\n> +\t\tANDROID_NOISE_REDUCTION_AVAILABLE_NOISE_REDUCTION_MODES,\n> +\t\tANDROID_NOISE_REDUCTION_MODE_HIGH_QUALITY);\n> +\tif (!found)\n> +\t\thwLevel = ANDROID_INFO_SUPPORTED_HARDWARE_LEVEL_LIMITED;\n> +\n> \thwLevel_ = hwLevel;\n> }\n> \n> diff --git a/src/android/camera_device.cpp b/src/android/camera_device.cpp\n> index ab31bdda..502dd10a 100644\n> --- a/src/android/camera_device.cpp\n> +++ b/src/android/camera_device.cpp\n> @@ -826,6 +826,40 @@ int CameraDevice::processControls(Camera3RequestDescriptor *descriptor)\n> \t\tcontrols.set(controls::draft::TestPatternMode, testPatternMode);\n> \t}\n> \n> +\tif (settings.getEntry(ANDROID_NOISE_REDUCTION_AVAILABLE_NOISE_REDUCTION_MODES, &entry)) {\n> +\t\tconst uint8_t data = *entry.data.u8;\n> +\t\tint32_t noiseReductionMode = controls::NoiseReductionModeOff;\n\nAs there's a default case you could skip initialization.\n\n> +\t\tswitch (data) {\n> +\t\tcase ANDROID_NOISE_REDUCTION_MODE_OFF:\n> +\t\t\tnoiseReductionMode = controls::NoiseReductionModeOff;\n> +\t\t\tbreak;\n> +\n> +\t\tcase ANDROID_NOISE_REDUCTION_MODE_FAST:\n> +\t\t\tnoiseReductionMode = controls::NoiseReductionModeFast;\n> +\t\t\tbreak;\n> +\n> +\t\tcase ANDROID_NOISE_REDUCTION_MODE_HIGH_QUALITY:\n> +\t\t\tnoiseReductionMode = controls::NoiseReductionModeHighQuality;\n> +\t\t\tbreak;\n> +\n> +\t\tcase ANDROID_NOISE_REDUCTION_MODE_MINIMAL:\n> +\t\t\tnoiseReductionMode = controls::NoiseReductionModeRaw;\n> +\t\t\tbreak;\n> +\n> +\t\tcase ANDROID_NOISE_REDUCTION_MODE_ZERO_SHUTTER_LAG:\n> +\t\t\tnoiseReductionMode = controls::NoiseReductionModeZSL;\n> +\t\t\tbreak;\n> +\n> +\t\tdefault:\n> +\t\t\tLOG(HAL, Error)\n> +\t\t\t\t<< \"Unknown noise reduction mode: \" << data;\n> +\n> +\t\t\treturn -EINVAL;\n> +\t\t}\n> +\n> +\t\tcontrols.set(controls::NoiseReductionMode, noiseReductionMode);\n> +\t}\n> +\n> \treturn 0;\n> }\n> \n> @@ -1379,6 +1413,13 @@ CameraDevice::getResultMetadata(const Camera3RequestDescriptor &descriptor) cons\n> \t\tresultMetadata->addEntry(ANDROID_SCALER_CROP_REGION, cropRect);\n> \t}\n> \n> +\tif (metadata.contains(controls::NoiseReductionMode)) {\n> +\t\tconst int32_t noiseReductionMode =\n> +\t\t\tmetadata.get(controls::NoiseReductionMode);\n> +\t\tresultMetadata->addEntry(ANDROID_NOISE_REDUCTION_MODE,\n> +\t\t\t\t\t noiseReductionMode);\n> +\t}\n\nShould you use a switch/case here too, if you use one in\nCameraDevice::processControls() ?\n\n> +\n> \tif (metadata.contains(controls::draft::TestPatternMode)) {\n> \t\tconst int32_t testPatternMode =\n> \t\t\tmetadata.get(controls::draft::TestPatternMode);","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 244EABDB1D\n\tfor ;\n\tWed, 15 Sep 2021 02:14:17 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id 8970969189;\n\tWed, 15 Sep 2021 04:14: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 1A58C60247\n\tfor ;\n\tWed, 15 Sep 2021 04:14:15 +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 811D724F;\n\tWed, 15 Sep 2021 04:14:14 +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=\"G8WTcJoQ\"; dkim-atps=neutral","DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com;\n\ts=mail; t=1631672054;\n\tbh=KlgYuwzaL0T/c5Qc8CeUV+xe9sL1a9OBNxFYBQ36c+s=;\n\th=Date:From:To:Cc:Subject:References:In-Reply-To:From;\n\tb=G8WTcJoQ/18X/GqEQxXZn9T6tH12XUUl7OBOMUNQJHpRvdS36Cxs28jHJL9Vpmv6O\n\tgAjSnzY0v2Y+gyKkTK4PGWrt2U7BEzQlSg24YJaslzZGiKzVgiszCa2QL+3Ihw3fo9\n\tcpmTYxMzywgxMUmUY00ebA5Vqpap2HuifqCLqQUc=","Date":"Wed, 15 Sep 2021 05:13:49 +0300","From":"Laurent Pinchart ","To":"Paul Elder ","Message-ID":"","References":"<20210913102007.2303225-1-paul.elder@ideasonboard.com>\n\t<20210913102007.2303225-3-paul.elder@ideasonboard.com>","MIME-Version":"1.0","Content-Type":"text/plain; charset=utf-8","Content-Disposition":"inline","In-Reply-To":"<20210913102007.2303225-3-paul.elder@ideasonboard.com>","Subject":"Re: [libcamera-devel] [PATCH 3/3] android: Plumb noise reduction","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","Errors-To":"libcamera-devel-bounces@lists.libcamera.org","Sender":"\"libcamera-devel\" "}},{"id":19700,"web_url":"https://patchwork.libcamera.org/comment/19700/","msgid":"<20210915030955.GH1857410@pyrite.rasen.tech>","date":"2021-09-15T03:09:55","subject":"Re: [libcamera-devel] [PATCH 3/3] android: Plumb noise reduction","submitter":{"id":17,"url":"https://patchwork.libcamera.org/api/people/17/","name":"Paul Elder","email":"paul.elder@ideasonboard.com"},"content":"Hi Laurent,\n\nOn Wed, Sep 15, 2021 at 05:13:49AM +0300, Laurent Pinchart wrote:\n> Hi Paul,\n> \n> Thank you for the patch.\n> \n> On Mon, Sep 13, 2021 at 07:20:07PM +0900, Paul Elder wrote:\n> > Add:\n> > - hardware level detection based on available noise reduction values\n> > (no sepcific capabilities seem to require it; only FULL in general)\n> \n> s/sepcific/specific/\n> \n> > - Conversion from android to libcamera noise reduction modes for request\n> > (use switch-case instead of direct copy just in case)\n> > - Conversion from libcamera to android noise reduction modes for result\n> > \n> > We already have the mechanism to report the available noise reduction\n> > modes, so that is not added.\n> > \n> > Signed-off-by: Paul Elder \n> > ---\n> > src/android/camera_capabilities.cpp | 19 +++++++++++++\n> > src/android/camera_device.cpp | 41 +++++++++++++++++++++++++++++\n> > 2 files changed, 60 insertions(+)\n> > \n> > diff --git a/src/android/camera_capabilities.cpp b/src/android/camera_capabilities.cpp\n> > index 08e44a1a..be6687d9 100644\n> > --- a/src/android/camera_capabilities.cpp\n> > +++ b/src/android/camera_capabilities.cpp\n> > @@ -375,6 +375,25 @@ void CameraCapabilities::computeHwLevel(\n> > \tif (!found || *entry.data.i32 != 0)\n> > \t\thwLevel = ANDROID_INFO_SUPPORTED_HARDWARE_LEVEL_LIMITED;\n> > \n> > +\tfound = staticMetadata_->entryContains(\n> > +\t\tANDROID_NOISE_REDUCTION_AVAILABLE_NOISE_REDUCTION_MODES,\n> > +\t\tANDROID_NOISE_REDUCTION_MODE_OFF);\n> > +\tif (!found)\n> > +\t\thwLevel = ANDROID_INFO_SUPPORTED_HARDWARE_LEVEL_LIMITED;\n> > +\n> > +\tfound = staticMetadata_->entryContains(\n> > +\t\tANDROID_NOISE_REDUCTION_AVAILABLE_NOISE_REDUCTION_MODES,\n> > +\t\tANDROID_NOISE_REDUCTION_MODE_FAST);\n> > +\tif (!found)\n> > +\t\thwLevel = ANDROID_INFO_SUPPORTED_HARDWARE_LEVEL_LIMITED;\n> > +\n> > +\t/* Android docs don't say this is required, but CTS does. */\n> > +\tfound = staticMetadata_->entryContains(\n> > +\t\tANDROID_NOISE_REDUCTION_AVAILABLE_NOISE_REDUCTION_MODES,\n> > +\t\tANDROID_NOISE_REDUCTION_MODE_HIGH_QUALITY);\n> > +\tif (!found)\n> > +\t\thwLevel = ANDROID_INFO_SUPPORTED_HARDWARE_LEVEL_LIMITED;\n> > +\n> > \thwLevel_ = hwLevel;\n> > }\n> > \n> > diff --git a/src/android/camera_device.cpp b/src/android/camera_device.cpp\n> > index ab31bdda..502dd10a 100644\n> > --- a/src/android/camera_device.cpp\n> > +++ b/src/android/camera_device.cpp\n> > @@ -826,6 +826,40 @@ int CameraDevice::processControls(Camera3RequestDescriptor *descriptor)\n> > \t\tcontrols.set(controls::draft::TestPatternMode, testPatternMode);\n> > \t}\n> > \n> > +\tif (settings.getEntry(ANDROID_NOISE_REDUCTION_AVAILABLE_NOISE_REDUCTION_MODES, &entry)) {\n> > +\t\tconst uint8_t data = *entry.data.u8;\n> > +\t\tint32_t noiseReductionMode = controls::NoiseReductionModeOff;\n> \n> As there's a default case you could skip initialization.\n> \n> > +\t\tswitch (data) {\n> > +\t\tcase ANDROID_NOISE_REDUCTION_MODE_OFF:\n> > +\t\t\tnoiseReductionMode = controls::NoiseReductionModeOff;\n> > +\t\t\tbreak;\n> > +\n> > +\t\tcase ANDROID_NOISE_REDUCTION_MODE_FAST:\n> > +\t\t\tnoiseReductionMode = controls::NoiseReductionModeFast;\n> > +\t\t\tbreak;\n> > +\n> > +\t\tcase ANDROID_NOISE_REDUCTION_MODE_HIGH_QUALITY:\n> > +\t\t\tnoiseReductionMode = controls::NoiseReductionModeHighQuality;\n> > +\t\t\tbreak;\n> > +\n> > +\t\tcase ANDROID_NOISE_REDUCTION_MODE_MINIMAL:\n> > +\t\t\tnoiseReductionMode = controls::NoiseReductionModeRaw;\n> > +\t\t\tbreak;\n> > +\n> > +\t\tcase ANDROID_NOISE_REDUCTION_MODE_ZERO_SHUTTER_LAG:\n> > +\t\t\tnoiseReductionMode = controls::NoiseReductionModeZSL;\n> > +\t\t\tbreak;\n> > +\n> > +\t\tdefault:\n> > +\t\t\tLOG(HAL, Error)\n> > +\t\t\t\t<< \"Unknown noise reduction mode: \" << data;\n> > +\n> > +\t\t\treturn -EINVAL;\n> > +\t\t}\n> > +\n> > +\t\tcontrols.set(controls::NoiseReductionMode, noiseReductionMode);\n> > +\t}\n> > +\n> > \treturn 0;\n> > }\n> > \n> > @@ -1379,6 +1413,13 @@ CameraDevice::getResultMetadata(const Camera3RequestDescriptor &descriptor) cons\n> > \t\tresultMetadata->addEntry(ANDROID_SCALER_CROP_REGION, cropRect);\n> > \t}\n> > \n> > +\tif (metadata.contains(controls::NoiseReductionMode)) {\n> > +\t\tconst int32_t noiseReductionMode =\n> > +\t\t\tmetadata.get(controls::NoiseReductionMode);\n> > +\t\tresultMetadata->addEntry(ANDROID_NOISE_REDUCTION_MODE,\n> > +\t\t\t\t\t noiseReductionMode);\n> > +\t}\n> \n> Should you use a switch/case here too, if you use one in\n> CameraDevice::processControls() ?\n\ntbh that's what I thought doo. But look at what TestPatternMode does\nright below ↓ :p\n\n\nimo switch-case is better though.\n\n\nThanks,\n\nPaul\n\n> \n> > +\n> > \tif (metadata.contains(controls::draft::TestPatternMode)) {\n> > \t\tconst int32_t testPatternMode =\n> > \t\t\tmetadata.get(controls::draft::TestPatternMode);","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 98224BDC71\n\tfor ;\n\tWed, 15 Sep 2021 03:10:05 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id 6408669189;\n\tWed, 15 Sep 2021 05:10:05 +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 10D7060247\n\tfor ;\n\tWed, 15 Sep 2021 05:10:04 +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 8033224F;\n\tWed, 15 Sep 2021 05:10:02 +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=\"hSNDNK8h\"; dkim-atps=neutral","DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com;\n\ts=mail; t=1631675403;\n\tbh=Q3d3BfF8w+pQnzX2CHQ5uKA3J1A0mOSuzP6lt5ayWd8=;\n\th=Date:From:To:Cc:Subject:References:In-Reply-To:From;\n\tb=hSNDNK8hc0ujgQLyol7o46yEUXswYL2zYJ9Tb+ijz0I0m4wOIeyrEG0dHfLkYVi/J\n\tVTKsyG4JqEDhkhSgFy1PzXoRY6pln/CrlOTVMP7XQeylnSPf5aIDFUFZVj+J8Yrrjw\n\tMfD/b6tgA37OpT3/AHtxpm5Nnc8+dWX2D7SAeIX0=","Date":"Wed, 15 Sep 2021 12:09:55 +0900","From":"paul.elder@ideasonboard.com","To":"Laurent Pinchart ","Message-ID":"<20210915030955.GH1857410@pyrite.rasen.tech>","References":"<20210913102007.2303225-1-paul.elder@ideasonboard.com>\n\t<20210913102007.2303225-3-paul.elder@ideasonboard.com>\n\t","MIME-Version":"1.0","Content-Type":"text/plain; charset=utf-8","Content-Disposition":"inline","Content-Transfer-Encoding":"8bit","In-Reply-To":"","Subject":"Re: [libcamera-devel] [PATCH 3/3] android: Plumb noise reduction","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","Errors-To":"libcamera-devel-bounces@lists.libcamera.org","Sender":"\"libcamera-devel\" "}},{"id":19702,"web_url":"https://patchwork.libcamera.org/comment/19702/","msgid":"","date":"2021-09-15T03:45:32","subject":"Re: [libcamera-devel] [PATCH 3/3] android: Plumb noise reduction","submitter":{"id":2,"url":"https://patchwork.libcamera.org/api/people/2/","name":"Laurent Pinchart","email":"laurent.pinchart@ideasonboard.com"},"content":"Hi Paul,\n\nOn Wed, Sep 15, 2021 at 12:09:55PM +0900, paul.elder@ideasonboard.com wrote:\n> On Wed, Sep 15, 2021 at 05:13:49AM +0300, Laurent Pinchart wrote:\n> > On Mon, Sep 13, 2021 at 07:20:07PM +0900, Paul Elder wrote:\n> > > Add:\n> > > - hardware level detection based on available noise reduction values\n> > > (no sepcific capabilities seem to require it; only FULL in general)\n> > \n> > s/sepcific/specific/\n> > \n> > > - Conversion from android to libcamera noise reduction modes for request\n> > > (use switch-case instead of direct copy just in case)\n> > > - Conversion from libcamera to android noise reduction modes for result\n> > > \n> > > We already have the mechanism to report the available noise reduction\n> > > modes, so that is not added.\n> > > \n> > > Signed-off-by: Paul Elder \n> > > ---\n> > > src/android/camera_capabilities.cpp | 19 +++++++++++++\n> > > src/android/camera_device.cpp | 41 +++++++++++++++++++++++++++++\n> > > 2 files changed, 60 insertions(+)\n> > > \n> > > diff --git a/src/android/camera_capabilities.cpp b/src/android/camera_capabilities.cpp\n> > > index 08e44a1a..be6687d9 100644\n> > > --- a/src/android/camera_capabilities.cpp\n> > > +++ b/src/android/camera_capabilities.cpp\n> > > @@ -375,6 +375,25 @@ void CameraCapabilities::computeHwLevel(\n> > > \tif (!found || *entry.data.i32 != 0)\n> > > \t\thwLevel = ANDROID_INFO_SUPPORTED_HARDWARE_LEVEL_LIMITED;\n> > > \n> > > +\tfound = staticMetadata_->entryContains(\n> > > +\t\tANDROID_NOISE_REDUCTION_AVAILABLE_NOISE_REDUCTION_MODES,\n> > > +\t\tANDROID_NOISE_REDUCTION_MODE_OFF);\n> > > +\tif (!found)\n> > > +\t\thwLevel = ANDROID_INFO_SUPPORTED_HARDWARE_LEVEL_LIMITED;\n> > > +\n> > > +\tfound = staticMetadata_->entryContains(\n> > > +\t\tANDROID_NOISE_REDUCTION_AVAILABLE_NOISE_REDUCTION_MODES,\n> > > +\t\tANDROID_NOISE_REDUCTION_MODE_FAST);\n> > > +\tif (!found)\n> > > +\t\thwLevel = ANDROID_INFO_SUPPORTED_HARDWARE_LEVEL_LIMITED;\n> > > +\n> > > +\t/* Android docs don't say this is required, but CTS does. */\n> > > +\tfound = staticMetadata_->entryContains(\n> > > +\t\tANDROID_NOISE_REDUCTION_AVAILABLE_NOISE_REDUCTION_MODES,\n> > > +\t\tANDROID_NOISE_REDUCTION_MODE_HIGH_QUALITY);\n> > > +\tif (!found)\n> > > +\t\thwLevel = ANDROID_INFO_SUPPORTED_HARDWARE_LEVEL_LIMITED;\n> > > +\n> > > \thwLevel_ = hwLevel;\n> > > }\n> > > \n> > > diff --git a/src/android/camera_device.cpp b/src/android/camera_device.cpp\n> > > index ab31bdda..502dd10a 100644\n> > > --- a/src/android/camera_device.cpp\n> > > +++ b/src/android/camera_device.cpp\n> > > @@ -826,6 +826,40 @@ int CameraDevice::processControls(Camera3RequestDescriptor *descriptor)\n> > > \t\tcontrols.set(controls::draft::TestPatternMode, testPatternMode);\n> > > \t}\n> > > \n> > > +\tif (settings.getEntry(ANDROID_NOISE_REDUCTION_AVAILABLE_NOISE_REDUCTION_MODES, &entry)) {\n> > > +\t\tconst uint8_t data = *entry.data.u8;\n> > > +\t\tint32_t noiseReductionMode = controls::NoiseReductionModeOff;\n> > \n> > As there's a default case you could skip initialization.\n> > \n> > > +\t\tswitch (data) {\n> > > +\t\tcase ANDROID_NOISE_REDUCTION_MODE_OFF:\n> > > +\t\t\tnoiseReductionMode = controls::NoiseReductionModeOff;\n> > > +\t\t\tbreak;\n> > > +\n> > > +\t\tcase ANDROID_NOISE_REDUCTION_MODE_FAST:\n> > > +\t\t\tnoiseReductionMode = controls::NoiseReductionModeFast;\n> > > +\t\t\tbreak;\n> > > +\n> > > +\t\tcase ANDROID_NOISE_REDUCTION_MODE_HIGH_QUALITY:\n> > > +\t\t\tnoiseReductionMode = controls::NoiseReductionModeHighQuality;\n> > > +\t\t\tbreak;\n> > > +\n> > > +\t\tcase ANDROID_NOISE_REDUCTION_MODE_MINIMAL:\n> > > +\t\t\tnoiseReductionMode = controls::NoiseReductionModeRaw;\n> > > +\t\t\tbreak;\n> > > +\n> > > +\t\tcase ANDROID_NOISE_REDUCTION_MODE_ZERO_SHUTTER_LAG:\n> > > +\t\t\tnoiseReductionMode = controls::NoiseReductionModeZSL;\n> > > +\t\t\tbreak;\n> > > +\n> > > +\t\tdefault:\n> > > +\t\t\tLOG(HAL, Error)\n> > > +\t\t\t\t<< \"Unknown noise reduction mode: \" << data;\n> > > +\n> > > +\t\t\treturn -EINVAL;\n> > > +\t\t}\n> > > +\n> > > +\t\tcontrols.set(controls::NoiseReductionMode, noiseReductionMode);\n> > > +\t}\n> > > +\n> > > \treturn 0;\n> > > }\n> > > \n> > > @@ -1379,6 +1413,13 @@ CameraDevice::getResultMetadata(const Camera3RequestDescriptor &descriptor) cons\n> > > \t\tresultMetadata->addEntry(ANDROID_SCALER_CROP_REGION, cropRect);\n> > > \t}\n> > > \n> > > +\tif (metadata.contains(controls::NoiseReductionMode)) {\n> > > +\t\tconst int32_t noiseReductionMode =\n> > > +\t\t\tmetadata.get(controls::NoiseReductionMode);\n> > > +\t\tresultMetadata->addEntry(ANDROID_NOISE_REDUCTION_MODE,\n> > > +\t\t\t\t\t noiseReductionMode);\n> > > +\t}\n> > \n> > Should you use a switch/case here too, if you use one in\n> > CameraDevice::processControls() ?\n> \n> tbh that's what I thought doo. But look at what TestPatternMode does\n> right below ↓ :p\n> \n> imo switch-case is better though.\n\nEither there's a guarantee that the values match, and you can then drop\nthe switch above, or there isn't, and you need one here.\n\n> > > +\n> > > \tif (metadata.contains(controls::draft::TestPatternMode)) {\n> > > \t\tconst int32_t testPatternMode =\n> > > \t\t\tmetadata.get(controls::draft::TestPatternMode);","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 1A2F8BDC71\n\tfor ;\n\tWed, 15 Sep 2021 03:45:59 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id 8283069189;\n\tWed, 15 Sep 2021 05:45: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 844E560247\n\tfor ;\n\tWed, 15 Sep 2021 05:45: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 0662024F;\n\tWed, 15 Sep 2021 05:45:56 +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=\"rlHjtoga\"; dkim-atps=neutral","DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com;\n\ts=mail; t=1631677557;\n\tbh=uGmV3x929EzdPZrlxKzLvL7RwNLMlddEqi6F9VTC2nw=;\n\th=Date:From:To:Cc:Subject:References:In-Reply-To:From;\n\tb=rlHjtogajxRX25kBZIw4+nHDlolC99pUIg62rZxf4j4bmjHYsNockMQ6K1Bv7xIi5\n\tjvq9vTID4Cu1hOsNVNgkEhVp54dyUKwu5H+CXo/yzHWYoEoXPRm3EhNumoJlZuSc6M\n\t9olnacOdf2unkuMG+ycAMkeL5kOs7gC7WIH01xN4=","Date":"Wed, 15 Sep 2021 06:45:32 +0300","From":"Laurent Pinchart ","To":"paul.elder@ideasonboard.com","Message-ID":"","References":"<20210913102007.2303225-1-paul.elder@ideasonboard.com>\n\t<20210913102007.2303225-3-paul.elder@ideasonboard.com>\n\t\n\t<20210915030955.GH1857410@pyrite.rasen.tech>","MIME-Version":"1.0","Content-Type":"text/plain; charset=utf-8","Content-Disposition":"inline","Content-Transfer-Encoding":"8bit","In-Reply-To":"<20210915030955.GH1857410@pyrite.rasen.tech>","Subject":"Re: [libcamera-devel] [PATCH 3/3] android: Plumb noise reduction","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","Errors-To":"libcamera-devel-bounces@lists.libcamera.org","Sender":"\"libcamera-devel\" "}}]