Seeking media fragment URI before loadeddata event

Source/core/html/HTMLMediaElement.cpp

 /*
  * Copyright (C) 2007, 2008, 2009, 2010, 2011, 2012, 2013 Apple Inc. All rights reserved.
  *
  * Redistribution and use in source and binary forms, with or without
  * modification, are permitted provided that the following conditions
  * are met:
  * 1. Redistributions of source code must retain the above copyright
  *    notice, this list of conditions and the following disclaimer.
  * 2. Redistributions in binary form must reproduce the above copyright
  *    notice, this list of conditions and the following disclaimer in the
@@ skipping 316 matching lines @@
     , m_defaultPlaybackStartPosition(0)
     , m_loadState(WaitingForSource)
     , m_deferredLoadState(NotDeferred)
     , m_deferredLoadTimer(this, &HTMLMediaElement::deferredLoadTimerFired)
     , m_webLayer(0)
     , m_preload(MediaPlayer::Auto)
     , m_displayMode(Unknown)
     , m_cachedTime(MediaPlayer::invalidTime())
     , m_cachedTimeWallClockUpdateTime(0)
     , m_minimumWallClockTimeToCacheMediaTime(0)
-    , m_fragmentStartTime(MediaPlayer::invalidTime())
     , m_fragmentEndTime(MediaPlayer::invalidTime())
     , m_pendingActionFlags(0)
     , m_userGestureRequiredForPlay(false)
     , m_playing(false)
     , m_shouldDelayLoadEvent(false)
     , m_haveFiredLoadedData(false)
     , m_active(true)
     , m_autoplaying(true)
     , m_muted(false)
     , m_paused(true)
@@ skipping 1458 matching lines @@
         if (wasPotentiallyPlaying && m_readyState < HAVE_FUTURE_DATA) {
             // 4.8.10.8
             scheduleTimeupdateEvent(false);
             scheduleEvent(EventTypeNames::waiting);
         }
     }
 
     if (m_readyState >= HAVE_METADATA && oldState < HAVE_METADATA) {
         createPlaceholderTracksIfNecessary();
 
-        prepareMediaFragmentURI();
+        selectInitialTracksIfNecessary();
 
-        selectInitialTracksIfNecessary();
+        MediaFragmentURIParser fragmentParser(m_currentSrc);
+        m_fragmentEndTime = fragmentParser.endTime();
 
         m_duration = duration();
         scheduleEvent(EventTypeNames::durationchange);
 
         if (isHTMLVideoElement())
             scheduleEvent(EventTypeNames::resize);
         scheduleEvent(EventTypeNames::loadedmetadata);
 
+        bool jumped = false;
         if (m_defaultPlaybackStartPosition > 0) {
             seek(m_defaultPlaybackStartPosition);
+            jumped = true;
             m_defaultPlaybackStartPosition = 0;
         }
 
+        if (!jumped) {
+            double start = fragmentParser.startTime();
+            if (start != MediaPlayer::invalidTime() && start > 0) {
+                m_sentEndEvent = false;
+                UseCounter::count(document(), UseCounter::HTMLMediaElementSeekToFragmentStart);
+                seek(start);
+                jumped = true;
+            }
+            // FIXME: Add support for selecting tracks by ID with the Media Fragments track dimension.
+        }
+
+        if (jumped && m_mediaController && m_mediaController->currentTime() < currentTime())

[philipj_slow, 2014/09/10 12:25:48]

This bit would be clearer if the initial playback position were made explicit.
I've filed a spec bug pointing out that it's only used locally in these steps:
https://www.w3.org/Bugs/Public/show_bug.cgi?id=26771

I suggest a double initialPlaybackPosition locally in these steps. The structure
should be something like:

if (m_mediaController) {
    if (jumped && initialPlaybackPosition > m_mediaController->currentTime())
        m_mediaController->setCurrentTime(initialPlaybackPosition);
    else
        seek(m_mediaController->currentTime());
}

Writing that out, it's clear that this step is trying to get the controller and
media element in sync, which is relevant to the bug you filed. Maybe there is no
bug in the spec?

[amogh.bihani, 2014/09/11 13:35:59]

I guess then it would be going away from media element load algorithm (step
8)... right?
Ah yes! It works as intended after this. :)

[philipj_slow, 2014/09/11 14:38:26]

Yes, AFAICT that step doesn't amount to anything that couldn't be kept local.
Should I close the spec bug then?

[amogh.bihani, 2014/09/12 11:02:23]

Yes sure :)

+            m_mediaController->setCurrentTime(currentTime());
+
         if (hasMediaControls())
             mediaControls()->reset();
         if (renderer())
             renderer()->updateFromElement();
     }
 
     bool shouldUpdateDisplayState = false;
 
     if (m_readyState >= HAVE_CURRENT_DATA && oldState < HAVE_CURRENT_DATA && !m_haveFiredLoadedData) {
         m_haveFiredLoadedData = true;
         shouldUpdateDisplayState = true;
         scheduleEvent(EventTypeNames::loadeddata);
         setShouldDelayLoadEvent(false);
-        applyMediaFragmentURI();
     }
 
     bool isPotentiallyPlaying = potentiallyPlaying();
     if (m_readyState == HAVE_FUTURE_DATA && oldState <= HAVE_CURRENT_DATA && tracksAreReady) {
         scheduleEvent(EventTypeNames::canplay);
         if (isPotentiallyPlaying)
             scheduleEvent(EventTypeNames::playing);
         shouldUpdateDisplayState = true;
     }
 
@@ skipping 2006 matching lines @@
     // or if its media controller position is either before the media resource's earliest possible
     // position relative to the MediaController's timeline or after the end of the media resource
     // relative to the MediaController's timeline.
     double mediaControllerPosition = m_mediaController->currentTime();
     if (mediaControllerPosition < 0 || mediaControllerPosition > duration())
         return true;
 
     return false;
 }
 
-void HTMLMediaElement::prepareMediaFragmentURI()
-{
-    MediaFragmentURIParser fragmentParser(m_currentSrc);
-    double dur = duration();
-
-    double start = fragmentParser.startTime();
-    if (start != MediaFragmentURIParser::invalidTimeValue() && start > 0) {
-        m_fragmentStartTime = start;
-        if (m_fragmentStartTime > dur)
-            m_fragmentStartTime = dur;
-    } else
-        m_fragmentStartTime = MediaPlayer::invalidTime();
-
-    double end = fragmentParser.endTime();
-    if (end != MediaFragmentURIParser::invalidTimeValue() && end > 0 && end > m_fragmentStartTime) {
-        m_fragmentEndTime = end;
-        if (m_fragmentEndTime > dur)
-            m_fragmentEndTime = dur;
-    } else
-        m_fragmentEndTime = MediaPlayer::invalidTime();
-
-    // FIXME: Add support for selecting tracks by ID with the Media Fragments track dimension.
-
-    if (m_fragmentStartTime != MediaPlayer::invalidTime() && m_readyState < HAVE_FUTURE_DATA)
-        prepareToPlay();
-}
-
-void HTMLMediaElement::applyMediaFragmentURI()
-{
-    if (m_fragmentStartTime != MediaPlayer::invalidTime()) {
-        m_sentEndEvent = false;
-        UseCounter::count(document(), UseCounter::HTMLMediaElementSeekToFragmentStart);
-        seek(m_fragmentStartTime);
-    }
-}
-
 WebMediaPlayer::CORSMode HTMLMediaElement::corsMode() const
 {
     const AtomicString& crossOriginMode = fastGetAttribute(crossoriginAttr);
     if (crossOriginMode.isNull())
         return WebMediaPlayer::CORSModeUnspecified;
     if (equalIgnoringCase(crossOriginMode, "use-credentials"))
         return WebMediaPlayer::CORSModeUseCredentials;
     return WebMediaPlayer::CORSModeAnonymous;
 }
 
@@ skipping 83 matching lines @@
 
 #if ENABLE(WEB_AUDIO)
 void HTMLMediaElement::clearWeakMembers(Visitor* visitor)
 {
     if (!visitor->isAlive(m_audioSourceNode) && audioSourceProvider())
         audioSourceProvider()->setClient(0);
 }
 #endif
 
 }