Don't underflow for background rendering. Ensure accurate counts.

media/renderers/video_renderer_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/renderers/video_renderer_impl.h"
 
 #include <utility>
 
 #include "base/bind.h"
 #include "base/callback.h"
@@ skipping 168 matching lines @@
   scoped_refptr<VideoFrame> result =
       algorithm_->Render(deadline_min, deadline_max, &frames_dropped);
 
   // Due to how the |algorithm_| holds frames, this should never be null if
   // we've had a proper startup sequence.
   DCHECK(result);
 
   // Declare HAVE_NOTHING if we reach a state where we can't progress playback
   // any further.  We don't want to do this if we've already done so, reached
   // end of stream, or have frames available.  We also don't want to do this in
-  // background rendering mode unless this isn't the first background render
-  // tick and we haven't seen any decoded frames since the last one.
+  // background rendering mode, as the frames aren't visible anyways.
   MaybeFireEndedCallback_Locked(true);
   if (buffering_state_ == BUFFERING_HAVE_ENOUGH && !received_end_of_stream_ &&
-      !algorithm_->effective_frames_queued() &&
-      (!background_rendering ||
-       (!frames_decoded_ && was_background_rendering_))) {
+      !algorithm_->effective_frames_queued() && !background_rendering &&
+      !was_background_rendering_) {

[chcunningham, 2016/09/09 21:27:06]

So, IIUC this is just giving us one more tick to get the queue full, right?
Expected to make a big difference?

[DaleCurtis, 2016/09/09 21:51:41]

Yes, because whenever the previous state was reached this would always be the
underflow state.

[chcunningham, 2016/09/09 22:17:46]

You're saying that every time we will *never* have frames decoded between the
transition of Background and Foreground rendering? I expect that this may
sometimes occur, but not always?

[DaleCurtis, 2016/09/09 22:20:46]

I mean every time we get into a state where we were previously flip flopping on
underflow we would always hit underflow on the first non-background-render tick.

     // Do not set |buffering_state_| here as the lock in FrameReady() may be
     // held already and it fire the state changes in the wrong order.
     DVLOG(3) << __func__ << " posted TransitionToHaveNothing.";
     task_runner_->PostTask(
         FROM_HERE, base::Bind(&VideoRendererImpl::TransitionToHaveNothing,
                               weak_factory_.GetWeakPtr()));
   }
 
   // We don't count dropped frames in the background to avoid skewing the count
   // and impacting JavaScript visible metrics used by web developers.
@@ skipping 478 matching lines @@
 
 void VideoRendererImpl::AttemptReadAndCheckForMetadataChanges(
     VideoPixelFormat pixel_format,
     const gfx::Size& natural_size) {
   base::AutoLock auto_lock(lock_);
   CheckForMetadataChanges(pixel_format, natural_size);
   AttemptRead_Locked();
 }
 
 }  // namespace media