media: Increase the default audio buffer size for encrypted streams

media: Increase the default audio buffer size for encrypted streams

Encrypted audio may need to be decrypted on the renderer main thread
which means that long running tasks on that thread can stall audio
decoding, causing noticeable audio gaps.

By increasing the default initial buffer size from 200ms to 500ms for
encrypted streams, we should drastically reduce the occurrence of
audio hitches caused by this.

There is ongoing work to move decryption to the media thread, at which
point we can revert this change (http://crbug.com/403462).

This change has an impact on preload time (load() to loaded) for audio
because we now decode more. On a linux z620 preload time for crowd.ogg
goes from 94ms to 130ms. (Release, component, dcheck_always_on build).

BUG=718161
Review-Url: https://codereview.chromium.org/2858393002
Cr-Commit-Position: refs/heads/master@{#471522}
Committed: https://chromium.googlesource.com/chromium/src/+/a51ee9488f2b82b1dc8e9c79881b97dc318f0b39

[watk, 2017-05-04 23:24:11]

watk@chromium.org changed reviewers:
+ xhwang@chromium.org

[watk, 2017-05-04 23:24:11]

xhwang: PTAL. I don't love the increase in preload time, but I think it's the
right trade off until we land SoDo. 

dalecurtis: FYI

[watk, 2017-05-04 23:24:56]

https://codereview.chromium.org/2858393002/diff/1/media/test/pipeline_integra...
File media/test/pipeline_integration_test.cc (right):

https://codereview.chromium.org/2858393002/diff/1/media/test/pipeline_integra...
media/test/pipeline_integration_test.cc:1490:
"-0.71,0.36,2.96,2.68,2.10,-1.08,")));
This changed because it hashes all audio it's seen. I Don't know why it changes
so little though.

[xhwang, 2017-05-06 03:52:13]

Sorry for the delay!

The idea looks good. But it seems we are missing some code in this CL?

https://codereview.chromium.org/2858393002/diff/1/media/renderers/audio_rende...
File media/renderers/audio_renderer_impl.h (right):

https://codereview.chromium.org/2858393002/diff/1/media/renderers/audio_rende...
media/renderers/audio_renderer_impl.h:249: bool encrypted_;
I am not seeing this initialized and/or set anywhere?

Style nit: is_encrypted_

[watk, 2017-05-09 17:47:58]

The CQ bit was checked by watk@chromium.org to run a CQ dry run

[watk, 2017-05-09 17:48:52]

Patchset #2 (id:20001) has been deleted

[commit-bot: I haz the power, 2017-05-09 17:49:44]

Dry run: CQ is trying da patch.

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

[watk, 2017-05-09 17:49:55]

The CQ bit was checked by watk@chromium.org to run a CQ dry run

[commit-bot: I haz the power, 2017-05-09 17:50:43]

Dry run: CQ is trying da patch.

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

[watk, 2017-05-09 17:51:10]

Oops, looks like I lost some changes. For testing I was always using 500ms for
unencrypted too.

[watk, 2017-05-09 17:51:28]

PTAL thanks!

https://codereview.chromium.org/2858393002/diff/1/media/renderers/audio_rende...
File media/renderers/audio_renderer_impl.h (right):

https://codereview.chromium.org/2858393002/diff/1/media/renderers/audio_rende...
media/renderers/audio_renderer_impl.h:249: bool encrypted_;
On 2017/05/06 03:52:13, xhwang wrote:
> I am not seeing this initialized and/or set anywhere?
> 
> Style nit: is_encrypted_

Done.

[xhwang, 2017-05-09 17:57:15]

LGTM, thanks for the fix!

[watk, 2017-05-09 18:06:26]

The CQ bit was unchecked by watk@chromium.org

[watk, 2017-05-09 18:06:26]

The CQ bit was checked by watk@chromium.org

[commit-bot: I haz the power, 2017-05-09 18:06:54]

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-05-09 18:35:21]

The CQ bit was unchecked by commit-bot@chromium.org

[commit-bot: I haz the power, 2017-05-09 18:35:22]

