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 =

[DaleCurtis, 2016/12/12 23:43:09]

Can you move these into InitializeAudioRenderer() and InitializeVideoRenderer()
respectively?

[servolk, 2016/12/12 23:59:23]

Yes, done.

       demuxer_stream_provider->GetStream(DemuxerStream::AUDIO);
   if (audio_stream)

[DaleCurtis, 2016/12/12 23:43:09]

Needs {}

[servolk, 2016/12/12 23:59:23]

The if is no longer necessary now that it's called from InitializeAudioRenderer

     audio_stream->SetStreamStatusChangeCB(base::Bind(
         &RendererImpl::RestartStreamPlayback, weak_this_, audio_stream));
   DemuxerStream* video_stream =
       demuxer_stream_provider->GetStream(DemuxerStream::VIDEO);
   if (video_stream)

[DaleCurtis, 2016/12/12 23:43:09]

Multiline if needs {}

[servolk, 2016/12/12 23:59:23]

ditto

     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();
@@ skipping 209 matching lines @@
 
 void RendererImpl::FinishInitialization(PipelineStatus status) {
   DCHECK(!init_cb_.is_null());
 
   if (!pending_cdm_attached_cb_.is_null())
     base::ResetAndReturn(&pending_cdm_attached_cb_).Run(status == PIPELINE_OK);
 
   base::ResetAndReturn(&init_cb_).Run(status);
 }
 
 void RendererImpl::InitializeAudioRenderer() {

[DaleCurtis, 2016/12/12 23:43:09]

Pass in DemuxerStream* here and add a comment about why we do so if you can't
move the setters.

[servolk, 2016/12/12 23:59:23]

I guess we could pass DemuxerStream* in here, but InitializeAudioRenderer might
get called either from RendererImpl::Initialize directly or from SetCdm if we
need to init CDM first. If we want to be strict about always calling GetStream
once per each stream type I guess we could call those once in
RendererImpl::Initialize and then always reuse the cached values. Or we could
just call GetStream once inside RendererImpl::InitializeAudio/VideoRenderer and
allow HasEncryptedStream to call GetStream one more time, it's probably not a
big deal.

   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_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_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