Ignore calls to Reset() when in error state

media/filters/decoder_stream.cc

 // Copyright 2014 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/filters/decoder_stream.h"
 
 #include <utility>
 
 #include "base/bind.h"
 #include "base/callback_helpers.h"
@@ skipping 41 matching lines @@
       state_(STATE_UNINITIALIZED),
       stream_(NULL),
       decoder_selector_(new DecoderSelector<StreamType>(task_runner,
                                                         std::move(decoders),
                                                         media_log)),
       decoded_frames_since_fallback_(0),
       active_splice_(false),
       decoding_eos_(false),
       pending_decode_requests_(0),
       duration_tracker_(8),
+      received_config_change_during_reinit_(false),
+      pending_demuxer_read_(false),
       weak_factory_(this),
       fallback_weak_factory_(this) {}
 
 template <DemuxerStream::Type StreamType>
 DecoderStream<StreamType>::~DecoderStream() {
   FUNCTION_DVLOG(2);
   DCHECK(task_runner_->BelongsToCurrentThread());
 
   decoder_selector_.reset();
 
@@ skipping 78 matching lines @@
 template <DemuxerStream::Type StreamType>
 void DecoderStream<StreamType>::Reset(const base::Closure& closure) {
   FUNCTION_DVLOG(2);
   DCHECK(task_runner_->BelongsToCurrentThread());
   DCHECK_NE(state_, STATE_UNINITIALIZED);
   DCHECK(reset_cb_.is_null());
 
   reset_cb_ = closure;
 
   if (!read_cb_.is_null()) {
-    task_runner_->PostTask(FROM_HERE, base::Bind(
-        base::ResetAndReturn(&read_cb_), ABORTED, scoped_refptr<Output>()));
+    task_runner_->PostTask(FROM_HERE,
+                           base::Bind(base::ResetAndReturn(&read_cb_), ABORTED,
+                                      scoped_refptr<Output>()));
   }
 
   ready_outputs_.clear();
 
+  // It's possible to have received a DECODE_ERROR and entered STATE_ERROR right
+  // before a Reset() is executed. If we are still waiting for a demuxer read,
+  // OnBufferReady() will handle the reset callback.
+  // See crbug.com/597605 and crbug.com/607454.
+  if (state_ == STATE_ERROR && !pending_demuxer_read_) {
+    task_runner_->PostTask(FROM_HERE, base::ResetAndReturn(&reset_cb_));
+    return;
+  }
+
   // During decoder reinitialization, the Decoder does not need to be and
   // cannot be Reset(). |decrypting_demuxer_stream_| was reset before decoder
   // reinitialization.
   if (state_ == STATE_REINITIALIZING_DECODER)
     return;
 
-  // During pending demuxer read and when not using DecryptingDemuxerStream,
-  // the Decoder will be reset after demuxer read is returned
-  // (in OnBufferReady()).
-  if (state_ == STATE_PENDING_DEMUXER_READ && !decrypting_demuxer_stream_)
-    return;
-
+  // |decrypting_demuxer_stream_| will fire all of its read requests when
+  // it resets. |reset_cb_| will be fired in OnDecoderReset(), after the
+  // decrypting demuxer stream finishes its reset.
   if (decrypting_demuxer_stream_) {
     decrypting_demuxer_stream_->Reset(base::Bind(
         &DecoderStream<StreamType>::ResetDecoder, weak_factory_.GetWeakPtr()));
     return;
   }
 
+  // During pending demuxer read and when not using DecryptingDemuxerStream,
+  // the Decoder will be reset after demuxer read is returned
+  // (in OnBufferReady()).
+  if (pending_demuxer_read_)
+    return;
+
   ResetDecoder();
 }
 
 template <DemuxerStream::Type StreamType>
 bool DecoderStream<StreamType>::CanReadWithoutStalling() const {
   DCHECK(task_runner_->BelongsToCurrentThread());
   return !ready_outputs_.empty() || decoder_->CanReadWithoutStalling();
 }
 
 template <>
@@ skipping 45 matching lines @@
 }
 
 template <DemuxerStream::Type StreamType>
 void DecoderStream<StreamType>::OnDecoderSelected(
     std::unique_ptr<Decoder> selected_decoder,
     std::unique_ptr<DecryptingDemuxerStream> decrypting_demuxer_stream) {
   FUNCTION_DVLOG(2) << ": "
                     << (selected_decoder ? selected_decoder->GetDisplayName()
                                          : "No decoder selected.");
   DCHECK(task_runner_->BelongsToCurrentThread());
-  DCHECK(state_ == STATE_INITIALIZING ||
-         state_ == STATE_REINITIALIZING_DECODER ||
-         state_ == STATE_CONFIG_CHANGE_RECEIVED_WHILE_REINITIALIZING_DECODER)
+  DCHECK(state_ == STATE_INITIALIZING || state_ == STATE_REINITIALIZING_DECODER)
       << state_;
   if (state_ == STATE_INITIALIZING) {
     DCHECK(!init_cb_.is_null());
     DCHECK(read_cb_.is_null());
     DCHECK(reset_cb_.is_null());
   } else if (state_ == STATE_REINITIALIZING_DECODER) {
     DCHECK(decoder_);
   }
 
   previous_decoder_ = std::move(decoder_);
   decoded_frames_since_fallback_ = 0;
   decoder_ = std::move(selected_decoder);
   if (decrypting_demuxer_stream) {
     decrypting_demuxer_stream_ = std::move(decrypting_demuxer_stream);
     stream_ = decrypting_demuxer_stream_.get();
   }
 
   // TODO(tguilbert): crbug.com/603713 support config changes on decoder reinit.
-  if (state_ == STATE_CONFIG_CHANGE_RECEIVED_WHILE_REINITIALIZING_DECODER) {
+  if (received_config_change_during_reinit_) {
     CompleteDecoderReinitialization(false);
     return;
   }
 
   // Attempt to decode buffers from previous decoders (when those decoders have
   // never successfully outputed a frame).
   fallback_buffers_ = pending_buffers_;
 
   if (!decoder_) {
     if (state_ == STATE_INITIALIZING) {
@@ skipping 218 matching lines @@
 
   if (!fallback_buffers_.empty()) {
     scoped_refptr<DecoderBuffer> buffer = fallback_buffers_.front();
     fallback_buffers_.pop_front();
 
     // Decode the buffer without re-appending it to |pending_buffers_|.
     DecodeInternal(buffer);
     return;
   }
 
+  // Set a flag in addition to the state, because the state can be overwritten
+  // when encountering an error. See crbug.com/597605.
+  DCHECK(!pending_demuxer_read_);
+  pending_demuxer_read_ = true;
+
   state_ = STATE_PENDING_DEMUXER_READ;
   stream_->Read(base::Bind(&DecoderStream<StreamType>::OnBufferReady,
                            weak_factory_.GetWeakPtr()));
 }
 
 template <DemuxerStream::Type StreamType>
 void DecoderStream<StreamType>::OnBufferReady(
     DemuxerStream::Status status,
     const scoped_refptr<DecoderBuffer>& buffer) {
   FUNCTION_DVLOG(2) << ": " << status << ", "
                     << (buffer.get() ? buffer->AsHumanReadableString()
                                      : "NULL");
 
   DCHECK(task_runner_->BelongsToCurrentThread());
   if (decoded_frames_since_fallback_) {
     DCHECK(state_ == STATE_PENDING_DEMUXER_READ || state_ == STATE_ERROR)
         << state_;
   } else {
     DCHECK(state_ == STATE_PENDING_DEMUXER_READ || state_ == STATE_ERROR ||
            STATE_REINITIALIZING_DECODER)
         << state_;
   }
   DCHECK_EQ(buffer.get() != NULL, status == DemuxerStream::kOk) << status;
 
+  // Unset the flag. STATE_PENDING_DEMUXER_READ might have been overwritten.
+  // See crbug.com/597605.
+  pending_demuxer_read_ = false;
+
   // If parallel decode requests are supported, multiple read requests might
   // have been sent to the demuxer. The buffers might arrive while the decoder
   // is reinitializing after falling back on first decode error.
   if (state_ == STATE_REINITIALIZING_DECODER &&
       !decoded_frames_since_fallback_) {
     switch (status) {
       case DemuxerStream::kOk:
         // Save valid buffers to be consumed by the new decoder.
         // |pending_buffers_| is copied to |fallback_buffers| in
         // OnDecoderSelected().
         pending_buffers_.push_back(buffer);
         break;
       case DemuxerStream::kConfigChanged:
         // TODO(tguilbert): crbug.com/603713
         // |decoder_| might have a stale config by the time it is reinitialized.
         // Ideally, we would save the config from |stream_| and reinitialize the
         // decoder by playing back the sequence of buffers and config changes.
-        state_ = STATE_CONFIG_CHANGE_RECEIVED_WHILE_REINITIALIZING_DECODER;
+        received_config_change_during_reinit_ = true;
         pending_buffers_.clear();
         break;
       case DemuxerStream::kAborted:
         // |this| will read from the demuxer stream again in OnDecoderSelected()
         // and receive a kAborted then.
         pending_buffers_.clear();
         break;
     }
     return;
   }
 
   // Decoding has been stopped (e.g due to an error).
   if (state_ != STATE_PENDING_DEMUXER_READ) {
     DCHECK(state_ == STATE_ERROR);
     DCHECK(read_cb_.is_null());
+
+    if (!reset_cb_.is_null()) {
+      // If we are using DecryptingDemuxerStream, we already called DDS::Reset()
+      // which will continue the resetting process in its callback.
+      if (!decrypting_demuxer_stream_)
+        Reset(base::ResetAndReturn(&reset_cb_));
+    }
     return;
   }
 
   state_ = STATE_NORMAL;
 
   if (status == DemuxerStream::kConfigChanged) {
     FUNCTION_DVLOG(2) << ": " << "ConfigChanged";
     DCHECK(stream_->SupportsConfigChanges());
 
     // Pending buffers might not match the reinitialized decoder's new config.
@@ skipping 15 matching lines @@
     //   happened, and read straight from the demuxer, which could lead to some
     //   lost frames if we were to fallback then).
     pending_buffers_.clear();
 
     if (!config_change_observer_cb_.is_null())
       config_change_observer_cb_.Run();
 
     state_ = STATE_FLUSHING_DECODER;
     if (!reset_cb_.is_null()) {
       // If we are using DecryptingDemuxerStream, we already called DDS::Reset()
-      // which will continue the resetting process in it's callback.
+      // which will continue the resetting process in its callback.
       if (!decrypting_demuxer_stream_)
         Reset(base::ResetAndReturn(&reset_cb_));
       // Reinitialization will continue after Reset() is done.
     } else {
       FlushDecoder();
     }
     return;
   }
 
   if (!reset_cb_.is_null()) {
     // If we are using DecryptingDemuxerStream, we already called DDS::Reset()
-    // which will continue the resetting process in it's callback.
+    // which will continue the resetting process in its callback.
     if (!decrypting_demuxer_stream_)
       Reset(base::ResetAndReturn(&reset_cb_));
     return;
   }
 
   if (status == DemuxerStream::kAborted) {
     if (!read_cb_.is_null())
       SatisfyRead(DEMUXER_READ_ABORTED, NULL);
     return;
   }
@@ skipping 116 matching lines @@
   }
 
   // The resetting process will be continued in OnDecoderReinitialized().
   ReinitializeDecoder();
 }
 
 template class DecoderStream<DemuxerStream::VIDEO>;
 template class DecoderStream<DemuxerStream::AUDIO>;
 
 }  // namespace media