`setManualWhiteBalance()` fails after `setAutoWhiteBalanceLock(true)`

diablodale commented 2 years ago

all calls to setManualWhiteBalance() after setAutoWhiteBalanceLock(true) silently fail to change the color balance. For manual white balance to succeed, the code must call setAutoWhiteBalanceLock(false) or never call setAutoWhiteBalanceLock()

This is a subtle issue, and the workaround is easy. I think this issue should align with the overall Luxonis philosophy on the topic of setting manual values with things that have auto/lock behaviors. I think this is similar, in philosophy, as https://github.com/luxonis/depthai-core/commit/70d4796c94c6741af73c09731c3786cb68c1d15f

Setup

Microsoft Windows [Version 10.0.19044.1706]
VS2019 v16.11.15
depthai-core v2.15.5

Repro

Create c++ app that will at runtime change the color camera white balance to auto or to a easily changed (key or UI element) integer between 1000-12000 pseudocode...

int32_t colorbalance
if (colorbalance) {
  dai::CameraControl ctrl;
  ctrl.setAutoWhiteBalanceMode(dai::CameraControl::AutoWhiteBalanceMode::OFF);
  ctrl.setAutoWhiteBalanceLock(true);
  ctrl.setManualWhiteBalance(colorbalance);
}
else {
  ctrl.setAutoWhiteBalanceMode(dai::CameraControl::AutoWhiteBalanceMode::AUTO);
  ctrl.setAutoWhiteBalanceLock(false);
}

build and run
change the whitebalance interactively starting at 2600 and increase to 8000

Result

The color balance is set to 2600 and then no further changes.

Expected

The color balance to be initially set to 2600 and continue to change all the way until 8000.

Workaround

Remove the single line ctrl.setAutoWhiteBalanceLock(true);

themarpe commented 2 years ago

Thanks for the report @diablodale

@alex-luxonis can we just modify the setManualWhiteBalance to also add in AWB_LOCK = false command?

CameraControl& CameraControl::setManualWhiteBalance(int colorTemperatureK) {
    cfg.setCommand(RawCameraControl::Command::AWB_LOCK);
    cfg.awbLockMode = false;
    cfg.setCommand(RawCameraControl::Command::WB_COLOR_TEMP);
    cfg.wbColorTemp = colorTemperatureK;
    return *this;
}

diablodale commented 2 years ago

Will that host-side sdk change work if I reorder the app's api calls?

if (colorbalance) {
  dai::CameraControl ctrl;
  ctrl.setAutoWhiteBalanceMode(dai::CameraControl::AutoWhiteBalanceMode::OFF);
  ctrl.setManualWhiteBalance(colorbalance);
  ctrl.setAutoWhiteBalanceLock(true);  // reordered to be after 🤪
}

themarpe commented 2 years ago

@diablodale I'm afraid it won't. We could modify setAutoWhiteBalanceLock as well to be NOOP if control has manual WB set.

Just so we are on the same page, is the main intention here to solve the case where one specifies lock beforehand but forgets to disable, or does it in the same control as manual WB?

diablodale commented 2 years ago

My intention is to prevent bugs like I wrote yesterday. Which can lead to disappointment, confusion, esclation to support, etc. The philosophy that a setManualxxx() always overrides is my preference (implemented in sdk or in firmware). An alternative is to add documentation in the headers+website that Lock overrides everything and therefore Manual will fail until unlocked.

My Use Case Yesterday

I wanted to add color white balance to my app. I knew OAK has an auto and manual mode for balance. I searched the SDK's headers for the api with balance. (Can also go to https://docs.luxonis.com/en/latest/ and search for balance). I found setAutoWhiteBalanceMode() and saw beside it the Lock and Manual apis.

Then I wrote the C++ code in the OP. It is how I mapped my thinking "I want to turn off auto balance, set it to a integer value, and lock it so the balance doesn't change" into the apis. At first the Lock was at the end but I moved it 2nd as in the OP since that paired the two apis about a "lock" topic together.

config, build, run, and experienced this OP issue.

Comments

I immediately knew the issue, but I have a lot of experience with depthai-core and its behavior. Not everyone will have the same.

I would prefer all the "auto" apis to have similar behaviors/calls.

Exposure <-> Balance

Auto-exposure apis are almost the same. I just now tested...the locking behavior is different. Exposure behaves as I would expect: setManualExposure() always works even when it was previously setAutoExposureLock(true). I think the auto-balance apis should follow the same philosophy/rules. 🤔 Pseudocode...

if (exposure) {
    ctrl.setManualExposure(exposure, 800); // this always works
    ctrl.setAutoExposureLock(true);
}
else {
    ctrl.setAutoExposureEnable();
    ctrl.setAutoExposureLock(false);
}

exposure	balance	notes
`setAutoExposureEnable`	`setAutoWhiteBalanceMode`	strange api to me; no param and no `Disable` 🤷‍♀️
`setAutoExposureLock`	`setAutoWhiteBalanceLock`
`setManualExposure`	`setManualWhiteBalance`	`setManualExposure()` overrides everything

Focus <-> Balance

Auto-focus apis avoid this Lock api issue by having mode=CONTINUOUS_VIDEO -or- mode=AUTO and the app calls setAutoFocusTrigger(). It is somewhat reverse...it is always locked except for the trigger moment.

focus	balance	notes
`setAutoFocusMode`	`setAutoWhiteBalanceMode`
`setAutoFocusTrigger`	`setAutoWhiteBalanceLock`	avoids this issue
`setManualFocus`	`setManualWhiteBalance`

luxonis / depthai-core