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_duration = duration();
         scheduleEvent(EventTypeNames::durationchange);
 
+        double end = fragmentParser.endTime();

[amogh.bihani, 2014/09/05 09:05:43]

Does end==0 needs to be taken care of? As per line 2430 (which uses >= for
comparison) I don't think so, but wanted to confirm first. If it is not needed
then we can also remove 'double end' and save one more variable. :)

something like
m_fragmentEndtime = fragmentParser.endTime();
if (m_fragmentEndTime != invalidTime())
    m_fraggmentEndTime = std::min((std::max(0, m_fraggmentEndTime), duration);

[philipj_slow, 2014/09/05 22:49:11]

At this point it seems like we should just set m_fragmentEndTime =
fragmentParser.endTime() and it seems like
HTMLMediaElement::playbackProgressTimerFired will behave just fine. Clamping to
the duration seems pretty useless, I'm not sure why that was done originally.

[amogh.bihani, 2014/09/08 09:33:46]

Done.

+        if (end != MediaPlayer::invalidTime() && end > 0)
+            m_fragmentEndTime = std::min(end, m_duration);
+        else
+            m_fragmentEndTime = MediaPlayer::invalidTime();
+
         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) {

[philipj_slow, 2014/09/05 22:49:12]

Missing whitespace after !=

+                m_sentEndEvent = false;
+                UseCounter::count(document(), UseCounter::HTMLMediaElementSeekToFragmentStart);
+                seek(start);
+                jumped = true;

[philipj_slow, 2014/09/05 22:49:11]

Don't need to set jumped, the spec doesn't AFAICT.

[amogh.bihani, 2014/09/08 09:33:45]

Step 10 says "if jumped is still false, seek to that time and let jumped be
true."

I had not implemented step 13 as I thought it is a big change and should be in a
new CL, although it turns out it wasn't so I am adding that in next PS.

[philipj_slow, 2014/09/08 14:42:31]

Oh sorry, sloppy spec reading on my part. That the use of jumped wasn't
implemented was indeed why I missed this.

+            }
+            // FIXME: Add support for selecting tracks by ID with the Media Fragments track dimension.
+        }
+
         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
 
 }