Allow media track switching.

media/renderers/audio_renderer_impl.cc

 // Copyright (c) 2012 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/audio_renderer_impl.h"
 
 #include <math.h>
 #include <stddef.h>
 #include <algorithm>
 #include <utility>
@@ skipping 18 matching lines @@
 #include "media/base/renderer_client.h"
 #include "media/base/timestamp_constants.h"
 #include "media/filters/audio_clock.h"
 #include "media/filters/decrypting_demuxer_stream.h"
 
 namespace media {
 
 AudioRendererImpl::AudioRendererImpl(
     const scoped_refptr<base::SingleThreadTaskRunner>& task_runner,
     media::AudioRendererSink* sink,
-    ScopedVector<AudioDecoder> decoders,
+    const CreateAudioDecodersCB& create_audio_decoders_cb,
     const scoped_refptr<MediaLog>& media_log)
     : task_runner_(task_runner),
       expecting_config_changes_(false),
       sink_(sink),
-      audio_buffer_stream_(
-          new AudioBufferStream(task_runner, std::move(decoders), media_log)),
       media_log_(media_log),
       client_(nullptr),
       tick_clock_(new base::DefaultTickClock()),
       last_audio_memory_usage_(0),
       last_decoded_sample_rate_(0),
       last_decoded_channel_layout_(CHANNEL_LAYOUT_NONE),
       playback_rate_(0.0),
       state_(kUninitialized),
+      create_audio_decoders_cb_(create_audio_decoders_cb),
       buffering_state_(BUFFERING_HAVE_NOTHING),
       rendering_(false),
       sink_playing_(false),
       pending_read_(false),
       received_end_of_stream_(false),
       rendered_end_of_stream_(false),
       is_suspending_(false),
       weak_factory_(this) {
-  audio_buffer_stream_->set_config_change_observer(base::Bind(
-      &AudioRendererImpl::OnConfigChange, weak_factory_.GetWeakPtr()));
-
+  DCHECK(!create_audio_decoders_cb_.is_null());
   // Tests may not have a power monitor.
   base::PowerMonitor* monitor = base::PowerMonitor::Get();
   if (!monitor)
     return;
 
   // PowerObserver's must be added and removed from the same thread, but we
   // won't remove the observer until we're destructed on |task_runner_| so we
   // must post it here if we're on the wrong thread.
   if (task_runner_->BelongsToCurrentThread()) {
     monitor->AddObserver(this);
@@ skipping 252 matching lines @@
 void AudioRendererImpl::Initialize(DemuxerStream* stream,
                                    CdmContext* cdm_context,
                                    RendererClient* client,
                                    const PipelineStatusCB& init_cb) {
   DVLOG(1) << __func__;
   DCHECK(task_runner_->BelongsToCurrentThread());
   DCHECK(client);
   DCHECK(stream);
   DCHECK_EQ(stream->type(), DemuxerStream::AUDIO);
   DCHECK(!init_cb.is_null());
-  DCHECK_EQ(kUninitialized, state_);
+  DCHECK(state_ == kUninitialized || state_ == kFlushed);
   DCHECK(sink_.get());
 
+  // If we are re-initializing playback (e.g. switching media tracks), stop the
+  // sink first.
+  if (state_ == kFlushed)

[DaleCurtis, 2017/03/24 19:35:22]

Sink should already be stopped by this point?

[servolk, 2017/03/24 21:15:32]

No, as far as I can see AudioRendererImpl stops the sink only in the destructor.
AudioRendererImpl::StopRendering_Locked just pauses the sink. Since calling
Initialize from kFlushed state only happens when switching audio tracks, I
figured it's probably safest to stop the sink in case the new track has a
different sampling rate or number of channels.

[DaleCurtis, 2017/03/24 21:30:38]

Sorry, I meant that the sink shouldn't be issuing callbacks at this point? Do we
need to Stop() the sink here or is a pause/play cycle enough? Otherwise I think
this will result in a AudioOutputDevice being dropped and recreated (w/ shared
memory and other overhead attached).

[servolk, 2017/03/24 22:18:13]

I think that currently a simple play/pause cycle may be not enough, because if I
remove this or replace sink_->Stop() with sink_->Pause(), I'm getting a DCHECK
when this method goes on and reinitializes the audio buffer stream:
[1:13:0324/151216.804515:FATAL:webaudiosourceprovider_impl.cc(188)] Check
failed: state_ == kStopped (1 vs. 0)
#0 0x7f14f6494ccb base::debug::StackTrace::StackTrace()
#1 0x7f14f649335c base::debug::StackTrace::StackTrace()
#2 0x7f14f650124f logging::LogMessage::~LogMessage()
#3 0x7f14e11f24c7 media::WebAudioSourceProviderImpl::Initialize()
#4 0x7f14ecceb2d9 media::AudioRendererImpl::OnAudioBufferStreamInitialized()

[DaleCurtis, 2017/03/24 22:20:02]

Yes, I was thinking the code would be smart enough to avoid the re-initialize
when possible. Though I just remembered you configuration may change, so perhaps
it's unavoidable.

+    sink_->Stop();
+
   state_ = kInitializing;
   client_ = client;
 
+  audio_buffer_stream_ = base::MakeUnique<AudioBufferStream>(
+      task_runner_, create_audio_decoders_cb_.Run(), media_log_);
+
+  audio_buffer_stream_->set_config_change_observer(base::Bind(
+      &AudioRendererImpl::OnConfigChange, weak_factory_.GetWeakPtr()));
+
   // Always post |init_cb_| because |this| could be destroyed if initialization
   // failed.
   init_cb_ = BindToCurrentLoop(init_cb);
 
   auto output_device_info = sink_->GetOutputDeviceInfo();
   const AudioParameters& hw_params = output_device_info.output_params();
   expecting_config_changes_ = stream->SupportsConfigChanges();
   if (!expecting_config_changes_ || !hw_params.IsValid() ||
       hw_params.format() == AudioParameters::AUDIO_FAKE) {
     // The actual buffer size is controlled via the size of the AudioBus
@@ skipping 635 matching lines @@
   // All channels with a zero mix are muted and can be ignored.
   std::vector<bool> channel_mask(audio_parameters_.channels(), false);
   for (size_t ch = 0; ch < matrix.size(); ++ch) {
     channel_mask[ch] = std::any_of(matrix[ch].begin(), matrix[ch].end(),
                                    [](float mix) { return !!mix; });
   }
   algorithm_->SetChannelMask(std::move(channel_mask));
 }
 
 }  // namespace media