New issue
Advanced search Search tips
Note: Color blocks (like or ) mean that a user may not be available. Tooltip shows the reason.

Issue 826968 link

Starred by 2 users

Issue metadata

Status: Fixed
Owner:
Closed: Apr 2018
Cc:
Components:
EstimatedDays: ----
NextAction: ----
OS: Chrome
Pri: 2
Type: Bug



Sign in to add a comment

Adaptive brightness algorithm is too sensitive to noise

Project Member Reported by puthik@chromium.org, Mar 28 2018

Issue description

There are reported that in some light condition, adaptive brightness algorithm is continuously make screen brighter and dimmer.

We should do the smoothing for the data we got from ALS
 
Project Member

Comment 1 by bugdroid1@chromium.org, Mar 31 2018

The following revision refers to this bug:
  https://chromium.googlesource.com/chromiumos/overlays/board-overlays/+/decdf357c3a279a39fff45f8479196d3496214a6

commit decdf357c3a279a39fff45f8479196d3496214a6
Author: Puthikorn Voravootivat <puthik@chromium.org>
Date: Sat Mar 31 02:46:28 2018

eve: Set als smoothing constant to 0.2

There are reports that eve ALS based display brightness adjustment
might be too sensitive in some lighting condition such as overhead
LED light with no other light source.

This CLs set als smoothing constant to 0.2 to do simple exponential
smoothing for ambient light lux value from ALS to filter out the
extreme value.

This also prevents screen to adjusting brightness for sequence of
ALS readings like [120, 115, 5, 3, 5, 120, 115] which is easily
occured when something covers the ALS sensor for a short time.

BUG= chromium:826968 
TEST=Eve won't dim screen if cover ALS for a short time.
CQ-DEPEND=CL:985118

Change-Id: Ief8fa76553eaa60689d99245d598ee9c1eee8000
Signed-off-by: Puthikorn Voravootivat <puthik@chromium.org>
Reviewed-on: https://chromium-review.googlesource.com/988258
Reviewed-by: Dan Erat <derat@chromium.org>

[add] https://crrev.com/decdf357c3a279a39fff45f8479196d3496214a6/overlay-eve/chromeos-base/chromeos-bsp-eve/files/powerd_prefs/als_smoothing_constant
[rename] https://crrev.com/decdf357c3a279a39fff45f8479196d3496214a6/overlay-eve/chromeos-base/chromeos-bsp-eve/chromeos-bsp-eve-0.0.1-r44.ebuild

Project Member

Comment 2 by bugdroid1@chromium.org, Mar 31 2018

The following revision refers to this bug:
  https://chromium.googlesource.com/chromiumos/platform2/+/325a6b915b37599da61cc4561dc68e409556d386

commit 325a6b915b37599da61cc4561dc68e409556d386
Author: Puthikorn Voravootivat <puthik@chromium.org>
Date: Sat Mar 31 02:46:28 2018

power: Use simple exponential smoothing for light level.

Use the following formula to do light level smoothing.
nex_lux = alpha * lux_now + (1 - alpha) * lux_previous

The alpha (aka smoothing constant) is default to be 1.0
which means smoothing is disabled. This can be override
for each platform using newly created powerd pref called
|als_smoothing_constant|.

With reasonable smoothing constant like 0.2, this will
makes screen to not adjusting brightness for sequence of
ALS readings like [120, 115, 5, 3, 5, 120, 115] which is easily
occured when something covers the ALS sensor for a short time.

BUG= chromium:826968 
TEST=newly created unit test, manual test.
eve won't dim screen if cover ALS for a short time with alpha=0.2

Change-Id: I32153eabdceaf685c38e28f02020e13fcde7c407
Signed-off-by: Puthikorn Voravootivat <puthik@chromium.org>
Reviewed-on: https://chromium-review.googlesource.com/985118

