Fix crash in renderer initialization when media pipeline is stopped

media/renderers/renderer_impl.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/renderers/renderer_impl.h"
 
 #include <utility>
 
 #include "base/bind.h"
 #include "base/callback.h"
@@ skipping 113 matching lines @@
 }
 
 void RendererImpl::Initialize(DemuxerStreamProvider* demuxer_stream_provider,
                               RendererClient* client,
                               const PipelineStatusCB& init_cb) {
   DVLOG(1) << __func__;
   DCHECK(task_runner_->BelongsToCurrentThread());
   DCHECK_EQ(state_, STATE_UNINITIALIZED);
   DCHECK(!init_cb.is_null());
   DCHECK(client);
-  DCHECK(demuxer_stream_provider->GetStream(DemuxerStream::AUDIO) ||
-         demuxer_stream_provider->GetStream(DemuxerStream::VIDEO));
 
   client_ = client;
   demuxer_stream_provider_ = demuxer_stream_provider;
   init_cb_ = init_cb;
 
-  DemuxerStream* audio_stream =
-      demuxer_stream_provider->GetStream(DemuxerStream::AUDIO);
-  if (audio_stream)
-    audio_stream->SetStreamStatusChangeCB(base::Bind(
-        &RendererImpl::RestartStreamPlayback, weak_this_, audio_stream));
-  DemuxerStream* video_stream =
-      demuxer_stream_provider->GetStream(DemuxerStream::VIDEO);
-  if (video_stream)
-    video_stream->SetStreamStatusChangeCB(base::Bind(
-        &RendererImpl::RestartStreamPlayback, weak_this_, video_stream));
-
   if (HasEncryptedStream() && !cdm_context_) {
     state_ = STATE_INIT_PENDING_CDM;
     return;
   }
 
   state_ = STATE_INITIALIZING;
   InitializeAudioRenderer();
 }
 
 void RendererImpl::SetCdm(CdmContext* cdm_context,
@@ skipping 215 matching lines @@
 
 void RendererImpl::InitializeAudioRenderer() {
   DVLOG(1) << __func__;
   DCHECK(task_runner_->BelongsToCurrentThread());
   DCHECK_EQ(state_, STATE_INITIALIZING);
   DCHECK(!init_cb_.is_null());
 
   PipelineStatusCB done_cb =
       base::Bind(&RendererImpl::OnAudioRendererInitializeDone, weak_this_);
 
-  if (!demuxer_stream_provider_->GetStream(DemuxerStream::AUDIO)) {
+  DemuxerStream* audio_stream =
+      demuxer_stream_provider_->GetStream(DemuxerStream::AUDIO);
+  if (!audio_stream) {
     audio_renderer_.reset();
     task_runner_->PostTask(FROM_HERE, base::Bind(done_cb, PIPELINE_OK));
     return;
   }
 
+  audio_stream->SetStreamStatusChangeCB(base::Bind(
+      &RendererImpl::RestartStreamPlayback, weak_this_, audio_stream));

[DaleCurtis, 2016/12/13 00:31:38]

Should we be binding in the audio_stream pointer like this? If the stream goes
away, what happens if this callback is called later. Seems like
demuxer_stream_provider should be passed in here?

[servolk, 2016/12/13 00:46:50]

As I've explained in my previous reply, this should be fine. WMPI guarantees
that Demuxer will outline Renderer, and any stream returned by
DemuxerStreamProvider::GetStream should have the same lifetime as the Demuxer
itself.

+
   audio_renderer_client_.reset(
       new RendererClientInternal(DemuxerStream::AUDIO, this));
   // Note: After the initialization of a renderer, error events from it may
   // happen at any time and all future calls must guard against STATE_ERROR.
-  audio_renderer_->Initialize(
-      demuxer_stream_provider_->GetStream(DemuxerStream::AUDIO), cdm_context_,
-      audio_renderer_client_.get(), done_cb);
+  audio_renderer_->Initialize(audio_stream, cdm_context_,
+                              audio_renderer_client_.get(), done_cb);
 }
 
 void RendererImpl::OnAudioRendererInitializeDone(PipelineStatus status) {
   DVLOG(1) << __func__ << ": " << status;
   DCHECK(task_runner_->BelongsToCurrentThread());
 
   // OnError() may be fired at any time by the renderers, even if they thought
   // they initialized successfully (due to delayed output device setup).
   if (state_ != STATE_INITIALIZING) {
     DCHECK(init_cb_.is_null());
@@ skipping 12 matching lines @@
 
 void RendererImpl::InitializeVideoRenderer() {
   DVLOG(1) << __func__;
   DCHECK(task_runner_->BelongsToCurrentThread());
   DCHECK_EQ(state_, STATE_INITIALIZING);
   DCHECK(!init_cb_.is_null());
 
   PipelineStatusCB done_cb =
       base::Bind(&RendererImpl::OnVideoRendererInitializeDone, weak_this_);
 
-  if (!demuxer_stream_provider_->GetStream(DemuxerStream::VIDEO)) {
+  DemuxerStream* video_stream =
+      demuxer_stream_provider_->GetStream(DemuxerStream::VIDEO);
+  if (!video_stream) {
     video_renderer_.reset();
     task_runner_->PostTask(FROM_HERE, base::Bind(done_cb, PIPELINE_OK));
     return;
   }
 
+  video_stream->SetStreamStatusChangeCB(base::Bind(
+      &RendererImpl::RestartStreamPlayback, weak_this_, video_stream));

[DaleCurtis, 2016/12/13 00:31:38]

Ditto.

[servolk, 2016/12/13 00:46:50]

Same.

+
   video_renderer_client_.reset(
       new RendererClientInternal(DemuxerStream::VIDEO, this));
   video_renderer_->Initialize(
-      demuxer_stream_provider_->GetStream(DemuxerStream::VIDEO), cdm_context_,
-      video_renderer_client_.get(),
+      video_stream, cdm_context_, video_renderer_client_.get(),
       base::Bind(&RendererImpl::GetWallClockTimes, base::Unretained(this)),
       done_cb);
 }
 
 void RendererImpl::OnVideoRendererInitializeDone(PipelineStatus status) {
   DVLOG(1) << __func__ << ": " << status;
   DCHECK(task_runner_->BelongsToCurrentThread());
 
   // OnError() may be fired at any time by the renderers, even if they thought
   // they initialized successfully (due to delayed output device setup).
@@ skipping 386 matching lines @@
   DCHECK(task_runner_->BelongsToCurrentThread());
   client_->OnVideoNaturalSizeChange(size);
 }
 
 void RendererImpl::OnVideoOpacityChange(bool opaque) {
   DCHECK(task_runner_->BelongsToCurrentThread());
   client_->OnVideoOpacityChange(opaque);
 }
 
 }  // namespace media