- Revision
- 280468
- Author
- [email protected]
- Date
- 2021-07-29 23:43:48 -0700 (Thu, 29 Jul 2021)
Log Message
Missing playing events when the ready state becomes HAVE_FUTURE_DATA/HAVE_ENOUGH_DATA from HAVE_METADATA state
https://bugs.webkit.org/show_bug.cgi?id=228531
Reviewed by Eric Carlson.
Source/WebCore:
The main issue is that missing playing event when the ready state becomes HAVE_FUTURE_DATA or HAVE_ENOUGH_DATA from HAVE_METADATA.
According to the specification, we need to "notify about playing" in the following cases:
- If the previous ready state was HAVE_CURRENT_DATA or less, and the new ready state is HAVE_FUTURE_DATA, and it's not paused.
- If the new ready state is HAVE_ENOUGH_DATA, and it's eligible for autoplay
The implementation didn't cover these cases and had web compatibility issues.
We also should move scheduleNotifyAboutPlaying from setPlaying to playInternal and checks the ready state.
- Without this change, playing event is fired twice. The first one is fired by setReadyState, and the second is called from setPlaying.
- According to the specification, scheduleNotifyAboutPlaying should be in "internal play steps" and check the ready state.
Checking ready state fixes the issue where playing event is fired twice.
Test: media/media-source/media-source-monitor-playing-event.html
* html/HTMLMediaElement.cpp:
(WebCore::HTMLMediaElement::setReadyState): Added missing scheduleNotifyAboutPlaying calls. Added do-while to make the implementation similar to the specification text
(WebCore::HTMLMediaElement::playInternal): Added scheduleNotifyAboutPlaying call.
According to the specification, "internal play steps" should "notify about playing" when
the ready state is HAVE_FUTURE_DATA or HAVE_ENOUGH_DATA.
(WebCore::HTMLMediaElement::setPlaying): Removed scheduleNotifyAboutPlaying call because playInternal now calls scheduleNotifyAboutPlaying instead.
Reference:
- https://html.spec.whatwg.org/multipage/media.html#internal-play-steps
- https://html.spec.whatwg.org/multipage/media.html#ready-states
LayoutTests:
Added the testcase to checks if the playing event is fired correctly on the ready state changes.
* media/media-source/media-source-monitor-playing-event-expected.txt: Added.
* media/media-source/media-source-monitor-playing-event.html: Added.
Modified Paths
Added Paths
Diff
Modified: trunk/LayoutTests/ChangeLog (280467 => 280468)
--- trunk/LayoutTests/ChangeLog 2021-07-30 03:53:49 UTC (rev 280467)
+++ trunk/LayoutTests/ChangeLog 2021-07-30 06:43:48 UTC (rev 280468)
@@ -1,3 +1,15 @@
+2021-07-29 Tomoki Imai <[email protected]>
+
+ Missing playing events when the ready state becomes HAVE_FUTURE_DATA/HAVE_ENOUGH_DATA from HAVE_METADATA state
+ https://bugs.webkit.org/show_bug.cgi?id=228531
+
+ Reviewed by Eric Carlson.
+
+ Added the testcase to checks if the playing event is fired correctly on the ready state changes.
+
+ * media/media-source/media-source-monitor-playing-event-expected.txt: Added.
+ * media/media-source/media-source-monitor-playing-event.html: Added.
+
2021-07-29 Myles C. Maxfield <[email protected]>
Stop building WebGPU and the WHLSL compiler to decrease binary size
Added: trunk/LayoutTests/media/media-source/media-source-monitor-playing-event-expected.txt (0 => 280468)
--- trunk/LayoutTests/media/media-source/media-source-monitor-playing-event-expected.txt (rev 0)
+++ trunk/LayoutTests/media/media-source/media-source-monitor-playing-event-expected.txt 2021-07-30 06:43:48 UTC (rev 280468)
@@ -0,0 +1,33 @@
+This test checks if the playing event fires when the ready state changes from HAVE_METADATA to HAVE_FUTURE_DATA or higher.
+
+EXPECTED (source.readyState == 'closed') OK
+EVENT(loadstart)
+EVENT(sourceopen)
+RUN(sourceBuffer.appendBuffer(initSegment))
+EVENT(loadedmetadata)
+EVENT(updateend)
+video.readyState : HAVE_METADATA
+RUN(sourceBuffer.appendBuffer(sample))
+EVENT(updateend)
+video.readyState : HAVE_FUTURE_DATA
+RUN(sourceBuffer.appendBuffer(sample))
+EVENT(loadeddata)
+EVENT(canplay)
+EVENT(updateend)
+EVENT(canplaythrough)
+EVENT(playing)
+video.readyState : HAVE_ENOUGH_DATA
+RUN(sourceBuffer.remove(0,10))
+EVENT(updateend)
+EVENT(waiting)
+video.readyState : HAVE_METADATA
+RUN(sourceBuffer.appendBuffer(sample))
+EVENT(updateend)
+video.readyState : HAVE_METADATA
+RUN(sourceBuffer.appendBuffer(sample))
+EVENT(updateend)
+EVENT(canplay)
+EVENT(playing)
+video.readyState : HAVE_ENOUGH_DATA
+END OF TEST
+
Added: trunk/LayoutTests/media/media-source/media-source-monitor-playing-event.html (0 => 280468)
--- trunk/LayoutTests/media/media-source/media-source-monitor-playing-event.html (rev 0)
+++ trunk/LayoutTests/media/media-source/media-source-monitor-playing-event.html 2021-07-30 06:43:48 UTC (rev 280468)
@@ -0,0 +1,92 @@
+<!DOCTYPE html>
+<html>
+<head>
+ <title>media-source-monitor-playing-event</title>
+ <script src=""
+ <script src=""
+ <script>
+ var source;
+ var sourceBuffer;
+ var initSegment;
+ var sample;
+ var handleVideoEvents = [
+ "loadstart",
+ "waiting",
+ "loadedmetadata",
+ "loadeddata",
+ "canplay",
+ "canplaythrough",
+ "pause",
+ "ended",
+ ];
+ var readyStateString = [
+ "HAVE_NOTHING",
+ "HAVE_METADATA",
+ "HAVE_CURRENT_DATA",
+ "HAVE_FUTURE_DATA",
+ "HAVE_ENOUGH_DATA"
+ ];
+
+ if (window.internals)
+ internals.initializeMockMediaSource();
+
+ window.addEventListener('load', async() => {
+ findMediaElement();
+
+ for (var i = 0; i < handleVideoEvents.length ; i++)
+ waitForEvent(handleVideoEvents[i]);
+
+ source = new MediaSource();
+ testExpected('source.readyState', 'closed');
+
+ const videoSource = document.createElement('source');
+ videoSource.type = 'video/mock; codecs=mock';
+ videoSource.src = ""
+ video.appendChild(videoSource);
+ await waitFor(source, 'sourceopen');
+
+ sourceBuffer = source.addSourceBuffer("video/mock; codecs=mock");
+ initSegment = makeAInit(100, [makeATrack(1, 'mock', TRACK_KIND.VIDEO)]);
+ run('sourceBuffer.appendBuffer(initSegment)');
+ await waitFor(sourceBuffer, 'updateend');
+
+ consoleWrite('video.readyState : ' + readyStateString[video.readyState]);
+ sample = makeASample(0, 0, 1, 1, 1, SAMPLE_FLAG.SYNC, 1);
+ // This append changes ready state to HAVE_FUTURE_DATA.
+ run('sourceBuffer.appendBuffer(sample)');
+ await waitFor(sourceBuffer, 'updateend');
+
+ consoleWrite('video.readyState : ' + readyStateString[video.readyState]);
+ sample = makeASample(1, 1, 9, 1, 1, SAMPLE_FLAG.SYNC, 1);
+ // This append changes the ready state to HAVE_ENOUGH_DATA and fires the playing event.
+ run('sourceBuffer.appendBuffer(sample)');
+ await Promise.all([waitFor(mediaElement, 'playing'), waitFor(sourceBuffer, 'updateend')]);
+
+ consoleWrite('video.readyState : ' + readyStateString[video.readyState]);
+ // This remove changes ready state to HAVE_METADATA.
+ run('sourceBuffer.remove(0,10)');
+ await waitFor(sourceBuffer, 'updateend');
+
+ await sleepFor(1000);
+
+ consoleWrite('video.readyState : ' + readyStateString[video.readyState]);
+ sample = makeASample(0, 0, 1, 1, 1, SAMPLE_FLAG.SYNC, 1);
+ run('sourceBuffer.appendBuffer(sample)');
+ await waitFor(sourceBuffer, 'updateend');
+
+ consoleWrite('video.readyState : ' + readyStateString[video.readyState]);
+ sample = makeASample(1, 1, 9, 1, 1, SAMPLE_FLAG.SYNC, 1);
+ // This append changes the ready state to HAVE_ENOUGH_DATA and fires the playing event.
+ run('sourceBuffer.appendBuffer(sample)');
+ await Promise.all([waitFor(mediaElement, 'playing'), waitFor(sourceBuffer, 'updateend')]);
+
+ consoleWrite('video.readyState : ' + readyStateString[video.readyState]);
+ endTest();
+ });
+ </script>
+</head>
+<body>
+ <div>This test checks if the playing event fires when the ready state changes from HAVE_METADATA to HAVE_FUTURE_DATA or higher.</div>
+ <video autoplay></video>
+</body>
+</html>
Modified: trunk/Source/WebCore/ChangeLog (280467 => 280468)
--- trunk/Source/WebCore/ChangeLog 2021-07-30 03:53:49 UTC (rev 280467)
+++ trunk/Source/WebCore/ChangeLog 2021-07-30 06:43:48 UTC (rev 280468)
@@ -1,3 +1,34 @@
+2021-07-29 Tomoki Imai <[email protected]>
+
+ Missing playing events when the ready state becomes HAVE_FUTURE_DATA/HAVE_ENOUGH_DATA from HAVE_METADATA state
+ https://bugs.webkit.org/show_bug.cgi?id=228531
+
+ Reviewed by Eric Carlson.
+
+ The main issue is that missing playing event when the ready state becomes HAVE_FUTURE_DATA or HAVE_ENOUGH_DATA from HAVE_METADATA.
+ According to the specification, we need to "notify about playing" in the following cases:
+ - If the previous ready state was HAVE_CURRENT_DATA or less, and the new ready state is HAVE_FUTURE_DATA, and it's not paused.
+ - If the new ready state is HAVE_ENOUGH_DATA, and it's eligible for autoplay
+ The implementation didn't cover these cases and had web compatibility issues.
+
+ We also should move scheduleNotifyAboutPlaying from setPlaying to playInternal and checks the ready state.
+ - Without this change, playing event is fired twice. The first one is fired by setReadyState, and the second is called from setPlaying.
+ - According to the specification, scheduleNotifyAboutPlaying should be in "internal play steps" and check the ready state.
+ Checking ready state fixes the issue where playing event is fired twice.
+
+ Test: media/media-source/media-source-monitor-playing-event.html
+
+ * html/HTMLMediaElement.cpp:
+ (WebCore::HTMLMediaElement::setReadyState): Added missing scheduleNotifyAboutPlaying calls. Added do-while to make the implementation similar to the specification text
+ (WebCore::HTMLMediaElement::playInternal): Added scheduleNotifyAboutPlaying call.
+ According to the specification, "internal play steps" should "notify about playing" when
+ the ready state is HAVE_FUTURE_DATA or HAVE_ENOUGH_DATA.
+ (WebCore::HTMLMediaElement::setPlaying): Removed scheduleNotifyAboutPlaying call because playInternal now calls scheduleNotifyAboutPlaying instead.
+
+ Reference:
+ - https://html.spec.whatwg.org/multipage/media.html#internal-play-steps
+ - https://html.spec.whatwg.org/multipage/media.html#ready-states
+
2021-07-29 Myles C. Maxfield <[email protected]>
Stop building WebGPU and the WHLSL compiler to decrease binary size
Modified: trunk/Source/WebCore/html/HTMLMediaElement.cpp (280467 => 280468)
--- trunk/Source/WebCore/html/HTMLMediaElement.cpp 2021-07-30 03:53:49 UTC (rev 280467)
+++ trunk/Source/WebCore/html/HTMLMediaElement.cpp 2021-07-30 06:43:48 UTC (rev 280468)
@@ -2403,65 +2403,105 @@
}
}
- if (m_readyState >= HAVE_METADATA && oldState < HAVE_METADATA) {
- prepareMediaFragmentURI();
- scheduleEvent(eventNames().durationchangeEvent);
- scheduleResizeEvent();
- scheduleEvent(eventNames().loadedmetadataEvent);
+ // Apply the first applicable set of substeps from the following list:
+ do {
+ // FIXME: The specification seems to only say HAVE_METADATA
+ // explicitly (rather than or higher) for this state. It's unclear
+ // if/how things like loadedmetadataEvent should happen if
+ // we go directly from below HAVE_METADATA to higher than
+ // HAVE_METADATA.
+ if (m_readyState >= HAVE_METADATA && oldState < HAVE_METADATA) {
+ prepareMediaFragmentURI();
+ scheduleEvent(eventNames().durationchangeEvent);
+ scheduleResizeEvent();
+ scheduleEvent(eventNames().loadedmetadataEvent);
#if ENABLE(WIRELESS_PLAYBACK_TARGET)
- if (hasEventListeners(eventNames().webkitplaybacktargetavailabilitychangedEvent))
- enqueuePlaybackTargetAvailabilityChangedEvent();
+ if (hasEventListeners(eventNames().webkitplaybacktargetavailabilitychangedEvent))
+ enqueuePlaybackTargetAvailabilityChangedEvent();
#endif
- m_initiallyMuted = m_volume < 0.05 || muted();
+ m_initiallyMuted = m_volume < 0.05 || muted();
- updateRenderer();
+ updateRenderer();
- if (is<MediaDocument>(document()))
- downcast<MediaDocument>(document()).mediaElementNaturalSizeChanged(expandedIntSize(m_player->naturalSize()));
+ if (is<MediaDocument>(document()))
+ downcast<MediaDocument>(document()).mediaElementNaturalSizeChanged(expandedIntSize(m_player->naturalSize()));
- logMediaLoadRequest(document().page(), m_player->engineDescription(), String(), true);
+ logMediaLoadRequest(document().page(), m_player->engineDescription(), String(), true);
#if ENABLE(WIRELESS_PLAYBACK_TARGET)
- scheduleUpdateMediaState();
+ scheduleUpdateMediaState();
#endif
- mediaSession().clientCharacteristicsChanged();
- }
+ mediaSession().clientCharacteristicsChanged();
- if (m_readyState >= HAVE_CURRENT_DATA && oldState < HAVE_CURRENT_DATA) {
- if (!m_haveFiredLoadedData) {
- m_haveFiredLoadedData = true;
- scheduleEvent(eventNames().loadeddataEvent);
- // FIXME: It's not clear that it's correct to skip these this operation just
- // because m_haveFiredLoadedData is already true. At one time we were skipping
- // the call to setShouldDelayLoadEvent, which was definitely incorrect.
- applyMediaFragmentURI();
+ // As the spec only mentiones HAVE_METADATA, run the later
+ // steps if we are moving to a higher state.
+ if (m_readyState == HAVE_METADATA)
+ break;
}
- setShouldDelayLoadEvent(false);
- }
- if (m_readyState == HAVE_FUTURE_DATA && oldState <= HAVE_CURRENT_DATA && tracksAreReady)
- scheduleEvent(eventNames().canplayEvent);
+ if (m_readyState >= HAVE_CURRENT_DATA && oldState < HAVE_CURRENT_DATA) {
+ if (!m_haveFiredLoadedData) {
+ m_haveFiredLoadedData = true;
+ scheduleEvent(eventNames().loadeddataEvent);
+ // FIXME: It's not clear that it's correct to skip these this operation just
+ // because m_haveFiredLoadedData is already true. At one time we were skipping
+ // the call to setShouldDelayLoadEvent, which was definitely incorrect.
+ applyMediaFragmentURI();
+ }
+ setShouldDelayLoadEvent(false);
- if (m_readyState == HAVE_ENOUGH_DATA && oldState < HAVE_ENOUGH_DATA && tracksAreReady) {
- if (oldState <= HAVE_CURRENT_DATA)
+ // If the new ready state is HAVE_FUTURE_DATA or HAVE_ENOUGH_DATA, then the relevant steps below must then be run also.
+ if (m_readyState < HAVE_FUTURE_DATA)
+ break;
+ }
+
+ if (!tracksAreReady)
+ break;
+
+ if (m_readyState == HAVE_FUTURE_DATA && oldState <= HAVE_CURRENT_DATA) {
scheduleEvent(eventNames().canplayEvent);
- scheduleEvent(eventNames().canplaythroughEvent);
+ // If the element’s paused attribute is false, the user agent must queue a task to fire a simple event named playing at the element.
+ if (!paused())
+ scheduleNotifyAboutPlaying();
+ break;
+ }
- auto canTransition = canTransitionFromAutoplayToPlay();
- if (canTransition) {
- m_paused = false;
- setShowPosterFlag(false);
- invalidateCachedTime();
- setAutoplayEventPlaybackState(AutoplayEventPlaybackState::StartedWithoutUserGesture);
- m_playbackStartedTime = currentMediaTime().toDouble();
- scheduleEvent(eventNames().playEvent);
- } else if (canTransition.error() == MediaPlaybackDenialReason::UserGestureRequired) {
- ALWAYS_LOG(LOGIDENTIFIER, "Autoplay blocked, user gesture required");
- setAutoplayEventPlaybackState(AutoplayEventPlaybackState::PreventedAutoplay);
+ if (m_readyState == HAVE_ENOUGH_DATA && oldState < HAVE_ENOUGH_DATA) {
+ // If the previous ready state was HAVE_CURRENT_DATA or less,
+ // the user agent must queue a media element task given the media element to fire an event named canplay at the element,
+ // and, if the element's paused attribute is false, notify about playing for the element.
+ if (oldState <= HAVE_CURRENT_DATA) {
+ scheduleEvent(eventNames().canplayEvent);
+ if (!paused())
+ scheduleNotifyAboutPlaying();
+ }
+
+ // The user agent must queue a media element task given the media element to fire an event named canplaythrough at the element.
+ scheduleEvent(eventNames().canplaythroughEvent);
+
+ // If the element is not eligible for autoplay, then the user agent must abort these substeps.
+ // The user agent may run the following substeps:
+ // Set the paused attribute to false.
+ // If the element's show poster flag is true, set it to false and run the time marches on steps.
+ // Queue a media element task given the element to fire an event named play at the element.
+ // Notify about playing for the element.
+ auto canTransition = canTransitionFromAutoplayToPlay();
+ if (canTransition) {
+ m_paused = false;
+ setShowPosterFlag(false);
+ invalidateCachedTime();
+ setAutoplayEventPlaybackState(AutoplayEventPlaybackState::StartedWithoutUserGesture);
+ m_playbackStartedTime = currentMediaTime().toDouble();
+ scheduleEvent(eventNames().playEvent);
+ scheduleNotifyAboutPlaying();
+ } else if (canTransition.error() == MediaPlaybackDenialReason::UserGestureRequired) {
+ ALWAYS_LOG(LOGIDENTIFIER, "Autoplay blocked, user gesture required");
+ setAutoplayEventPlaybackState(AutoplayEventPlaybackState::PreventedAutoplay);
+ }
}
- }
+ } while (false);
// If we transition to the Future Data state and we're about to begin playing, ensure playback is actually permitted first,
// honoring any playback denial reasons such as the requirement of a user gesture.
@@ -3545,8 +3585,14 @@
scheduleEvent(eventNames().playEvent);
+ // If the media element's readyState attribute has the value HAVE_NOTHING, HAVE_METADATA, or HAVE_CURRENT_DATA,
+ // queue a media element task given the media element to fire an event named waiting at the element.
+ // Otherwise, the media element's readyState attribute has the value HAVE_FUTURE_DATA or HAVE_ENOUGH_DATA:
+ // notify about playing for the element.
if (m_readyState <= HAVE_CURRENT_DATA)
scheduleEvent(eventNames().waitingEvent);
+ else
+ scheduleNotifyAboutPlaying();
} else if (m_readyState >= HAVE_FUTURE_DATA)
scheduleResolvePendingPlayPromises();
@@ -5482,9 +5528,6 @@
m_playing = playing;
- if (m_playing)
- scheduleNotifyAboutPlaying();
-
document().updateIsPlayingMedia();
#if ENABLE(WIRELESS_PLAYBACK_TARGET)