[modify] https://crrev.com/325a6b915b37599da61cc4561dc68e409556d386/power_manager/common/power_constants.cc
[modify] https://crrev.com/325a6b915b37599da61cc4561dc68e409556d386/power_manager/powerd/policy/keyboard_backlight_controller_unittest.cc
[modify] https://crrev.com/325a6b915b37599da61cc4561dc68e409556d386/power_manager/common/power_constants.h
[add] https://crrev.com/325a6b915b37599da61cc4561dc68e409556d386/power_manager/default_prefs/als_smoothing_constant
[modify] https://crrev.com/325a6b915b37599da61cc4561dc68e409556d386/power_manager/powerd/policy/keyboard_backlight_controller.cc
[modify] https://crrev.com/325a6b915b37599da61cc4561dc68e409556d386/power_manager/powerd/policy/ambient_light_handler_unittest.cc
[modify] https://crrev.com/325a6b915b37599da61cc4561dc68e409556d386/power_manager/powerd/policy/internal_backlight_controller.cc
[modify] https://crrev.com/325a6b915b37599da61cc4561dc68e409556d386/power_manager/powerd/policy/internal_backlight_controller_unittest.cc
[modify] https://crrev.com/325a6b915b37599da61cc4561dc68e409556d386/power_manager/powerd/policy/ambient_light_handler.cc
[modify] https://crrev.com/325a6b915b37599da61cc4561dc68e409556d386/power_manager/powerd/policy/ambient_light_handler.h

Cc: josa...@chromium.org
Labels: Merge-Request-66 M-66
+josafat for M66 merge request

Low risk because this only affects Pixelbook.
Project Member

Comment 4 by sheriffbot@chromium.org, Apr 4 2018

Labels: -Merge-Request-66 Merge-Review-66 Hotlist-Merge-Review
This bug requires manual review: We are only 12 days from stable.
Please contact the milestone owner if you have questions.
Owners: cmasso@(Android), cmasso@(iOS), josafat@(ChromeOS), abdulsyed@(Desktop)

For more details visit https://www.chromium.org/issue-tracking/autotriage - Your friendly Sheriffbot
Labels: -Hotlist-Merge-Review -Merge-Review-66 Merge-Approved-66
If this is verified on canary consider it merge approved.
Project Member

Comment 6 by bugdroid1@chromium.org, Apr 6 2018

Labels: merge-merged-release-R66-10452.B
The following revision refers to this bug:
  https://chromium.googlesource.com/chromiumos/platform2/+/857ebd445d3f3683120f96564f51a420feff14f7

commit 857ebd445d3f3683120f96564f51a420feff14f7
Author: Puthikorn Voravootivat <puthik@chromium.org>
Date: Fri Apr 06 01:56:19 2018

power: Use simple exponential smoothing for light level.

Use the following formula to do light level smoothing.
nex_lux = alpha * lux_now + (1 - alpha) * lux_previous

The alpha (aka smoothing constant) is default to be 1.0
which means smoothing is disabled. This can be override
for each platform using newly created powerd pref called
|als_smoothing_constant|.

With reasonable smoothing constant like 0.2, this will
makes screen to not adjusting brightness for sequence of
ALS readings like [120, 115, 5, 3, 5, 120, 115] which is easily
occured when something covers the ALS sensor for a short time.

BUG= chromium:826968 
TEST=newly created unit test, manual test.
eve won't dim screen if cover ALS for a short time with alpha=0.2

Change-Id: I32153eabdceaf685c38e28f02020e13fcde7c407
Signed-off-by: Puthikorn Voravootivat <puthik@chromium.org>
Reviewed-on: https://chromium-review.googlesource.com/985118
(cherry picked from commit 325a6b915b37599da61cc4561dc68e409556d386)
Reviewed-on: https://chromium-review.googlesource.com/999332

