Sequential WebVTT cues with the same contents can be dropped

https://bugs.webkit.org/show_bug.cgi?id=240893 Reviewed by Eric Carlson. When sequential WebVTT cues are parsed with the same contents, instead of dropping the new cue, extend its endTime of the existing cue to include the endTime of the newly parsed one. * LayoutTests/http/tests/media/hls/hls-webvtt-flashing-expected.txt: Added. * LayoutTests/http/tests/media/hls/hls-webvtt-flashing.html: Added. * LayoutTests/http/tests/media/resources/hls/subtitles/flashSequence0.webvtt: Added. * LayoutTests/http/tests/media/resources/hls/subtitles/prog_flashing.m3u8: Added. * LayoutTests/http/tests/media/resources/hls/test-webvtt-flashing.m3u8: Added. * LayoutTests/platform/glib/TestExpectations: * Source/WebCore/html/track/InbandGenericTextTrack.cpp: (WebCore::InbandGenericTextTrack::newCuesParsed): * Source/WebCore/html/track/InbandWebVTTTextTrack.cpp: (WebCore::InbandWebVTTTextTrack::newCuesParsed): * Source/WebCore/html/track/TextTrack.cpp: (WebCore::TextTrack::matchCue): (WebCore::TextTrack::hasCue): Deleted. * Source/WebCore/html/track/TextTrack.h: (WebCore::TextTrack::hasCue): Canonical link: https://commits.webkit.org/250988@main git-svn-id: https://svn.webkit.org/repository/webkit/trunk@294854 268f45cc-cd09-0410-ab3c-d52691b4dbfc
WebKit · May 26, 2022 · ee32ad0efe9dd47175967717df78fade593317d3 · ee32ad0
1 parent a533bb5
commit ee32ad0efe9dd47175967717df78fade593317d3
Showing 10 changed files with 103 additions and 18 deletions.
diff --git a/LayoutTests/http/tests/media/hls/hls-webvtt-flashing-expected.txt b/LayoutTests/http/tests/media/hls/hls-webvtt-flashing-expected.txt
@@ -0,0 +1,10 @@
+
+EVENT(addtrack)
+EXPECTED (video.textTracks.length == '1') OK
+RUN(video.textTracks[0].mode = 'showing')
+EVENT(cuechange)
+EXPECTED (video.textTracks[0].activeCues.length == '1') OK
+EXPECTED (video.textTracks[0].activeCues[0].text == 'Subtitle Test') OK
+EXPECTED (video.textTracks[0].activeCues[0].endTime > '0.100') OK
+END OF TEST
+
diff --git a/LayoutTests/http/tests/media/hls/hls-webvtt-flashing.html b/LayoutTests/http/tests/media/hls/hls-webvtt-flashing.html
@@ -0,0 +1,35 @@
+<!DOCTYPE html>
+<html>
+<head>
+    <script src=../../media-resources/video-test.js></script>
+    <script>
+        if (window.testRunner) {
+            testRunner.dumpAsText();
+            testRunner.waitUntilDone();
+        }
+
+        function start() {
+            video = document.getElementById('video');
+            video.src = "../resources/hls/test-webvtt-flashing.m3u8";
+            video.play();
+            waitForEventOnceOn(video.textTracks, 'addtrack', trackAdded);
+        }
+
+        function trackAdded() {
+            testExpected("video.textTracks.length", "1");
+            run("video.textTracks[0].mode = 'showing'");
+            waitForEventOnceOn(video.textTracks[0], 'cuechange', cueChanged);
+        }
+
+        function cueChanged() {
+            testExpected("video.textTracks[0].activeCues.length", "1");
+            testExpected("video.textTracks[0].activeCues[0].text", "Subtitle Test");
+            testExpected("video.textTracks[0].activeCues[0].endTime", "0.100", ">");
+            endTest();
+        }
+    </script>
+</head>
+<body onload="start()">
+    <video id="video" muted loop></video>
+</body>
+</html>
diff --git a/LayoutTests/http/tests/media/resources/hls/subtitles/flashSequence0.webvtt b/LayoutTests/http/tests/media/resources/hls/subtitles/flashSequence0.webvtt
@@ -0,0 +1,8 @@
+WEBVTT
+X-TIMESTAMP-MAP=MPEGTS:900000,LOCAL:00:00:00.000
+
+00:00:00.000 --> 00:00:00.100
+Subtitle Test
+
+00:00:00.100 --> 00:00:01.000
+Subtitle Test
diff --git a/LayoutTests/http/tests/media/resources/hls/subtitles/prog_flashing.m3u8 b/LayoutTests/http/tests/media/resources/hls/subtitles/prog_flashing.m3u8
@@ -0,0 +1,8 @@
+#EXTM3U
+#EXT-X-TARGETDURATION:30
+#EXT-X-VERSION:3
+#EXT-X-MEDIA-SEQUENCE:0
+#EXT-X-PLAYLIST-TYPE:VOD
+#EXTINF:10,
+flashSequence0.webvtt
+#EXT-X-ENDLIST
diff --git a/LayoutTests/http/tests/media/resources/hls/test-webvtt-flashing.m3u8 b/LayoutTests/http/tests/media/resources/hls/test-webvtt-flashing.m3u8
@@ -0,0 +1,6 @@
+#EXTM3U
+
+#EXT-X-MEDIA:TYPE=SUBTITLES,GROUP-ID="subtitles",LANGUAGE="en-US",NAME="English",AUTOSELECT=YES,DEFAULT=YES,URI="subtitles/prog_flashing.m3u8"
+
+#EXT-X-STREAM-INF:BANDWIDTH=634451,CODECS="mp4a.40.2, avc1.4d401e",RESOLUTION=640x480,SUBTITLES="subtitles"
+bipbop/prog_index.m3u8
diff --git a/LayoutTests/platform/glib/TestExpectations b/LayoutTests/platform/glib/TestExpectations
@@ -2483,6 +2483,7 @@ webkit.org/b/168373 http/tests/media/track-in-band-hls-metadata-crash.html [ Tim
 webkit.org/b/168373 media/media-fullscreen-loop-inline.html [ Timeout ]
 webkit.org/b/174242 media/media-fullscreen-pause-inline.html [ Skip ]
 webkit.org/b/182108 http/tests/media/hls/hls-webvtt-tracks.html [ Timeout ]
+webkit.org/b/182108 http/tests/media/hls/hls-webvtt-flashing.html [ Timeout ]
 webkit.org/b/137311 media/video-fullscreen-only-playback.html [ Timeout Crash ]
 
 webkit.org/b/194044 imported/w3c/web-platform-tests/media-source/mediasource-addsourcebuffer.html [ Failure ]

diff --git a/Source/WebCore/html/track/InbandGenericTextTrack.cpp b/Source/WebCore/html/track/InbandGenericTextTrack.cpp
@@ -190,12 +190,20 @@ void InbandGenericTextTrack::newCuesParsed()
 {
     for (auto& cueData : parser().takeCues()) {
         auto cue = VTTCue::create(document(), cueData);
-        if (hasCue(cue, TextTrackCue::IgnoreDuration)) {
+        auto existingCue = matchCue(cue, TextTrackCue::IgnoreDuration);
+        if (!existingCue) {
+            INFO_LOG(LOGIDENTIFIER, cue.get());
+            addCue(WTFMove(cue));
+            continue;
+        }
+
+        if (existingCue->endTime() >= cue->endTime()) {
             INFO_LOG(LOGIDENTIFIER, "ignoring already added cue: ", cue.get());
-            return;
+            continue;
         }
-        INFO_LOG(LOGIDENTIFIER, cue.get());
-        addCue(WTFMove(cue));
+
+        ALWAYS_LOG(LOGIDENTIFIER, "extending endTime of existing cue: ", *existingCue, " to ", cue->endTime());
+        existingCue->setEndTime(cue->endTime());
     }
 }
 

diff --git a/Source/WebCore/html/track/InbandWebVTTTextTrack.cpp b/Source/WebCore/html/track/InbandWebVTTTextTrack.cpp
@@ -74,12 +74,20 @@ void InbandWebVTTTextTrack::newCuesParsed()
 {
     for (auto& cueData : parser().takeCues()) {
         auto cue = VTTCue::create(document(), cueData);
-        if (hasCue(cue, TextTrackCue::IgnoreDuration)) {
+        auto existingCue = matchCue(cue, TextTrackCue::IgnoreDuration);
+        if (!existingCue) {
+            INFO_LOG(LOGIDENTIFIER, cue.get());
+            addCue(WTFMove(cue));
+            continue;
+        }
+
+        if (existingCue->endTime() >= cue->endTime()) {
             INFO_LOG(LOGIDENTIFIER, "ignoring already added cue: ", cue.get());
-            return;
+            continue;
         }
-        INFO_LOG(LOGIDENTIFIER, cue.get());
-        addCue(WTFMove(cue));
+
+        ALWAYS_LOG(LOGIDENTIFIER, "extending endTime of existing cue: ", *existingCue, " to ", cue->endTime());
+        existingCue->setEndTime(cue->endTime());
     }
 }
 

diff --git a/Source/WebCore/html/track/TextTrack.cpp b/Source/WebCore/html/track/TextTrack.cpp
@@ -526,13 +526,13 @@ int TextTrack::trackIndexRelativeToRenderedTracks()
     return m_renderedTrackIndex.value();
 }
 
-bool TextTrack::hasCue(TextTrackCue& cue, TextTrackCue::CueMatchRules match)
+RefPtr<TextTrackCue> TextTrack::matchCue(TextTrackCue& cue, TextTrackCue::CueMatchRules match)
 {
     if (cue.startMediaTime() < MediaTime::zeroTime() || cue.endMediaTime() < MediaTime::zeroTime())
-        return false;
+        return nullptr;
 
     if (!m_cues || !m_cues->length())
-        return false;
+        return nullptr;
 
     size_t searchStart = 0;
     size_t searchEnd = m_cues->length();
@@ -546,7 +546,7 @@ bool TextTrack::hasCue(TextTrackCue& cue, TextTrackCue::CueMatchRules match)
         // Cues in the TextTrackCueList are maintained in start time order.
         if (searchStart == searchEnd) {
             if (!searchStart)
-                return false;
+                return nullptr;
 
             // If there is more than one cue with the same start time, back up to first one so we
             // consider all of them.
@@ -559,17 +559,17 @@ bool TextTrack::hasCue(TextTrackCue& cue, TextTrackCue::CueMatchRules match)
                     ++searchStart;
                 firstCompare = false;
                 if (searchStart > m_cues->length())
-                    return false;
+                    return nullptr;
 
                 existingCue = m_cues->item(searchStart - 1);
                 if (!existingCue)
-                    return false;
+                    return nullptr;
 
                 if (cue.startMediaTime() > (existingCue->startMediaTime() + startTimeVariance()))
-                    return false;
+                    return nullptr;
 
                 if (existingCue->isEqual(cue, match))
-                    return true;
+                    return existingCue;
             }
         }
 
@@ -582,7 +582,7 @@ bool TextTrack::hasCue(TextTrackCue& cue, TextTrackCue::CueMatchRules match)
     }
 
     ASSERT_NOT_REACHED();
-    return false;
+    return nullptr;
 }
 
 bool TextTrack::isMainProgramContent() const

diff --git a/Source/WebCore/html/track/TextTrack.h b/Source/WebCore/html/track/TextTrack.h
@@ -138,7 +138,8 @@ class TextTrack : public TrackBase, public EventTargetWithInlineData, public Act
 
     Document& document() const;
 
-    bool hasCue(TextTrackCue&, TextTrackCue::CueMatchRules = TextTrackCue::MatchAllFields);
+    RefPtr<TextTrackCue> matchCue(TextTrackCue&, TextTrackCue::CueMatchRules = TextTrackCue::MatchAllFields);
+    bool hasCue(TextTrackCue& cue, TextTrackCue::CueMatchRules rules = TextTrackCue::MatchAllFields) { return matchCue(cue, rules); }
     void setKind(Kind);
 
     void newCuesAvailable(const TextTrackCueList&);