Don't discard in-band cues with negative start times

https://bugs.webkit.org/show_bug.cgi?id=153867
<rdar://problem/19588632>

Reviewed by Jer Noble.

Source/WebCore:

No new tests, updated and un-skipped http/tests/media/track-in-band-hls-metadata.html.

* platform/graphics/avfoundation/InbandMetadataTextTrackPrivateAVF.cpp:
(WebCore::InbandMetadataTextTrackPrivateAVF::addDataCue):  ASSERT if passed negative time value.
(WebCore::InbandMetadataTextTrackPrivateAVF::updatePendingCueEndTimes): Ditto. Correct logging.

* platform/graphics/avfoundation/objc/MediaPlayerPrivateAVFoundationObjC.mm:
(WebCore::MediaPlayerPrivateAVFoundationObjC::processCue): ASSERT if passed negative time value.
(WebCore::MediaPlayerPrivateAVFoundationObjC::metadataDidArrive): Convert negative cue times to zero.
(-[WebCoreAVFMovieObserver legibleOutput:didOutputAttributedStrings:nativeSampleBuffers:forItemTime:]):
  Ditto.

LayoutTests:

* http/tests/media/track-in-band-hls-metadata-expected.txt:
* http/tests/media/track-in-band-hls-metadata.html: Test more attributes for correctness,
  fail test immediately if cue.value is undefined so the test doesn't generate an exception
  and exit without any results.
* platform/mac/TestExpectations: Unskip test.

git-svn-id: https://svn.webkit.org/repository/webkit/trunk@196131 268f45cc-cd09-0410-ab3c-d52691b4dbfc

diff --git a/LayoutTests/ChangeLog b/LayoutTests/ChangeLog
index 5a999f6..a2b9a30 100644 (file)
--- a/LayoutTests/ChangeLog
+++ b/LayoutTests/ChangeLog
@@ -1,3 +1,17 @@
+2016-02-04  Eric Carlson  <eric.carlson@apple.com>
+
+        Don't discard in-band cues with negative start times
+        https://bugs.webkit.org/show_bug.cgi?id=153867
+        <rdar://problem/19588632>
+
+        Reviewed by Jer Noble.
+
+        * http/tests/media/track-in-band-hls-metadata-expected.txt:
+        * http/tests/media/track-in-band-hls-metadata.html: Test more attributes for correctness,
+          fail test immediately if cue.value is undefined so the test doesn't generate an exception
+          and exit without any results.
+        * platform/mac/TestExpectations: Unskip test.
+
 2016-02-04  Hyemi Shin  <hyemi.sin@samsung.com>
 
         Specify an exception for createChannelMerger, createChannelSplitter and createPeriodicWave

diff --git a/LayoutTests/http/tests/media/track-in-band-hls-metadata-expected.txt b/LayoutTests/http/tests/media/track-in-band-hls-metadata-expected.txt
index 76182cf..40e6cc7 100644 (file)
--- a/LayoutTests/http/tests/media/track-in-band-hls-metadata-expected.txt
+++ b/LayoutTests/http/tests/media/track-in-band-hls-metadata-expected.txt
@@ -24,26 +24,34 @@ EVENT(cuechange)
 
 ** Validate cue data
 * 1
+EXPECTED (typeof(cue) != 'undefined') OK
 EXPECTED (cue.type == 'org.id3') OK
 EXPECTED (cue.data == 'null') OK
+EXPECTED (cue.value != 'null') OK
 EXPECTED (cue.value.key == '"TIT2"') OK
 EXPECTED (cue.value.data == '"Stream Counting"') OK
 
 * 2
+EXPECTED (typeof(cue) != 'undefined') OK
 EXPECTED (cue.type == 'org.id3') OK
 EXPECTED (cue.data == 'null') OK
+EXPECTED (cue.value != 'null') OK
 EXPECTED (cue.value.key == '"TPE1"') OK
 EXPECTED (cue.value.data == '"Andy"') OK
 
 * 3
+EXPECTED (typeof(cue) != 'undefined') OK
 EXPECTED (cue.type == 'org.id3') OK
 EXPECTED (cue.data == 'null') OK
