Overlapping VTTCues carried by InbandTextTrack (LegibleOutput) does n…

…ot handle correctly https://bugs.webkit.org/show_bug.cgi?id=251771 rdar://105372930 Reviewed by Jer Noble. Parse every cue in each CMSampleBuffer provided the the AVPlayerItemLegibleOutput delegate, not just the first one. * LayoutTests/http/tests/media/hls/track-in-band-multiple-cues-expected.txt: Added. * LayoutTests/http/tests/media/hls/track-in-band-multiple-cues.html: Added. * LayoutTests/http/tests/media/resources/hls/multiple-cues-per-sample/master.m3u8: Added. * LayoutTests/http/tests/media/resources/hls/multiple-cues-per-sample/multiple0.vtt: Added. * LayoutTests/http/tests/media/resources/hls/multiple-cues-per-sample/multiple1.vtt: Added. * LayoutTests/http/tests/media/resources/hls/multiple-cues-per-sample/test-0.ts: Added. * LayoutTests/http/tests/media/resources/hls/multiple-cues-per-sample/test-1.ts: Added. * LayoutTests/http/tests/media/resources/hls/multiple-cues-per-sample/video.m3u8: Added. * LayoutTests/http/tests/media/resources/hls/multiple-cues-per-sample/vtt.m3u8: Added. * Source/WebCore/html/track/InbandGenericTextTrack.cpp: (WebCore::InbandGenericTextTrack::cueToExtend): Look for an existing cue with the same contents and start time and duration within the allowed delta. (WebCore::InbandGenericTextTrack::newCuesParsed): Use cueToExtend. * Source/WebCore/html/track/InbandGenericTextTrack.h: * Source/WebCore/html/track/InbandTextTrack.h: * Source/WebCore/html/track/TextTrackCue.h: * Source/WebCore/platform/graphics/avfoundation/InbandTextTrackPrivateAVF.cpp: (WebCore::InbandTextTrackPrivateAVF::processNativeSamples): There may be more than one cue in each CMSampleBuffer. Canonical link: https://commits.webkit.org/266260@main
WebKit · Jul 24, 2023 · 6f89f78 · 6f89f78
1 parent 90608fd
commit 6f89f78
Show file tree

Hide file tree

