Implemented proper pause-then-seek behaviour for the media pipeline.

media/filters/decoder_base.h

 // Copyright (c) 2009 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.
 
 // A base class that provides the plumbing for a decoder filters.
 
 #ifndef MEDIA_FILTERS_DECODER_BASE_H_
 #define MEDIA_FILTERS_DECODER_BASE_H_
 
 #include <deque>
@@ skipping 51 matching lines @@
     // TODO(scherkus): change the callback format to pass a scoped_refptr<> or
     // better yet see if we can get away with not using reference counting.
     scoped_refptr<Buffer> buffer_ref = buffer;
     this->message_loop()->PostTask(FROM_HERE,
         NewRunnableMethod(this, &DecoderBase::ReadCompleteTask, buffer_ref));
   }
 
  protected:
   DecoderBase()
       : pending_reads_(0),
-        seeking_(false),
-        state_(UNINITIALIZED) {
+        expecting_discontinuous_(false),
+        state_(kUninitialized) {
   }
 
   virtual ~DecoderBase() {
-    DCHECK(state_ == UNINITIALIZED || state_ == STOPPED);
+    DCHECK(state_ == kUninitialized || state_ == kStopped);
     DCHECK(result_queue_.empty());
     DCHECK(read_queue_.empty());
   }
 
   // This method is called by the derived class from within the OnDecode method.
   // It places an output buffer in the result queue.  It must be called from
   // within the OnDecode method.
   void EnqueueResult(Output* output) {
     DCHECK_EQ(MessageLoop::current(), this->message_loop());
     if (!IsStopped()) {
@@ skipping 20 matching lines @@
   virtual void OnSeek(base::TimeDelta time) {}
 
   // Method that must be implemented by the derived class.  If the decode
   // operation produces one or more outputs, the derived class should call
   // the EnequeueResult() method from within this method.
   virtual void OnDecode(Buffer* input) = 0;
 
   MediaFormat media_format_;
 
  private:
-  bool IsStopped() { return state_ == STOPPED; }
+  bool IsStopped() { return state_ == kStopped; }
 
   void StopTask() {
     DCHECK_EQ(MessageLoop::current(), this->message_loop());
 
     // Delegate to the subclass first.
     OnStop();
 
     // Throw away all buffers in all queues.
     result_queue_.clear();
     STLDeleteElements(&read_queue_);
-    state_ = STOPPED;
+    state_ = kStopped;
   }
 
   void SeekTask(base::TimeDelta time, FilterCallback* callback) {
     DCHECK_EQ(MessageLoop::current(), this->message_loop());
+    DCHECK_EQ(0u, pending_reads_) << "Pending reads should have completed";
+    DCHECK(read_queue_.empty()) << "Read requests should be empty";
     scoped_ptr<FilterCallback> c(callback);
 
     // Delegate to the subclass first.
+    //
+    // TODO(scherkus): if we have the strong assertion that there are no pending
+    // reads in the entire pipeline when we receive Seek(), subclasses could
+    // either flush their buffers here or wait for IsDiscontinuous().  I'm
+    // inclined to say that they should still wait for IsDiscontinuous() so they
+    // don't have duplicated logic for Seek() and actual discontinuous frames.
     OnSeek(time);
 
-    // Flush the result queue.
+    // Flush our decoded results.  We'll set a boolean that we can DCHECK to
+    // verify our assertion that the first buffer received after a Seek() should
+    // always be discontinuous.
     result_queue_.clear();
+    expecting_discontinuous_ = true;
 
-    // Turn on the seeking flag so that we can discard buffers until a
-    // discontinuous buffer is received.
-    seeking_ = true;
-
-    // For now, signal that we're done seeking.
-    // TODO(scherkus): implement asynchronous seeking for decoder_base.h
+    // Signal that we're done seeking.
     callback->Run();
   }
 
   void InitializeTask(DemuxerStream* demuxer_stream, FilterCallback* callback) {
     DCHECK_EQ(MessageLoop::current(), this->message_loop());
-    DCHECK(state_ == UNINITIALIZED);
-    DCHECK(!demuxer_stream_);
+    CHECK(kUninitialized == state_);
+    CHECK(!demuxer_stream_);
     scoped_ptr<FilterCallback> c(callback);
     demuxer_stream_ = demuxer_stream;
 
     // Delegate to subclass first.
     if (!OnInitialize(demuxer_stream_)) {
       this->host()->SetError(PIPELINE_ERROR_DECODE);
       callback->Run();
       return;
     }
 
     // TODO(scherkus): subclass shouldn't mutate superclass media format.
     DCHECK(!media_format_.empty()) << "Subclass did not set media_format_";
-    state_ = INITIALIZED;
+    state_ = kInitialized;
     callback->Run();
   }
 
   void ReadTask(ReadCallback* read_callback) {
     DCHECK_EQ(MessageLoop::current(), this->message_loop());
 
     // TODO(scherkus): should reply with a null operation (empty buffer).
     if (IsStopped()) {
       delete read_callback;
       return;
@@ skipping 11 matching lines @@
   }
 
   void ReadCompleteTask(scoped_refptr<Buffer> buffer) {
     DCHECK_EQ(MessageLoop::current(), this->message_loop());
     DCHECK_GT(pending_reads_, 0u);
     --pending_reads_;
     if (IsStopped()) {
       return;
     }
 
-    // Once the |seeking_| flag is set we ignore every buffers here
-    // until we receive a discontinuous buffer and we will turn off the
-    // |seeking_| flag.
+    // TODO(scherkus): remove this when we're less paranoid about our seeking
+    // invariants.
     if (buffer->IsDiscontinuous()) {
-      // TODO(hclam): put a DCHECK here to assert |seeking_| being true.
-      // I cannot do this now because seek operation is not fully
-      // asynchronous. There may be pending seek requests even before the
-      // previous was finished.
-      seeking_ = false;
-    }
-    if (seeking_) {
-      return;
+      DCHECK(expecting_discontinuous_);
+      expecting_discontinuous_ = false;
     }
 
     // Decode the frame right away.
     OnDecode(buffer);
 
     // Attempt to fulfill a pending read callback and schedule additional reads
     // if necessary.
     FulfillPendingRead();
 
     // Issue reads as necessary.
@@ skipping 23 matching lines @@
     read_queue_.pop_front();
 
     // Execute the callback!
     read_callback->Run(output);
   }
 
   // Tracks the number of asynchronous reads issued to |demuxer_stream_|.
   // Using size_t since it is always compared against deque::size().
   size_t pending_reads_;
 
-  // An internal state of the decoder that indicates that are waiting for seek
-  // to complete. We expect to receive a discontinuous frame/packet from the
-  // demuxer to signal that seeking is completed.
-  bool seeking_;
+  // A flag used for debugging that we expect our next read to be discontinuous.
+  bool expecting_discontinuous_;
 
   // Pointer to the demuxer stream that will feed us compressed buffers.
   scoped_refptr<DemuxerStream> demuxer_stream_;
 
   // Queue of decoded samples produced in the OnDecode() method of the decoder.
   // Any samples placed in this queue will be assigned to the OutputQueue
   // buffers once the OnDecode() method returns.
   //
   // TODO(ralphl): Eventually we want to have decoders get their destination
   // buffer from the OutputQueue and write to it directly.  Until we change
   // from the Assignable buffer to callbacks and renderer-allocated buffers,
   // we need this extra queue.
   typedef std::deque<scoped_refptr<Output> > ResultQueue;
   ResultQueue result_queue_;
 
   // Queue of callbacks supplied by the renderer through the Read() method.
   typedef std::deque<ReadCallback*> ReadQueue;
   ReadQueue read_queue_;
 
+  // Pause callback.
+  scoped_ptr<FilterCallback> pause_callback_;
+
   // Simple state tracking variable.
   enum State {
-    UNINITIALIZED,
-    INITIALIZED,
-    STOPPED,
+    kUninitialized,
+    kInitialized,
+    kStopped,
   };
   State state_;
 
   DISALLOW_COPY_AND_ASSIGN(DecoderBase);
 };
 
 }  // namespace media
 
 #endif  // MEDIA_FILTERS_DECODER_BASE_H_