[libcamera-devel] [PATCH v5 6/6] android: CameraDevice: Report queried test pattern modes

[Jacopo Mondi]

Hi Hiro,

On Wed, May 19, 2021 at 04:59:41PM +0900, Hirokazu Honda wrote:
> Report to the Android camera stack the list of supported test
> pattern modes constructed by inspecting the values reported
> by libcamera through the controls::draft::TestPatternMode control.
>
> Signed-off-by: Hirokazu Honda <hiroh at chromium.org>
> ---
>  src/android/camera_device.cpp | 47 ++++++++++++++++++++++++++++++++---
>  1 file changed, 44 insertions(+), 3 deletions(-)
>
> diff --git a/src/android/camera_device.cpp b/src/android/camera_device.cpp
> index b32e8be5..a07679a3 100644
> --- a/src/android/camera_device.cpp
> +++ b/src/android/camera_device.cpp
> @@ -1035,11 +1035,52 @@ const camera_metadata_t *CameraDevice::getStaticMetadata()
>
>  	staticMetadata_->addEntry(ANDROID_SENSOR_ORIENTATION, orientation_);
>
> -	std::vector<int32_t> testPatterModes = {
> -		ANDROID_SENSOR_TEST_PATTERN_MODE_OFF,
> +	std::vector<int32_t> testPatternModes = {
> +		ANDROID_SENSOR_TEST_PATTERN_MODE_OFF
>  	};
> +	if (const auto &testPatternsInfo =
> +		    controlsInfo.find(&controls::draft::TestPatternMode);
> +	    testPatternsInfo != controlsInfo.end()) {

Is this intentional ? It compiles as it is legal, I'm surprised
testPatternsInfo is visibile in the if() { } block scope.

Honestly, it's kind of unusual, but maybe it's just me. If it compiles
and it works...

> +		const auto &values = testPatternsInfo->second.values();
> +		ASSERT(!values.empty());
> +		for (const auto &value : values) {
> +			switch (value.get<int32_t>()) {
> +			case controls::draft::TestPatternModeOff:
> +				/*
> +				 * ANDROID_SENSOR_TEST_PATTERN_MODE_OFF is
> +				 * already in testPatternModes.
> +				 */
> +				break;
> +			case controls::draft::TestPatternModeSolidColor:
> +				testPatternModes.push_back(
> +					ANDROID_SENSOR_TEST_PATTERN_MODE_SOLID_COLOR);
> +				break;
> +			case controls::draft::TestPatternModeColorBars:
> +				testPatternModes.push_back(
> +					ANDROID_SENSOR_TEST_PATTERN_MODE_COLOR_BARS);
> +				break;
> +			case controls::draft::TestPatternModeColorBarsFadeToGray:
> +				testPatternModes.push_back(
> +					ANDROID_SENSOR_TEST_PATTERN_MODE_COLOR_BARS_FADE_TO_GRAY);
> +				break;
> +			case controls::draft::TestPatternModePn9:
> +				testPatternModes.push_back(
> +					ANDROID_SENSOR_TEST_PATTERN_MODE_PN9);
> +				break;
> +			case controls::draft::TestPatternModeCustom1:
> +				testPatternModes.push_back(
> +					ANDROID_SENSOR_TEST_PATTERN_MODE_CUSTOM1);
> +				break;
> +			default:
> +				LOG(HAL, Error)
> +					<< "Unknown test pattern mode: "

Doesn't it fit on the previous line ?

The patch looks good
Reviewed-by: Jacopo Mondi <jacopo at jmondi.org>

Thanks
  j


> +					<< value.get<int32_t>();
> +				continue;
> +			}
> +		}
> +	}
>  	staticMetadata_->addEntry(ANDROID_SENSOR_AVAILABLE_TEST_PATTERN_MODES,
> -				  testPatterModes);
> +				  testPatternModes);
>
>  	uint8_t timestampSource = ANDROID_SENSOR_INFO_TIMESTAMP_SOURCE_UNKNOWN;
>  	staticMetadata_->addEntry(ANDROID_SENSOR_INFO_TIMESTAMP_SOURCE,
> --
> 2.31.1.751.gd2f1c929bd-goog
>