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

[Hirokazu Honda]

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>