Changeset 188390 in webkit


Ignore:
Timestamp:
Aug 13, 2015 12:16:09 PM (9 years ago)
Author:
eric.carlson@apple.com
Message:

Don't short circuit seeking
​https://bugs.webkit.org/show_bug.cgi?id=147892

Reviewed by Jer Noble.

Source/WebCore:

Test: media/video-seek-to-current-time.html

  • html/HTMLMediaElement.cpp:

(WebCore::HTMLMediaElement::prepareForLoad): Call clearSeeking.
(WebCore::HTMLMediaElement::fastSeek): Add logging.
(WebCore::HTMLMediaElement::seekWithTolerance): Add logging. Set m_pendingSeekType.
(WebCore::HTMLMediaElement::seekTask): Call clearSeeking. Don't short circuit a

if the current or pending seek is a fast seek. Set m_seeking to true immediately
before calling media engine as it may have been cleared before the seek task
queue ran.

(WebCore::HTMLMediaElement::clearSeeking): New.

  • html/HTMLMediaElement.h:
  • html/HTMLMediaElementEnums.h:
  • platform/GenericTaskQueue.h:

(WebCore::GenericTaskQueue::enqueueTask): Clear m_pendingTasks.

  • platform/graphics/avfoundation/MediaPlayerPrivateAVFoundation.cpp:

(WebCore::MediaPlayerPrivateAVFoundation::seekWithTolerance): Don't return early

when asked to seek to the current time.

(WebCore::MediaPlayerPrivateAVFoundation::invalidateCachedDuration): Remove some

extremely noisy logging.

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

(WebCore::MediaPlayerPrivateAVFoundationObjC::seekToTime): Add logging.

LayoutTests:

  • media/event-attributes-expected.txt: Update for test change.
  • media/event-attributes.html: There is no reason to expect that a 'timeupdate' will have been sent before 'canplaythrough'.
  • media/video-seek-to-current-time-expected.txt: Added.
  • media/video-seek-to-current-time.html: Added.
  • platform/efl/TestExpectations: Skip new test.
  • platform/gtk/TestExpectations: Ditto.
  • platform/mac/TestExpectations: Mark the new test as sometimes failing because of webkit.org/b/147944.
  • platform/win/TestExpectations: Skip new test.
Location:
trunk
Files:
2 added
14 edited

Legend:

