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

Issue 762104 link

Starred by 1 user

Issue metadata

Status: Assigned
Owner:
Cc:
Components:
EstimatedDays: ----
NextAction: ----
OS: Chrome
Pri: 2
Type: Bug

Blocking:
issue 716066



Sign in to add a comment

SuspendMediaSessions for suspend will cause short glitch of sound when system resumes

Project Member Reported by warx@chromium.org, Sep 5 2017

Issue description

Moved from email discussion:

The WIP CL: https://codereview.chromium.org/2916733002/ will SuspendMediaSessions for  issue 716066 . It is like calling pause() on all audio outputs of all Webcontents. However, for the case in  issue 716066 , there will be a short glitch of media sounds.

Dylan comments: "Normal stream playback behavior is chrome sending a bunch of samples to cras, then removing the stream. After the stream is removed, crash finishes playing all the samples from that stream, normally 20-40 ms worth. On suspend, cras will shut down all the devices, but leave the streams. Then on resume it will continue playback of the streams. This was added so chrome didn't have to deal with suspend/resume in the audio path. Now that chrome is dealing with suspend/resume in its audio path we might want to reconsider that decision."

Based on the discussion from engineers involved, we have:

Solution 1: chrome side provides an option to drop the remaining samples when removing the stream if chrome doesn't want the stream to finish playing, like chrome receives the notification of suspend imminent.
Solution 2: CRAS side dumps queued samples across suspend/resume. Some amount of audio would be lost, but might be ok for users. This could be done by SetSuspendAudio call.

Solution 1 & 2 probably should be considered together, depends on whether chrome drops samples when it pauses the stream.

Solution 3: SetAudioMuted(true) before SuspendImminent and SetAudioMuted(false) after SuspendDone. This will make the resumed sample unnoticeable to user. However, IsAudioMuted() could be a user setting, we should respect that.

Daniel's comment for solution 3: make sure we are not in a bad state if Chrome crashes after we set audio muted globally and before ask it unmuted.

Solution 3 might be the quickest engineer effort for this problem, considering in either case last audio samples before suspend are "lost" for users. But it doesn't fix the root cause. I am not sure which direction we should go forward.
 

Comment 1 by warx@chromium.org, Sep 5 2017

Blocking: 716066
I like the idea of having a separate interface for times when chrome wants to remove a stream and drop the remaining samples. This will help the case where a user presses pause in addition to the suspend/resume case.

Comment 3 by warx@chromium.org, Sep 8 2017

Dylan:

Chrome removes a stream. I believe it is here: https://cs.chromium.org/chromium/src/media/audio/cras/cras_unified.cc?q=cras_unif&sq=package:chromium&l=237. It is called with some non-trivial delay after audio is paused. Do you know why there is a delay here?

For dropping the remaining samples, does that mean we ignore the AudioSourceCallback::OnMoreData() when needed?

Thanks!
The delay is there because cras will finish playing the samples that were already sent. This was added as otherwise we would totally miss some short sounds.

I was suggesting adding a separate path for removing a stream and dropping the remaining samples, chrome would have to use the correct API for suspend, but continue to use the current api when removing a stream after playback completes.

Comment 5 by warx@chromium.org, Sep 8 2017

Cc: warx@chromium.org
Owner: hychao@chromium.org
OK, I think we firstly need the API support from cras_client. hychao@, could you take it?
warx, assuming you have a new API "cras_client_remove_stream_flush', can you prototype the chrome side change and make sure it's possible to use?

Comment 7 by warx@chromium.org, Sep 11 2017

Dylan, I wrote a doc for this issue: https://docs.google.com/a/google.com/document/d/15E0iAm7GdGsubvBWX_cQhT_oUFzq7r00MYe7B6WYpDc/edit?usp=sharing

I would like to know your ideas about it. Thanks!
Cc: wuchengli@chromium.org
Components: -Internals>Media OS>Kernel>Audio
Updating components per warx@

Sign in to add a comment