RESOLVED FIXED 116145
Playing many sounds with HTML5 Audio makes WebKit unresponsive
https://bugs.webkit.org/show_bug.cgi?id=116145
Summary Playing many sounds with HTML5 Audio makes WebKit unresponsive
Dieter Komendera
Reported 2013-05-15 03:30:03 PDT
Created attachment 201813 [details] test case Tested with WebKit nightly r150098, Safari Version 6.0.4, Mac OS X 10.8.3 Playing sounds with HTML5 Audio regularly (e.g every 0.5 seconds) makes WebKit unresponsive after a while (about 1 or 2 minutes), eventually making Safari beachballing and sound playback stops. To reproduce: * open the attached file * let sounds play for a while (about 1 or 2 minutes) * notice that Safari starts beachballing and gets unresponsive, sound playback gets stuck Maybe related to https://bugs.webkit.org/show_bug.cgi?id=72698 ?
Attachments
test case (263 bytes, text/html)
2013-05-15 03:30 PDT, Dieter Komendera
no flags
process sample (184.05 KB, text/plain)
2013-05-15 03:32 PDT, Dieter Komendera
no flags
HTML5 audio timing test page (loads hosted demo MP3) (7.04 KB, text/html)
2013-11-03 14:44 PST, Scott Schiller
no flags
WIP Patch (32.53 KB, patch)
2013-11-04 12:41 PST, Jer Noble
no flags
Archive of layout-test-results from webkit-ews-14 for mac-mountainlion-wk2 (705.15 KB, application/zip)
2013-11-04 14:21 PST, Build Bot
no flags
Patch (28.89 KB, patch)
2013-11-04 14:35 PST, Jer Noble
no flags
Patch (25.73 KB, patch)
2013-11-04 17:00 PST, Jer Noble
eric.carlson: review+
Patch for landing (30.11 KB, patch)
2013-11-05 13:09 PST, Jer Noble
buildbot: commit-queue-
Archive of layout-test-results from webkit-ews-15 for mac-mountainlion-wk2 (454.08 KB, application/zip)
2013-11-05 14:02 PST, Build Bot
no flags
Archive of layout-test-results from webkit-ews-02 for mac-mountainlion (563.56 KB, application/zip)
2013-11-05 14:58 PST, Build Bot
no flags
Archive of layout-test-results from webkit-ews-01 for mac-mountainlion (518.83 KB, application/zip)
2013-11-05 15:14 PST, Build Bot
no flags
Dieter Komendera
Comment 1 2013-05-15 03:32:04 PDT
Created attachment 201814 [details] process sample sample taken when sound playback got stuck
Chris Rogers
Comment 2 2013-05-15 12:02:52 PDT
Looks like it's using <audio> here, and is unrelated to Web Audio API
Jer Noble
Comment 3 2013-05-15 12:52:28 PDT
Looks like, for this specific case, there could be a large gain by changing this line in HTMLMediaElement::shouldDisableSleep() from: return m_player && !m_player->paused() && hasVideo() && hasAudio() && !loop(); to: return m_player && hasVideo() && hasAudio() && !loop() && !m_player->paused(); Or, we could cache the value of [m_avPlayer rate] so as not to call back into AVFoundation so often.
Dieter Komendera
Comment 4 2013-10-11 03:34:56 PDT
I just retested with my testcase with Safari Version 7.0 (9537.71) on Mavericks GM. It looks like the situation is somewhat improved, as I can't get Safari be unresponsive any more. Though I noticed that the resource consumption is pretty high, as Activity Monitor shows for the Safari Web Content process an always growing thread count (tested to more than 1000) and allocating several hundred MB of memory.
Scott Schiller
Comment 5 2013-11-03 14:43:04 PST
I found the new Safari 7.0 (9537.71) on OS X 10.9 had notable stuttering and UI "jank" when playing a number of HTML5 Audio() objects simultaneously, so I made a test page to demonstrate the effect. Live demo page: http://isflashdeadyet.com/tests/html5-audio-timing/ As far as tests show, Chrome, IE 9 and Firefox do not become intermittently unresponsive in the way Safari does. I first noted the issue when working on a browser-based game prototype. http://schillmania.com/armor-alley/
Scott Schiller
Comment 6 2013-11-03 14:44:29 PST
Created attachment 215877 [details] HTML5 audio timing test page (loads hosted demo MP3)
Jer Noble
Comment 7 2013-11-03 21:22:46 PST
(In reply to comment #6) > Created an attachment (id=215877) [details] > HTML5 audio timing test page (loads hosted demo MP3) Looks like we're spending a large amount of time asking whether we're paused: 1855 Thread_1464288 DispatchQueue_1: com.apple.main-thread (serial) + 1855 start (in libdyld.dylib) + 1 [0x7fff8720d5fd] + 1855 ??? (in com.apple.WebKit.WebContent) load address 0x10a94b000 + 0xba0 [0x10a94bba0] + 1855 xpc_main (in libxpc.dylib) + 399 [0x7fff8c927b2e] + 1855 _xpc_main (in XPCService) + 385 [0x7fff85befc0f] + 1855 NSApplicationMain (in AppKit) + 940 [0x7fff8d60f803] + 1855 -[NSApplication run] (in AppKit) + 553 [0x7fff8d6249cc] + 1855 -[NSApplication nextEventMatchingMask:untilDate:inMode:dequeue:] (in AppKit) + 122 [0x7fff8d6308db] + 1855 _DPSNextEvent (in AppKit) + 1434 [0x7fff8d63128e] + 1855 _BlockUntilNextEventMatchingListInModeWithFilter (in HIToolbox) + 65 [0x7fff85e23abc] + 1855 ReceiveNextEventCommon (in HIToolbox) + 479 [0x7fff85e23cb7] + 1855 RunCurrentEventLoopInMode (in HIToolbox) + 226 [0x7fff85e23f0d] + 1849 CFRunLoopRunSpecific (in CoreFoundation) + 309 [0x7fff88803275] + ! 1563 __CFRunLoopRun (in CoreFoundation) + 1525 [0x7fff88803aa5] + ! : 1563 __CFRunLoopDoTimers (in CoreFoundation) + 298 [0x7fff888b976a] + ! : 1563 __CFRunLoopDoTimer (in CoreFoundation) + 1151 [0x7fff8884825f] + ! : 1563 __CFRUNLOOP_IS_CALLING_OUT_TO_A_TIMER_CALLBACK_FUNCTION__ (in CoreFoundation) + 20 [0x7fff88848724] + ! : 1559 WTF::dispatchFunctionsFromMainThread() (in JavaScriptCore) + 308 [0x7fff8849d6d4] + ! : | 1558 non-virtual thunk to WebCore::HTMLMediaElement::mediaPlayerRateChanged(WebCore::MediaPlayer*) (in WebCore) + 94 [0x7fff8680c95e] + ! : | + 1558 WebCore::HTMLMediaElement::updateDisableSleep() (in WebCore) + 18 [0x7fff8680c992] + ! : | + 1558 WebCore::HTMLMediaElement::shouldDisableSleep() const (in WebCore) + 34 [0x7fff8680ca42] + ! : | + 1558 WebCore::MediaPlayer::paused() const (in WebCore) + 17 [0x7fff8670ef81] + ! : | + 1558 WebCore::MediaPlayerPrivateAVFoundation::paused() const (in WebCore) + 24 [0x7fff8680b6e8] + ! : | + 1558 -[AVPlayer rate] (in AVFoundation) + 120 [0x7fff8b5b508a] + ! : | + 1558 -[AVPropertyStorage objectForKey:defaultObjectBlock:] (in AVFoundation) + 79 [0x7fff8b5b0fa3] + ! : | + 1558 __16-[AVPlayer rate]_block_invoke (in AVFoundation) + 37 [0x7fff8b60c4d5] + ! : | + 1558 -[AVPlayer _rate] (in AVFoundation) + 70 [0x7fff8b5b50eb] + ! : | + 1558 playerasync_GetRate (in MediaToolbox) + 123 [0x7fff89b855de] FigPlayer_Async.c:2509 + ! : | + 1558 playerasync_tryLockSynchronousCommand (in MediaToolbox) + 83 [0x7fff89b850b7] FigPlayer_Async.c:870 + ! : | + 1558 _dispatch_semaphore_wait_slow (in libdispatch.dylib) + 206 [0x7fff83033a15] + ! : | + 1558 semaphore_wait_trap (in libsystem_kernel.dylib) + 10 [0x7fff889f7a56]
Jer Noble
Comment 8 2013-11-03 21:46:51 PST
Seems like we should be caching the value of [m_avPlayer rate].
Jer Noble
Comment 9 2013-11-04 12:41:34 PST
Created attachment 215945 [details] WIP Patch
Eric Carlson
Comment 10 2013-11-04 13:06:06 PST
Comment on attachment 215945 [details] WIP Patch View in context: https://bugs.webkit.org/attachment.cgi?id=215945&action=review > Source/WebCore/platform/graphics/avfoundation/MediaPlayerPrivateAVFoundation.cpp:-844 > - case Notification::ItemTracksChanged: > - tracksChanged(); > - updateStates(); > - break; > - case Notification::ItemStatusChanged: > - updateStates(); > - break; > - case Notification::ItemSeekableTimeRangesChanged: > - seekableTimeRangesChanged(); > - updateStates(); > - break; > - case Notification::ItemLoadedTimeRangesChanged: > - loadedTimeRangesChanged(); > - updateStates(); > - break; > - case Notification::ItemPresentationSizeChanged: > - sizeChanged(); > - updateStates(); > - break; > - case Notification::ItemIsPlaybackLikelyToKeepUpChanged: > - updateStates(); > - break; > - case Notification::ItemIsPlaybackBufferEmptyChanged: > - updateStates(); > - break; > - case Notification::ItemIsPlaybackBufferFullChanged: > - updateStates(); > - break; > - case Notification::PlayerRateChanged: > - updateStates(); > - rateChanged(); > - break; Won't this break Windows? > Source/WebCore/platform/graphics/avfoundation/MediaPlayerPrivateAVFoundation.cpp:-861 > - case Notification::DurationChanged: > - invalidateCachedDuration(); > - break; Ditto. > Source/WebCore/platform/graphics/avfoundation/objc/MediaPlayerPrivateAVFoundationObjC.mm:1618 > +void MediaPlayerPrivateAVFoundationObjC::playbackLikelyToKeepUpDidChange(bool likelyToKeepUp) > +{ ASSERT(m_pendingStatusChanges); > Source/WebCore/platform/graphics/avfoundation/objc/MediaPlayerPrivateAVFoundationObjC.mm:1632 > +void MediaPlayerPrivateAVFoundationObjC::playbackBufferEmptyDidChange(bool bufferEmpty) > +{ Ditto. > Source/WebCore/platform/graphics/avfoundation/objc/MediaPlayerPrivateAVFoundationObjC.mm:1646 > +void MediaPlayerPrivateAVFoundationObjC::playbackBufferFullDidChange(bool bufferFull) > +{ Ditto. > Source/WebCore/platform/graphics/avfoundation/objc/MediaPlayerPrivateAVFoundationObjC.mm:1827 > + callOnMainThread(WTF::bind(&MediaPlayerPrivateAVFoundationObjC::playerItemStatusDidChange, m_callback, [newValue intValue])); What will happen if MediaPlayerPrivateAVFoundationObjC::cancelLoad is called before the callOnMainThread completes? Maybe it makes sense to trampoline through a method in the helper object so you can check m_callback again?
Jer Noble
Comment 11 2013-11-04 13:12:29 PST
Comment on attachment 215945 [details] WIP Patch View in context: https://bugs.webkit.org/attachment.cgi?id=215945&action=review >> Source/WebCore/platform/graphics/avfoundation/MediaPlayerPrivateAVFoundation.cpp:-844 >> - break; > > Won't this break Windows? Ahh, good point. AVCF would have to adopt a similar callOnMainThread(WTF::bind(...)) semantic. It's safe to put these in for now though. >> Source/WebCore/platform/graphics/avfoundation/objc/MediaPlayerPrivateAVFoundationObjC.mm:1618 >> +{ > > ASSERT(m_pendingStatusChanges); Added. >> Source/WebCore/platform/graphics/avfoundation/objc/MediaPlayerPrivateAVFoundationObjC.mm:1827 >> + callOnMainThread(WTF::bind(&MediaPlayerPrivateAVFoundationObjC::playerItemStatusDidChange, m_callback, [newValue intValue])); > > What will happen if MediaPlayerPrivateAVFoundationObjC::cancelLoad is called before the callOnMainThread completes? Maybe it makes sense to trampoline through a method in the helper object so you can check m_callback again? Perhaps we should only trampoline to the main thread if we're not there already. I think that on Mavericks these notifications are always issued on the main thread now.
Build Bot
Comment 12 2013-11-04 14:21:25 PST
Comment on attachment 215945 [details] WIP Patch Attachment 215945 [details] did not pass mac-wk2-ews (mac-wk2): Output: http://webkit-queues.appspot.com/results/19608842 New failing tests: fullscreen/video-controls-timeline.html compositing/self-painting-layers2.html compositing/video-page-visibility.html fast/events/media-focus-in-standalone-media-document.html http/tests/media/video-play-progress.html compositing/overflow/overflow-compositing-descendant.html media/W3C/audio/events/event_canplay.html compositing/layers-inside-overflow-scroll.html compositing/self-painting-layers.html compositing/overflow/scroll-ancestor-update.html http/tests/media/video-useragent.html compositing/geometry/clipped-video-controller.html compositing/regions/transform-transparent-positioned-video-inside-region.html http/tests/canvas/webgl/origin-clean-conformance.html compositing/geometry/video-opacity-overlay.html fast/canvas/webgl/oes-texture-half-float-not-supported.html compositing/geometry/video-fixed-scrolling.html accessibility/media-element.html http/tests/media/video-load-suspend.html http/tests/media/video-query-url.html fast/events/tabindex-focus-blur-all.html compositing/video/video-poster.html http/tests/media/media-document.html media/audio-constructor-preload.html media/audio-concurrent-supported.html compositing/visibility/visibility-simple-video-layer.html media/W3C/audio/paused/paused_false_during_play.html http/tests/media/video-load-twice.html fast/canvas/webgl/tex-image-and-sub-image-2d-with-video.html http/tests/media/video-redirect.html
Build Bot
Comment 13 2013-11-04 14:21:27 PST
Created attachment 215951 [details] Archive of layout-test-results from webkit-ews-14 for mac-mountainlion-wk2 The attached test failures were seen while running run-webkit-tests on the mac-wk2-ews. Bot: webkit-ews-14 Port: mac-mountainlion-wk2 Platform: Mac OS X 10.8.5
Jer Noble
Comment 14 2013-11-04 14:35:44 PST
Jer Noble
Comment 15 2013-11-04 17:00:49 PST
Created attachment 215974 [details] Patch Removed the CMTimebase functionality, as it requires Mavericks. Instead, we'll enable the HTMLMediaElement-level curretTime caching.
Eric Carlson
Comment 16 2013-11-04 17:09:18 PST
Comment on attachment 215974 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=215974&action=review > Source/WebCore/platform/graphics/avfoundation/objc/MediaPlayerPrivateAVFoundationObjC.h:134 > + virtual double maximumDurationToCacheMediaTime() const { return 5; } This will only change the behavior on the Mac. If you don't think it is OK to make this change on Windows yet, please file a bug so we can test fix. > Source/WebCore/platform/graphics/avfoundation/objc/MediaPlayerPrivateAVFoundationObjC.h:256 > + unsigned m_pendingStatusChanges; > + int m_cachedItemStatus; > + bool m_cachedLikelyToKeepUp; > + bool m_cachedBufferEmpty; > + bool m_cachedBufferFull; > + RetainPtr<NSArray> m_cachedSeekableRanges; > + RetainPtr<NSArray> m_cachedLoadedRanges; > + RetainPtr<NSArray> m_cachedTracks; > + bool m_cachedHasEnabledAudio; > + FloatSize m_cachedPresentationSize; > + double m_cachedDuration; > + double m_cachedRate; Nit: sorting these by size will make the object slightly smaller. > Source/WebCore/platform/graphics/avfoundation/objc/MediaPlayerPrivateAVFoundationObjC.mm:249 > + , m_cachedItemStatus(MediaPlayerAVPlayerItemStatusDoesNotExist) > + , m_cachedHasEnabledAudio(false) > + , m_cachedDuration(MediaPlayer::invalidTime()) > + , m_cachedRate(0) Shouldn't you initialize m_pendingStatusChanges, m_cachedLikelyToKeepUp, m_cachedBufferEmpty, and m_cachedBufferFull?
Jer Noble
Comment 17 2013-11-04 17:25:51 PST
(In reply to comment #16) > (From update of attachment 215974 [details]) > View in context: https://bugs.webkit.org/attachment.cgi?id=215974&action=review > > > Source/WebCore/platform/graphics/avfoundation/objc/MediaPlayerPrivateAVFoundationObjC.h:134 > > + virtual double maximumDurationToCacheMediaTime() const { return 5; } > > This will only change the behavior on the Mac. If you don't think it is OK to make this change on Windows yet, please file a bug so we can test fix. I purposefully didn't want to affect the Windows engine. While it's probably fine to turn it on in both engines, it's entirely unrelated to this bug. I'll file another bug for AVCF. > > Source/WebCore/platform/graphics/avfoundation/objc/MediaPlayerPrivateAVFoundationObjC.h:256 > > + unsigned m_pendingStatusChanges; > > + int m_cachedItemStatus; > > + bool m_cachedLikelyToKeepUp; > > + bool m_cachedBufferEmpty; > > + bool m_cachedBufferFull; > > + RetainPtr<NSArray> m_cachedSeekableRanges; > > + RetainPtr<NSArray> m_cachedLoadedRanges; > > + RetainPtr<NSArray> m_cachedTracks; > > + bool m_cachedHasEnabledAudio; > > + FloatSize m_cachedPresentationSize; > > + double m_cachedDuration; > > + double m_cachedRate; > > Nit: sorting these by size will make the object slightly smaller. Will do. > > Source/WebCore/platform/graphics/avfoundation/objc/MediaPlayerPrivateAVFoundationObjC.mm:249 > > + , m_cachedItemStatus(MediaPlayerAVPlayerItemStatusDoesNotExist) > > + , m_cachedHasEnabledAudio(false) > > + , m_cachedDuration(MediaPlayer::invalidTime()) > > + , m_cachedRate(0) > > Shouldn't you initialize m_pendingStatusChanges, m_cachedLikelyToKeepUp, m_cachedBufferEmpty, and m_cachedBufferFull? I totally should!
Jer Noble
Comment 18 2013-11-04 17:27:45 PST
For Dieter and Scott, please note that while this patch eliminates most of the stalls/jankiness by caching as much as possible in WebKit, there are still times where AVFoundation will cause it's own stall by taking a lock on the main thread. Still, with this patch in place, I can get 60fps in Scott's test most of the time. :)
Dieter Komendera
Comment 19 2013-11-05 00:24:44 PST
Jer, sounds good, thanks a bunch! Will give it a ride as soon as it hits the Nightly build. This should enable us to remove the Flash fallback for playing sounds in our game client. Scott, thanks for putting together the testcase!
Jer Noble
Comment 20 2013-11-05 13:09:48 PST
Created attachment 216063 [details] Patch for landing
WebKit Commit Bot
Comment 21 2013-11-05 13:12:42 PST
Attachment 216063 [details] did not pass style-queue: Failed to run "['Tools/Scripts/check-webkit-style', '--diff-files', u'Source/WebCore/ChangeLog', u'Source/WebCore/html/HTMLMediaElement.cpp', u'Source/WebCore/platform/graphics/avfoundation/MediaPlayerPrivateAVFoundation.cpp', u'Source/WebCore/platform/graphics/avfoundation/MediaPlayerPrivateAVFoundation.h', u'Source/WebCore/platform/graphics/avfoundation/objc/MediaPlayerPrivateAVFoundationObjC.h', u'Source/WebCore/platform/graphics/avfoundation/objc/MediaPlayerPrivateAVFoundationObjC.mm']" exit_code: 1 Source/WebCore/platform/graphics/avfoundation/MediaPlayerPrivateAVFoundation.h:111: Extra space before ( in function call [whitespace/parens] [4] Source/WebCore/platform/graphics/avfoundation/MediaPlayerPrivateAVFoundation.h:129: Extra space before ( in function call [whitespace/parens] [4] Total errors found: 2 in 6 files If any of these errors are false positives, please file a bug against check-webkit-style.
Build Bot
Comment 22 2013-11-05 14:01:59 PST
Comment on attachment 216063 [details] Patch for landing Attachment 216063 [details] did not pass mac-wk2-ews (mac-wk2): Output: http://webkit-queues.appspot.com/results/21238013 New failing tests: webgl/1.0.1/conformance/textures/texture-npot-video.html webgl/1.0.2/conformance/textures/texture-npot-video.html
Build Bot
Comment 23 2013-11-05 14:02:01 PST
Created attachment 216078 [details] Archive of layout-test-results from webkit-ews-15 for mac-mountainlion-wk2 The attached test failures were seen while running run-webkit-tests on the mac-wk2-ews. Bot: webkit-ews-15 Port: mac-mountainlion-wk2 Platform: Mac OS X 10.8.5
Build Bot
Comment 24 2013-11-05 14:58:09 PST
Comment on attachment 216063 [details] Patch for landing Attachment 216063 [details] did not pass mac-ews (mac): Output: http://webkit-queues.appspot.com/results/21388001 New failing tests: media/video-loop.html webgl/1.0.1/conformance/textures/texture-npot-video.html webgl/1.0.2/conformance/textures/texture-npot-video.html
Build Bot
Comment 25 2013-11-05 14:58:13 PST
Created attachment 216085 [details] Archive of layout-test-results from webkit-ews-02 for mac-mountainlion The attached test failures were seen while running run-webkit-tests on the mac-ews. Bot: webkit-ews-02 Port: mac-mountainlion Platform: Mac OS X 10.8.5
Build Bot
Comment 26 2013-11-05 15:14:46 PST
Comment on attachment 216063 [details] Patch for landing Attachment 216063 [details] did not pass mac-ews (mac): Output: http://webkit-queues.appspot.com/results/21268029 New failing tests: media/video-loop.html webgl/1.0.1/conformance/textures/texture-npot-video.html webgl/1.0.2/conformance/textures/texture-npot-video.html
Build Bot
Comment 27 2013-11-05 15:14:49 PST
Created attachment 216089 [details] Archive of layout-test-results from webkit-ews-01 for mac-mountainlion The attached test failures were seen while running run-webkit-tests on the mac-ews. Bot: webkit-ews-01 Port: mac-mountainlion Platform: Mac OS X 10.8.5
Jer Noble
Comment 28 2013-11-06 08:14:15 PST
Note You need to log in before you can comment on or make changes to this bug.