Set audio_sync_reader timeout as half buffer duration on ChromeOS

Set audio_sync_reader timeout as half buffer duration on ChromeOS

Set audio_sync_reader timeout as half buffer duration instead of fixed
20ms.
When system is too busy and renderer can not prepare enough data,
audio_sync_reader will timeout and fill zeros. Chrome replies the fetch
request in time so CRAS does not see underrun. User will hear silence
instead of annoying pop noise.

BUG=b:36119859
TEST=On kevin, play youtube with stressapptest -M 100 -m 6 -s 3600. Hear
silence instead of pop noise. Check there is no underrun in CRAS audio
thread log.

Review-Url: https://codereview.chromium.org/2760243002
Cr-Commit-Position: refs/heads/master@{#460321}
Committed: https://chromium.googlesource.com/chromium/src/+/36527589aedb56b1696c30f4b548b9d3a319fdfc

[cychiang, 2017-03-21 06:38:46]

Description was changed from

==========
Set audio_sync_reader timeout as half buffer duration on ChromeOS

Set audio_sync_reader timeout as half buffer duration instead of fixed
20ms.
When system is too busy and renderer can not prepare enough data,
audio_sync_reader will timeout and fill zeros. Chrome replies the fetch
request in time so CRAS does not see underrun. User will hear silence
instead of annoying pop noise.

BUG=b:36119859
TEST=On kevin, play youtube with stressapptest -M 100 -m 6 -s 3600. Hear
silence instead of pop noise. Check there is no underrun in CRAS audio
thread log.
==========

to

==========
Set audio_sync_reader timeout as half buffer duration on ChromeOS

Set audio_sync_reader timeout as half buffer duration instead of fixed
20ms.
When system is too busy and renderer can not prepare enough data,
audio_sync_reader will timeout and fill zeros. Chrome replies the fetch
request in time so CRAS does not see underrun. User will hear silence
instead of annoying pop noise.

BUG=b:36119859
TEST=On kevin, play youtube with stressapptest -M 100 -m 6 -s 3600. Hear
silence instead of pop noise. Check there is no underrun in CRAS audio
thread log.
==========

[cychiang, 2017-03-21 06:38:46]

cychiang@chromium.org changed reviewers:
+ dalecurtis@chromium.org, tommi@chromium.org

[cychiang, 2017-03-27 06:35:41]

On 2017/03/21 06:38:46, cychiang wrote:
> mailto:cychiang@chromium.org changed reviewers:
> + mailto:dalecurtis@chromium.org, mailto:tommi@chromium.org

Hi Dale and Tommi, could you please review this CL?
Thanks!

[DaleCurtis, 2017-03-27 18:54:58]

I thought this was already fixed by the thread priorities? I'd be wary about
merging this CL back a release without proper soak time.

[cychiang1, 2017-03-28 09:53:09]

On 2017/03/27 18:54:58, DaleCurtis wrote:
> I thought this was already fixed by the thread priorities? I'd be wary about
> merging this CL back a release without proper soak time.

Hi Dale, the thread priority change only affects audio_sync_reader. So when
renderer is starved, audio_sync_reader will still need to wait for 20 ms to
timeout, and this causes underrun in CRAS and ALSA, so there will be pop noise.
Using a timeout that is related to buffer size can avoid underrun seen by CRAS
and ALSA. User will hear silence instead of unpleasant pop noise.
We can put this on canary for a longer time and see if it works well. Currently
there are changes on CrOS avoiding pop noise (use min_buffer_level UCM config to
fill zeros on some boards), but solving this for all boards seems a better
direction.

Thanks!

[DaleCurtis, 2017-03-28 17:23:20]

Okay, so long as you're not planning to merge this back right away  this lgtm

[cychiang, 2017-03-29 06:45:46]

The CQ bit was checked by cychiang@chromium.org

[cychiang, 2017-03-29 06:46:05]

On 2017/03/28 17:23:20, DaleCurtis wrote:
> Okay, so long as you're not planning to merge this back right away  this lgtm

Got it. Thanks a lot!

[commit-bot: I haz the power, 2017-03-29 06:46:15]

CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[commit-bot: I haz the power, 2017-03-29 07:50:53]

CQ is committing da patch.

Bot data: {"patchset_id": 1, "attempt_start_ts": 1490769946529430, "parent_rev":
"10004604d31dfc0134b334d946f7f62550b2b6da", "commit_rev":
"36527589aedb56b1696c30f4b548b9d3a319fdfc"}

[commit-bot: I haz the power, 2017-03-29 07:51:41]

Description was changed from

==========
Set audio_sync_reader timeout as half buffer duration on ChromeOS

Set audio_sync_reader timeout as half buffer duration instead of fixed
20ms.
When system is too busy and renderer can not prepare enough data,
audio_sync_reader will timeout and fill zeros. Chrome replies the fetch
request in time so CRAS does not see underrun. User will hear silence
instead of annoying pop noise.

BUG=b:36119859
TEST=On kevin, play youtube with stressapptest -M 100 -m 6 -s 3600. Hear
silence instead of pop noise. Check there is no underrun in CRAS audio
thread log.
==========

to

==========
Set audio_sync_reader timeout as half buffer duration on ChromeOS

Set audio_sync_reader timeout as half buffer duration instead of fixed
20ms.
When system is too busy and renderer can not prepare enough data,
audio_sync_reader will timeout and fill zeros. Chrome replies the fetch
request in time so CRAS does not see underrun. User will hear silence
instead of annoying pop noise.

BUG=b:36119859
TEST=On kevin, play youtube with stressapptest -M 100 -m 6 -s 3600. Hear
silence instead of pop noise. Check there is no underrun in CRAS audio
thread log.

Review-Url: https://codereview.chromium.org/2760243002
Cr-Commit-Position: refs/heads/master@{#460321}
Committed:
https://chromium.googlesource.com/chromium/src/+/36527589aedb56b1696c30f4b548...
==========

[commit-bot: I haz the power, 2017-03-29 07:51:42]

Committed patchset #1 (id:1) as
https://chromium.googlesource.com/chromium/src/+/36527589aedb56b1696c30f4b548...