Fix ChunkDemuxer seek deadlock

media/filters/video_renderer_base.cc

 // Copyright (c) 2012 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 "base/bind.h"
 #include "base/callback.h"
 #include "base/threading/platform_thread.h"
 #include "media/base/buffers.h"
 #include "media/base/filter_host.h"
 #include "media/base/limits.h"
@@ skipping 318 matching lines @@
   // frame when this is true.
   frame_available_.Signal();
   if (state_ == kFlushing) {
     AttemptFlush_Locked();
   } else if (state_ == kError || state_ == kStopped) {
     DoStopOrError_Locked();
   }
 }
 
 void VideoRendererBase::FrameReady(scoped_refptr<VideoFrame> frame) {
-  DCHECK(frame);
-
   base::AutoLock auto_lock(lock_);
   DCHECK_NE(state_, kUninitialized);
   DCHECK_NE(state_, kStopped);
   DCHECK_NE(state_, kError);
   DCHECK_NE(state_, kFlushed);
   CHECK(pending_read_);
 
   pending_read_ = false;
 
   if (state_ == kFlushing) {
     AttemptFlush_Locked();
     return;
   }
 
-  // Discard frames until we reach our desired seek timestamp.
-  if (state_ == kSeeking && !frame->IsEndOfStream() &&
-      (frame->GetTimestamp() + frame->GetDuration()) <= seek_timestamp_) {
-    AttemptRead_Locked();
-    return;
-  }
+  if (frame) {

[Ami GONE FROM CHROMIUM, 2012/01/27 23:44:58]

reverse test & early-return?

[acolwell GONE FROM CHROMIUM, 2012/01/29 03:00:41]

Done.

+    // Discard frames until we reach our desired seek timestamp.
+    if (state_ == kSeeking && !frame->IsEndOfStream() &&
+        (frame->GetTimestamp() + frame->GetDuration()) <= seek_timestamp_) {
+      AttemptRead_Locked();
+      return;
+    }
 
-  // Adjust the incoming frame if its rendering stop time is past the duration
-  // of the video itself. This is typically the last frame of the video and
-  // occurs if the container specifies a duration that isn't a multiple of the
-  // frame rate.  Another way for this to happen is for the container to state a
-  // smaller duration than the largest packet timestamp.
-  if (!frame->IsEndOfStream()) {
-    if (frame->GetTimestamp() > host()->GetDuration())
-      frame->SetTimestamp(host()->GetDuration());
-    if ((frame->GetTimestamp() + frame->GetDuration()) > host()->GetDuration())
-      frame->SetDuration(host()->GetDuration() - frame->GetTimestamp());
-  }
+    // Adjust the incoming frame if its rendering stop time is past the duration
+    // of the video itself. This is typically the last frame of the video and
+    // occurs if the container specifies a duration that isn't a multiple of the
+    // frame rate.  Another way for this to happen is for the container to state
+    // a smaller duration than the largest packet timestamp.
+    if (!frame->IsEndOfStream()) {
+      base::TimeDelta duration = host()->GetDuration();

[Ami GONE FROM CHROMIUM, 2012/01/27 23:44:58]

IMO this is a worsening of readability, since there are two "durations" involved
here (frame & host).  Revert?

[acolwell GONE FROM CHROMIUM, 2012/01/29 03:00:41]

Done.

+      if (frame->GetTimestamp() > duration)
+        frame->SetTimestamp(duration);
+      if ((frame->GetTimestamp() + frame->GetDuration()) > duration)
+        frame->SetDuration(duration - frame->GetTimestamp());
+    }
 
-  // This one's a keeper! Place it in the ready queue.
-  ready_frames_.push_back(frame);
-  DCHECK_LE(NumFrames_Locked(), limits::kMaxVideoFrames);
-  frame_available_.Signal();
+    // This one's a keeper! Place it in the ready queue.
+    ready_frames_.push_back(frame);
+    DCHECK_LE(NumFrames_Locked(), limits::kMaxVideoFrames);
+    frame_available_.Signal();
 
-  PipelineStatistics statistics;
-  statistics.video_frames_decoded = 1;
-  statistics_callback_.Run(statistics);
+    PipelineStatistics statistics;
+    statistics.video_frames_decoded = 1;
+    statistics_callback_.Run(statistics);
 
-  // Always request more decoded video if we have capacity. This serves two
-  // purposes:
-  //   1) Prerolling while paused
-  //   2) Keeps decoding going if video rendering thread starts falling behind
-  if (NumFrames_Locked() < limits::kMaxVideoFrames && !frame->IsEndOfStream()) {
-    AttemptRead_Locked();
-    return;
+    // Always request more decoded video if we have capacity. This serves two
+    // purposes:
+    //   1) Prerolling while paused
+    //   2) Keeps decoding going if video rendering thread starts falling behind
+    if (NumFrames_Locked() < limits::kMaxVideoFrames &&
+        !frame->IsEndOfStream()) {
+      AttemptRead_Locked();
+      return;
+    }
   }
 
   // If we're at capacity or end of stream while seeking we need to transition
   // to prerolled.
   if (state_ == kSeeking) {
     DCHECK(!current_frame_);
     state_ = kPrerolled;
 
     // Because we might remain in the prerolled state for an undetermined amount
     // of time (i.e., we were not playing before we received a seek), we'll
@@ skipping 70 matching lines @@
 
 int VideoRendererBase::NumFrames_Locked() const {
   lock_.AssertAcquired();
   int outstanding_frames =
       (current_frame_ ? 1 : 0) + (last_available_frame_ ? 1 : 0) +
       (current_frame_ && (current_frame_ == last_available_frame_) ? -1 : 0);
   return ready_frames_.size() + outstanding_frames;
 }
 
 }  // namespace media