Try jobs failed on following builders:
  mac_chromium_rel_ng on master.tryserver.chromium.mac (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.mac/builders/mac_chromium_rel_...)

[watk, 2017-05-09 21:29:53]

The CQ bit was checked by watk@chromium.org

[watk, 2017-05-09 21:29:53]

The patchset sent to the CQ was uploaded after l-g-t-m from xhwang@chromium.org
Link to the patchset: https://codereview.chromium.org/2858393002/#ps60001
(title: "fix cast's FakeMediaSource")

[commit-bot: I haz the power, 2017-05-09 21:31:27]

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-05-09 23:07:56]

The CQ bit was unchecked by commit-bot@chromium.org

[commit-bot: I haz the power, 2017-05-09 23:07:56]

Try jobs failed on following builders:
  chromeos_amd64-generic_chromium_compile_only_ng on
master.tryserver.chromium.linux (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.linux/builders/chromeos_amd64-...)

[watk, 2017-05-09 23:28:51]

The CQ bit was checked by watk@chromium.org

[commit-bot: I haz the power, 2017-05-09 23:30:26]

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-05-10 01:34:55]

The CQ bit was unchecked by commit-bot@chromium.org

[commit-bot: I haz the power, 2017-05-10 01:34:56]

Try jobs failed on following builders:
  chromeos_amd64-generic_chromium_compile_only_ng on
master.tryserver.chromium.linux (JOB_TIMED_OUT, build has not started yet;
builder either lacks capacity or does not exist (misspelled?))
  chromium_presubmit on master.tryserver.chromium.linux (JOB_TIMED_OUT, build
has not started yet; builder either lacks capacity or does not exist
(misspelled?))

[watk, 2017-05-10 20:11:34]

The CQ bit was checked by watk@chromium.org

[watk, 2017-05-10 20:11:34]

The patchset sent to the CQ was uploaded after l-g-t-m from xhwang@chromium.org
Link to the patchset: https://codereview.chromium.org/2858393002/#ps80001
(title: "Revert test")

[commit-bot: I haz the power, 2017-05-10 20:13:25]

CQ is trying da patch.

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

[watk, 2017-05-10 20:13:32]

Oops. The test needed changing originally because I increased the buffer for
non-encrypted. Now that it's encrypted only, it doesn't need to change.

[commit-bot: I haz the power, 2017-05-10 20:48:02]

The CQ bit was unchecked by commit-bot@chromium.org

[commit-bot: I haz the power, 2017-05-10 20:48:03]

Try jobs failed on following builders:
  cast_shell_linux on master.tryserver.chromium.linux (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.linux/builders/cast_shell_linu...)

[watk, 2017-05-10 20:58:46]

The CQ bit was checked by watk@chromium.org

[watk, 2017-05-10 20:58:46]

The patchset sent to the CQ was uploaded after l-g-t-m from xhwang@chromium.org
Link to the patchset: https://codereview.chromium.org/2858393002/#ps100001
(title: "Fix another chromecast usage of AudioRendererAlgorithm")

[commit-bot: I haz the power, 2017-05-10 21:00:18]

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-05-10 21:18:08]

The CQ bit was unchecked by commit-bot@chromium.org

[commit-bot: I haz the power, 2017-05-10 21:18:09]

Try jobs failed on following builders:
  chromium_presubmit on master.tryserver.chromium.linux (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.linux/builders/chromium_presub...)

[watk, 2017-05-10 21:19:16]

watk@chromium.org changed reviewers:
+ lcwu@chromium.org

[watk, 2017-05-10 21:19:16]

lcwu@chromium.org: Please review changes in chromecast

[lcwu1, 2017-05-10 21:33:13]

chromecast/ lgtm

[watk, 2017-05-10 21:35:36]

The CQ bit was checked by watk@chromium.org

[commit-bot: I haz the power, 2017-05-10 21:37: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-05-11 00:30:32]

The CQ bit was unchecked by commit-bot@chromium.org

[commit-bot: I haz the power, 2017-05-11 00:30:34]

