[libcamera-devel] [PATCH v4 6/6] android: CameraDevice: Report queried test pattern modes
Hirokazu Honda
hiroh at chromium.org
Wed May 19 09:36:10 CEST 2021
Hi Jacopo, thank you for reviewing.
On Tue, May 18, 2021 at 9:41 PM Jacopo Mondi <jacopo at 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 at 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 at lists.libcamera.org
> > https://lists.libcamera.org/listinfo/libcamera-devel
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <https://lists.libcamera.org/pipermail/libcamera-devel/attachments/20210519/1a61c36a/attachment.htm>
More information about the libcamera-devel
mailing list