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

[Hirokazu Honda]

Hi Jacopo, thank you for reviewing.

On Thu, May 27, 2021 at 6:24 AM Jacopo Mondi <jacopo at jmondi.org> wrote:

> 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...
>
>

Yes, it is. It is C++ grammer introduced since C++17.
https://skebanga.github.io/if-with-initializer/
I think it is useful if a variable is used only in if-condition and its
clause.


> > +             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
> >
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <https://lists.libcamera.org/pipermail/libcamera-devel/attachments/20210527/670206dd/attachment.htm>