| Message ID | 20210618103351.1642060-5-paul.elder@ideasonboard.com |
|---|---|
| State | Superseded |
| Delegated to: | Paul Elder |
| Headers | show |
| Series |
|
| Related | show |
Hi Paul, Thank you for your patch. On Fri, 18 Jun 2021 at 11:34, Paul Elder <paul.elder@ideasonboard.com> wrote: > Previously it was possible to have AwbEnable set to false, yet have > AwbMode on anything. This caused a confusion situation, so merge the two > into AwbMode. While at it, pull in the android AWB modes. > > Adjust the previous users of AwbEnable accordingly. > > Signed-off-by: Paul Elder <paul.elder@ideasonboard.com> > --- > include/libcamera/ipa/raspberrypi.h | 1 - > src/ipa/raspberrypi/raspberrypi.cpp | 27 ++++++++---------------- > src/libcamera/control_ids.yaml | 32 +++++++++++++++-------------- > test/controls/control_list.cpp | 6 +++--- > 4 files changed, 29 insertions(+), 37 deletions(-) > > diff --git a/include/libcamera/ipa/raspberrypi.h > b/include/libcamera/ipa/raspberrypi.h > index a8790000..63392a26 100644 > --- a/include/libcamera/ipa/raspberrypi.h > +++ b/include/libcamera/ipa/raspberrypi.h > @@ -35,7 +35,6 @@ static const ControlInfoMap Controls = { > { &controls::AeConstraintMode, > ControlInfo(controls::AeConstraintModeValues) }, > { &controls::AeExposureMode, > ControlInfo(controls::AeExposureModeValues) }, > { &controls::ExposureValue, ControlInfo(0.0f, 16.0f) }, > - { &controls::AwbEnable, ControlInfo(false, true) }, > { &controls::ColourGains, ControlInfo(0.0f, 32.0f) }, > { &controls::AwbMode, ControlInfo(controls::AwbModeValues) }, > { &controls::Brightness, ControlInfo(-1.0f, 1.0f) }, > diff --git a/src/ipa/raspberrypi/raspberrypi.cpp > b/src/ipa/raspberrypi/raspberrypi.cpp > index ad0132c0..ed5f1250 100644 > --- a/src/ipa/raspberrypi/raspberrypi.cpp > +++ b/src/ipa/raspberrypi/raspberrypi.cpp > @@ -745,24 +745,6 @@ void IPARPi::queueRequest(const ControlList &controls) > break; > } > > - case controls::AWB_ENABLE: { > - RPiController::Algorithm *awb = > controller_.GetAlgorithm("awb"); > - if (!awb) { > - LOG(IPARPI, Warning) > - << "Could not set AWB_ENABLE - no > AWB algorithm"; > - break; > - } > - > - if (ctrl.second.get<bool>() == false) > - awb->Pause(); > - else > - awb->Resume(); > - > - libcameraMetadata_.set(controls::AwbEnable, > - ctrl.second.get<bool>()); > - break; > - } > - > case controls::AWB_MODE: { > RPiController::AwbAlgorithm *awb = > dynamic_cast<RPiController::AwbAlgorithm *>( > controller_.GetAlgorithm("awb")); > @@ -773,6 +755,15 @@ void IPARPi::queueRequest(const ControlList &controls) > } > > int32_t idx = ctrl.second.get<int32_t>(); > + > + if (idx == controls::AwbOff) { > + awb->Pause(); > + break; > + } > + > + if (idx == controls::AwbAuto) > + awb->Resume(); > I think the logic here may need adjusting such that any state that is not controls::AwbOff will need to call awb->Resume(), or conditionally call resume if adb->IsPaused() returns true. Thanks, Naush > if (AwbModeTable.count(idx)) { > awb->SetMode(AwbModeTable.at(idx)); > libcameraMetadata_.set(controls::AwbMode, > idx); > diff --git a/src/libcamera/control_ids.yaml > b/src/libcamera/control_ids.yaml > index 5717bc1f..2e62f61b 100644 > --- a/src/libcamera/control_ids.yaml > +++ b/src/libcamera/control_ids.yaml > @@ -229,13 +229,6 @@ controls: > Report an estimate of the current illuminance level in lux. The > Lux > control can only be returned in metadata. > > - - AwbEnable: > - type: bool > - description: | > - Enable or disable the AWB. > - > - \sa ColourGains > - > # AwbMode needs further attention: > # - Auto-generate max enum value. > # - Better handling of custom types. > @@ -245,29 +238,38 @@ controls: > Specify the range of illuminants to use for the AWB algorithm. > The modes > supported are platform specific, and not all modes may be > supported. > enum: > - - name: AwbAuto > + - name: AwbOff > value: 0 > + description: The AWB routune is disabled. > + - name: AwbAuto > + value: 1 > description: Search over the whole colour temperature range. > - name: AwbIncandescent > - value: 1 > - description: Incandescent AWB lamp mode. > - - name: AwbTungsten > value: 2 > - description: Tungsten AWB lamp mode. > + description: Incandescent AWB lamp mode. > - name: AwbFluorescent > value: 3 > description: Fluorescent AWB lamp mode. > - - name: AwbIndoor > + - name: AwbWarmFluorescent > value: 4 > - description: Indoor AWB lighting mode. > + description: Warm fluorescent AWB lamp mode. > - name: AwbDaylight > value: 5 > description: Daylight AWB lighting mode. > - name: AwbCloudy > value: 6 > description: Cloudy AWB lighting mode. > - - name: AwbCustom > + - name: AwbTwilight > value: 7 > + description: Twilight AWB lamp mode. > + - name: AwbTungsten > + value: 8 > + description: Tungsten AWB lamp mode. > + - name: AwbIndoor > + value: 9 > + description: Indoor AWB lighting mode. > + - name: AwbCustom > + value: 10 > description: Custom AWB mode. > > - AwbLocked: > diff --git a/test/controls/control_list.cpp > b/test/controls/control_list.cpp > index 70cf61b8..ce55d09b 100644 > --- a/test/controls/control_list.cpp > +++ b/test/controls/control_list.cpp > @@ -143,10 +143,10 @@ protected: > * Attempt to set an invalid control and verify that the > * operation failed. > */ > - list.set(controls::AwbEnable, true); > + list.set(controls::AwbMode, true); > > - if (list.contains(controls::AwbEnable)) { > - cout << "List shouldn't contain AwbEnable control" > << endl; > + if (list.contains(controls::AwbMode)) { > + cout << "List shouldn't contain AwbMode control" > << endl; > return TestFail; > } > > -- > 2.27.0 > >
Hi Paul, Thank you for your patch. On Fri, Jun 18, 2021 at 01:23:03PM +0100, Naushir Patuck wrote: > On Fri, 18 Jun 2021 at 11:34, Paul Elder wrote: > > Previously it was possible to have AwbEnable set to false, yet have > > AwbMode on anything. This caused a confusion situation, so merge the two > > into AwbMode. While at it, pull in the android AWB modes. I'd say "pull in additional AWB modes defined by Android" (or s/pull in/add). > > > > Adjust the previous users of AwbEnable accordingly. > > > > Signed-off-by: Paul Elder <paul.elder@ideasonboard.com> > > --- > > include/libcamera/ipa/raspberrypi.h | 1 - > > src/ipa/raspberrypi/raspberrypi.cpp | 27 ++++++++---------------- > > src/libcamera/control_ids.yaml | 32 +++++++++++++++-------------- > > test/controls/control_list.cpp | 6 +++--- > > 4 files changed, 29 insertions(+), 37 deletions(-) > > > > diff --git a/include/libcamera/ipa/raspberrypi.h b/include/libcamera/ipa/raspberrypi.h > > index a8790000..63392a26 100644 > > --- a/include/libcamera/ipa/raspberrypi.h > > +++ b/include/libcamera/ipa/raspberrypi.h > > @@ -35,7 +35,6 @@ static const ControlInfoMap Controls = { > > { &controls::AeConstraintMode, ControlInfo(controls::AeConstraintModeValues) }, > > { &controls::AeExposureMode, ControlInfo(controls::AeExposureModeValues) }, > > { &controls::ExposureValue, ControlInfo(0.0f, 16.0f) }, > > - { &controls::AwbEnable, ControlInfo(false, true) }, > > { &controls::ColourGains, ControlInfo(0.0f, 32.0f) }, > > { &controls::AwbMode, ControlInfo(controls::AwbModeValues) }, > > { &controls::Brightness, ControlInfo(-1.0f, 1.0f) }, > > diff --git a/src/ipa/raspberrypi/raspberrypi.cpp b/src/ipa/raspberrypi/raspberrypi.cpp > > index ad0132c0..ed5f1250 100644 > > --- a/src/ipa/raspberrypi/raspberrypi.cpp > > +++ b/src/ipa/raspberrypi/raspberrypi.cpp > > @@ -745,24 +745,6 @@ void IPARPi::queueRequest(const ControlList &controls) > > break; > > } > > > > - case controls::AWB_ENABLE: { > > - RPiController::Algorithm *awb = controller_.GetAlgorithm("awb"); > > - if (!awb) { > > - LOG(IPARPI, Warning) > > - << "Could not set AWB_ENABLE - no AWB algorithm"; > > - break; > > - } > > - > > - if (ctrl.second.get<bool>() == false) > > - awb->Pause(); > > - else > > - awb->Resume(); > > - > > - libcameraMetadata_.set(controls::AwbEnable, > > - ctrl.second.get<bool>()); > > - break; > > - } > > - > > case controls::AWB_MODE: { > > RPiController::AwbAlgorithm *awb = dynamic_cast<RPiController::AwbAlgorithm *>( > > controller_.GetAlgorithm("awb")); > > @@ -773,6 +755,15 @@ void IPARPi::queueRequest(const ControlList &controls) > > } > > > > int32_t idx = ctrl.second.get<int32_t>(); > > + > > + if (idx == controls::AwbOff) { > > + awb->Pause(); > > + break; > > + } > > + > > + if (idx == controls::AwbAuto) > > + awb->Resume(); > > I think the logic here may need adjusting such that any state that is not > controls::AwbOff will need to call awb->Resume(), or conditionally call > resume if adb->IsPaused() returns true. Agreed. The RPi AWB implementation differs from the behaviour specified by Android in that the RPi AWB algorithm isn't disabled when the mode is set to one of the presets. The presets instead serve to limit the search range of the AWB algorithm, instead of setting hardcoded manual values as in Android. Naush, what would happen if the tuning file specified a fixed colour temperature (by setting the min and max to the same value) for AWB presets ? Would the AWB algorithm then always produce fixed values for the colour gains ? > > if (AwbModeTable.count(idx)) { > > awb->SetMode(AwbModeTable.at(idx)); > > libcameraMetadata_.set(controls::AwbMode, idx); > > diff --git a/src/libcamera/control_ids.yaml b/src/libcamera/control_ids.yaml > > index 5717bc1f..2e62f61b 100644 > > --- a/src/libcamera/control_ids.yaml > > +++ b/src/libcamera/control_ids.yaml > > @@ -229,13 +229,6 @@ controls: > > Report an estimate of the current illuminance level in lux. The Lux > > control can only be returned in metadata. > > > > - - AwbEnable: > > - type: bool > > - description: | > > - Enable or disable the AWB. > > - > > - \sa ColourGains > > - > > # AwbMode needs further attention: > > # - Auto-generate max enum value. > > # - Better handling of custom types. > > @@ -245,29 +238,38 @@ controls: > > Specify the range of illuminants to use for the AWB algorithm. The modes > > supported are platform specific, and not all modes may be supported. > > enum: > > - - name: AwbAuto > > + - name: AwbOff > > value: 0 > > + description: The AWB routune is disabled. s/routine/ > > + - name: AwbAuto > > + value: 1 > > description: Search over the whole colour temperature range. > > - name: AwbIncandescent > > - value: 1 > > - description: Incandescent AWB lamp mode. > > - - name: AwbTungsten > > value: 2 > > - description: Tungsten AWB lamp mode. > > + description: Incandescent AWB lamp mode. > > - name: AwbFluorescent > > value: 3 > > description: Fluorescent AWB lamp mode. > > - - name: AwbIndoor > > + - name: AwbWarmFluorescent > > value: 4 > > - description: Indoor AWB lighting mode. > > + description: Warm fluorescent AWB lamp mode. > > - name: AwbDaylight > > value: 5 > > description: Daylight AWB lighting mode. > > - name: AwbCloudy > > value: 6 > > description: Cloudy AWB lighting mode. > > - - name: AwbCustom > > + - name: AwbTwilight > > value: 7 > > + description: Twilight AWB lamp mode. > > + - name: AwbTungsten > > + value: 8 > > + description: Tungsten AWB lamp mode. > > + - name: AwbIndoor > > + value: 9 > > + description: Indoor AWB lighting mode. > > + - name: AwbCustom > > + value: 10 > > description: Custom AWB mode. > > > > - AwbLocked: > > diff --git a/test/controls/control_list.cpp b/test/controls/control_list.cpp > > index 70cf61b8..ce55d09b 100644 > > --- a/test/controls/control_list.cpp > > +++ b/test/controls/control_list.cpp > > @@ -143,10 +143,10 @@ protected: > > * Attempt to set an invalid control and verify that the > > * operation failed. > > */ > > - list.set(controls::AwbEnable, true); > > + list.set(controls::AwbMode, true); > > > > - if (list.contains(controls::AwbEnable)) { > > - cout << "List shouldn't contain AwbEnable control" << endl; > > + if (list.contains(controls::AwbMode)) { > > + cout << "List shouldn't contain AwbMode control" << endl; > > return TestFail; > > } > >
Hi Paul, A related question, will there be a similar patch for AE ? On Mon, Jun 28, 2021 at 04:06:30AM +0300, Laurent Pinchart wrote: > On Fri, Jun 18, 2021 at 01:23:03PM +0100, Naushir Patuck wrote: > > On Fri, 18 Jun 2021 at 11:34, Paul Elder wrote: > > > Previously it was possible to have AwbEnable set to false, yet have > > > AwbMode on anything. This caused a confusion situation, so merge the two > > > into AwbMode. While at it, pull in the android AWB modes. > > I'd say "pull in additional AWB modes defined by Android" (or s/pull > in/add). > > > > > > > Adjust the previous users of AwbEnable accordingly. > > > > > > Signed-off-by: Paul Elder <paul.elder@ideasonboard.com> > > > --- > > > include/libcamera/ipa/raspberrypi.h | 1 - > > > src/ipa/raspberrypi/raspberrypi.cpp | 27 ++++++++---------------- > > > src/libcamera/control_ids.yaml | 32 +++++++++++++++-------------- > > > test/controls/control_list.cpp | 6 +++--- > > > 4 files changed, 29 insertions(+), 37 deletions(-) > > > > > > diff --git a/include/libcamera/ipa/raspberrypi.h b/include/libcamera/ipa/raspberrypi.h > > > index a8790000..63392a26 100644 > > > --- a/include/libcamera/ipa/raspberrypi.h > > > +++ b/include/libcamera/ipa/raspberrypi.h > > > @@ -35,7 +35,6 @@ static const ControlInfoMap Controls = { > > > { &controls::AeConstraintMode, ControlInfo(controls::AeConstraintModeValues) }, > > > { &controls::AeExposureMode, ControlInfo(controls::AeExposureModeValues) }, > > > { &controls::ExposureValue, ControlInfo(0.0f, 16.0f) }, > > > - { &controls::AwbEnable, ControlInfo(false, true) }, > > > { &controls::ColourGains, ControlInfo(0.0f, 32.0f) }, > > > { &controls::AwbMode, ControlInfo(controls::AwbModeValues) }, > > > { &controls::Brightness, ControlInfo(-1.0f, 1.0f) }, > > > diff --git a/src/ipa/raspberrypi/raspberrypi.cpp b/src/ipa/raspberrypi/raspberrypi.cpp > > > index ad0132c0..ed5f1250 100644 > > > --- a/src/ipa/raspberrypi/raspberrypi.cpp > > > +++ b/src/ipa/raspberrypi/raspberrypi.cpp > > > @@ -745,24 +745,6 @@ void IPARPi::queueRequest(const ControlList &controls) > > > break; > > > } > > > > > > - case controls::AWB_ENABLE: { > > > - RPiController::Algorithm *awb = controller_.GetAlgorithm("awb"); > > > - if (!awb) { > > > - LOG(IPARPI, Warning) > > > - << "Could not set AWB_ENABLE - no AWB algorithm"; > > > - break; > > > - } > > > - > > > - if (ctrl.second.get<bool>() == false) > > > - awb->Pause(); > > > - else > > > - awb->Resume(); > > > - > > > - libcameraMetadata_.set(controls::AwbEnable, > > > - ctrl.second.get<bool>()); > > > - break; > > > - } > > > - > > > case controls::AWB_MODE: { > > > RPiController::AwbAlgorithm *awb = dynamic_cast<RPiController::AwbAlgorithm *>( > > > controller_.GetAlgorithm("awb")); > > > @@ -773,6 +755,15 @@ void IPARPi::queueRequest(const ControlList &controls) > > > } > > > > > > int32_t idx = ctrl.second.get<int32_t>(); > > > + > > > + if (idx == controls::AwbOff) { > > > + awb->Pause(); > > > + break; > > > + } > > > + > > > + if (idx == controls::AwbAuto) > > > + awb->Resume(); > > > > I think the logic here may need adjusting such that any state that is not > > controls::AwbOff will need to call awb->Resume(), or conditionally call > > resume if adb->IsPaused() returns true. > > Agreed. The RPi AWB implementation differs from the behaviour specified > by Android in that the RPi AWB algorithm isn't disabled when the mode is > set to one of the presets. The presets instead serve to limit the search > range of the AWB algorithm, instead of setting hardcoded manual values > as in Android. > > Naush, what would happen if the tuning file specified a fixed colour > temperature (by setting the min and max to the same value) for AWB > presets ? Would the AWB algorithm then always produce fixed values for > the colour gains ? > > > > if (AwbModeTable.count(idx)) { > > > awb->SetMode(AwbModeTable.at(idx)); > > > libcameraMetadata_.set(controls::AwbMode, idx); > > > diff --git a/src/libcamera/control_ids.yaml b/src/libcamera/control_ids.yaml > > > index 5717bc1f..2e62f61b 100644 > > > --- a/src/libcamera/control_ids.yaml > > > +++ b/src/libcamera/control_ids.yaml > > > @@ -229,13 +229,6 @@ controls: > > > Report an estimate of the current illuminance level in lux. The Lux > > > control can only be returned in metadata. > > > > > > - - AwbEnable: > > > - type: bool > > > - description: | > > > - Enable or disable the AWB. > > > - > > > - \sa ColourGains > > > - > > > # AwbMode needs further attention: > > > # - Auto-generate max enum value. > > > # - Better handling of custom types. > > > @@ -245,29 +238,38 @@ controls: > > > Specify the range of illuminants to use for the AWB algorithm. The modes > > > supported are platform specific, and not all modes may be supported. > > > enum: > > > - - name: AwbAuto > > > + - name: AwbOff > > > value: 0 > > > + description: The AWB routune is disabled. > > s/routine/ > > > > + - name: AwbAuto > > > + value: 1 > > > description: Search over the whole colour temperature range. > > > - name: AwbIncandescent > > > - value: 1 > > > - description: Incandescent AWB lamp mode. > > > - - name: AwbTungsten > > > value: 2 > > > - description: Tungsten AWB lamp mode. > > > + description: Incandescent AWB lamp mode. > > > - name: AwbFluorescent > > > value: 3 > > > description: Fluorescent AWB lamp mode. > > > - - name: AwbIndoor > > > + - name: AwbWarmFluorescent > > > value: 4 > > > - description: Indoor AWB lighting mode. > > > + description: Warm fluorescent AWB lamp mode. > > > - name: AwbDaylight > > > value: 5 > > > description: Daylight AWB lighting mode. > > > - name: AwbCloudy > > > value: 6 > > > description: Cloudy AWB lighting mode. > > > - - name: AwbCustom > > > + - name: AwbTwilight > > > value: 7 > > > + description: Twilight AWB lamp mode. > > > + - name: AwbTungsten > > > + value: 8 > > > + description: Tungsten AWB lamp mode. > > > + - name: AwbIndoor > > > + value: 9 > > > + description: Indoor AWB lighting mode. > > > + - name: AwbCustom > > > + value: 10 > > > description: Custom AWB mode. > > > > > > - AwbLocked: > > > diff --git a/test/controls/control_list.cpp b/test/controls/control_list.cpp > > > index 70cf61b8..ce55d09b 100644 > > > --- a/test/controls/control_list.cpp > > > +++ b/test/controls/control_list.cpp > > > @@ -143,10 +143,10 @@ protected: > > > * Attempt to set an invalid control and verify that the > > > * operation failed. > > > */ > > > - list.set(controls::AwbEnable, true); > > > + list.set(controls::AwbMode, true); > > > > > > - if (list.contains(controls::AwbEnable)) { > > > - cout << "List shouldn't contain AwbEnable control" << endl; > > > + if (list.contains(controls::AwbMode)) { > > > + cout << "List shouldn't contain AwbMode control" << endl; > > > return TestFail; > > > } > > >
Hi Laurent, On Mon, 28 Jun 2021 at 02:06, Laurent Pinchart < laurent.pinchart@ideasonboard.com> wrote: > Hi Paul, > > Thank you for your patch. > > On Fri, Jun 18, 2021 at 01:23:03PM +0100, Naushir Patuck wrote: > > On Fri, 18 Jun 2021 at 11:34, Paul Elder wrote: > > > Previously it was possible to have AwbEnable set to false, yet have > > > AwbMode on anything. This caused a confusion situation, so merge the > two > > > into AwbMode. While at it, pull in the android AWB modes. > > I'd say "pull in additional AWB modes defined by Android" (or s/pull > in/add). > > > > > > > Adjust the previous users of AwbEnable accordingly. > > > > > > Signed-off-by: Paul Elder <paul.elder@ideasonboard.com> > > > --- > > > include/libcamera/ipa/raspberrypi.h | 1 - > > > src/ipa/raspberrypi/raspberrypi.cpp | 27 ++++++++---------------- > > > src/libcamera/control_ids.yaml | 32 +++++++++++++++-------------- > > > test/controls/control_list.cpp | 6 +++--- > > > 4 files changed, 29 insertions(+), 37 deletions(-) > > > > > > diff --git a/include/libcamera/ipa/raspberrypi.h > b/include/libcamera/ipa/raspberrypi.h > > > index a8790000..63392a26 100644 > > > --- a/include/libcamera/ipa/raspberrypi.h > > > +++ b/include/libcamera/ipa/raspberrypi.h > > > @@ -35,7 +35,6 @@ static const ControlInfoMap Controls = { > > > { &controls::AeConstraintMode, > ControlInfo(controls::AeConstraintModeValues) }, > > > { &controls::AeExposureMode, > ControlInfo(controls::AeExposureModeValues) }, > > > { &controls::ExposureValue, ControlInfo(0.0f, 16.0f) }, > > > - { &controls::AwbEnable, ControlInfo(false, true) }, > > > { &controls::ColourGains, ControlInfo(0.0f, 32.0f) }, > > > { &controls::AwbMode, ControlInfo(controls::AwbModeValues) }, > > > { &controls::Brightness, ControlInfo(-1.0f, 1.0f) }, > > > diff --git a/src/ipa/raspberrypi/raspberrypi.cpp > b/src/ipa/raspberrypi/raspberrypi.cpp > > > index ad0132c0..ed5f1250 100644 > > > --- a/src/ipa/raspberrypi/raspberrypi.cpp > > > +++ b/src/ipa/raspberrypi/raspberrypi.cpp > > > @@ -745,24 +745,6 @@ void IPARPi::queueRequest(const ControlList > &controls) > > > break; > > > } > > > > > > - case controls::AWB_ENABLE: { > > > - RPiController::Algorithm *awb = > controller_.GetAlgorithm("awb"); > > > - if (!awb) { > > > - LOG(IPARPI, Warning) > > > - << "Could not set AWB_ENABLE - > no AWB algorithm"; > > > - break; > > > - } > > > - > > > - if (ctrl.second.get<bool>() == false) > > > - awb->Pause(); > > > - else > > > - awb->Resume(); > > > - > > > - libcameraMetadata_.set(controls::AwbEnable, > > > - > ctrl.second.get<bool>()); > > > - break; > > > - } > > > - > > > case controls::AWB_MODE: { > > > RPiController::AwbAlgorithm *awb = > dynamic_cast<RPiController::AwbAlgorithm *>( > > > controller_.GetAlgorithm("awb")); > > > @@ -773,6 +755,15 @@ void IPARPi::queueRequest(const ControlList > &controls) > > > } > > > > > > int32_t idx = ctrl.second.get<int32_t>(); > > > + > > > + if (idx == controls::AwbOff) { > > > + awb->Pause(); > > > + break; > > > + } > > > + > > > + if (idx == controls::AwbAuto) > > > + awb->Resume(); > > > > I think the logic here may need adjusting such that any state that is not > > controls::AwbOff will need to call awb->Resume(), or conditionally call > > resume if adb->IsPaused() returns true. > > Agreed. The RPi AWB implementation differs from the behaviour specified > by Android in that the RPi AWB algorithm isn't disabled when the mode is > set to one of the presets. The presets instead serve to limit the search > range of the AWB algorithm, instead of setting hardcoded manual values > as in Android. > > Naush, what would happen if the tuning file specified a fixed colour > temperature (by setting the min and max to the same value) for AWB > presets ? Would the AWB algorithm then always produce fixed values for > the colour gains ? > In this case the RPi AWB will (or at least should, I've never tried :)) will still search a small patch around the region. So you may not necessarily get the exact same gain values for every frame, but they will be close. Regards, Naush > > > > if (AwbModeTable.count(idx)) { > > > awb->SetMode(AwbModeTable.at(idx)); > > > > libcameraMetadata_.set(controls::AwbMode, idx); > > > diff --git a/src/libcamera/control_ids.yaml > b/src/libcamera/control_ids.yaml > > > index 5717bc1f..2e62f61b 100644 > > > --- a/src/libcamera/control_ids.yaml > > > +++ b/src/libcamera/control_ids.yaml > > > @@ -229,13 +229,6 @@ controls: > > > Report an estimate of the current illuminance level in lux. > The Lux > > > control can only be returned in metadata. > > > > > > - - AwbEnable: > > > - type: bool > > > - description: | > > > - Enable or disable the AWB. > > > - > > > - \sa ColourGains > > > - > > > # AwbMode needs further attention: > > > # - Auto-generate max enum value. > > > # - Better handling of custom types. > > > @@ -245,29 +238,38 @@ controls: > > > Specify the range of illuminants to use for the AWB > algorithm. The modes > > > supported are platform specific, and not all modes may be > supported. > > > enum: > > > - - name: AwbAuto > > > + - name: AwbOff > > > value: 0 > > > + description: The AWB routune is disabled. > > s/routine/ > > > > + - name: AwbAuto > > > + value: 1 > > > description: Search over the whole colour temperature range. > > > - name: AwbIncandescent > > > - value: 1 > > > - description: Incandescent AWB lamp mode. > > > - - name: AwbTungsten > > > value: 2 > > > - description: Tungsten AWB lamp mode. > > > + description: Incandescent AWB lamp mode. > > > - name: AwbFluorescent > > > value: 3 > > > description: Fluorescent AWB lamp mode. > > > - - name: AwbIndoor > > > + - name: AwbWarmFluorescent > > > value: 4 > > > - description: Indoor AWB lighting mode. > > > + description: Warm fluorescent AWB lamp mode. > > > - name: AwbDaylight > > > value: 5 > > > description: Daylight AWB lighting mode. > > > - name: AwbCloudy > > > value: 6 > > > description: Cloudy AWB lighting mode. > > > - - name: AwbCustom > > > + - name: AwbTwilight > > > value: 7 > > > + description: Twilight AWB lamp mode. > > > + - name: AwbTungsten > > > + value: 8 > > > + description: Tungsten AWB lamp mode. > > > + - name: AwbIndoor > > > + value: 9 > > > + description: Indoor AWB lighting mode. > > > + - name: AwbCustom > > > + value: 10 > > > description: Custom AWB mode. > > > > > > - AwbLocked: > > > diff --git a/test/controls/control_list.cpp > b/test/controls/control_list.cpp > > > index 70cf61b8..ce55d09b 100644 > > > --- a/test/controls/control_list.cpp > > > +++ b/test/controls/control_list.cpp > > > @@ -143,10 +143,10 @@ protected: > > > * Attempt to set an invalid control and verify that > the > > > * operation failed. > > > */ > > > - list.set(controls::AwbEnable, true); > > > + list.set(controls::AwbMode, true); > > > > > > - if (list.contains(controls::AwbEnable)) { > > > - cout << "List shouldn't contain AwbEnable > control" << endl; > > > + if (list.contains(controls::AwbMode)) { > > > + cout << "List shouldn't contain AwbMode > control" << endl; > > > return TestFail; > > > } > > > > > -- > Regards, > > Laurent Pinchart >
Just to add that I think it's wrong to fix the colour gains when a preset AWB mode is chosen. Illuminants can differ in other ways besides colour temperature - for example, some lamps are greener than others, especially true of some modern fluorescent lights. The RPi approach will still take care of this, but algorithms that simply use fixed colour gains can leave you with greenish or purplish images. (Of course, we still let you set explicit values using the control mechanism.) David On Mon, 28 Jun 2021 at 10:13, Naushir Patuck <naush@raspberrypi.com> wrote: > Hi Laurent, > > On Mon, 28 Jun 2021 at 02:06, Laurent Pinchart < > laurent.pinchart@ideasonboard.com> wrote: > >> Hi Paul, >> >> Thank you for your patch. >> >> On Fri, Jun 18, 2021 at 01:23:03PM +0100, Naushir Patuck wrote: >> > On Fri, 18 Jun 2021 at 11:34, Paul Elder wrote: >> > > Previously it was possible to have AwbEnable set to false, yet have >> > > AwbMode on anything. This caused a confusion situation, so merge the >> two >> > > into AwbMode. While at it, pull in the android AWB modes. >> >> I'd say "pull in additional AWB modes defined by Android" (or s/pull >> in/add). >> >> > > >> > > Adjust the previous users of AwbEnable accordingly. >> > > >> > > Signed-off-by: Paul Elder <paul.elder@ideasonboard.com> >> > > --- >> > > include/libcamera/ipa/raspberrypi.h | 1 - >> > > src/ipa/raspberrypi/raspberrypi.cpp | 27 ++++++++---------------- >> > > src/libcamera/control_ids.yaml | 32 >> +++++++++++++++-------------- >> > > test/controls/control_list.cpp | 6 +++--- >> > > 4 files changed, 29 insertions(+), 37 deletions(-) >> > > >> > > diff --git a/include/libcamera/ipa/raspberrypi.h >> b/include/libcamera/ipa/raspberrypi.h >> > > index a8790000..63392a26 100644 >> > > --- a/include/libcamera/ipa/raspberrypi.h >> > > +++ b/include/libcamera/ipa/raspberrypi.h >> > > @@ -35,7 +35,6 @@ static const ControlInfoMap Controls = { >> > > { &controls::AeConstraintMode, >> ControlInfo(controls::AeConstraintModeValues) }, >> > > { &controls::AeExposureMode, >> ControlInfo(controls::AeExposureModeValues) }, >> > > { &controls::ExposureValue, ControlInfo(0.0f, 16.0f) }, >> > > - { &controls::AwbEnable, ControlInfo(false, true) }, >> > > { &controls::ColourGains, ControlInfo(0.0f, 32.0f) }, >> > > { &controls::AwbMode, ControlInfo(controls::AwbModeValues) }, >> > > { &controls::Brightness, ControlInfo(-1.0f, 1.0f) }, >> > > diff --git a/src/ipa/raspberrypi/raspberrypi.cpp >> b/src/ipa/raspberrypi/raspberrypi.cpp >> > > index ad0132c0..ed5f1250 100644 >> > > --- a/src/ipa/raspberrypi/raspberrypi.cpp >> > > +++ b/src/ipa/raspberrypi/raspberrypi.cpp >> > > @@ -745,24 +745,6 @@ void IPARPi::queueRequest(const ControlList >> &controls) >> > > break; >> > > } >> > > >> > > - case controls::AWB_ENABLE: { >> > > - RPiController::Algorithm *awb = >> controller_.GetAlgorithm("awb"); >> > > - if (!awb) { >> > > - LOG(IPARPI, Warning) >> > > - << "Could not set AWB_ENABLE >> - no AWB algorithm"; >> > > - break; >> > > - } >> > > - >> > > - if (ctrl.second.get<bool>() == false) >> > > - awb->Pause(); >> > > - else >> > > - awb->Resume(); >> > > - >> > > - libcameraMetadata_.set(controls::AwbEnable, >> > > - >> ctrl.second.get<bool>()); >> > > - break; >> > > - } >> > > - >> > > case controls::AWB_MODE: { >> > > RPiController::AwbAlgorithm *awb = >> dynamic_cast<RPiController::AwbAlgorithm *>( >> > > controller_.GetAlgorithm("awb")); >> > > @@ -773,6 +755,15 @@ void IPARPi::queueRequest(const ControlList >> &controls) >> > > } >> > > >> > > int32_t idx = ctrl.second.get<int32_t>(); >> > > + >> > > + if (idx == controls::AwbOff) { >> > > + awb->Pause(); >> > > + break; >> > > + } >> > > + >> > > + if (idx == controls::AwbAuto) >> > > + awb->Resume(); >> > >> > I think the logic here may need adjusting such that any state that is >> not >> > controls::AwbOff will need to call awb->Resume(), or conditionally call >> > resume if adb->IsPaused() returns true. >> >> Agreed. The RPi AWB implementation differs from the behaviour specified >> by Android in that the RPi AWB algorithm isn't disabled when the mode is >> set to one of the presets. The presets instead serve to limit the search >> range of the AWB algorithm, instead of setting hardcoded manual values >> as in Android. >> >> Naush, what would happen if the tuning file specified a fixed colour >> temperature (by setting the min and max to the same value) for AWB >> presets ? Would the AWB algorithm then always produce fixed values for >> the colour gains ? >> > > In this case the RPi AWB will (or at least should, I've never tried :)) > will still > search a small patch around the region. So you may not necessarily get the > exact same gain values for every frame, but they will be close. > > Regards, > Naush > > >> >> > > if (AwbModeTable.count(idx)) { >> > > awb->SetMode(AwbModeTable.at(idx)); >> > > >> libcameraMetadata_.set(controls::AwbMode, idx); >> > > diff --git a/src/libcamera/control_ids.yaml >> b/src/libcamera/control_ids.yaml >> > > index 5717bc1f..2e62f61b 100644 >> > > --- a/src/libcamera/control_ids.yaml >> > > +++ b/src/libcamera/control_ids.yaml >> > > @@ -229,13 +229,6 @@ controls: >> > > Report an estimate of the current illuminance level in lux. >> The Lux >> > > control can only be returned in metadata. >> > > >> > > - - AwbEnable: >> > > - type: bool >> > > - description: | >> > > - Enable or disable the AWB. >> > > - >> > > - \sa ColourGains >> > > - >> > > # AwbMode needs further attention: >> > > # - Auto-generate max enum value. >> > > # - Better handling of custom types. >> > > @@ -245,29 +238,38 @@ controls: >> > > Specify the range of illuminants to use for the AWB >> algorithm. The modes >> > > supported are platform specific, and not all modes may be >> supported. >> > > enum: >> > > - - name: AwbAuto >> > > + - name: AwbOff >> > > value: 0 >> > > + description: The AWB routune is disabled. >> >> s/routine/ >> >> > > + - name: AwbAuto >> > > + value: 1 >> > > description: Search over the whole colour temperature >> range. >> > > - name: AwbIncandescent >> > > - value: 1 >> > > - description: Incandescent AWB lamp mode. >> > > - - name: AwbTungsten >> > > value: 2 >> > > - description: Tungsten AWB lamp mode. >> > > + description: Incandescent AWB lamp mode. >> > > - name: AwbFluorescent >> > > value: 3 >> > > description: Fluorescent AWB lamp mode. >> > > - - name: AwbIndoor >> > > + - name: AwbWarmFluorescent >> > > value: 4 >> > > - description: Indoor AWB lighting mode. >> > > + description: Warm fluorescent AWB lamp mode. >> > > - name: AwbDaylight >> > > value: 5 >> > > description: Daylight AWB lighting mode. >> > > - name: AwbCloudy >> > > value: 6 >> > > description: Cloudy AWB lighting mode. >> > > - - name: AwbCustom >> > > + - name: AwbTwilight >> > > value: 7 >> > > + description: Twilight AWB lamp mode. >> > > + - name: AwbTungsten >> > > + value: 8 >> > > + description: Tungsten AWB lamp mode. >> > > + - name: AwbIndoor >> > > + value: 9 >> > > + description: Indoor AWB lighting mode. >> > > + - name: AwbCustom >> > > + value: 10 >> > > description: Custom AWB mode. >> > > >> > > - AwbLocked: >> > > diff --git a/test/controls/control_list.cpp >> b/test/controls/control_list.cpp >> > > index 70cf61b8..ce55d09b 100644 >> > > --- a/test/controls/control_list.cpp >> > > +++ b/test/controls/control_list.cpp >> > > @@ -143,10 +143,10 @@ protected: >> > > * Attempt to set an invalid control and verify that >> the >> > > * operation failed. >> > > */ >> > > - list.set(controls::AwbEnable, true); >> > > + list.set(controls::AwbMode, true); >> > > >> > > - if (list.contains(controls::AwbEnable)) { >> > > - cout << "List shouldn't contain AwbEnable >> control" << endl; >> > > + if (list.contains(controls::AwbMode)) { >> > > + cout << "List shouldn't contain AwbMode >> control" << endl; >> > > return TestFail; >> > > } >> > > >> >> -- >> Regards, >> >> Laurent Pinchart >> >
Hi Laurent, On Mon, Jun 28, 2021 at 04:22:46AM +0300, Laurent Pinchart wrote: > Hi Paul, > > A related question, will there be a similar patch for AE ? The android's AE modes besides off/auto are all just variations of "needs flash", and when we discussed this last iirc the conclusion was that we don't need them, so off/on is sufficient. Paul > > On Mon, Jun 28, 2021 at 04:06:30AM +0300, Laurent Pinchart wrote: > > On Fri, Jun 18, 2021 at 01:23:03PM +0100, Naushir Patuck wrote: > > > On Fri, 18 Jun 2021 at 11:34, Paul Elder wrote: > > > > Previously it was possible to have AwbEnable set to false, yet have > > > > AwbMode on anything. This caused a confusion situation, so merge the two > > > > into AwbMode. While at it, pull in the android AWB modes. > > > > I'd say "pull in additional AWB modes defined by Android" (or s/pull > > in/add). > > > > > > > > > > Adjust the previous users of AwbEnable accordingly. > > > > > > > > Signed-off-by: Paul Elder <paul.elder@ideasonboard.com> > > > > --- > > > > include/libcamera/ipa/raspberrypi.h | 1 - > > > > src/ipa/raspberrypi/raspberrypi.cpp | 27 ++++++++---------------- > > > > src/libcamera/control_ids.yaml | 32 +++++++++++++++-------------- > > > > test/controls/control_list.cpp | 6 +++--- > > > > 4 files changed, 29 insertions(+), 37 deletions(-) > > > > > > > > diff --git a/include/libcamera/ipa/raspberrypi.h b/include/libcamera/ipa/raspberrypi.h > > > > index a8790000..63392a26 100644 > > > > --- a/include/libcamera/ipa/raspberrypi.h > > > > +++ b/include/libcamera/ipa/raspberrypi.h > > > > @@ -35,7 +35,6 @@ static const ControlInfoMap Controls = { > > > > { &controls::AeConstraintMode, ControlInfo(controls::AeConstraintModeValues) }, > > > > { &controls::AeExposureMode, ControlInfo(controls::AeExposureModeValues) }, > > > > { &controls::ExposureValue, ControlInfo(0.0f, 16.0f) }, > > > > - { &controls::AwbEnable, ControlInfo(false, true) }, > > > > { &controls::ColourGains, ControlInfo(0.0f, 32.0f) }, > > > > { &controls::AwbMode, ControlInfo(controls::AwbModeValues) }, > > > > { &controls::Brightness, ControlInfo(-1.0f, 1.0f) }, > > > > diff --git a/src/ipa/raspberrypi/raspberrypi.cpp b/src/ipa/raspberrypi/raspberrypi.cpp > > > > index ad0132c0..ed5f1250 100644 > > > > --- a/src/ipa/raspberrypi/raspberrypi.cpp > > > > +++ b/src/ipa/raspberrypi/raspberrypi.cpp > > > > @@ -745,24 +745,6 @@ void IPARPi::queueRequest(const ControlList &controls) > > > > break; > > > > } > > > > > > > > - case controls::AWB_ENABLE: { > > > > - RPiController::Algorithm *awb = controller_.GetAlgorithm("awb"); > > > > - if (!awb) { > > > > - LOG(IPARPI, Warning) > > > > - << "Could not set AWB_ENABLE - no AWB algorithm"; > > > > - break; > > > > - } > > > > - > > > > - if (ctrl.second.get<bool>() == false) > > > > - awb->Pause(); > > > > - else > > > > - awb->Resume(); > > > > - > > > > - libcameraMetadata_.set(controls::AwbEnable, > > > > - ctrl.second.get<bool>()); > > > > - break; > > > > - } > > > > - > > > > case controls::AWB_MODE: { > > > > RPiController::AwbAlgorithm *awb = dynamic_cast<RPiController::AwbAlgorithm *>( > > > > controller_.GetAlgorithm("awb")); > > > > @@ -773,6 +755,15 @@ void IPARPi::queueRequest(const ControlList &controls) > > > > } > > > > > > > > int32_t idx = ctrl.second.get<int32_t>(); > > > > + > > > > + if (idx == controls::AwbOff) { > > > > + awb->Pause(); > > > > + break; > > > > + } > > > > + > > > > + if (idx == controls::AwbAuto) > > > > + awb->Resume(); > > > > > > I think the logic here may need adjusting such that any state that is not > > > controls::AwbOff will need to call awb->Resume(), or conditionally call > > > resume if adb->IsPaused() returns true. > > > > Agreed. The RPi AWB implementation differs from the behaviour specified > > by Android in that the RPi AWB algorithm isn't disabled when the mode is > > set to one of the presets. The presets instead serve to limit the search > > range of the AWB algorithm, instead of setting hardcoded manual values > > as in Android. > > > > Naush, what would happen if the tuning file specified a fixed colour > > temperature (by setting the min and max to the same value) for AWB > > presets ? Would the AWB algorithm then always produce fixed values for > > the colour gains ? > > > > > > if (AwbModeTable.count(idx)) { > > > > awb->SetMode(AwbModeTable.at(idx)); > > > > libcameraMetadata_.set(controls::AwbMode, idx); > > > > diff --git a/src/libcamera/control_ids.yaml b/src/libcamera/control_ids.yaml > > > > index 5717bc1f..2e62f61b 100644 > > > > --- a/src/libcamera/control_ids.yaml > > > > +++ b/src/libcamera/control_ids.yaml > > > > @@ -229,13 +229,6 @@ controls: > > > > Report an estimate of the current illuminance level in lux. The Lux > > > > control can only be returned in metadata. > > > > > > > > - - AwbEnable: > > > > - type: bool > > > > - description: | > > > > - Enable or disable the AWB. > > > > - > > > > - \sa ColourGains > > > > - > > > > # AwbMode needs further attention: > > > > # - Auto-generate max enum value. > > > > # - Better handling of custom types. > > > > @@ -245,29 +238,38 @@ controls: > > > > Specify the range of illuminants to use for the AWB algorithm. The modes > > > > supported are platform specific, and not all modes may be supported. > > > > enum: > > > > - - name: AwbAuto > > > > + - name: AwbOff > > > > value: 0 > > > > + description: The AWB routune is disabled. > > > > s/routine/ > > > > > > + - name: AwbAuto > > > > + value: 1 > > > > description: Search over the whole colour temperature range. > > > > - name: AwbIncandescent > > > > - value: 1 > > > > - description: Incandescent AWB lamp mode. > > > > - - name: AwbTungsten > > > > value: 2 > > > > - description: Tungsten AWB lamp mode. > > > > + description: Incandescent AWB lamp mode. > > > > - name: AwbFluorescent > > > > value: 3 > > > > description: Fluorescent AWB lamp mode. > > > > - - name: AwbIndoor > > > > + - name: AwbWarmFluorescent > > > > value: 4 > > > > - description: Indoor AWB lighting mode. > > > > + description: Warm fluorescent AWB lamp mode. > > > > - name: AwbDaylight > > > > value: 5 > > > > description: Daylight AWB lighting mode. > > > > - name: AwbCloudy > > > > value: 6 > > > > description: Cloudy AWB lighting mode. > > > > - - name: AwbCustom > > > > + - name: AwbTwilight > > > > value: 7 > > > > + description: Twilight AWB lamp mode. > > > > + - name: AwbTungsten > > > > + value: 8 > > > > + description: Tungsten AWB lamp mode. > > > > + - name: AwbIndoor > > > > + value: 9 > > > > + description: Indoor AWB lighting mode. > > > > + - name: AwbCustom > > > > + value: 10 > > > > description: Custom AWB mode. > > > > > > > > - AwbLocked: > > > > diff --git a/test/controls/control_list.cpp b/test/controls/control_list.cpp > > > > index 70cf61b8..ce55d09b 100644 > > > > --- a/test/controls/control_list.cpp > > > > +++ b/test/controls/control_list.cpp > > > > @@ -143,10 +143,10 @@ protected: > > > > * Attempt to set an invalid control and verify that the > > > > * operation failed. > > > > */ > > > > - list.set(controls::AwbEnable, true); > > > > + list.set(controls::AwbMode, true); > > > > > > > > - if (list.contains(controls::AwbEnable)) { > > > > - cout << "List shouldn't contain AwbEnable control" << endl; > > > > + if (list.contains(controls::AwbMode)) { > > > > + cout << "List shouldn't contain AwbMode control" << endl; > > > > return TestFail; > > > > } > > > >
diff --git a/include/libcamera/ipa/raspberrypi.h b/include/libcamera/ipa/raspberrypi.h index a8790000..63392a26 100644 --- a/include/libcamera/ipa/raspberrypi.h +++ b/include/libcamera/ipa/raspberrypi.h @@ -35,7 +35,6 @@ static const ControlInfoMap Controls = { { &controls::AeConstraintMode, ControlInfo(controls::AeConstraintModeValues) }, { &controls::AeExposureMode, ControlInfo(controls::AeExposureModeValues) }, { &controls::ExposureValue, ControlInfo(0.0f, 16.0f) }, - { &controls::AwbEnable, ControlInfo(false, true) }, { &controls::ColourGains, ControlInfo(0.0f, 32.0f) }, { &controls::AwbMode, ControlInfo(controls::AwbModeValues) }, { &controls::Brightness, ControlInfo(-1.0f, 1.0f) }, diff --git a/src/ipa/raspberrypi/raspberrypi.cpp b/src/ipa/raspberrypi/raspberrypi.cpp index ad0132c0..ed5f1250 100644 --- a/src/ipa/raspberrypi/raspberrypi.cpp +++ b/src/ipa/raspberrypi/raspberrypi.cpp @@ -745,24 +745,6 @@ void IPARPi::queueRequest(const ControlList &controls) break; } - case controls::AWB_ENABLE: { - RPiController::Algorithm *awb = controller_.GetAlgorithm("awb"); - if (!awb) { - LOG(IPARPI, Warning) - << "Could not set AWB_ENABLE - no AWB algorithm"; - break; - } - - if (ctrl.second.get<bool>() == false) - awb->Pause(); - else - awb->Resume(); - - libcameraMetadata_.set(controls::AwbEnable, - ctrl.second.get<bool>()); - break; - } - case controls::AWB_MODE: { RPiController::AwbAlgorithm *awb = dynamic_cast<RPiController::AwbAlgorithm *>( controller_.GetAlgorithm("awb")); @@ -773,6 +755,15 @@ void IPARPi::queueRequest(const ControlList &controls) } int32_t idx = ctrl.second.get<int32_t>(); + + if (idx == controls::AwbOff) { + awb->Pause(); + break; + } + + if (idx == controls::AwbAuto) + awb->Resume(); + if (AwbModeTable.count(idx)) { awb->SetMode(AwbModeTable.at(idx)); libcameraMetadata_.set(controls::AwbMode, idx); diff --git a/src/libcamera/control_ids.yaml b/src/libcamera/control_ids.yaml index 5717bc1f..2e62f61b 100644 --- a/src/libcamera/control_ids.yaml +++ b/src/libcamera/control_ids.yaml @@ -229,13 +229,6 @@ controls: Report an estimate of the current illuminance level in lux. The Lux control can only be returned in metadata. - - AwbEnable: - type: bool - description: | - Enable or disable the AWB. - - \sa ColourGains - # AwbMode needs further attention: # - Auto-generate max enum value. # - Better handling of custom types. @@ -245,29 +238,38 @@ controls: Specify the range of illuminants to use for the AWB algorithm. The modes supported are platform specific, and not all modes may be supported. enum: - - name: AwbAuto + - name: AwbOff value: 0 + description: The AWB routune is disabled. + - name: AwbAuto + value: 1 description: Search over the whole colour temperature range. - name: AwbIncandescent - value: 1 - description: Incandescent AWB lamp mode. - - name: AwbTungsten value: 2 - description: Tungsten AWB lamp mode. + description: Incandescent AWB lamp mode. - name: AwbFluorescent value: 3 description: Fluorescent AWB lamp mode. - - name: AwbIndoor + - name: AwbWarmFluorescent value: 4 - description: Indoor AWB lighting mode. + description: Warm fluorescent AWB lamp mode. - name: AwbDaylight value: 5 description: Daylight AWB lighting mode. - name: AwbCloudy value: 6 description: Cloudy AWB lighting mode. - - name: AwbCustom + - name: AwbTwilight value: 7 + description: Twilight AWB lamp mode. + - name: AwbTungsten + value: 8 + description: Tungsten AWB lamp mode. + - name: AwbIndoor + value: 9 + description: Indoor AWB lighting mode. + - name: AwbCustom + value: 10 description: Custom AWB mode. - AwbLocked: diff --git a/test/controls/control_list.cpp b/test/controls/control_list.cpp index 70cf61b8..ce55d09b 100644 --- a/test/controls/control_list.cpp +++ b/test/controls/control_list.cpp @@ -143,10 +143,10 @@ protected: * Attempt to set an invalid control and verify that the * operation failed. */ - list.set(controls::AwbEnable, true); + list.set(controls::AwbMode, true); - if (list.contains(controls::AwbEnable)) { - cout << "List shouldn't contain AwbEnable control" << endl; + if (list.contains(controls::AwbMode)) { + cout << "List shouldn't contain AwbMode control" << endl; return TestFail; }
Previously it was possible to have AwbEnable set to false, yet have AwbMode on anything. This caused a confusion situation, so merge the two into AwbMode. While at it, pull in the android AWB modes. Adjust the previous users of AwbEnable accordingly. Signed-off-by: Paul Elder <paul.elder@ideasonboard.com> --- include/libcamera/ipa/raspberrypi.h | 1 - src/ipa/raspberrypi/raspberrypi.cpp | 27 ++++++++---------------- src/libcamera/control_ids.yaml | 32 +++++++++++++++-------------- test/controls/control_list.cpp | 6 +++--- 4 files changed, 29 insertions(+), 37 deletions(-)