Changeset 138017 in webkit

Timestamp:

Dec 18, 2012 7:23:29 AM (11 years ago)

Author:

eric.carlson@apple.com

Message:

Add in-band text track cues only once
​https://bugs.webkit.org/show_bug.cgi?id=104593

Reviewed by Dean Jackson.

Source/WebCore:

Test: media/track/track-in-band-cues-added-once.html

• html/track/InbandTextTrack.cpp:

(WebCore::InbandTextTrack::hasCue): New.

• html/track/InbandTextTrack.h:

• html/track/TextTrackCue.cpp:

(WebCore::TextTrackCue::setCueSettings): Remember the raw cue settings so they can be accessed later.

• html/track/TextTrackCue.h:

• platform/graphics/InbandTextTrackPrivateClient.h: Declare hasCue.

• platform/graphics/avfoundation/InbandTextTrackPrivateAVF.cpp:

(WebCore::InbandTextTrackPrivateAVF::processCue): Early return if m_player has been cleared.
(WebCore::InbandTextTrackPrivateAVF::setMode): Ditto.

• platform/graphics/avfoundation/MediaPlayerPrivateAVFoundation.cpp:

(WebCore::MediaPlayerPrivateAVFoundation::seek): Clear the partially accumulated cue when the seek

starts, not when it completes.

(WebCore::MediaPlayerPrivateAVFoundation::seekCompleted): Ditto.
(WebCore::MediaPlayerPrivateAVFoundation::flushCurrentCue): Don't add a cue if it is already in the

text track cue list.

• platform/graphics/avfoundation/objc/MediaPlayerPrivateAVFoundationObjC.mm:

(WebCore::MediaPlayerPrivateAVFoundationObjC::clearTextTracks): Drive-by cleanup, clear the track list completely.

LayoutTests:

Test to ensure that in-band text track cues are not added to the the cue list more than once.

• media/track/track-in-band-cues-added-once-expected.txt: Added.
• media/track/track-in-band-cues-added-once.html: Added.
• platform/chromium/TestExpectations:
• platform/efl/TestExpectations:
• platform/gtk/TestExpectations:
• platform/mac/TestExpectations:
• platform/qt/TestExpectations:
• platform/win/TestExpectations:

Location:

trunk

Files:

• LayoutTests/ChangeLog (modified) (1 diff)
• LayoutTests/media/track/track-in-band-cues-added-once-expected.txt (added)
• LayoutTests/media/track/track-in-band-cues-added-once.html (added)
• LayoutTests/platform/chromium/TestExpectations (modified) (1 diff)
• LayoutTests/platform/efl/TestExpectations (modified) (1 diff)
• LayoutTests/platform/gtk/TestExpectations (modified) (1 diff)
• LayoutTests/platform/mac/TestExpectations (modified) (1 diff)
• LayoutTests/platform/qt/TestExpectations (modified) (1 diff)
• LayoutTests/platform/win/TestExpectations (modified) (1 diff)
• Source/WebCore/ChangeLog (modified) (1 diff)
• Source/WebCore/html/track/InbandTextTrack.cpp (modified) (1 diff)
• Source/WebCore/html/track/InbandTextTrack.h (modified) (1 diff)
• Source/WebCore/html/track/TextTrackCue.cpp (modified) (1 diff)
• Source/WebCore/html/track/TextTrackCue.h (modified) (2 diffs)
• Source/WebCore/platform/graphics/InbandTextTrackPrivateClient.h (modified) (1 diff)
• Source/WebCore/platform/graphics/avfoundation/InbandTextTrackPrivateAVF.cpp (modified) (2 diffs)
• Source/WebCore/platform/graphics/avfoundation/MediaPlayerPrivateAVFoundation.cpp (modified) (3 diffs)
• Source/WebCore/platform/graphics/avfoundation/objc/MediaPlayerPrivateAVFoundationObjC.mm (modified) (1 diff)

trunk/LayoutTests/ChangeLog