[modify] https://crrev.com/857ebd445d3f3683120f96564f51a420feff14f7/power_manager/common/power_constants.cc
[modify] https://crrev.com/857ebd445d3f3683120f96564f51a420feff14f7/power_manager/powerd/policy/keyboard_backlight_controller_unittest.cc
[modify] https://crrev.com/857ebd445d3f3683120f96564f51a420feff14f7/power_manager/common/power_constants.h
[add] https://crrev.com/857ebd445d3f3683120f96564f51a420feff14f7/power_manager/default_prefs/als_smoothing_constant
[modify] https://crrev.com/857ebd445d3f3683120f96564f51a420feff14f7/power_manager/powerd/policy/keyboard_backlight_controller.cc
[modify] https://crrev.com/857ebd445d3f3683120f96564f51a420feff14f7/power_manager/powerd/policy/ambient_light_handler_unittest.cc
[modify] https://crrev.com/857ebd445d3f3683120f96564f51a420feff14f7/power_manager/powerd/policy/internal_backlight_controller.cc
[modify] https://crrev.com/857ebd445d3f3683120f96564f51a420feff14f7/power_manager/powerd/policy/internal_backlight_controller_unittest.cc
[modify] https://crrev.com/857ebd445d3f3683120f96564f51a420feff14f7/power_manager/powerd/policy/ambient_light_handler.cc
[modify] https://crrev.com/857ebd445d3f3683120f96564f51a420feff14f7/power_manager/powerd/policy/ambient_light_handler.h

Project Member

Comment 7 by bugdroid1@chromium.org, Apr 6 2018

The following revision refers to this bug:
  https://chromium.googlesource.com/chromiumos/overlays/board-overlays/+/577d842d2d4bbba1651bd33b2f51335c2a1a99a1

commit 577d842d2d4bbba1651bd33b2f51335c2a1a99a1
Author: Puthikorn Voravootivat <puthik@chromium.org>
Date: Fri Apr 06 02:03:59 2018

eve: Set als smoothing constant to 0.2

There are reports that eve ALS based display brightness adjustment
might be too sensitive in some lighting condition such as overhead
LED light with no other light source.

This CLs set als smoothing constant to 0.2 to do simple exponential
smoothing for ambient light lux value from ALS to filter out the
extreme value.

This also prevents screen to adjusting brightness for sequence of
ALS readings like [120, 115, 5, 3, 5, 120, 115] which is easily
occured when something covers the ALS sensor for a short time.

BUG= chromium:826968 
TEST=Eve won't dim screen if cover ALS for a short time.
CQ-DEPEND=CL:985118

Change-Id: Ief8fa76553eaa60689d99245d598ee9c1eee8000
Signed-off-by: Puthikorn Voravootivat <puthik@chromium.org>
Reviewed-on: https://chromium-review.googlesource.com/988258
Reviewed-by: Dan Erat <derat@chromium.org>
(cherry picked from commit decdf357c3a279a39fff45f8479196d3496214a6)
Reviewed-on: https://chromium-review.googlesource.com/998846

[rename] https://crrev.com/577d842d2d4bbba1651bd33b2f51335c2a1a99a1/overlay-eve/chromeos-base/chromeos-bsp-eve/chromeos-bsp-eve-0.0.1-r42.ebuild
[add] https://crrev.com/577d842d2d4bbba1651bd33b2f51335c2a1a99a1/overlay-eve/chromeos-base/chromeos-bsp-eve/files/powerd_prefs/als_smoothing_constant

Labels: -Merge-Approved-66
Status: Fixed (was: Started)
Project Member

Comment 9 by bugdroid1@chromium.org, Apr 10 2018

The following revision refers to this bug:
  https://chromium.googlesource.com/chromiumos/platform2/+/b519781f25aaef7dda3eec3a97fc72e9f0067710

commit b519781f25aaef7dda3eec3a97fc72e9f0067710
Author: Puthikorn Voravootivat <puthik@chromium.org>
Date: Tue Apr 10 23:17:42 2018

power: Update screen brightness doc for Pixelbook

Pixelbook implementation of screen brightness has some
improvement not mentioned in doc. Update doc accordingly.

BUG= chromium:826968 , b:38208252
TEST=None

Change-Id: I97378babcb8082e12456559c4964b2d0cc7b68cb
Signed-off-by: Puthikorn Voravootivat <puthik@chromium.org>
Reviewed-on: https://chromium-review.googlesource.com/1003276
Reviewed-by: Dan Erat <derat@chromium.org>
Commit-Queue: ChromeOS CL Exonerator Bot <chromiumos-cl-exonerator@appspot.gserviceaccount.com>

[modify] https://crrev.com/b519781f25aaef7dda3eec3a97fc72e9f0067710/power_manager/docs/screen_brightness.md

Sign in to add a comment