+EXPECTED (cue.value != 'null') OK
 EXPECTED (cue.value.key == '"TALB"') OK
 EXPECTED (cue.value.data == '"Greatest Hits"') OK
 
 * 4
+EXPECTED (typeof(cue) != 'undefined') OK
 EXPECTED (cue.type == 'org.id3') OK
 EXPECTED (cue.data == 'null') OK
+EXPECTED (cue.value != 'null') OK
 EXPECTED (cue.value.key == '"GEOB"') OK
 EXPECTED (cue.value.data == '{}') OK
 EXPECTED (cue.value.type == '"image/png"') OK
@@ -51,15 +59,19 @@ EXPECTED (cue.value.info == '"Our Hero"') OK
 EXPECTED (cue.value.name == '"abe.png"') OK
 
 * 5
+EXPECTED (typeof(cue) != 'undefined') OK
 EXPECTED (cue.type == 'org.id3') OK
 EXPECTED (cue.data == 'null') OK
+EXPECTED (cue.value != 'null') OK
 EXPECTED (cue.value.key == '"APIC"') OK
 EXPECTED (cue.value.data == '{}') OK
 EXPECTED (cue.value.type == '"image/png"') OK
 
 * 6
+EXPECTED (typeof(cue) != 'undefined') OK
 EXPECTED (cue.type == 'org.id3') OK
 EXPECTED (cue.data == 'null') OK
+EXPECTED (cue.value != 'null') OK
 EXPECTED (cue.value.key == '"TXXX"') OK
 EXPECTED (cue.value.data == '"Text Blob"') OK
 

diff --git a/LayoutTests/http/tests/media/track-in-band-hls-metadata.html b/LayoutTests/http/tests/media/track-in-band-hls-metadata.html
index 1e055f2..8f953ce 100644 (file)
--- a/LayoutTests/http/tests/media/track-in-band-hls-metadata.html
+++ b/LayoutTests/http/tests/media/track-in-band-hls-metadata.html
@@ -19,6 +19,7 @@
                  {"key" : "TXXX", "data" : "Text Blob"},
             ];
             var imageSizes = [ [76, 103], [100, 100] ];
+            var cue;
         
             function addtrack(event)
             {
@@ -56,7 +57,7 @@
 
             function compareCues(observed, expected)
             {
-                // Make a mutable copy of the cue so we can remove attributes as they are processed.
+                // Make a mutable copy of observed so we can remove attributes as they are processed.
                 observed = JSON.parse(JSON.stringify(observed));
                 for (property in expected) {
                     observedValue = observed[property];
@@ -65,14 +66,14 @@
 
                     observedValue = JSON.stringify(observedValue);
                     expectedValue = JSON.stringify(expectedValue);
-                    reportExpected(observedValue == expectedValue, "cue.value." + property, "==", expectedValue, observedValue);
+                    reportExpected(observedValue == expectedValue, `cue.value.${property}`, "==", expectedValue, observedValue);
                     delete observed[property];
                 }
 
                 for (property in observed) {
                     observedName = property;
                     observedValue = observed[property];
-                    logResult(Failed, "Error: unexpected cue property <em>cue.value." + observedName + "</em> = <em>'" + observedValue + "'</em> ");
+                    logResult(Failed, `Error: unexpected cue property <em>cue.value. ${observedName}</em> = <em>' ${observedValue}'</em>`);
                 }
             }
 
@@ -87,10 +88,18 @@
                 video.pause();
 
                 for (var i = 0; i < 6; i++) {
-                    consoleWrite("<em>* " + (i + 1) + "</em>");
+                    consoleWrite(`<em>* ${i + 1}</em>`);
                     cue = track.cues[i];
+
+                    testExpected("typeof(cue)", undefined, "!=");
                     testExpected("cue.type", "org.id3");
                     testExpected("cue.data", null);
+                    testExpected("cue.value", null, "!=");
+                    if (typeof(cue.value) == undefined) {
+                        failTest(`Error: cue.value is undefined!`);
+                        return;
+                    }
+
                     compareCues(cue.value, cueData[i]);
                     consoleWrite("");
                 }
@@ -114,7 +123,7 @@
                 run("video.src = 'http://127.0.0.1:8000/media/resources/hls/metadata/prog_index.m3u8'");
 
                 consoleWrite("");
-                waitForEvent("canplaythrough", canplaythrough);
+                waitForEventOnce("canplaythrough", canplaythrough);
                 waitForEvent('addtrack', addtrack, false, false, video.textTracks);
             }
         </script>

