Transition media element to HAVE_CURRENT_DATA upon underflow.

media/blink/webmediaplayer_impl.cc

 // Copyright 2013 The Chromium Authors. All rights reserved.
 // Use of this source code is governed by a BSD-style license that can be
 // found in the LICENSE file.
 
 #include "media/blink/webmediaplayer_impl.h"
 
 #include <algorithm>
 #include <cmath>
 #include <limits>
 #include <string>
@@ skipping 1017 matching lines @@
 
 void WebMediaPlayerImpl::OnBufferingStateChange(BufferingState state) {
   DVLOG(1) << __FUNCTION__ << "(" << state << ")";
   DCHECK(main_task_runner_->BelongsToCurrentThread());
 
   // Ignore buffering state changes until we've completed all outstanding
   // operations.
   if (!pipeline_controller_.IsStable())
     return;
 
-  // TODO(scherkus): Handle other buffering states when Pipeline starts using
-  // them and translate them ready state changes http://crbug.com/144683
-  DCHECK_EQ(state, BUFFERING_HAVE_ENOUGH);
-  SetReadyState(WebMediaPlayer::ReadyStateHaveEnoughData);
+  if (state == BUFFERING_HAVE_ENOUGH) {
+    // TODO(chcunningham): Monitor playback position vs buffered. Potentially
+    // transition to HAVE_FUTURE_DATA here if not enough is buffered.
+    SetReadyState(WebMediaPlayer::ReadyStateHaveEnoughData);
 
-  // Let the DataSource know we have enough data. It may use this information to
-  // release unused network connections.
-  if (data_source_)
-    data_source_->OnBufferingHaveEnough(false);
+    // Let the DataSource know we have enough data. It may use this information
+    // to release unused network connections.
+    if (data_source_)
+      data_source_->OnBufferingHaveEnough(false);
+  } else {
+    // Buffering has underflowed.
+    DCHECK_EQ(state, BUFFERING_HAVE_NOTHING);
+    // It shouldn't be possible to underflow if we've not advanced past
+    // HAVE_CURRENT_DATA.
+    DCHECK_GT(highest_ready_state_, WebMediaPlayer::ReadyStateHaveCurrentData);
+    SetReadyState(WebMediaPlayer::ReadyStateHaveCurrentData);
+  }
 
   // Blink expects a timeChanged() in response to a seek().
   if (should_notify_time_changed_)
     client_->timeChanged();

[wolenetz, 2016/07/07 21:32:03]

Is timeChanged() notification also always required on underflow?

[sandersd (OOO until July 31), 2016/07/07 21:49:19]

Changing the ready state does not directly change anything that HTMLMediaElement
cares about, so I don't think so.

(There is also a separate readyStateChanged() call that happens inside
SetReadyState().)

[chcunningham, 2016/07/07 22:29:41]

sandersd@, do we need to bother calling timeChanged in the HAVE_ENOUGH case
then? Can we just remove the call altogether?

I initially thought this was needed to fire the timeupdate event which the spec
asks for at the transition from HAVE_CURRENT to less than HAVE_CURRENT. But I
see that this is already taken care of in HTMLMediaElement's SetReadyState

[sandersd (OOO until July 31), 2016/07/07 22:33:12]

HTMLMediaElement actually hooks this to fire the 'seeked' event, we do need it.

[chcunningham, 2016/07/12 00:47:17]

Ack. Moved this up. FYI I also moved ReportMemoryUsage into the HAVE_ENOUGH case
because its comment mentions "have enough" ... double check me here.

[wolenetz, 2016/07/13 22:27:54]

I think this is correct.

 
   // Once we have enough, start reporting the total memory usage. We'll also
   // report once playback starts.
   ReportMemoryUsage();
 
   UpdatePlayState();

[wolenetz, 2016/07/07 21:32:03]

Does underflow allow us to suspend if idle (stalled) long enough? From chat with
sandersd@, the behavior expectation is that (barring being in the middle of a
seek), an underflow should enable suspend to occur if underflowed long enough.
Please add appropriate wmpi_unittest(s) to verify underflow during, and not
during, seek, results on suspend behavior. +sandersd@ to CR that portion of this
CL, too.

[sandersd (OOO until July 31), 2016/07/07 21:49:19]

In general these conditions should be unchanged; the state that matters is
whether the current state is >= HAVE_METADATA, whether we have ever been at
HAVE_FUTURE_DATA.

Se we expect the usual behavior: suspend while paused or when backgrounded,
don't suspend during seek (except for MSE), etc.

[chcunningham, 2016/07/07 22:29:41]

sandersd@ do we already have a test for this usual behavior? Is a new test
needed?

[sandersd (OOO until July 31), 2016/07/07 22:33:12]

There are already tests for the main cases of note, you'll just want to add some
to make sure we don't regress handling of this new ready state.

[chcunningham, 2016/07/12 00:47:17]

Underflow is a very different user experience then intentional pause. After
chatting with sanders, we agree that underflow should never be capable of
triggering idle suspend. We agreed on 2 main scenarios:

1. simulating underflow does not causes idle suspend
2. simulating underflow does not interfere with legit background susp

I've added a test to cover both these cases.

[wolenetz, 2016/07/13 22:27:54]

Acknowledged.

 }
 
 void WebMediaPlayerImpl::OnDurationChange() {
   DCHECK(main_task_runner_->BelongsToCurrentThread());
 
   // TODO(sandersd): We should call delegate_->DidPlay() with the new duration,
   // especially if it changed from  <5s to >5s.
   if (ready_state_ == WebMediaPlayer::ReadyStateHaveNothing)
     return;
 
@@ skipping 623 matching lines @@
   if (isRemote())
     return;
 #endif
 
   // Idle timeout chosen arbitrarily.
   background_pause_timer_.Start(FROM_HERE, base::TimeDelta::FromSeconds(5),
                                 this, &WebMediaPlayerImpl::OnPause);
 }
 
 }  // namespace media