Partially revert change to not post AttemptRead() every Render().

http://crrev.com/328649 changed VideoRendererImpl so that it only
posted AttemptRead() when there was space in the queue.  Unfortunately
this causes more dropped frames for demanding content (H.264 4K in
particular) than always posting since the AttemptRead() call may be
delayed for some time:

AttemptRead() TimeFromPostUntilExecuted: 0.076 ms
AttemptRead() TimeFromPostUntilExecuted: 4.613 ms
AttemptRead() TimeFromPostUntilExecuted: 56.158 ms
AttemptRead() TimeFromPostUntilExecuted: 56.197 ms
AttemptRead() TimeFromPostUntilExecuted: 52.214 ms
AttemptRead() TimeFromPostUntilExecuted: 35.555 ms
AttemptRead() TimeFromPostUntilExecuted: 18.917 ms

At the same time, the performance improvements I saw locally on my
MacBook are not reflected by the performance bots.  I further see the
dropped frame regression locally on Windows, so let's revert.

I considered a couple other solutions to avoid always posting, but
none were very glamorous:
- Release lock before calling AttemptRead(), fixes cases where Render()
was blocked on the lock in FrameReady().
- Checking the end time / ideal render count for the current frame and
posting on when we're almost expired...

In both cases, neither resolved all problem cases and both add more
complexity than just posting.  The task will abort trivially if there
is no work to do, so just revert.

BUG=439548
TEST=telemetry dropped frame count is lower.

Review URL: https://codereview.chromium.org/1135143002

Cr-Commit-Position: refs/heads/master@{#329281}

media/renderers/video_renderer_impl.cc

@@ -239,13 +239,11 @@ scoped_refptr<VideoFrame> VideoRendererImpl::Render(
         base::TimeDelta::FromMilliseconds(250));
   }
 
-  // To avoid unnecessary work, only post this task if there is a chance of work
-  // to be done.  AttemptRead() may still ignore this call for other reasons.
-  if (!rendered_end_of_stream_ && !HaveReachedBufferingCap(effective_frames)) {
-    task_runner_->PostTask(FROM_HERE,
-                           base::Bind(&VideoRendererImpl::AttemptRead,
-                                      weak_factory_.GetWeakPtr()));
-  }
+  // Always post this task, it will acquire new frames if necessary and since it
+  // happens on another thread, even if we don't have room in the queue now, by
+  // the time it runs (may be delayed up to 50ms for complex decodes!) we might.
+  task_runner_->PostTask(FROM_HERE, base::Bind(&VideoRendererImpl::AttemptRead,
+                                               weak_factory_.GetWeakPtr()));
 
   return result;
 }
@@ -685,22 +683,17 @@ void VideoRendererImpl::MaybeStopSinkAfterFirstPaint() {
 
 bool VideoRendererImpl::HaveReachedBufferingCap() {
   DCHECK(task_runner_->BelongsToCurrentThread());
-  return HaveReachedBufferingCap(use_new_video_renderering_path_
-                                     ? algorithm_->EffectiveFramesQueued()
-                                     : 0);
-}
+  const size_t kMaxVideoFrames = limits::kMaxVideoFrames;
 
-bool VideoRendererImpl::HaveReachedBufferingCap(size_t effective_frames) {
   if (use_new_video_renderering_path_) {
     // When the display rate is less than the frame rate, the effective frames
     // queued may be much smaller than the actual number of frames queued.  Here
     // we ensure that frames_queued() doesn't get excessive.
-    return effective_frames >= static_cast<size_t>(limits::kMaxVideoFrames) ||
-           algorithm_->frames_queued() >=
-               static_cast<size_t>(3 * limits::kMaxVideoFrames);
+    return algorithm_->EffectiveFramesQueued() >= kMaxVideoFrames ||
+           algorithm_->frames_queued() >= 3 * kMaxVideoFrames;
   }
 
-  return ready_frames_.size() >= static_cast<size_t>(limits::kMaxVideoFrames);
+  return ready_frames_.size() >= kMaxVideoFrames;
 }
 
 void VideoRendererImpl::StartSink() {

media/renderers/video_renderer_impl.h

@@ -135,11 +135,8 @@ class MEDIA_EXPORT VideoRendererImpl
   // false it Stop() on |sink_|.
   void MaybeStopSinkAfterFirstPaint();
 
-  // Returns true if there is no more room for additional buffered frames.  The
-  // overloaded method is the same, but allows skipping an internal call to
-  // EffectiveFramesQueued() if that value is already known.
+  // Returns true if there is no more room for additional buffered frames.
   bool HaveReachedBufferingCap();
-  bool HaveReachedBufferingCap(size_t effective_frames);
 
   // Starts or stops |sink_| respectively. Do not call while |lock_| is held.
   void StartSink();