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 ?
Created attachment 201814 [details] process sample sample taken when sound playback got stuck
Looks like it's using <audio> here, and is unrelated to Web Audio API
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.
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.
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/
Created attachment 215877 [details] HTML5 audio timing test page (loads hosted demo MP3)
(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]
Seems like we should be caching the value of [m_avPlayer rate].
Created attachment 215945 [details] WIP Patch
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?
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.
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
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
Created attachment 215955 [details] Patch
Created attachment 215974 [details] Patch Removed the CMTimebase functionality, as it requires Mavericks. Instead, we'll enable the HTMLMediaElement-level curretTime caching.
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?
(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!
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. :)
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!
Created attachment 216063 [details] Patch for landing
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.
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
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
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
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
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
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
Committed r158745: <http://trac.webkit.org/changeset/158745>