diff --git a/LayoutTests/platform/mac/TestExpectations b/LayoutTests/platform/mac/TestExpectations
index 0e65869..7dabdf8 100644 (file)
--- a/LayoutTests/platform/mac/TestExpectations
+++ b/LayoutTests/platform/mac/TestExpectations
@@ -906,7 +906,6 @@ webkit.org/b/122507 media/track/track-cue-rendering.html [ Pass Failure ]
 webkit.org/b/123010 media/W3C/audio/networkState/networkState_during_loadstart.html [ Pass Timeout ]
 webkit.org/b/123099 media/media-controller-time-clamp.html [ Pass Timeout ]
 webkit.org/b/123522 media/track/track-in-band-legacy-api.html [ Pass Failure Crash ]
-webkit.org/b/128312 media/video-load-preload-metadata.html [ Pass Failure ]
 webkit.org/b/130490 media/video-remote-control-playpause.html [ Pass Failure ]
 webkit.org/b/130971 media/track/track-remove-track.html [ Pass Crash Failure Timeout ]
 webkit.org/b/131855 media/event-attributes.html [ Pass Failure Timeout ]

diff --git a/Source/WebCore/ChangeLog b/Source/WebCore/ChangeLog
index fbea9f9..0882b7c 100644 (file)
--- a/Source/WebCore/ChangeLog
+++ b/Source/WebCore/ChangeLog
@@ -1,3 +1,23 @@
+2016-02-04  Eric Carlson  <eric.carlson@apple.com>
+
+        Don't discard in-band cues with negative start times
+        https://bugs.webkit.org/show_bug.cgi?id=153867
+        <rdar://problem/19588632>
+
+        Reviewed by Jer Noble.
+
+        No new tests, updated and un-skipped http/tests/media/track-in-band-hls-metadata.html.
+
+        * platform/graphics/avfoundation/InbandMetadataTextTrackPrivateAVF.cpp:
+        (WebCore::InbandMetadataTextTrackPrivateAVF::addDataCue):  ASSERT if passed negative time value.
+        (WebCore::InbandMetadataTextTrackPrivateAVF::updatePendingCueEndTimes): Ditto. Correct logging.
+
+        * platform/graphics/avfoundation/objc/MediaPlayerPrivateAVFoundationObjC.mm:
+        (WebCore::MediaPlayerPrivateAVFoundationObjC::processCue): ASSERT if passed negative time value.
+        (WebCore::MediaPlayerPrivateAVFoundationObjC::metadataDidArrive): Convert negative cue times to zero.
+        (-[WebCoreAVFMovieObserver legibleOutput:didOutputAttributedStrings:nativeSampleBuffers:forItemTime:]):
+          Ditto.
+
 2016-02-04  Hyemi Shin  <hyemi.sin@samsung.com>
 
         Specify an exception for createChannelMerger, createChannelSplitter and createPeriodicWave

