Audio glitches on Posix machines when number of file descriptors are high |
|||
Issue descriptionOriginal 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.
,
May 17 2017
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.
,
May 17 2017
And thanks Dale for the history in comment #1.
,
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
,
May 23 2017
I have tested to go over 1023 FDs with appr.tc; no issues observed.
,
May 23 2017
Thanks for the fix! When will this roll out to the stable channel?
,
May 23 2017
Beta is expected early-mid June, stable end of July.
,
May 23 2017
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!
,
May 23 2017
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.
,
May 23 2017
Is this fix in Version 60.0.3100.0 (Official Build) dev (64-bit) version? If not what version will it be in?
,
May 23 2017
Re #10: it's not in that version. It will be in the next Canary and Dev. For ChromeOS it's Dev.
,
May 23 2017
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.
,
May 25 2017
I've contacted a number of people who had glitches fairly recently to ask if someone is willing to verify in Dev channel.
,
Jun 15 2017
The fix is in 60.0.3109.0.
,
Jun 19 2017
Any data or reviews that indicate the impact of this fix?
,
Jun 19 2017
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.
,
Aug 22 2017
Issue 583351 has been merged into this issue.
,
Oct 18 2017
Issue 487060 has been merged into this issue. |
|||
►
Sign in to add a comment |
|||
Comment 1 by dalecur...@chromium.org
, May 16 2017