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