Attempt decoder fallback if first decode fails

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),
-      weak_factory_(this) {}
+      weak_factory_(this),
+      decode_weak_factory_(this) {}
 
 template <DemuxerStream::Type StreamType>
 DecoderStream<StreamType>::~DecoderStream() {
   FUNCTION_DVLOG(2);
   DCHECK(task_runner_->BelongsToCurrentThread());
 
   decoder_selector_.reset();
 
   if (!init_cb_.is_null()) {
     task_runner_->PostTask(FROM_HERE,
@@ skipping 163 matching lines @@
 }
 
 template <DemuxerStream::Type StreamType>
 void DecoderStream<StreamType>::OnDecoderSelected(
     scoped_ptr<Decoder> selected_decoder,
     scoped_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)
+  DCHECK(state_ == STATE_INITIALIZING ||
+         state_ == STATE_REINITIALIZING_DECODER ||
+         state_ == STATE_CONFIG_CHANGE_RECEIVED_WHILE_REINITIALIZING_DECODER)
       << state_;
   if (state_ == STATE_INITIALIZING) {
     DCHECK(!init_cb_.is_null());
     DCHECK(read_cb_.is_null());
     DCHECK(reset_cb_.is_null());
-  } else {
+  } 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) {
+    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) {
       state_ = STATE_UNINITIALIZED;
       MEDIA_LOG(ERROR, media_log_) << GetStreamTypeString()
                                    << " decoder initialization failed";
       base::ResetAndReturn(&init_cb_).Run(false);
     } else {
       CompleteDecoderReinitialization(false);
     }
     return;
@@ skipping 21 matching lines @@
     Status status,
     const scoped_refptr<Output>& output) {
   DCHECK(!read_cb_.is_null());
   base::ResetAndReturn(&read_cb_).Run(status, output);
 }
 
 template <DemuxerStream::Type StreamType>
 void DecoderStream<StreamType>::Decode(
     const scoped_refptr<DecoderBuffer>& buffer) {
   FUNCTION_DVLOG(2);
+
+  // We don't know if the decoder will error out on first decode yet. Save the
+  // buffer to feed it to the fallback decoder later if needed.
+  if (!decoded_frames_since_fallback_)
+    pending_buffers_.push_back(buffer);
+
+  DecodeInternal(buffer);
+}
+
+template <DemuxerStream::Type StreamType>
+void DecoderStream<StreamType>::DecodeInternal(
+    const scoped_refptr<DecoderBuffer>& buffer) {
+  FUNCTION_DVLOG(2);
   DCHECK(state_ == STATE_NORMAL || state_ == STATE_FLUSHING_DECODER) << state_;
   DCHECK_LT(pending_decode_requests_, GetMaxDecodeRequests());
   DCHECK(reset_cb_.is_null());
   DCHECK(buffer.get());
 
   int buffer_size = buffer->end_of_stream() ? 0 : buffer->data_size();
 
   TRACE_EVENT_ASYNC_BEGIN2(
       "media", GetTraceString<StreamType>(), this, "key frame",
       !buffer->end_of_stream() && buffer->is_key_frame(), "timestamp (ms)",
       !buffer->end_of_stream() ? buffer->timestamp().InMilliseconds() : 0);
 
   if (buffer->end_of_stream())
     decoding_eos_ = true;
   else if (buffer->duration() != kNoTimestamp())
     duration_tracker_.AddSample(buffer->duration());
 
   ++pending_decode_requests_;
-  decoder_->Decode(buffer,
-                   base::Bind(&DecoderStream<StreamType>::OnDecodeDone,
-                              weak_factory_.GetWeakPtr(),
-                              buffer_size,
-                              buffer->end_of_stream()));
+  decoder_->Decode(buffer, base::Bind(&DecoderStream<StreamType>::OnDecodeDone,
+                                      decode_weak_factory_.GetWeakPtr(),
+                                      buffer_size, buffer->end_of_stream()));
 }
 
 template <DemuxerStream::Type StreamType>
 void DecoderStream<StreamType>::FlushDecoder() {
   Decode(DecoderBuffer::CreateEOSBuffer());
 }
 
 template <DemuxerStream::Type StreamType>
 void DecoderStream<StreamType>::OnDecodeDone(int buffer_size,
                                              bool end_of_stream,
@@ skipping 18 matching lines @@
     return;
   }
 
   // Drop decoding result if Reset() was called during decoding.
   // The resetting process will be handled when the decoder is reset.
   if (!reset_cb_.is_null())
     return;
 
   switch (status) {
     case DecodeStatus::DECODE_ERROR:
+      if (!decoded_frames_since_fallback_) {
+        pending_decode_requests_ = 0;
+
+        // Prevent all pending decode requests from being called back.
+        decode_weak_factory_.InvalidateWeakPtrs();
+
+        state_ = STATE_REINITIALIZING_DECODER;
+        decoder_selector_->SelectDecoder(
+            stream_, nullptr,
+            base::Bind(&DecoderStream<StreamType>::OnDecoderSelected,
+                       weak_factory_.GetWeakPtr()),
+            base::Bind(&DecoderStream<StreamType>::OnDecodeOutputReady,
+                       weak_factory_.GetWeakPtr()),
+            waiting_for_decryption_key_cb_);
+        return;
+      }
       state_ = STATE_ERROR;
       MEDIA_LOG(ERROR, media_log_) << GetStreamTypeString() << " decode error";
       ready_outputs_.clear();
       if (!read_cb_.is_null())
         SatisfyRead(DECODE_ERROR, NULL);
       return;
 
     case DecodeStatus::ABORTED:
       // Decoder can return DecodeStatus::ABORTED during Reset() or during
       // destruction.
@@ skipping 35 matching lines @@
   if (state_ == STATE_ERROR) {
     DCHECK(read_cb_.is_null());
     return;
   }
 
   // Drop decoding result if Reset() was called during decoding.
   // The resetting process will be handled when the decoder is reset.
   if (!reset_cb_.is_null())
     return;
 
+  ++decoded_frames_since_fallback_;
+
+  // |decoder_| sucessfully decoded a frame. No need to keep buffers for a
+  // fallback decoder.
+  // Note: |fallback_buffers_| might still have buffers, and we will keep
+  // reading from there before requesting new buffers from |stream_|.
+  pending_buffers_.clear();
+
   if (!read_cb_.is_null()) {
     // If |ready_outputs_| was non-empty, the read would have already been
     // satisifed by Read().
     DCHECK(ready_outputs_.empty());
     SatisfyRead(OK, output);
     return;
   }
 
   // Store decoded output.
   ready_outputs_.push_back(output);
 
   // Destruct any previous decoder once we've decoded enough frames to ensure
   // that it's no longer in use.
   if (previous_decoder_ &&
-      ++decoded_frames_since_fallback_ > limits::kMaxVideoFrames) {
+      decoded_frames_since_fallback_ > limits::kMaxVideoFrames) {
     previous_decoder_.reset();
   }
 }
 
 template <DemuxerStream::Type StreamType>
 void DecoderStream<StreamType>::ReadFromDemuxerStream() {
   FUNCTION_DVLOG(2);
   DCHECK_EQ(state_, STATE_NORMAL);
   DCHECK(CanDecodeMore());
   DCHECK(reset_cb_.is_null());
 
+  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;
+  }
+
   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());