@@ +1 @@
+2012-12-18  Eric Carlson  <eric.carlson@apple.com>
+
+        Add in-band text track cues only once
+        https://bugs.webkit.org/show_bug.cgi?id=104593
+
+        Reviewed by Dean Jackson.
+
+        Test to ensure that in-band text track cues are not added to the the cue list more than once.
+
+        * media/track/track-in-band-cues-added-once-expected.txt: Added.
+        * media/track/track-in-band-cues-added-once.html: Added.
+        * platform/chromium/TestExpectations:
+        * platform/efl/TestExpectations:
+        * platform/gtk/TestExpectations:
+        * platform/mac/TestExpectations:
+        * platform/qt/TestExpectations:
+        * platform/win/TestExpectations:
+
 2012-12-18  Thiago Marcos P. Santos  <thiago.santos@intel.com>
 

trunk/LayoutTests/platform/chromium/TestExpectations

@@ -4232 +4232 @@
 # No support for exposing in-band text tracks
 webkit.org/b/103767  [ Win Mac Linux ] media/track/track-in-band.html [ Skip ]
+webkit.org/b/103767  [ Win Mac Linux ] media/track/track-in-band-cues-added-once.html [ Skip ]
 
 # Flaky on Win (perhaps due to lighttpd?)

trunk/LayoutTests/platform/efl/TestExpectations

@@ -1654 +1654 @@
 # No support for exposing in-band text tracks
 Bug(EFL) media/track/track-in-band.html [ Skip ]
+Bug(EFL) media/track/track-in-band-cues-added-once.html [ Skip ]
 
 # Newly added test in r136225 fails on GTK bot too.

trunk/LayoutTests/platform/gtk/TestExpectations

@@ -458 +458 @@
 # No support for exposing in-band text tracks
 webkit.org/b/103771 media/track/track-in-band.html [ Failure ]
+webkit.org/b/103771 media/track/track-in-band-cues-added-once.html [ Failure ]
 
 #////////////////////////////////////////////////////////////////////////////////////////

trunk/LayoutTests/platform/mac/TestExpectations

@@ -1242 +1242 @@
 # Mountain Lion and prior do not allow access to in-band text tracks
 webkit.org/b/103663 [ MountainLion Lion SnowLeopard ] media/track/track-in-band.html
+webkit.org/b/103663 [ MountainLion Lion SnowLeopard ] media/track/track-in-band-cues-added-once.html
 
 webkit.org/b/104104 fast/overflow/scrollbar-click-retains-focus.html [ Failure ]

trunk/LayoutTests/platform/qt/TestExpectations

@@ -2443 +2443 @@
 # No support for exposing in-band text tracks
 webkit.org/b/103769 media/track/track-in-band.html [ Skip ]
+webkit.org/b/103769 media/track/track-in-band-cues-added-once.html [ Skip ]
 
 webkit.org/b/104150 fast/media/implicit-media-all.html [ ImageOnlyFailure ]

trunk/LayoutTests/platform/win/TestExpectations

@@ -2477 +2477 @@
 # No support for exposing in-band text tracks
 webkit.org/b/103770 media/track/track-in-band.html [ Skip ]
+webkit.org/b/103770 media/track/track-in-band-cues-added-once.html [ Skip ]
 
 # https://bugs.webkit.org/show_bug.cgi?id=97026

trunk/Source/WebCore/ChangeLog

