Bug 116145 – Playing many sounds with HTML5 Audio makes WebKit unresponsive

• Component: Media
• Status: RESOLVED FIXED

[Dieter Komendera]

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 ?

[Dieter Komendera]

Created attachment 201814 [details]
process sample

sample taken when sound playback got stuck

[Chris Rogers]

Looks like it's using <audio> here, and is unrelated to Web Audio API

[Jer Noble]

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]

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]

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]

Created attachment 215877 [details]
HTML5 audio timing test page (loads hosted demo MP3)

[Jer Noble]

(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]

Seems like we should be caching the value of [m_avPlayer rate].

[Jer Noble]

Created attachment 215945 [details]
WIP Patch

[Eric Carlson]

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 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 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]

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]

Created attachment 215955 [details]
Patch

[Jer Noble]

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 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]

(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]

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]

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]

Created attachment 216063 [details]
Patch for landing

[WebKit Commit Bot]

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 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]

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 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]

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 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]

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]

Committed r158745: <http://trac.webkit.org/changeset/158745>