-  DCHECK(state_ == STATE_PENDING_DEMUXER_READ || state_ == STATE_ERROR)
-      << state_;
+  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;
 
+  // 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;
+        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());
     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
+    pending_buffers_.clear();

[sandersd (OOO until July 31), 2016/04/19 21:12:19]

If the current decoder fails after this, we should enter state
STATE_CONFIG_CHANGE_RECEIVED_WHILE_REINITIALIZING_DECODER. Should there be a
flag we set instead of this clear?

[tguilbert, 2016/04/20 01:13:17]

After trying to change the above mentioned state to a flag, I feel as though it
added complexity to all possible interactions between OnDecoderReinitialized(),
OnDecoderSelected(), OnDecodeOutputReady() and FlushDecoder(), without an easy
way to signal why we were doing this. I reverted my changes, and propose
deferring the changes until we address 603713. The only cases where there is a
significant difference in behavior are:

- Decoder returns a DECODE_ERROR during flushing (As is: fallback, losing some
frames. With the flag, error out completely).

- Decoder returns no DECODE_ERROR and no during flushing (As is: keep going,
losing some buffers/frames if we fallback. With the flag: error out completely).

Does this seem acceptable? Are there any significant issues that might arise
from not error-ing out?

+
     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.
       if (!decrypting_demuxer_stream_)
         Reset(base::ResetAndReturn(&reset_cb_));
       // Reinitialization will continue after Reset() is done.
@@ skipping 116 matching lines @@
 void DecoderStream<StreamType>::OnDecoderReset() {
   FUNCTION_DVLOG(2);
   DCHECK(task_runner_->BelongsToCurrentThread());
   DCHECK(state_ == STATE_NORMAL || state_ == STATE_FLUSHING_DECODER ||
          state_ == STATE_ERROR || state_ == STATE_END_OF_STREAM) << state_;
   // If Reset() was called during pending read, read callback should be fired
   // before the reset callback is fired.
   DCHECK(read_cb_.is_null());
   DCHECK(!reset_cb_.is_null());
 
+  // Make sure we read directly from the demuxer after a reset.
+  fallback_buffers_.clear();
+  pending_buffers_.clear();
+
   if (state_ != STATE_FLUSHING_DECODER) {
     state_ = STATE_NORMAL;
     active_splice_ = false;
     base::ResetAndReturn(&reset_cb_).Run();
     return;
   }
 
   // The resetting process will be continued in OnDecoderReinitialized().
   ReinitializeDecoder();
 }
 
 template class DecoderStream<DemuxerStream::VIDEO>;
 template class DecoderStream<DemuxerStream::AUDIO>;
 
 }  // namespace media