Skip to content

Commit 6003b71

Browse files
committed
Merge M72: "Convert <audio> pipeline to use async device info requests."
This is part 4/4 CLs to move the <audio>/<video> elements off of a synchronous API that can lead to renderer hangs and premature audio renderer errors. This changes the AudioRendererMixerPool API to require an AudioRendererSink and OutputDeviceInfo when providing a mixer. AudioRendererMixerInputs are subsequently changed to use the new API. Likewise AudioRendererImpl also now uses the asynchronous API. To simplify the async process, AudioRendererMixerInputs will only setup correctly when OutputDeviceInfo has been requested ahead of time, since that's the pattern that AudioRendererImpl will use. This also moves the NullAudioSink setup from WebAudioSourceProvider over to the AudioRendererImpl. This causes WebAudio to be disconnected from the element, but if audio isn't work anyways, it shouldn't matter. BUG=905506 TEST=updated tests, compiles, runs. R=​olka Change-Id: I4edf89bb1e20cc91191a6eb97a0e38b6aeba68f8 Reviewed-on: https://chromium-review.googlesource.com/c/1347795 Commit-Queue: Dale Curtis <[email protected]> Reviewed-by: Jesse Doherty <[email protected]> Reviewed-by: Olga Sharonova <[email protected]> Reviewed-by: Chrome Cunningham <[email protected]> Cr-Original-Commit-Position: refs/heads/master@{#612526}(cherry picked from commit 41607b5) Reviewed-on: https://chromium-review.googlesource.com/c/1362295 Reviewed-by: Dale Curtis <[email protected]> Cr-Commit-Position: refs/branch-heads/3626@{#49} Cr-Branched-From: d897fb1-refs/heads/master@{#612437}
1 parent d83c002 commit 6003b71

20 files changed

+460
-481
lines changed

content/renderer/media/audio/audio_device_factory.cc

Lines changed: 6 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -78,10 +78,9 @@ scoped_refptr<media::SwitchableAudioRendererSink> NewMixableSink(
7878
DCHECK(render_thread) << "RenderThreadImpl is not instantiated, or "
7979
<< "GetOutputDeviceInfo() is called on a wrong thread ";
8080
DCHECK(!params.processing_id.has_value());
81-
return scoped_refptr<media::AudioRendererMixerInput>(
82-
render_thread->GetAudioRendererMixerManager()->CreateInput(
83-
render_frame_id, params.session_id, params.device_id,
84-
AudioDeviceFactory::GetSourceLatencyType(source_type)));
81+
return render_thread->GetAudioRendererMixerManager()->CreateInput(
82+
render_frame_id, params.session_id, params.device_id,
83+
AudioDeviceFactory::GetSourceLatencyType(source_type));
8584
}
8685

8786
} // namespace
@@ -110,8 +109,9 @@ scoped_refptr<media::AudioRendererSink>
110109
AudioDeviceFactory::NewAudioRendererMixerSink(
111110
int render_frame_id,
112111
const media::AudioSinkParameters& params) {
113-
return NewFinalAudioRendererSink(render_frame_id, params,
114-
GetDefaultAuthTimeout());
112+
// AudioRendererMixer sinks are always used asynchronously and thus can
113+
// operate without a timeout value.
114+
return NewFinalAudioRendererSink(render_frame_id, params, base::TimeDelta());
115115
}
116116

117117
// static

content/renderer/media/audio/audio_device_factory.h

Lines changed: 5 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -51,9 +51,11 @@ class CONTENT_EXPORT AudioDeviceFactory {
5151
static media::AudioLatency::LatencyType GetSourceLatencyType(
5252
SourceType source);
5353

54-
// Creates a sink for AudioRendererMixer.
55-
// |render_frame_id| refers to the RenderFrame containing the entity
56-
// producing the audio.
54+
// Creates a sink for AudioRendererMixer. |render_frame_id| refers to the
55+
// RenderFrame containing the entity producing the audio. Note: These sinks do
56+
// not support the blocking GetOutputDeviceInfo() API and instead clients are
57+
// required to use the GetOutputDeviceInfoAsync() API. As such they are
58+
// configured with no authorization timeout value.
5759
static scoped_refptr<media::AudioRendererSink> NewAudioRendererMixerSink(
5860
int render_frame_id,
5961
const media::AudioSinkParameters& params);

content/renderer/media/audio/audio_renderer_mixer_manager.cc

Lines changed: 28 additions & 34 deletions
Original file line numberDiff line numberDiff line change
@@ -147,31 +147,35 @@ std::unique_ptr<AudioRendererMixerManager> AudioRendererMixerManager::Create() {
147147
base::BindRepeating(&AudioDeviceFactory::NewAudioRendererMixerSink)));
148148
}
149149