@@ +1 @@
+2012-12-18  Eric Carlson  <eric.carlson@apple.com>
+
+        Add in-band text track cues only once
+        https://bugs.webkit.org/show_bug.cgi?id=104593
+
+        Reviewed by Dean Jackson.
+
+        Test: media/track/track-in-band-cues-added-once.html
+
+        * html/track/InbandTextTrack.cpp:
+        (WebCore::InbandTextTrack::hasCue): New.
+        * html/track/InbandTextTrack.h:
+
+        * html/track/TextTrackCue.cpp:
+        (WebCore::TextTrackCue::setCueSettings): Remember the raw cue settings so they can be accessed later.
+        * html/track/TextTrackCue.h:
+
+        * platform/graphics/InbandTextTrackPrivateClient.h: Declare hasCue.
+
+        * platform/graphics/avfoundation/InbandTextTrackPrivateAVF.cpp:
+        (WebCore::InbandTextTrackPrivateAVF::processCue): Early return if m_player has been cleared.
+        (WebCore::InbandTextTrackPrivateAVF::setMode): Ditto.
+
+        * platform/graphics/avfoundation/MediaPlayerPrivateAVFoundation.cpp:
+        (WebCore::MediaPlayerPrivateAVFoundation::seek): Clear the partially accumulated cue when the seek 
+            starts, not when it completes.
+        (WebCore::MediaPlayerPrivateAVFoundation::seekCompleted): Ditto.
+        (WebCore::MediaPlayerPrivateAVFoundation::flushCurrentCue): Don't add a cue if it is already in the
+            text track cue list.
+        * platform/graphics/avfoundation/objc/MediaPlayerPrivateAVFoundationObjC.mm:
+        (WebCore::MediaPlayerPrivateAVFoundationObjC::clearTextTracks): Drive-by cleanup, clear the track list completely.
+
 2012-12-18  Eugene Klyuchnikov  <eustas@chromium.org>
 

trunk/Source/WebCore/html/track/InbandTextTrack.cpp

@@ -116 +116 @@
         client()->textTrackAddCues(this, m_cues.get());
 }
+
+bool InbandTextTrack::hasCue(InbandTextTrackPrivate*, double startTime, double endTime, const String& id, const String& content, const String& settings)
+{
+
+    if (startTime < 0 || endTime < 0)
+        return false;
+
+    if (!cues()->length())
+        return false;
+
+    size_t searchStart = 0;
+    size_t searchEnd = cues()->length();
+
+    while (1) {
+        ASSERT(searchStart <= cues()->length());
+        ASSERT(searchEnd <= cues()->length());
+        
+        TextTrackCue* cue;
+        
+        // Cues in the TextTrackCueList are maintained in start time order.
+        if (searchStart == searchEnd) {
+            if (!searchStart)
+                return false;
+            
+            cue = cues()->item(searchStart - 1);
+            if (!cue)
+                return false;
+            if (cue->startTime() != startTime)
+                return false;
+            if (cue->endTime() != endTime)
+                return false;
+            if (cue->text() != content)
+                return false;
+            if (cue->cueSettings() != settings)
+                return false;
+            if (cue->id() != id)
+                return false;
+            
+            return true;
+        }
+        
+        size_t index = (searchStart + searchEnd) / 2;
+        cue = cues()->item(index);
+        if (startTime < cue->startTime() || (startTime == cue->startTime() && endTime > cue->endTime()))
+            searchEnd = index;
+        else
+            searchStart = index + 1;
+    }
     
+    ASSERT_NOT_REACHED();
+    return false;
+}
+
 } // namespace WebCore
 

trunk/Source/WebCore/html/track/InbandTextTrack.h

@@ -53 +53 @@
 
     virtual void addCue(InbandTextTrackPrivate*, double, double, const String&, const String&, const String&) OVERRIDE;
+    virtual bool hasCue(InbandTextTrackPrivate*, double, double, const String&, const String&, const String&) OVERRIDE;
 
     RefPtr<InbandTextTrackPrivate> m_private;

trunk/Source/WebCore/html/track/TextTrackCue.cpp