Try jobs failed on following builders:
  linux_chromium_rel_ng on master.tryserver.chromium.linux (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.linux/builders/linux_chromium_...)

[watk, 2017-05-11 00:32:45]

The CQ bit was checked by watk@chromium.org

[commit-bot: I haz the power, 2017-05-11 00:34:47]

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-05-11 03:48:17]

The CQ bit was unchecked by commit-bot@chromium.org

[commit-bot: I haz the power, 2017-05-11 03:48:18]

Try jobs failed on following builders:
  linux_chromium_rel_ng on master.tryserver.chromium.linux (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.linux/builders/linux_chromium_...)

[watk, 2017-05-11 19:20:59]

The CQ bit was checked by watk@chromium.org

[commit-bot: I haz the power, 2017-05-11 19:21:51]

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-05-11 22:06:11]

The CQ bit was unchecked by commit-bot@chromium.org

[commit-bot: I haz the power, 2017-05-11 22:06:13]

Try jobs failed on following builders:
  linux_chromium_rel_ng on master.tryserver.chromium.linux (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.linux/builders/linux_chromium_...)

[watk, 2017-05-12 19:20:58]

The CQ bit was checked by watk@chromium.org

[commit-bot: I haz the power, 2017-05-12 19:21:56]

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-05-12 22:19:10]

The CQ bit was unchecked by commit-bot@chromium.org

[commit-bot: I haz the power, 2017-05-12 22:19:11]

Try jobs failed on following builders:
  linux_chromium_asan_rel_ng on master.tryserver.chromium.linux (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.linux/builders/linux_chromium_...)

[watk, 2017-05-12 22:20:55]

The CQ bit was checked by watk@chromium.org

[commit-bot: I haz the power, 2017-05-12 22:21:52]

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-05-13 01:08:38]

CQ is committing da patch.

Bot data: {"patchset_id": 100001, "attempt_start_ts": 1494627655646690,
"parent_rev": "cf212f476a0b162cb0971da9dd30e89facd943bb", "commit_rev":
"a51ee9488f2b82b1dc8e9c79881b97dc318f0b39"}

[commit-bot: I haz the power, 2017-05-13 01:08:45]

Description was changed from

==========
media: Increase the default audio buffer size for encrypted streams

Encrypted audio may need to be decrypted on the renderer main thread
which means that long running tasks on that thread can stall audio
decoding, causing noticeable audio gaps.

By increasing the default initial buffer size from 200ms to 500ms for
encrypted streams, we should drastically reduce the occurrence of
audio hitches caused by this.

There is ongoing work to move decryption to the media thread, at which
point we can revert this change (http://crbug.com/403462).

This change has an impact on preload time (load() to loaded) for audio
because we now decode more. On a linux z620 preload time for crowd.ogg
goes from 94ms to 130ms. (Release, component, dcheck_always_on build).

BUG=718161
==========

to

==========
media: Increase the default audio buffer size for encrypted streams

Encrypted audio may need to be decrypted on the renderer main thread
which means that long running tasks on that thread can stall audio
decoding, causing noticeable audio gaps.

By increasing the default initial buffer size from 200ms to 500ms for
encrypted streams, we should drastically reduce the occurrence of
audio hitches caused by this.

There is ongoing work to move decryption to the media thread, at which
point we can revert this change (http://crbug.com/403462).

This change has an impact on preload time (load() to loaded) for audio
because we now decode more. On a linux z620 preload time for crowd.ogg
goes from 94ms to 130ms. (Release, component, dcheck_always_on build).

BUG=718161

Review-Url: https://codereview.chromium.org/2858393002
Cr-Commit-Position: refs/heads/master@{#471522}
Committed:
https://chromium.googlesource.com/chromium/src/+/a51ee9488f2b82b1dc8e9c79881b...
==========

[commit-bot: I haz the power, 2017-05-13 01:08:46]

Committed patchset #5 (id:100001) as
https://chromium.googlesource.com/chromium/src/+/a51ee9488f2b82b1dc8e9c79881b...