Showing 14 changed files with 181 additions and 46 deletions.
diff --git a/LayoutTests/http/tests/media/hls/track-in-band-multiple-cues-expected.txt b/LayoutTests/http/tests/media/hls/track-in-band-multiple-cues-expected.txt
@@ -0,0 +1,10 @@
+
+Test that multiple cues in a sample buffer are parsed correctly.
+
+EVENT(addtrack)
+EVENT(playing)
+EVENT(cuechange)
+EXPECTED (track.cues.length === '5') OK
+
+END OF TEST
+
diff --git a/LayoutTests/http/tests/media/hls/track-in-band-multiple-cues.html b/LayoutTests/http/tests/media/hls/track-in-band-multiple-cues.html
@@ -0,0 +1,40 @@
+<!DOCTYPE html>
+<html>
+    <head>
+        <meta http-equiv="Content-Type" content="text/html; charset=utf-8" />
+
+        <script src=../../../media-resources/video-test.js></script>
+        <script src=../../../media-resources/media-file.js></script>
+
+        <script>
+            async function test()
+            {
+                findMediaElement();
+
+                waitForAndFail(video, 'error')
+                video.src = "http://127.0.0.1:8000/media/resources/hls/multiple-cues-per-sample/master.m3u8";
+
+                await waitFor(video.textTracks, 'addtrack')
+                track = video.textTracks[0];
+                track.mode = 'showing';
+
+                await Promise.all([
+                    waitFor(video, 'playing'),
+                    waitFor(track, 'cuechange')
+                ]);
+                await testExpectedEventually('track.cues.length', 5, '===', 4000);
+
+                consoleWrite('');
+            }
+
+            window.addEventListener('load', async (event) => {
+                await test();
+                endTest();
+            });
+        </script>
+    </head>
+    <body>
+        <video controls muted autoplay></video>
+        <p>Test that multiple cues in a sample buffer are parsed correctly.</p>
+    </body>
+</html>
diff --git a/LayoutTests/http/tests/media/resources/hls/multiple-cues-per-sample/master.m3u8 b/LayoutTests/http/tests/media/resources/hls/multiple-cues-per-sample/master.m3u8
@@ -0,0 +1,5 @@
+#EXTM3U
+#EXT-X-VERSION:3
+#EXT-X-MEDIA:TYPE=SUBTITLES,GROUP-ID="subtitle",NAME="subtitle",DEFAULT=YES,URI="vtt.m3u8"
+#EXT-X-STREAM-INF:BANDWIDTH=140800,CODECS="avc1.64001f,mp4a.40.2",SUBTITLES="subtitle"
+video.m3u8
diff --git a/LayoutTests/http/tests/media/resources/hls/multiple-cues-per-sample/multiple0.vtt b/LayoutTests/http/tests/media/resources/hls/multiple-cues-per-sample/multiple0.vtt
@@ -0,0 +1,10 @@
+WEBVTT
+X-TIMESTAMP-MAP=MPEGTS:127920,LOCAL:00:00:00.000
+
+Cue-1
+00:00.000 --> 00:01.900 line:1
+sample 1, line 1
+
+Cue-2
+00:00.000 --> 00:01.900 line:2
+sample 1, line 2
diff --git a/LayoutTests/http/tests/media/resources/hls/multiple-cues-per-sample/multiple1.vtt b/LayoutTests/http/tests/media/resources/hls/multiple-cues-per-sample/multiple1.vtt
@@ -0,0 +1,13 @@
+WEBVTT
+
+Cue-3
+00:00:02.000 --> 00:00:04.000 line:1
+sample 2, line 1
+
+Cue-4
+00:00:02.000 --> 00:00:04.000 line:2
+sample 2, line 2
+
+Cue-5
+00:00:02.000 --> 00:00:04.000 line:3
+sample 2, line 3
diff --git a/LayoutTests/http/tests/media/resources/hls/multiple-cues-per-sample/test-0.ts b/LayoutTests/http/tests/media/resources/hls/multiple-cues-per-sample/test-0.ts
diff --git a/LayoutTests/http/tests/media/resources/hls/multiple-cues-per-sample/test-1.ts b/LayoutTests/http/tests/media/resources/hls/multiple-cues-per-sample/test-1.ts
diff --git a/LayoutTests/http/tests/media/resources/hls/multiple-cues-per-sample/video.m3u8 b/LayoutTests/http/tests/media/resources/hls/multiple-cues-per-sample/video.m3u8
@@ -0,0 +1,10 @@
+#EXTM3U
+#EXT-X-VERSION:3
+#EXT-X-TARGETDURATION:2
+#EXT-X-MEDIA-SEQUENCE:0
+#EXT-X-PLAYLIST-TYPE:VOD
+#EXTINF:2.000000,
+test-0.ts
+#EXTINF:2.000000,
+test-1.ts
+#EXT-X-ENDLIST
diff --git a/LayoutTests/http/tests/media/resources/hls/multiple-cues-per-sample/vtt.m3u8 b/LayoutTests/http/tests/media/resources/hls/multiple-cues-per-sample/vtt.m3u8
@@ -0,0 +1,10 @@
+#EXTM3U
+#EXT-X-VERSION:3
+#EXT-X-TARGETDURATION:2
+#EXT-X-MEDIA-SEQUENCE:0
+#EXT-X-PLAYLIST-TYPE:VOD
+#EXTINF:2.000000,
+multiple0.vtt
+#EXTINF:2.000000,
+multiple1.vtt
+#EXT-X-ENDLIST
diff --git a/Source/WebCore/html/track/InbandGenericTextTrack.cpp b/Source/WebCore/html/track/InbandGenericTextTrack.cpp
@@ -185,11 +185,42 @@ void InbandGenericTextTrack::parseWebVTTFileHeader(String&& header)
     parser().parseFileHeader(WTFMove(header));
 }
 