@@ -836 +836 @@
 void TextTrackCue::setCueSettings(const String& input)
 {
+    m_settings = input;
     unsigned position = 0;
 

trunk/Source/WebCore/html/track/TextTrackCue.h

@@ -120 +120 @@
     void setText(const String&);
 
+    const String& cueSettings() const { return m_settings; }
     void setCueSettings(const String&);
 
@@ -186 +187 @@
     double m_endTime;
     String m_content;
+    String m_settings;
     int m_linePosition;
     int m_computedLinePosition;

trunk/Source/WebCore/platform/graphics/InbandTextTrackPrivateClient.h

@@ -40 +40 @@
     
     virtual void addCue(InbandTextTrackPrivate*, double /*start*/, double /*end*/, const String& /*id*/, const String& /*content*/, const String& /*settings*/) = 0;
+    virtual bool hasCue(InbandTextTrackPrivate*, double /*start*/, double /*end*/, const String& /*id*/, const String& /*content*/, const String& /*settings*/) = 0;
 };
 

trunk/Source/WebCore/platform/graphics/avfoundation/InbandTextTrackPrivateAVF.cpp

@@ -223 +223 @@
 void InbandTextTrackPrivateAVF::processCue(CFArrayRef attributedStrings, double time)
 {
+    if (!m_player)
+        return;
+    
     if (m_havePartialCue) {
         // Cues do not have an explicit duration, they are displayed until the next "cue" (which might be empty) is emitted.
@@ -269 +272 @@
 void InbandTextTrackPrivateAVF::setMode(InbandTextTrackPrivate::Mode newMode)
 {
+    if (!m_player)
+        return;
+
     InbandTextTrackPrivate::Mode oldMode = mode();
     InbandTextTrackPrivate::setMode(newMode);

trunk/Source/WebCore/platform/graphics/avfoundation/MediaPlayerPrivateAVFoundation.cpp

@@ -267 +267 @@
         return;
 
+#if HAVE(AVFOUNDATION_TEXT_TRACK_SUPPORT)
+    // Forget any partially accumulated cue data as the seek could be to a time outside of the cue's
+    // range, which will mean that the next cue delivered will result in the current cue getting the
+    // incorrect duration.
+    if (currentTrack())
+        currentTrack()->resetCueValues();
+#endif
+    
     LOG(Media, "MediaPlayerPrivateAVFoundation::seek(%p) - seeking to %f", this, time);
     m_seekTo = time;
@@ -575 +583 @@
     UNUSED_PARAM(finished);
 
-#if HAVE(AVFOUNDATION_TEXT_TRACK_SUPPORT)
-    // Forget any partially accumulated cue data as the seek could be to a time outside of the cue's
-    // range, which will mean that the next cue delivered will result in the current cue getting the
-    // incorrect duration.
-    if (currentTrack())
-        currentTrack()->resetCueValues();
-#endif
-
     m_seekTo = MediaPlayer::invalidTime();
     updateStates();
@@ -817 +817 @@
         return;
 
+    // AVFoundation returns a cue every time the data is buffered, only add it once.
+    if (track->client()->hasCue(track, track->start(), track->end(), track->id(), track->content(), track->settings())) {
+        LOG(Media, "MediaPlayerPrivateAVFoundation::flushCurrentCue(%p) - already have cue for time %.2f", this, track->start());
+        return;
+    }
+
+    LOG(Media, "MediaPlayerPrivateAVFoundation::flushCurrentCue(%p) - adding cue for time %.2f", this, track->start());
     track->client()->addCue(track, track->start(), track->end(), track->id(), track->content(), track->settings());
 }

trunk/Source/WebCore/platform/graphics/avfoundation/objc/MediaPlayerPrivateAVFoundationObjC.mm

@@ -1198 +1198 @@
 void MediaPlayerPrivateAVFoundationObjC::clearTextTracks()
 {
-    for (unsigned i = 0; i < m_textTracks.size(); ++i)
-        player()->removeTextTrack(m_textTracks[i].get());
+    for (unsigned i = 0; i < m_textTracks.size(); ++i) {
+        RefPtr<InbandTextTrackPrivateAVF> track = m_textTracks[i];
+        player()->removeTextTrack(track);
+        track->disconnect();
+    }
+    m_textTracks.clear();
 }