New issue
Advanced search Search tips

Issue 761990 link

Starred by 1 user
Status: Fixed
Owner:
grunell@chromium.org
Closed: Oct 2017
Cc:
ossu@chromium.org
solenberg@chromium.org
guidou@chromium.org
Components:
EstimatedDays: ----
NextAction: ----
OS: Linux , Android , Windows , Chrome , Mac
Pri: 1
Type: Bug



Sign in to add a comment

Suspend/resume can cause MediaStream track onended event to be fired.

Project Member Reported by grunell@chromium.org, Sep 5 2017 
On my Linux machine:

1. Go to https://webrtc.github.io/samples/src/content/getusermedia/audio/. Verify that mic audio is heard in speakers.
2. Suspend by running 'sudo pm-suspend' in terminal.
3. Wake up/resume machine.

Expected:
Audio should still be heard.

Actual:
No audio heard.

I tracked it down to the missing audio detection being triggered in AudioInputDevice, thus reporting an error which closes the stream. Fwiw, I printed the time that had passed when detected. On my machine, four tries reported 16 s twice and 17 s twice.

One solution is to use the base::PowerObserver to be notified when suspending. But it doesn't seem to work - no notification is sent. Maybe it isn't supported on Linux. There's no documentation in any of those files that mentions platform support.

The quick not-robust fix is to increase the timeout, but the time without callbacks could potentially be longer on other machines.

Comment 1 by grunell@chromium.org, Sep 6 2017

Labels: OS-Android OS-Chrome OS-iOS OS-Mac OS-Windows
Status: Started (was: Assigned)
I tested on my Macbook, same thing but doesn't happen that often. On my Linux machine I repro'd consistently. On Linux there's no support for suspend/resume notifications in Chrome, but all other platforms appear to have support.

Using the notifications to not check until we have resumed (and then reset the time) should be plausible, but on Linux we have to increase the threshold or remove the detection. Add support for the notifications would be nice of course.

Comment 2 by grunell@chromium.org, Sep 6 2017 

I have gotten up to 35 seconds between suspend and resume notifications on my Macbook.

Comment 3 by solenberg@chromium.org, Sep 6 2017 

What do you mean in #2 - the time between notifications shouldn't matter? Do you mean the delay from waking the device until you get the "resume" notification?

Comment 4 by grunell@chromium.org, Sep 6 2017 

I'm not familiar with the suspend/resume details on Mac. Should the time between suspend notification and being suspended not matter? I'm thinking that CoreAudio could be stopped early and thus no callbacks coming, and at some later point Chrome is stopped. Also, this could depend on what TimeTicks in Chrome is using under the hood.

Anyway, between suspend and resume notifications we shouldn't detect missing callbacks, then start over at resume notification.

Comment 5 by grunell@chromium.org, Sep 6 2017 

I just realised that it wasn't clear that I'm measuring using TimeTicks, which pauses during suspend. So the time is between suspend notification and suspend + between resume and resume notification, where "suspend" and "resume" really mean when Chrome's TimeTicks "pauses" and "resumes".
Project Member

Comment 6 by bugdroid1@chromium.org, Oct 11 2017 

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/9a40504e4362b2ae157e8b7634d4d6482bd12047

commit 9a40504e4362b2ae157e8b7634d4d6482bd12047
Author: Henrik Grunell <grunell@chromium.org>
Date: Wed Oct 11 13:37:08 2017

Don't detect missing input audio when suspending.

This fixes false positives.

* Move out the existing alive check from AudioInputDevice to a separate class AliveChecker.
* Introduce PowerObserverHelper.
* The AliveChecker has a PowerObserverHelper and is informed by it about suspend/resume.

Bug:  761990 
Cq-Include-Trybots: master.tryserver.chromium.android:android_optional_gpu_tests_rel;master.tryserver.chromium.linux:linux_optional_gpu_tests_rel;master.tryserver.chromium.mac:mac_optional_gpu_tests_rel;master.tryserver.chromium.win:win_optional_gpu_tests_rel
Change-Id: I091d7b41209d2f64fd7a6fd56db1f820389a9615
Reviewed-on: https://chromium-review.googlesource.com/655082
Reviewed-by: Olga Sharonova <olka@chromium.org>
Commit-Queue: Henrik Grunell <grunell@chromium.org>
Cr-Commit-Position: refs/heads/master@{#507965}
[modify] https://crrev.com/9a40504e4362b2ae157e8b7634d4d6482bd12047/media/audio/BUILD.gn
[add] https://crrev.com/9a40504e4362b2ae157e8b7634d4d6482bd12047/media/audio/alive_checker.cc
[add] https://crrev.com/9a40504e4362b2ae157e8b7634d4d6482bd12047/media/audio/alive_checker.h
[add] https://crrev.com/9a40504e4362b2ae157e8b7634d4d6482bd12047/media/audio/alive_checker_unittest.cc
[modify] https://crrev.com/9a40504e4362b2ae157e8b7634d4d6482bd12047/media/audio/audio_input_device.cc
[modify] https://crrev.com/9a40504e4362b2ae157e8b7634d4d6482bd12047/media/audio/audio_input_device.h
[add] https://crrev.com/9a40504e4362b2ae157e8b7634d4d6482bd12047/media/audio/power_observer_helper.cc
[add] https://crrev.com/9a40504e4362b2ae157e8b7634d4d6482bd12047/media/audio/power_observer_helper.h
[add] https://crrev.com/9a40504e4362b2ae157e8b7634d4d6482bd12047/media/audio/power_observer_helper_unittest.cc

Comment 7 by pinkerton@chromium.org, Oct 11 2017

Labels: -OS-iOS

Comment 8 by grunell@chromium.org, Oct 12 2017

Labels: M-63
Status: Fixed (was: Started)
Fixed in M63.
►

Sign in to add a comment