Unmodified
Added
Removed

  • trunk/LayoutTests/ChangeLog

    r188383 r188390  
     12015-08-13  Eric Carlson  <eric.carlson@apple.com>
     2
     3        Don't short circuit seeking
     4        https://bugs.webkit.org/show_bug.cgi?id=147892
     5
     6        Reviewed by Jer Noble.
     7
     8        * media/event-attributes-expected.txt: Update for test change.
     9        * media/event-attributes.html: There is no reason to expect that a 'timeupdate' will have
     10          been sent before 'canplaythrough'.
     11        * media/video-seek-to-current-time-expected.txt: Added.
     12        * media/video-seek-to-current-time.html: Added.
     13        * platform/efl/TestExpectations: Skip new test.
     14        * platform/gtk/TestExpectations: Ditto.
     15        * platform/mac/TestExpectations: Mark the new test as sometimes failing because of
     16          webkit.org/b/147944.
     17        * platform/win/TestExpectations: Skip new test.
     18
    1192015-08-13  Alexey Proskuryakov  <ap@apple.com>
    220

  • trunk/LayoutTests/media/event-attributes-expected.txt

    r158743 r188390  
    66EVENT(canplay)
    77EVENT(canplaythrough)
    8 EXPECTED (progressEventCount >= '1') OK
    98
    109*** starting playback

  • trunk/LayoutTests/media/event-attributes.html

    r125102 r188390  
    1919                {
    2020                    case "canplaythrough":
    21                         testExpected('progressEventCount', 1, '>=');
    2221                        consoleWrite("<br>*** starting playback");
    2322                        run("video.play()");

  • trunk/LayoutTests/platform/efl/TestExpectations

    r188263 r188390  
    22572257# This test relies on iOS-specific font fallback.
    22582258fast/text/arabic-glyph-cache-fill-combine.html [ ImageOnlyFailure ]
     2259
     2260# This test uses an MPEG-4 video
     2261media/video-seek-to-current-time.html [ Skip ]

  • trunk/LayoutTests/platform/gtk/TestExpectations

    r188263 r188390  
    24242424# This test relies on iOS-specific font fallback.
    24252425fast/text/arabic-glyph-cache-fill-combine.html [ ImageOnlyFailure ]
     2426
     2427# This test uses an MPEG-4 video
     2428media/video-seek-to-current-time.html [ Skip ]

  • trunk/LayoutTests/platform/mac/TestExpectations

    r188380 r188390  
    990990webkit.org/b/141294 compositing/reflections/masked-reflection-on-composited.html [ Pass Crash ]
    991991webkit.org/b/142152 media/track/track-in-band-cues-added-once.html [ Pass Failure ]
     992webkit.org/b/147944 media/video-seek-to-current-time.html [ Pass Failure ]
    992993## --- End flaky media tests
    993994

  • trunk/LayoutTests/platform/win/TestExpectations

    r188307 r188390  
    31713171fast/forms/indeterminate-progress-inline-height.html [ Failure ]
    31723172fast/forms/listbox-scrollbar-hit-test.html [ Failure ]
     3173
     3174# This test uses an MPEG-4 video
     3175media/video-seek-to-current-time.html [ Skip ]

  • trunk/Source/WebCore/ChangeLog

    r188389 r188390  
     12015-08-13  Eric Carlson  <eric.carlson@apple.com>
     2
     3        Don't short circuit seeking
     4        https://bugs.webkit.org/show_bug.cgi?id=147892
     5
     6        Reviewed by Jer Noble.
     7
     8        Test: media/video-seek-to-current-time.html
     9
     10        * html/HTMLMediaElement.cpp:
     11        (WebCore::HTMLMediaElement::prepareForLoad): Call clearSeeking.
     12        (WebCore::HTMLMediaElement::fastSeek): Add logging.
     13        (WebCore::HTMLMediaElement::seekWithTolerance): Add logging. Set m_pendingSeekType.
     14        (WebCore::HTMLMediaElement::seekTask):  Call clearSeeking. Don't short circuit a
     15          if the current or pending seek is a fast seek. Set m_seeking to true immediately
     16          before calling media engine as it may have been cleared before the seek task
     17          queue ran.
     18        (WebCore::HTMLMediaElement::clearSeeking): New.
     19        * html/HTMLMediaElement.h:
     20        * html/HTMLMediaElementEnums.h:
     21
     22        * platform/GenericTaskQueue.h:
     23        (WebCore::GenericTaskQueue::enqueueTask): Clear m_pendingTasks.
     24
     25        * platform/graphics/avfoundation/MediaPlayerPrivateAVFoundation.cpp:
     26        (WebCore::MediaPlayerPrivateAVFoundation::seekWithTolerance): Don't return early
     27          when asked to seek to the current time.
     28        (WebCore::MediaPlayerPrivateAVFoundation::invalidateCachedDuration): Remove some
     29          extremely noisy logging.
     30
     31        * platform/graphics/avfoundation/objc/MediaPlayerPrivateAVFoundationObjC.mm:
     32        (WebCore::MediaPlayerPrivateAVFoundationObjC::seekToTime): Add logging.
     33
    1342015-08-13  Simon Fraser  <simon.fraser@apple.com>
    235

  • trunk/Source/WebCore/html/HTMLMediaElement.cpp

    r188234 r188390  
    10181018
    10191019        // 4.6 - If seeking is true, set it to false.
    1020         m_seeking = false;
     1020        clearSeeking();
    10211021
    10221022        // 4.7 - Set the current playback position to 0.
    … …  
    24942494    // it is running to complete.
    24952495    if (m_seekTaskQueue.hasPendingTasks()) {
     2496        LOG(Media, "HTMLMediaElement::seekWithTolerance(%p) - cancelling pending seeks", this);
    24962497        m_seekTaskQueue.cancelAllTasks();
    24972498        m_pendingSeek = nullptr;
     2499        m_pendingSeekType = NoSeek;
    24982500    }
    24992501
    … …  
    25102512    // the script. The remainder of these steps must be run asynchronously.
    25112513    m_pendingSeek = std::make_unique<PendingSeek>(now, time, negativeTolerance, positiveTolerance);
    2512     if (fromDOM)
     2514    if (fromDOM) {
     2515        LOG(Media, "HTMLMediaElement::seekWithTolerance(%p) - enqueuing seek from %s to %s", this, toString(now).utf8().data(), toString(time).utf8().data());
    25132516        m_seekTaskQueue.enqueueTask(std::bind(&HTMLMediaElement::seekTask, this));
    2514     else
     2517    } else
    25152518        seekTask();
    25162519}
    … …  
    25182521void HTMLMediaElement::seekTask()
    25192522{
     2523    LOG(Media, "HTMLMediaElement::seekTask(%p)", this);
     2524
    25202525    if (!m_player) {
    2521         m_seeking = false;
     2526        clearSeeking();
    25222527        return;
    25232528    }
    … …  
    25462551    MediaTime mediaTime = m_player->mediaTimeForTimeValue(time);
    25472552    if (time != mediaTime)
    2548         LOG(Media, "HTMLMediaElement::seekTimerFired(%p) - %s - media timeline equivalent is %s", this, toString(time).utf8().data(), toString(mediaTime).utf8().data());
     2553        LOG(Media, "HTMLMediaElement::seekTask(%p) - %s - media timeline equivalent is %s", this, toString(time).utf8().data(), toString(mediaTime).utf8().data());
    25492554#endif
    25502555    time = m_player->mediaTimeForTimeValue(time);
    … …  
    25552560    // attribute then set the seeking IDL attribute to false and abort these steps.
    25562561    RefPtr<TimeRanges> seekableRanges = seekable();
     2562    bool noSeekRequired = !seekableRanges->length();
    25572563
    25582564    // Short circuit seeking to the current time by just firing the events if no seek is required.
    2559     // Don't skip calling the media engine if we are in poster mode because a seek should always
    2560     // cancel poster display.
    2561     bool noSeekRequired = !seekableRanges->length() || (time == now && displayMode() != Poster);
     2565    // Don't skip calling the media engine if 1) we are in poster mode (because a seek should always cancel
     2566    // poster display), or 2) if there is a pending fast seek, or 3) if this seek is not an exact seek
     2567    SeekType thisSeekType = (negativeTolerance == MediaTime::zeroTime() && positiveTolerance == MediaTime::zeroTime()) ? Precise : Fast;
     2568    if (!noSeekRequired && time == now && thisSeekType == Precise && m_pendingSeekType != Fast && displayMode() != Poster)
     2569        noSeekRequired = true;
    25622570
    25632571#if ENABLE(MEDIA_SOURCE)
    … …  
    25692577
    25702578    if (noSeekRequired) {
     2579        LOG(Media, "HTMLMediaElement::seekTask(%p) - seek to %s ignored", this, toString(time).utf8().data());
    25712580        if (time == now) {
    25722581            scheduleEvent(eventNames().seekingEvent);
    … …  
    25742583            scheduleEvent(eventNames().seekedEvent);
    25752584        }
    2576         m_seeking = false;
     2585        clearSeeking();
    25772586        return;
    25782587    }
    … …  
    25812590    m_sentEndEvent = false;
    25822591    m_lastSeekTime = time;
     2592    m_pendingSeekType = thisSeekType;
     2593    m_seeking = true;
    25832594
    25842595    // 10 - Queue a task to fire a simple event named seeking at the element.
    … …  
    25932604}
    25942605
     2606void HTMLMediaElement::clearSeeking()
     2607{
     2608    m_seeking = false;
     2609    m_pendingSeekType = NoSeek;
     2610    invalidateCachedTime();
     2611}
     2612
    25952613void HTMLMediaElement::finishSeek()
    25962614{
    2597     LOG(Media, "HTMLMediaElement::finishSeek(%p)", this);
    2598 
    25992615    // 4.8.10.9 Seeking
    26002616    // 14 - Set the seeking IDL attribute to false.
    2601     m_seeking = false;
     2617    clearSeeking();
     2618
     2619    LOG(Media, "HTMLMediaElement::finishSeek(%p) - current time = %s", this, toString(currentMediaTime()).utf8().data());
    26022620
    26032621    // 15 - Run the time maches on steps.

  • trunk/Source/WebCore/html/HTMLMediaElement.h

    r188234 r188390  
    636636    void seekWithTolerance(const MediaTime&, const MediaTime& negativeTolerance, const MediaTime& positiveTolerance, bool fromDOM);
    637637    void finishSeek();
    638     void checkIfSeekNeeded();
     638    void clearSeeking();
    639639    void addPlayedRange(const MediaTime& start, const MediaTime& end);
    640640   
    … …  
    798798    };
    799799    std::unique_ptr<PendingSeek> m_pendingSeek;
     800    SeekType m_pendingSeekType { NoSeek };
    800801
    801802    double m_volume;

  • trunk/Source/WebCore/html/HTMLMediaElementEnums.h

    r186016 r188390  
    5050    enum TextTrackVisibilityCheckType { CheckTextTrackVisibility, AssumeTextTrackVisibilityChanged };
    5151    enum InvalidURLAction { DoNothing, Complain };
     52
     53    typedef enum {
     54        NoSeek,
     55        Fast,
     56        Precise
     57    } SeekType;
    5258};
    5359

  • trunk/Source/WebCore/platform/GenericTaskQueue.h

    r187655 r188390  
    103103            if (!weakThis)
    104104                return;
     105            ASSERT(weakThis->m_pendingTasks);
     106            --weakThis->m_pendingTasks;
    105107            task();
    106108        });

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

    r188002 r188390  
    274274    if (time > durationMediaTime())
    275275        time = durationMediaTime();
    276 
    277     if (currentMediaTime() == time)
    278         return;
    279276
    280277    if (currentTextTrack())
    … …  
    685682void MediaPlayerPrivateAVFoundation::invalidateCachedDuration()
    686683{
    687     LOG(Media, "MediaPlayerPrivateAVFoundation::invalidateCachedDuration(%p)", this);
    688    
    689684    m_cachedDuration = MediaTime::invalidTime();
    690685

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

    r187044 r188390  
    13061306    auto weakThis = createWeakPtr();
    13071307
     1308    LOG(Media, "MediaPlayerPrivateAVFoundationObjC::seekToTime(%p) - calling seekToTime", this);
     1309
    13081310    [m_avPlayerItem.get() seekToTime:cmTime toleranceBefore:cmBefore toleranceAfter:cmAfter completionHandler:^(BOOL finished) {
    1309         callOnMainThread([weakThis, finished] {
     1311        double currentTime = CMTimeGetSeconds([m_avPlayerItem currentTime]);
     1312        callOnMainThread([weakThis, finished, currentTime] {
     1313            UNUSED_PARAM(currentTime);
    13101314            auto _this = weakThis.get();
     1315            LOG(Media, "MediaPlayerPrivateAVFoundationObjC::seekToTime(%p) - completion handler called, currentTime = %f", _this, currentTime);
    13111316            if (!_this)
    13121317                return;
Note: See TracChangeset for help on using the changeset viewer.