150-
media::AudioRendererMixerInput* AudioRendererMixerManager::CreateInput(
150+
scoped_refptr<media::AudioRendererMixerInput>
151+
AudioRendererMixerManager::CreateInput(
151152
int source_render_frame_id,
152153
int session_id,
153154
const std::string& device_id,
154155
media::AudioLatency::LatencyType latency) {
155156
// AudioRendererMixerManager lives on the renderer thread and is destroyed on
156157
// renderer thread destruction, so it's safe to pass its pointer to a mixer
157158
// input.
158-
return new media::AudioRendererMixerInput(
159-
this, source_render_frame_id,
160-
media::AudioDeviceDescription::UseSessionIdToSelectDevice(session_id,
161-
device_id)
162-
? GetOutputDeviceInfo(source_render_frame_id, session_id, device_id)
163-
.device_id()
164-
: device_id,
165-
latency);
159+
//
160+
// TODO(olka, grunell): |session_id| is always zero, delete since
161+
// NewAudioRenderingMixingStrategy didn't ship, https://crbug.com/870836.
162+
DCHECK_EQ(session_id, 0);
163+
return base::MakeRefCounted<media::AudioRendererMixerInput>(
164+
this, source_render_frame_id, device_id, latency);
166165
}
167166

168167
media::AudioRendererMixer* AudioRendererMixerManager::GetMixer(
169168
int source_render_frame_id,
170169
const media::AudioParameters& input_params,
171170
media::AudioLatency::LatencyType latency,
172-
const std::string& device_id,
173-
media::OutputDeviceStatus* device_status) {
174-
const MixerKey key(source_render_frame_id, input_params, latency, device_id);
171+
const media::OutputDeviceInfo& sink_info,
172+
scoped_refptr<media::AudioRendererSink> sink) {
173+
// Ownership of the sink must be given to GetMixer().
174+
DCHECK(sink->HasOneRef());
175+
DCHECK_EQ(sink_info.device_status(), media::OUTPUT_DEVICE_STATUS_OK);
176+
177+
const MixerKey key(source_render_frame_id, input_params, latency,
178+
sink_info.device_id());
175179
base::AutoLock auto_lock(mixers_lock_);
176180

177181
// Update latency map when the mixer is requested, i.e. there is an attempt to
@@ -189,27 +193,22 @@ media::AudioRendererMixer* AudioRendererMixerManager::GetMixer(
189193

190194
auto it = mixers_.find(key);
191195
if (it != mixers_.end()) {
192-
if (device_status)
193-
*device_status = media::OUTPUT_DEVICE_STATUS_OK;
194-
195196
it->second.ref_count++;
196197
DVLOG(1) << "Reusing mixer: " << it->second.mixer;
197-
return it->second.mixer;
198-
}
199198

200-
scoped_refptr<media::AudioRendererSink> sink = create_sink_cb_.Run(
201-
source_render_frame_id, media::AudioSinkParameters(0, device_id));
202-
203-
const media::OutputDeviceInfo& device_info = sink->GetOutputDeviceInfo();
204-
if (device_status)
205-
*device_status = device_info.device_status();
206-
if (device_info.device_status() != media::OUTPUT_DEVICE_STATUS_OK) {
199+
// Sink will now be released unused, but still must be stopped.
200+
//
201+
// TODO(dalecurtis): Is it worth caching this sink instead for a future
202+
// GetSink() call? We should experiment with a few top sites. We can't just
203+
// drop in AudioRendererSinkCache here since it doesn't reuse sinks once
204+
// they've been vended externally to the class.
207205
sink->Stop();
208-
return nullptr;
206+
207+
return it->second.mixer;
209208
}
210209

211210
const media::AudioParameters& mixer_output_params =
212-
GetMixerOutputParams(input_params, device_info.output_params(), latency);
211+
GetMixerOutputParams(input_params, sink_info.output_params(), latency);
213212
media::AudioRendererMixer* mixer = new media::AudioRendererMixer(
214213
mixer_output_params, std::move(sink),
215214
base::BindRepeating(LogMixerUmaHistogram, latency));
@@ -237,16 +236,11 @@ void AudioRendererMixerManager::ReturnMixer(media::AudioRendererMixer* mixer) {
237236
}
238237
}
239238

240-
media::OutputDeviceInfo AudioRendererMixerManager::GetOutputDeviceInfo(
239+
scoped_refptr<media::AudioRendererSink> AudioRendererMixerManager::GetSink(
241240
int source_render_frame_id,
242-
int session_id,
243241
const std::string& device_id) {
244-
auto sink =
245-
create_sink_cb_.Run(source_render_frame_id,
246-
media::AudioSinkParameters(session_id, device_id));
247-
auto info = sink->GetOutputDeviceInfo();
248-
sink->Stop();
249-
return info;
242+
return create_sink_cb_.Run(source_render_frame_id,
243+
media::AudioSinkParameters(0, device_id));
250244
}
251245

252246
AudioRendererMixerManager::MixerKey::MixerKey(

content/renderer/media/audio/audio_renderer_mixer_manager.h

Lines changed: 4 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -53,7 +53,7 @@ class CONTENT_EXPORT AudioRendererMixerManager
5353
// device to use. If |device_id| is empty and |session_id| is nonzero,
5454
// output device associated with the opened input device designated by
5555
// |session_id| is used. Otherwise, |session_id| is ignored.
56-
media::AudioRendererMixerInput* CreateInput(
56+
scoped_refptr<media::AudioRendererMixerInput> CreateInput(
5757
int source_render_frame_id,
5858
int session_id,
5959
const std::string& device_id,
@@ -65,14 +65,13 @@ class CONTENT_EXPORT AudioRendererMixerManager
6565
int source_render_frame_id,
6666
const media::AudioParameters& input_params,
6767
media::AudioLatency::LatencyType latency,
68-
const std::string& device_id,
69-
media::OutputDeviceStatus* device_status) final;
68+
const media::OutputDeviceInfo& sink_info,
69+
scoped_refptr<media::AudioRendererSink> sink) final;
7070

7171
void ReturnMixer(media::AudioRendererMixer* mixer) final;
7272

73-
media::OutputDeviceInfo GetOutputDeviceInfo(
73+
scoped_refptr<media::AudioRendererSink> GetSink(
7474
int source_render_frame_id,
75-
int session_id,
7675
const std::string& device_id) final;
7776

7877
protected:

0 commit comments

Comments
 (0)