+RefPtr<TextTrackCue> InbandGenericTextTrack::cueToExtend(TextTrackCue& newCue)
+{
+    if (newCue.startMediaTime() < MediaTime::zeroTime() || newCue.endMediaTime() < MediaTime::zeroTime())
+        return nullptr;
+
+    if (!m_cues || m_cues->length() < 2)
+        return nullptr;
+
+    return [this, &newCue]() -> RefPtr<TextTrackCue> {
+        for (size_t i = 0; i < m_cues->length(); ++i) {
+            auto existingCue = m_cues->item(i);
+            ASSERT(existingCue->track() == this);
+
+            if (abs(newCue.startMediaTime() - existingCue->startMediaTime()) > startTimeVariance())
+                continue;
+
+            if (abs(newCue.startMediaTime() - existingCue->endMediaTime()) > startTimeVariance())
+                return nullptr;
+
+            if (existingCue->cueContentsMatch(newCue))
+                return existingCue;
+        }
+
+        return nullptr;
+    }();
+}
+
 void InbandGenericTextTrack::newCuesParsed()
 {
     for (auto& cueData : parser().takeCues()) {
         auto cue = VTTCue::create(document(), cueData);
-        auto existingCue = matchCue(cue, TextTrackCue::IgnoreDuration);
+
+        auto existingCue = cueToExtend(cue);
+        if (!existingCue)
+            existingCue = matchCue(cue, TextTrackCue::IgnoreDuration);
+
         if (!existingCue) {
             INFO_LOG(LOGIDENTIFIER, cue.get());
             addCue(WTFMove(cue));

diff --git a/Source/WebCore/html/track/InbandGenericTextTrack.h b/Source/WebCore/html/track/InbandGenericTextTrack.h
@@ -66,6 +66,8 @@ class InbandGenericTextTrack final : public InbandTextTrack, private WebVTTParse
 
     void updateCueFromCueData(TextTrackCueGeneric&, InbandGenericCue&);
 
+    RefPtr<TextTrackCue> cueToExtend(TextTrackCue&);
+
     WebVTTParser& parser();
     void parseWebVTTCueData(ISOWebVTTCue&&) final;
     void parseWebVTTFileHeader(String&&) final;

diff --git a/Source/WebCore/html/track/InbandTextTrack.h b/Source/WebCore/html/track/InbandTextTrack.h
@@ -62,6 +62,8 @@ class InbandTextTrack : public TextTrack, private InbandTextTrackPrivateClient {
 
     Ref<InbandTextTrackPrivate> m_private;
 
+    MediaTime startTimeVariance() const override;
+
 private:
     bool isInband() const final { return true; }
     void idChanged(const AtomString&) override;
@@ -84,8 +86,6 @@ class InbandTextTrack : public TextTrack, private InbandTextTrackPrivateClient {
     void parseWebVTTFileHeader(String&&) override { ASSERT_NOT_REACHED(); }
     void parseWebVTTCueData(const uint8_t*, unsigned) override { ASSERT_NOT_REACHED(); }
     void parseWebVTTCueData(ISOWebVTTCue&&) override { ASSERT_NOT_REACHED(); }
-
-    MediaTime startTimeVariance() const override;
 };
 
 } // namespace WebCore

diff --git a/Source/WebCore/html/track/TextTrackCue.h b/Source/WebCore/html/track/TextTrackCue.h
@@ -132,12 +132,13 @@ class TextTrackCue : public RefCounted<TextTrackCue>, public EventTarget, public
     virtual void pauseSpeaking() { }
     virtual void cancelSpeaking() { }
 
+    virtual bool cueContentsMatch(const TextTrackCue&) const;
+
 protected:
     TextTrackCue(Document&, const MediaTime& start, const MediaTime& end);
 
     Document* document() const;
 
-    virtual bool cueContentsMatch(const TextTrackCue&) const;
     virtual void toJSON(JSON::Object&) const;
 
 private:

diff --git a/Source/WebCore/platform/graphics/avfoundation/InbandTextTrackPrivateAVF.cpp b/Source/WebCore/platform/graphics/avfoundation/InbandTextTrackPrivateAVF.cpp
@@ -485,7 +485,7 @@ void InbandTextTrackPrivateAVF::processNativeSamples(CFArrayRef nativeSamples, c
     if (!count)
         return;
 
-    INFO_LOG(LOGIDENTIFIER, count, " sample buffers at time ", presentationTime);
+    ALWAYS_LOG(LOGIDENTIFIER, count, " sample buffers at time ", presentationTime);
 
     for (CFIndex i = 0; i < count; i++) {
         RefPtr<ArrayBuffer> buffer;
@@ -494,57 +494,60 @@ void InbandTextTrackPrivateAVF::processNativeSamples(CFArrayRef nativeSamples, c
         if (!readNativeSampleBuffer(nativeSamples, i, buffer, duration, formatDescription))
             continue;
 
-        auto view = JSC::DataView::create(WTFMove(buffer), 0, buffer->byteLength());
-        auto peekResult = ISOBox::peekBox(view, 0);
-        if (!peekResult)
-            continue;
-
-        auto type = peekResult.value().first;
-        auto boxLength = peekResult.value().second;
-        if (boxLength > view->byteLength()) {
-            ERROR_LOG(LOGIDENTIFIER, "chunk  type = '", type, "', size = ", boxLength, " larger than buffer length!");
-            continue;
-        }
+        while (true) {
+            buffer = ArrayBuffer::create(m_sampleInputBuffer.data(), m_sampleInputBuffer.size());
+            auto view = JSC::DataView::create(WTFMove(buffer), 0, buffer->byteLength());
 
-        INFO_LOG(LOGIDENTIFIER, "chunk  type = '", type, "', size = ", boxLength);
-
-        do {
-            if (m_haveReportedVTTHeader || !formatDescription)
+            auto peekResult = ISOBox::peekBox(view, 0);
+            if (!peekResult)
                 break;
 
-            CFDictionaryRef extensions = CMFormatDescriptionGetExtensions(formatDescription);
-            if (!extensions)
+            auto type = peekResult.value().first;
+            auto boxLength = peekResult.value().second;
+            if (boxLength > view->byteLength()) {
+                ERROR_LOG(LOGIDENTIFIER, "chunk  type = '", type, "', size = ", boxLength, " larger than buffer length!");
                 break;
+            }
 
-            CFDictionaryRef sampleDescriptionExtensions = static_cast<CFDictionaryRef>(CFDictionaryGetValue(extensions, kCMFormatDescriptionExtension_SampleDescriptionExtensionAtoms));
-            if (!sampleDescriptionExtensions)
-                break;
-
-            CFDataRef webvttHeaderData = static_cast<CFDataRef>(CFDictionaryGetValue(sampleDescriptionExtensions, CFSTR("vttC")));
-            if (!webvttHeaderData)
-                break;
+            INFO_LOG(LOGIDENTIFIER, "chunk  type = '", type, "', size = ", boxLength);
 
-            unsigned length = CFDataGetLength(webvttHeaderData);
-            if (!length)
-                break;
+            do {
+                if (m_haveReportedVTTHeader || !formatDescription)
+                    break;
 
-            // A WebVTT header is terminated by "One or more WebVTT line terminators" so append two line feeds to make sure the parser
-            // reccognized this string as a full header.
-            auto header = makeString(StringView { CFDataGetBytePtr(webvttHeaderData), length }, "\n\n");
+                CFDictionaryRef extensions = CMFormatDescriptionGetExtensions(formatDescription);
+                if (!extensions)
+                    break;
 
-            INFO_LOG(LOGIDENTIFIER, "VTT header ", header);
-            client()->parseWebVTTFileHeader(WTFMove(header));
-            m_haveReportedVTTHeader = true;
-        } while (0);
+                CFDictionaryRef sampleDescriptionExtensions = static_cast<CFDictionaryRef>(CFDictionaryGetValue(extensions, kCMFormatDescriptionExtension_SampleDescriptionExtensionAtoms));
+                if (!sampleDescriptionExtensions)
+                    break;
 
-        if (type == ISOWebVTTCue::boxTypeName()) {
-            ISOWebVTTCue cueData = ISOWebVTTCue(presentationTime, duration);
-            cueData.read(view);
-            INFO_LOG(LOGIDENTIFIER, "VTT cue data ", cueData);
-            client()->parseWebVTTCueData(WTFMove(cueData));
-        }
+                CFDataRef webvttHeaderData = static_cast<CFDataRef>(CFDictionaryGetValue(sampleDescriptionExtensions, CFSTR("vttC")));
+                if (!webvttHeaderData)
+                    break;
+
+                unsigned length = CFDataGetLength(webvttHeaderData);
+                if (!length)
+                    break;
+
+                // A WebVTT header is terminated by "One or more WebVTT line terminators" so append two line feeds to make sure the parser
+                // reccognized this string as a full header.
+                auto header = makeString(StringView { CFDataGetBytePtr(webvttHeaderData), length }, "\n\n");
 
-        m_sampleInputBuffer.remove(0, (size_t)boxLength);
+                INFO_LOG(LOGIDENTIFIER, "VTT header ", header);
+                client()->parseWebVTTFileHeader(WTFMove(header));
+                m_haveReportedVTTHeader = true;
+            } while (0);
+
+            if (type == ISOWebVTTCue::boxTypeName()) {
+                ISOWebVTTCue cueData = ISOWebVTTCue(presentationTime, duration);
+                cueData.read(view);
+                client()->parseWebVTTCueData(WTFMove(cueData));
+            }
+
+            m_sampleInputBuffer.remove(0, (size_t)boxLength);
+        }
     }
 }