<div dir="ltr"><div dir="ltr">Hi Jacopo, thank you for reviewing.</div><br><div class="gmail_quote"><div dir="ltr" class="gmail_attr">On Tue, May 18, 2021 at 9:41 PM Jacopo Mondi <<a href="mailto:jacopo@jmondi.org">jacopo@jmondi.org</a>> wrote:<br></div><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left:1px solid rgb(204,204,204);padding-left:1ex">Hi Hiro,<br>
<br>
On Thu, May 06, 2021 at 04:54:49PM +0900, Hirokazu Honda wrote:<br>
> Report test pattern modes obtained by Camera::controls().<br>
<br>
Report to the Android camera stack the list of supported test<br>
pattern modes constructed by inspecting the values reported<br>
by libcamera through the controls::draft::TestPatternMode control.<br>
<br>
><br>
> Signed-off-by: Hirokazu Honda <<a href="mailto:hiroh@chromium.org" target="_blank">hiroh@chromium.org</a>><br>
> ---<br>
> src/android/camera_device.cpp | 46 ++++++++++++++++++++++++++++++++---<br>
> 1 file changed, 42 insertions(+), 4 deletions(-)<br>
><br>
> diff --git a/src/android/camera_device.cpp b/src/android/camera_device.cpp<br>
> index 3d5b5f24..a96bc09b 100644<br>
> --- a/src/android/camera_device.cpp<br>
> +++ b/src/android/camera_device.cpp<br>
> @@ -1090,12 +1090,50 @@ const camera_metadata_t *CameraDevice::getStaticMetadata()<br>
><br>
> staticMetadata_->addEntry(ANDROID_SENSOR_ORIENTATION, &orientation_, 1);<br>
><br>
> - std::vector<int32_t> testPatterModes = {<br>
> - ANDROID_SENSOR_TEST_PATTERN_MODE_OFF,<br>
> + const auto &testPatternsInfo =<br>
> + controlsInfo.find(&controls::draft::TestPatternMode);<br>
> + std::vector<int32_t> testPatternModes = {<br>
> + ANDROID_SENSOR_TEST_PATTERN_MODE_OFF<br>
> };<br>
<br>
Should we report OFF if testPatternsInfo == controlsInfo.end() ? Or<br>
should it be reported unconditionally, as if a driver reports any<br>
test pattern mode, it supports disabling it as well... <br></blockquote><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left:1px solid rgb(204,204,204);padding-left:1ex">
<br>
> + if (testPatternsInfo != controlsInfo.end()) {<br>
> + const auto &values = testPatternsInfo->second.values();<br>
> + if (!values.empty()) {<br>
<br>
Do we need this ? If the control is reported, it should be populated,<br>
right ?<br>
<br></blockquote><div><br></div><div>Changed to ASSERT.</div><div> </div><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left:1px solid rgb(204,204,204);padding-left:1ex">
> + testPatternModes.clear();<br>
> + for (const auto &value : values) {<br>
> + int aValue = -1;<br>
> + switch (value.get<int32_t>()) {<br>
> + case controls::draft::TestPatternModeOff:<br>
> + aValue = ANDROID_SENSOR_TEST_PATTERN_MODE_OFF;<br>
> + break;<br>
> + case controls::draft::TestPatternModeSolidColor:<br>
> + aValue = ANDROID_SENSOR_TEST_PATTERN_MODE_SOLID_COLOR;<br>
> + break;<br>
> + case controls::draft::TestPatternModeColorBars:<br>
> + aValue = ANDROID_SENSOR_TEST_PATTERN_MODE_COLOR_BARS;<br>
> + break;<br>
> + case controls::draft::TestPatternModeColorBarsFadeToGray:<br>
> + aValue = ANDROID_SENSOR_TEST_PATTERN_MODE_COLOR_BARS_FADE_TO_GRAY;<br>
> + break;<br>
> + case controls::draft::TestPatternModePn9:<br>
> + aValue = ANDROID_SENSOR_TEST_PATTERN_MODE_PN9;<br>
> + break;<br>
> + case controls::draft::TestPatternModeCustom1:<br>
> + aValue = ANDROID_SENSOR_TEST_PATTERN_MODE_CUSTOM1;<br>
> + break;<br>
> + default:<br>
> + LOG(HAL, Error)<br>
> + << "Unknown test pattern mode: "<br>
> + << value.get<int32_t>();<br>
> + continue;<br>
> + }<br>
> +<br>
> + testPatternModes.push_back(aValue);<br>
> + }<br>
> + }<br>
> + }<br>
<br>
What about:<br>
<br>
std::vector<int32_t> testPatternModes = {<br>
ANDROID_SENSOR_TEST_PATTERN_MODE_OFF,<br>
};<br>
const auto &testPatternsInfo = controlsInfo.find(&controls::draft::TestPatternMode);<br>
if (testPatternInfo == controlsInfo.end()) {<br>
const auto &values = testPatternsInfo->second.values();<br>
for (const auto &value : values) {<br>
switch (value.get<int32_t>()) {<br>
case controls::draft::TestPatternModeSolidColor:<br>
testPatternModes.push_back(<br>
ANDROID_SENSOR_TEST_PATTERN_MODE_SOLID_COLOR);<br>
break;<br>
case controls::draft::TestPatternModeColorBars:<br>
testPatternModes.push_back(<br>
ANDROID_SENSOR_TEST_PATTERN_MODE_COLOR_BARS);<br>
break;<br>
case controls::draft::TestPatternModeColorBarsFadeToGray:<br>
testPatternModes.push_back(<br>
ANDROID_SENSOR_TEST_PATTERN_MODE_COLOR_BARS_FADE_TO_GRAY);<br>
break;<br>
case controls::draft::TestPatternModePn9:<br>
testPatternModes.push_back(<br>
ANDROID_SENSOR_TEST_PATTERN_MODE_PN9);<br>
break;<br>
case controls::draft::TestPatternModeCustom1:<br>
testPatternModes.push_back(<br>
ANDROID_SENSOR_TEST_PATTERN_MODE_CUSTOM1);<br>
break;<br>
default:<br>
LOG(HAL, Error) << "Unknown test pattern mode: "<br>
<< value.get<int32_t>();<br>
continue;<br>
}<br>
}<br>
}<br>
<br>
<br>
> staticMetadata_->addEntry(ANDROID_SENSOR_AVAILABLE_TEST_PATTERN_MODES,<br>
> - testPatterModes.data(),<br>
> - testPatterModes.size());<br>
> + testPatternModes.data(),<br>
> + testPatternModes.size());<br>
<br>
You should enlarge the byteSize in calculateStaticMetadataSize() to<br>
reserve space for all the available test pattern modes. Although<br>
Paul's "android: camera_metadata: Auto-resize CameraMetadata" went in<br>
already, so probably this should just be rebased and that's it. </blockquote><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left:1px solid rgb(204,204,204);padding-left:1ex">
Thanks<br>
j<br>
<br>
><br>
> uint8_t timestampSource = ANDROID_SENSOR_INFO_TIMESTAMP_SOURCE_UNKNOWN;<br>
> staticMetadata_->addEntry(ANDROID_SENSOR_INFO_TIMESTAMP_SOURCE,<br>
> --<br>
> 2.31.1.607.g51e8a6a459-goog<br>
><br>
> _______________________________________________<br>
> libcamera-devel mailing list<br>
> <a href="mailto:libcamera-devel@lists.libcamera.org" target="_blank">libcamera-devel@lists.libcamera.org</a><br>
> <a href="https://lists.libcamera.org/listinfo/libcamera-devel" rel="noreferrer" target="_blank">https://lists.libcamera.org/listinfo/libcamera-devel</a><br>
</blockquote></div></div>