Refactor config changes flow

The original code has unnecessary DCHECKs that will cause crashes when
audio/video config got changed more than once. This CL removed them and
also refactor the config changes flow.

Bug: 1111702
Change-Id: Id3a7ba39cc5a5d433f4a0a791f10c75baa43ed79
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2332082Reviewed-by: Yuri Wiitala <miu@chromium.org>
Reviewed-by: mark a. foltz <mfoltz@chromium.org>
Commit-Queue: Chih-Hsuan Kuo <chkuo@google.com>
Auto-Submit: Chih-Hsuan Kuo <chkuo@google.com>
Cr-Commit-Position: refs/heads/master@{#794784}

media/remoting/stream_provider.cc

@@ -199,12 +199,20 @@ void StreamProvider::MediaStream::OnInitializeCallback(
       callback_message.has_audio_decoder_config()) {
     const pb::AudioDecoderConfig audio_message =
         callback_message.audio_decoder_config();
-    UpdateAudioConfig(audio_message);
+    ConvertProtoToAudioDecoderConfig(audio_message, &audio_decoder_config_);
+    if (!audio_decoder_config_.IsValidConfig()) {
+      OnError("Invalid audio config");
+      return;
+    }
   } else if (type_ == DemuxerStream::VIDEO &&
              callback_message.has_video_decoder_config()) {
     const pb::VideoDecoderConfig video_message =
         callback_message.video_decoder_config();
-    UpdateVideoConfig(video_message);
+    ConvertProtoToVideoDecoderConfig(video_message, &video_decoder_config_);
+    if (!video_decoder_config_.IsValidConfig()) {
+      OnError("Invalid video config");
+      return;
+    }
   } else {
     OnError("Config missing");
     return;
@@ -244,8 +252,6 @@ void StreamProvider::MediaStream::OnReadUntilCallback(
   total_received_frame_count_ = callback_message.count();
 
   if (ToDemuxerStreamStatus(callback_message.status()) == kConfigChanged) {
-    config_changed_ = true;
-
     if (callback_message.has_audio_decoder_config()) {
       const pb::AudioDecoderConfig audio_message =
           callback_message.audio_decoder_config();
@@ -277,14 +283,7 @@ void StreamProvider::MediaStream::UpdateAudioConfig(
     OnError("Invalid audio config");
     return;
   }
-  if (config_changed_) {
-    DCHECK(audio_decoder_config_.IsValidConfig());
-    DCHECK(!next_audio_decoder_config_.IsValidConfig());
-    next_audio_decoder_config_ = audio_config;
-  } else {
-    DCHECK(!audio_decoder_config_.IsValidConfig());
-    audio_decoder_config_ = audio_config;
-  }
+  next_audio_decoder_config_ = audio_config;
 }
 
 void StreamProvider::MediaStream::UpdateVideoConfig(
@@ -296,14 +295,7 @@ void StreamProvider::MediaStream::UpdateVideoConfig(
     OnError("Invalid video config");
     return;
   }
-  if (config_changed_) {
-    DCHECK(video_decoder_config_.IsValidConfig());
-    DCHECK(!next_video_decoder_config_.IsValidConfig());
-    next_video_decoder_config_ = video_config;
-  } else {
-    DCHECK(!video_decoder_config_.IsValidConfig());
-    video_decoder_config_ = video_config;
-  }
+  next_video_decoder_config_ = video_config;
 }
 
 void StreamProvider::MediaStream::SendReadUntil() {
@@ -326,7 +318,8 @@ void StreamProvider::MediaStream::Read(ReadCB read_cb) {
   DCHECK(read_cb);
 
   read_complete_callback_ = std::move(read_cb);
-  if (buffers_.empty() && config_changed_) {
+  if (buffers_.empty() && (next_audio_decoder_config_.IsValidConfig() ||
+                           next_video_decoder_config_.IsValidConfig())) {
     CompleteRead(DemuxerStream::kConfigChanged);
     return;
   }
@@ -345,14 +338,14 @@ void StreamProvider::MediaStream::CompleteRead(DemuxerStream::Status status) {
 
   switch (status) {
     case DemuxerStream::kConfigChanged:
-      if (type_ == AUDIO) {
-        DCHECK(next_audio_decoder_config_.IsValidConfig());
+      if (next_audio_decoder_config_.IsValidConfig()) {
         audio_decoder_config_ = next_audio_decoder_config_;
-      } else {
-        DCHECK(next_video_decoder_config_.IsValidConfig());
+        next_audio_decoder_config_ = media::AudioDecoderConfig();
+      }
+      if (next_video_decoder_config_.IsValidConfig()) {
         video_decoder_config_ = next_video_decoder_config_;
+        next_video_decoder_config_ = media::VideoDecoderConfig();
       }
-      config_changed_ = false;
       std::move(read_complete_callback_).Run(status, nullptr);
       return;
     case DemuxerStream::kAborted:

media/remoting/stream_provider.h

@@ -184,11 +184,6 @@ class StreamProvider final : public Demuxer {
     // contains how many frames are sent.
     uint32_t total_received_frame_count_ = 0;
 
-    // Indicates whether Audio/VideoDecoderConfig changed and the frames with
-    // the old config are not yet consumed. The new config is stored in the end
-    // of |audio/video_decoder_config_|.
-    bool config_changed_ = false;
-
     // Indicates whether a ReadUntil RPC message was sent without receiving the
     // ReadUntilCallback message yet.
     bool read_until_sent_ = false;