diff --git a/Source/WebCore/platform/graphics/avfoundation/InbandMetadataTextTrackPrivateAVF.cpp b/Source/WebCore/platform/graphics/avfoundation/InbandMetadataTextTrackPrivateAVF.cpp
index 655d44f..9c6ca87 100644 (file)
--- a/Source/WebCore/platform/graphics/avfoundation/InbandMetadataTextTrackPrivateAVF.cpp
+++ b/Source/WebCore/platform/graphics/avfoundation/InbandMetadataTextTrackPrivateAVF.cpp
@@ -57,6 +57,8 @@ InbandMetadataTextTrackPrivateAVF::~InbandMetadataTextTrackPrivateAVF()
 void InbandMetadataTextTrackPrivateAVF::addDataCue(const MediaTime& start, const MediaTime& end, PassRefPtr<SerializedPlatformRepresentation> prpCueData, const String& type)
 {
     ASSERT(cueFormat() == Data);
+    ASSERT(start >= MediaTime::zeroTime());
+
     if (!client())
         return;
 
@@ -69,15 +71,17 @@ void InbandMetadataTextTrackPrivateAVF::addDataCue(const MediaTime& start, const
 
 void InbandMetadataTextTrackPrivateAVF::updatePendingCueEndTimes(const MediaTime& time)
 {
+    ASSERT(time >= MediaTime::zeroTime());
+
     if (time >= m_currentCueStartTime) {
         if (client()) {
             for (auto& partialCue : m_incompleteCues) {
-                LOG(Media, "InbandMetadataTextTrackPrivateAVF::addDataCue(%p) - updating cue: start=%s, end=%s", this, toString(partialCue.startTime).utf8().data(), toString(time).utf8().data());
+                LOG(Media, "InbandMetadataTextTrackPrivateAVF::updatePendingCueEndTimes(%p) - updating cue: start=%s, end=%s", this, toString(partialCue.startTime).utf8().data(), toString(time).utf8().data());
                 client()->updateDataCue(this, partialCue.startTime, time, partialCue.cueData);
             }
         }
     } else
-        LOG(Media, "InbandMetadataTextTrackPrivateAVF::addDataCue negative length cue(s) ignored: start=%s, end=%s\n", toString(m_currentCueStartTime).utf8().data(), toString(time).utf8().data());
+        LOG(Media, "InbandMetadataTextTrackPrivateAVF::updatePendingCueEndTimes negative length cue(s) ignored: start=%s, end=%s\n", toString(m_currentCueStartTime).utf8().data(), toString(time).utf8().data());
 
     m_incompleteCues.resize(0);
     m_currentCueStartTime = MediaTime::zeroTime();

diff --git a/Source/WebCore/platform/graphics/avfoundation/objc/MediaPlayerPrivateAVFoundationObjC.mm b/Source/WebCore/platform/graphics/avfoundation/objc/MediaPlayerPrivateAVFoundationObjC.mm
index efa3d70..59c2c23 100644 (file)
--- a/Source/WebCore/platform/graphics/avfoundation/objc/MediaPlayerPrivateAVFoundationObjC.mm
+++ b/Source/WebCore/platform/graphics/avfoundation/objc/MediaPlayerPrivateAVFoundationObjC.mm
@@ -2861,6 +2861,8 @@ void MediaPlayerPrivateAVFoundationObjC::processMetadataTrack()
 
 void MediaPlayerPrivateAVFoundationObjC::processCue(NSArray *attributedStrings, NSArray *nativeSamples, const MediaTime& time)
 {
+    ASSERT(time >= MediaTime::zeroTime());
+
     if (!m_currentTextTrack)
         return;
 
@@ -3239,14 +3241,14 @@ void MediaPlayerPrivateAVFoundationObjC::metadataDidArrive(RetainPtr<NSArray> me
     // Set the duration of all incomplete cues before adding new ones.
     MediaTime earliestStartTime = MediaTime::positiveInfiniteTime();
     for (AVMetadataItemType *item in m_currentMetaData.get()) {
-        MediaTime start = toMediaTime(item.time);
+        MediaTime start = std::max(toMediaTime(item.time), MediaTime::zeroTime());
         if (start < earliestStartTime)
             earliestStartTime = start;
     }
     m_metadataTrack->updatePendingCueEndTimes(earliestStartTime);
 
     for (AVMetadataItemType *item in m_currentMetaData.get()) {
-        MediaTime start = toMediaTime(item.time);
+        MediaTime start = std::max(toMediaTime(item.time), MediaTime::zeroTime());
         MediaTime end = MediaTime::positiveInfiniteTime();
         if (CMTIME_IS_VALID(item.duration))
             end = start + toMediaTime(item.duration);
@@ -3560,7 +3562,8 @@ NSArray* playerKVOProperties()
         MediaPlayerPrivateAVFoundationObjC* callback = strongSelf->m_callback;
         if (!callback)
             return;
-        callback->processCue(strongStrings.get(), strongSamples.get(), toMediaTime(itemTime));
+        MediaTime time = std::max(toMediaTime(itemTime), MediaTime::zeroTime());
+        callback->processCue(strongStrings.get(), strongSamples.get(), time);
     });
 }