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

Issue 722720 link

Starred by 22 users

Issue metadata

Status: Fixed
Owner:
Closed: May 2017
Cc:
Components:
EstimatedDays: ----
NextAction: ----
OS: Linux , Android , Chrome , Mac
Pri: 2
Type: Bug

Blocking:
issue 487060



Sign in to add a comment

Audio glitches on Posix machines when number of file descriptors are high

Project Member Reported by grunell@chromium.org, May 16 2017

Issue description

Original bug is issue 487060. I'm filing a new bug for this Posix wide problem. This is likely the root cause for the original bug, but let's keep it separate until confirmed.

I can repro glitches fairly reliably by opening tabs with different content until number of file descriptors for Chrome's browser process reaches over 1024 (FD_SETSIZE). (Can be seen in the task manager by right-clicking and enabling viewing file descriptors.) Then do a Hangouts call.

This came out from a trace from cwmorris@ were it could be seen that we reported a timeout although we has not reached the deadline. By looking at the code this was suspicious:

https://cs.chromium.org/chromium/src/base/sync_socket_posix.cc?q=package:%5Echromium$&l=149

We should change select() to poll()/epoll() which can deal with larger fd values, and remove the special case. We should also log correctly at failure, and perhaps add UMA stats.
 
It's unfortunate this ends up causing glitches, it was assumed safe at the time since this was how all of Chrome's audio stack worked prior to the select() based mechanism I introduced. 
In a custom release build on Linux, were I force that code path (removing the if statement for checking the handle value), it's reproduced 100 % using WebRTC. It doesn't happen with a media element (e.g. YouTube video) or with WebAudio. ossu@ could repro on Mac, but only fairly reliably with a debug build iiuc.
Status: Started (was: Assigned)
And thanks Dale for the history in comment #1.
Project Member

Comment 4 by bugdroid1@chromium.org, May 23 2017

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

commit b464b3efcea3392d5254e4bb55053916801fcef7
Author: grunell <grunell@chromium.org>
Date: Tue May 23 11:15:26 2017

Change SyncSocket::ReceiveWithTimeout() to use poll() instead of select() on Posix platforms.

Removing the special case for fd > 1023 which can lead to audio glitches.

Code originally written by ossu@.

BUG= 722720 

Review-Url: https://codereview.chromium.org/2888403005
Cr-Commit-Position: refs/heads/master@{#473856}

[modify] https://crrev.com/b464b3efcea3392d5254e4bb55053916801fcef7/base/sync_socket_posix.cc

Status: Fixed (was: Started)
I have tested to go over 1023 FDs with appr.tc; no issues observed.
Thanks for the fix! When will this roll out to the stable channel?
Beta is expected early-mid June, stable end of July.
Is it possible to switch track for a CrOS device to e.g. Canary? It'd be great if you could help us verify this fix, Chris!
It is possible: https://support.google.com/chromebook/answer/1086915?hl=en

Note that when switching back to a more stable channel, all data will be erased. That is probably not true when the device is managed, it seems like that on the description.
Is this fix in Version 60.0.3100.0 (Official Build) dev (64-bit) version? If not what version will it be in?
Re #10: it's not in that version. It will be in the next Canary and Dev. For ChromeOS it's Dev.
I think that enough people have been able to reproduce this issue that I'll let them verify the fix (I use the stable channel), though it seems that Henrik has already verified it.
I've contacted a number of people who had glitches fairly recently to ask if someone is willing to verify in Dev channel.
The fix is in 60.0.3109.0.

Comment 15 by huib@chromium.org, Jun 19 2017

Any data or reviews that indicate the impact of this fix?
We only have data up to Dev so far; too little time in Beta yet. No change in the stats (Media.AudioRendererAudioGlitches and Media.AudioRendererMissedDeadline) can be seen, but that is not unexpected. I suspect this is because it is rare and when you get it you probably stop playout quickly. And if the issue persists you likely take some workaround action to fix it like restart the browser.

From those who have reported glitch issues and have tested in Dev I had no report so far of glitches. I'll follow up again with them this week.
Issue 583351 has been merged into this issue.
Issue 487060 has been merged into this issue.

Sign in to add a comment