Chromium Code Reviews
chromiumcodereview-hr@appspot.gserviceaccount.com (chromiumcodereview-hr) | Please choose your nickname with Settings | Help | Chromium Project | Gerrit Changes | Sign out
(58)

Side by Side Diff: media/base/audio_renderer_mixer_input.cc

Issue 1771463002: Revert of Add lock to fix race in AudioRendererMixerInput. (Closed) Base URL: https://chromium.googlesource.com/chromium/src.git@master
Patch Set: Created 4 years, 9 months ago
Use n/p to move between diff chunks; N/P to move between comments. Draft comments are only viewable by you.
Jump to:
View unified diff | Download patch
« no previous file with comments | « media/base/audio_renderer_mixer_input.h ('k') | no next file » | no next file with comments »
Toggle Intra-line Diffs ('i') | Expand Comments ('e') | Collapse Comments ('c') | Show Comments Hide Comments ('s')
OLDNEW
1 // Copyright (c) 2012 The Chromium Authors. All rights reserved. 1 // Copyright (c) 2012 The Chromium Authors. All rights reserved.
2 // Use of this source code is governed by a BSD-style license that can be 2 // Use of this source code is governed by a BSD-style license that can be
3 // found in the LICENSE file. 3 // found in the LICENSE file.
4 4
5 #include "media/base/audio_renderer_mixer_input.h" 5 #include "media/base/audio_renderer_mixer_input.h"
6 6
7 #include <cmath> 7 #include <cmath>
8 8
9 #include "base/bind.h" 9 #include "base/bind.h"
10 #include "base/callback_helpers.h" 10 #include "base/callback_helpers.h"
(...skipping 11 matching lines...) Expand all
22 playing_(false), 22 playing_(false),
23 volume_(1.0f), 23 volume_(1.0f),
24 get_mixer_cb_(get_mixer_cb), 24 get_mixer_cb_(get_mixer_cb),
25 remove_mixer_cb_(remove_mixer_cb), 25 remove_mixer_cb_(remove_mixer_cb),
26 get_hardware_params_cb_(get_hardware_params_cb), 26 get_hardware_params_cb_(get_hardware_params_cb),
27 device_id_(device_id), 27 device_id_(device_id),
28 security_origin_(security_origin), 28 security_origin_(security_origin),
29 mixer_(nullptr), 29 mixer_(nullptr),
30 callback_(nullptr), 30 callback_(nullptr),
31 error_cb_(base::Bind(&AudioRendererMixerInput::OnRenderError, 31 error_cb_(base::Bind(&AudioRendererMixerInput::OnRenderError,
32 base::Unretained(this))) { 32 base::Unretained(this))) {}
33 // Can be constructed on any thread, but sink operations should all occur
34 // on same thread.
35 thread_checker_.DetachFromThread();
36 }
37 33
38 AudioRendererMixerInput::~AudioRendererMixerInput() { 34 AudioRendererMixerInput::~AudioRendererMixerInput() {
39 DCHECK(!started_);
40 DCHECK(!mixer_); 35 DCHECK(!mixer_);
41 } 36 }
42 37
43 void AudioRendererMixerInput::Initialize( 38 void AudioRendererMixerInput::Initialize(
44 const AudioParameters& params, 39 const AudioParameters& params,
45 AudioRendererSink::RenderCallback* callback) { 40 AudioRendererSink::RenderCallback* callback) {
46 DCHECK(thread_checker_.CalledOnValidThread());
47 DCHECK(!started_);
48 DCHECK(!mixer_); 41 DCHECK(!mixer_);
49 DCHECK(callback); 42 DCHECK(callback);
50 43
51 params_ = params; 44 params_ = params;
52 callback_ = callback; 45 callback_ = callback;
53 } 46 }
54 47
55 void AudioRendererMixerInput::Start() { 48 void AudioRendererMixerInput::Start() {
56 DCHECK(thread_checker_.CalledOnValidThread());
57 DCHECK(!started_); 49 DCHECK(!started_);
58 DCHECK(!mixer_); 50 DCHECK(!mixer_);
59 DCHECK(callback_); // Initialized. 51 DCHECK(callback_); // Initialized.
60 52
61 started_ = true; 53 started_ = true;
62 mixer_ = get_mixer_cb_.Run(params_, device_id_, security_origin_, nullptr); 54 mixer_ = get_mixer_cb_.Run(params_, device_id_, security_origin_, nullptr);
63 if (!mixer_) { 55 if (!mixer_) {
64 callback_->OnRenderError(); 56 callback_->OnRenderError();
65 return; 57 return;
66 } 58 }
67 59
68 // Note: OnRenderError() may be called immediately after this call returns. 60 // Note: OnRenderError() may be called immediately after this call returns.
69 mixer_->AddErrorCallback(error_cb_); 61 mixer_->AddErrorCallback(error_cb_);
70 62
71 if (!pending_switch_callback_.is_null()) { 63 if (!pending_switch_callback_.is_null()) {
72 SwitchOutputDevice(pending_switch_device_id_, 64 SwitchOutputDevice(pending_switch_device_id_,
73 pending_switch_security_origin_, 65 pending_switch_security_origin_,
74 base::ResetAndReturn(&pending_switch_callback_)); 66 base::ResetAndReturn(&pending_switch_callback_));
75 } 67 }
76 } 68 }
77 69
78 void AudioRendererMixerInput::Stop() { 70 void AudioRendererMixerInput::Stop() {
79 DCHECK(thread_checker_.CalledOnValidThread());
80
81 // Stop() may be called at any time, if Pause() hasn't been called we need to 71 // Stop() may be called at any time, if Pause() hasn't been called we need to
82 // remove our mixer input before shutdown. 72 // remove our mixer input before shutdown.
83 Pause(); 73 Pause();
84 74
85 if (mixer_) { 75 if (mixer_) {
86 // TODO(dalecurtis): This is required so that |callback_| isn't called after 76 // TODO(dalecurtis): This is required so that |callback_| isn't called after
87 // Stop() by an error event since it may outlive this ref-counted object. We 77 // Stop() by an error event since it may outlive this ref-counted object. We
88 // should instead have sane ownership semantics: http://crbug.com/151051 78 // should instead have sane ownership semantics: http://crbug.com/151051
89 mixer_->RemoveErrorCallback(error_cb_); 79 mixer_->RemoveErrorCallback(error_cb_);
90 remove_mixer_cb_.Run(params_, device_id_, security_origin_); 80 remove_mixer_cb_.Run(params_, device_id_, security_origin_);
91 mixer_ = nullptr; 81 mixer_ = nullptr;
92 } 82 }
93 83
94 started_ = false; 84 started_ = false;
95 85
96 if (!pending_switch_callback_.is_null()) { 86 if (!pending_switch_callback_.is_null()) {
97 base::ResetAndReturn(&pending_switch_callback_) 87 base::ResetAndReturn(&pending_switch_callback_)
98 .Run(OUTPUT_DEVICE_STATUS_ERROR_INTERNAL); 88 .Run(OUTPUT_DEVICE_STATUS_ERROR_INTERNAL);
99 } 89 }
100 } 90 }
101 91
102 void AudioRendererMixerInput::Play() { 92 void AudioRendererMixerInput::Play() {
103 DCHECK(thread_checker_.CalledOnValidThread());
104
105 if (playing_ || !mixer_) 93 if (playing_ || !mixer_)
106 return; 94 return;
107 95
108 mixer_->AddMixerInput(params_, this); 96 mixer_->AddMixerInput(params_, this);
109 playing_ = true; 97 playing_ = true;
110 } 98 }
111 99
112 void AudioRendererMixerInput::Pause() { 100 void AudioRendererMixerInput::Pause() {
113 DCHECK(thread_checker_.CalledOnValidThread());
114
115 if (!playing_ || !mixer_) 101 if (!playing_ || !mixer_)
116 return; 102 return;
117 103
118 mixer_->RemoveMixerInput(params_, this); 104 mixer_->RemoveMixerInput(params_, this);
119 playing_ = false; 105 playing_ = false;
120 } 106 }
121 107
122 bool AudioRendererMixerInput::SetVolume(double volume) { 108 bool AudioRendererMixerInput::SetVolume(double volume) {
123 DCHECK(thread_checker_.CalledOnValidThread());
124 base::AutoLock auto_lock(volume_lock_);
125 volume_ = volume; 109 volume_ = volume;
126 return true; 110 return true;
127 } 111 }
128 112
129 OutputDevice* AudioRendererMixerInput::GetOutputDevice() { 113 OutputDevice* AudioRendererMixerInput::GetOutputDevice() {
130 return this; 114 return this;
131 } 115 }
132 116
133 void AudioRendererMixerInput::SwitchOutputDevice( 117 void AudioRendererMixerInput::SwitchOutputDevice(
134 const std::string& device_id, 118 const std::string& device_id,
135 const url::Origin& security_origin, 119 const url::Origin& security_origin,
136 const SwitchOutputDeviceCB& callback) { 120 const SwitchOutputDeviceCB& callback) {
137 DCHECK(thread_checker_.CalledOnValidThread());
138
139 if (!mixer_) { 121 if (!mixer_) {
140 if (pending_switch_callback_.is_null()) { 122 if (pending_switch_callback_.is_null()) {
141 pending_switch_callback_ = callback; 123 pending_switch_callback_ = callback;
142 pending_switch_device_id_ = device_id; 124 pending_switch_device_id_ = device_id;
143 pending_switch_security_origin_ = security_origin; 125 pending_switch_security_origin_ = security_origin;
144 } else { 126 } else {
145 callback.Run(OUTPUT_DEVICE_STATUS_ERROR_INTERNAL); 127 callback.Run(OUTPUT_DEVICE_STATUS_ERROR_INTERNAL);
146 } 128 }
147 129
148 return; 130 return;
(...skipping 21 matching lines...) Expand all
170 mixer_->AddErrorCallback(error_cb_); 152 mixer_->AddErrorCallback(error_cb_);
171 started_ = true; 153 started_ = true;
172 154
173 if (was_playing) 155 if (was_playing)
174 Play(); 156 Play();
175 157
176 callback.Run(OUTPUT_DEVICE_STATUS_OK); 158 callback.Run(OUTPUT_DEVICE_STATUS_OK);
177 } 159 }
178 160
179 AudioParameters AudioRendererMixerInput::GetOutputParameters() { 161 AudioParameters AudioRendererMixerInput::GetOutputParameters() {
180 DCHECK(thread_checker_.CalledOnValidThread());
181
182 if (mixer_) 162 if (mixer_)
183 return mixer_->GetOutputDevice()->GetOutputParameters(); 163 return mixer_->GetOutputDevice()->GetOutputParameters();
184 return get_hardware_params_cb_.Run(device_id_, security_origin_); 164 return get_hardware_params_cb_.Run(device_id_, security_origin_);
185 } 165 }
186 166
187 OutputDeviceStatus AudioRendererMixerInput::GetDeviceStatus() { 167 OutputDeviceStatus AudioRendererMixerInput::GetDeviceStatus() {
188 DCHECK(thread_checker_.CalledOnValidThread());
189
190 if (mixer_) 168 if (mixer_)
191 return mixer_->GetOutputDevice()->GetDeviceStatus(); 169 return mixer_->GetOutputDevice()->GetDeviceStatus();
192 170
193 if (started_) 171 if (started_)
194 return OUTPUT_DEVICE_STATUS_ERROR_INTERNAL; 172 return OUTPUT_DEVICE_STATUS_ERROR_INTERNAL;
195 173
196 return OUTPUT_DEVICE_STATUS_OK; 174 return OUTPUT_DEVICE_STATUS_OK;
197 } 175 }
198 176
199 double AudioRendererMixerInput::ProvideInput(AudioBus* audio_bus, 177 double AudioRendererMixerInput::ProvideInput(AudioBus* audio_bus,
200 base::TimeDelta buffer_delay) { 178 base::TimeDelta buffer_delay) {
201 // No thread checker here. This method is called on a different thread as part
202 // of audio rendering. AudioRendererMixer has locks that protect us from
203 // things like attempting to ProvideInput while simultaneously removing
204 // ourselves from mixer inputs (see Pause()).
205
206 // TODO(chcunningham): Delete this conversion and change ProvideInput to more 179 // TODO(chcunningham): Delete this conversion and change ProvideInput to more
207 // precisely describe delay as a count of frames delayed instead of TimeDelta. 180 // precisely describe delay as a count of frames delayed instead of TimeDelta.
208 // See http://crbug.com/587522. 181 // See http://crbug.com/587522.
209 uint32_t frames_delayed = std::round(buffer_delay.InMicroseconds() / 182 uint32_t frames_delayed = std::round(buffer_delay.InMicroseconds() /
210 params_.GetMicrosecondsPerFrame()); 183 params_.GetMicrosecondsPerFrame());
211 184
212 int frames_filled = callback_->Render(audio_bus, frames_delayed, 0); 185 int frames_filled = callback_->Render(audio_bus, frames_delayed, 0);
213 186
214 // AudioConverter expects unfilled frames to be zeroed. 187 // AudioConverter expects unfilled frames to be zeroed.
215 if (frames_filled < audio_bus->frames()) { 188 if (frames_filled < audio_bus->frames()) {
216 audio_bus->ZeroFramesPartial( 189 audio_bus->ZeroFramesPartial(
217 frames_filled, audio_bus->frames() - frames_filled); 190 frames_filled, audio_bus->frames() - frames_filled);
218 } 191 }
219 192
220 // Synchronize access to |volume_| with SetVolume(). 193 return frames_filled > 0 ? volume_ : 0;
221 {
222 base::AutoLock auto_lock(volume_lock_);
223 return frames_filled > 0 ? volume_ : 0;
224 }
225 } 194 }
226 195
227 void AudioRendererMixerInput::OnRenderError() { 196 void AudioRendererMixerInput::OnRenderError() {
228 callback_->OnRenderError(); 197 callback_->OnRenderError();
229 } 198 }
230 199
231 } // namespace media 200 } // namespace media
OLDNEW
« no previous file with comments | « media/base/audio_renderer_mixer_input.h ('k') | no next file » | no next file with comments »

Powered by Google App Engine
This is Rietveld 408576698