Index: content/renderer/media/webrtc/webrtc_local_audio_track_adapter.cc |
diff --git a/content/renderer/media/webrtc/webrtc_local_audio_track_adapter.cc b/content/renderer/media/webrtc/webrtc_local_audio_track_adapter.cc |
index 20a3969d11312d81c0b99354bc08c3f476666884..d7384ca0d225c0cb31fbcdc960bee2cd69edab27 100644 |
--- a/content/renderer/media/webrtc/webrtc_local_audio_track_adapter.cc |
+++ b/content/renderer/media/webrtc/webrtc_local_audio_track_adapter.cc |
@@ -23,34 +23,30 @@ WebRtcLocalAudioTrackAdapter::Create( |
webrtc::AudioSourceInterface* track_source) { |
// TODO(tommi): Change this so that the signaling thread is one of the |
// parameters to this method. |
- scoped_refptr<base::SingleThreadTaskRunner> signaling_thread; |
+ scoped_refptr<base::SingleThreadTaskRunner> signaling_task_runner; |
RenderThreadImpl* current = RenderThreadImpl::current(); |
if (current) { |
PeerConnectionDependencyFactory* pc_factory = |
current->GetPeerConnectionDependencyFactory(); |
- signaling_thread = pc_factory->GetWebRtcSignalingThread(); |
+ signaling_task_runner = pc_factory->GetWebRtcSignalingThread(); |
} |
- LOG_IF(ERROR, !signaling_thread.get()) << "No signaling thread!"; |
+ LOG_IF(ERROR, !signaling_task_runner.get()) << "No signaling thread!"; |
rtc::RefCountedObject<WebRtcLocalAudioTrackAdapter>* adapter = |
new rtc::RefCountedObject<WebRtcLocalAudioTrackAdapter>( |
- label, track_source, signaling_thread); |
+ label, track_source, signaling_task_runner); |
return adapter; |
} |
WebRtcLocalAudioTrackAdapter::WebRtcLocalAudioTrackAdapter( |
const std::string& label, |
webrtc::AudioSourceInterface* track_source, |
- const scoped_refptr<base::SingleThreadTaskRunner>& signaling_thread) |
+ const scoped_refptr<base::SingleThreadTaskRunner>& signaling_task_runner) |
: webrtc::MediaStreamTrack<webrtc::AudioTrackInterface>(label), |
owner_(NULL), |
track_source_(track_source), |
- signaling_thread_(signaling_thread), |
- signal_level_(0) { |
- signaling_thread_checker_.DetachFromThread(); |
- capture_thread_.DetachFromThread(); |
-} |
+ signaling_task_runner_(signaling_task_runner) {} |
o1ka
2016/02/29 14:28:04
How thread safety of the class is guaranteed if si
miu
2016/03/01 09:43:54
Many of the unit tests run single-threaded. See l
o1ka
2016/03/01 14:18:58
Acknowledged.
|
WebRtcLocalAudioTrackAdapter::~WebRtcLocalAudioTrackAdapter() { |
} |
@@ -63,15 +59,22 @@ void WebRtcLocalAudioTrackAdapter::Initialize(WebRtcLocalAudioTrack* owner) { |
void WebRtcLocalAudioTrackAdapter::SetAudioProcessor( |
const scoped_refptr<MediaStreamAudioProcessor>& processor) { |
- // SetAudioProcessor will be called when a new capture thread has been |
- // initialized, so we need to detach from any current capture thread we're |
- // checking and attach to the current one. |
- capture_thread_.DetachFromThread(); |
- DCHECK(capture_thread_.CalledOnValidThread()); |
- base::AutoLock auto_lock(lock_); |
+ // |audio_processor_| must only be set once. This eliminates the need to |
+ // protect |audio_processor_| with a mutex. |
o1ka
2016/02/29 14:28:04
Is there any guarantee that there won't be a race
miu
2016/03/01 09:43:55
No, but the header comments and the DCHECK on line
o1ka
2016/03/01 14:18:59
Agree that calling SetAudioProcessor() second time
miu
2016/03/02 01:12:50
Where is the race condition? I don't see what els
o1ka
2016/03/02 16:31:13
Acknowledged.
|
+ DCHECK(processor.get()); |
+ DCHECK(!audio_processor_); |
audio_processor_ = processor; |
} |
+void WebRtcLocalAudioTrackAdapter::SetLevel( |
+ scoped_refptr<MediaStreamAudioLevelCalculator::Level> level) { |
+ // |level_| must only be set once. This eliminates the need to protect |
+ // |level_| with a mutex. |
o1ka
2016/02/29 14:28:04
Same questions as for SetAudioProcessor()
miu
2016/03/01 09:43:55
Good questions. Answered in other comment.
o1ka
2016/03/01 14:18:58
Same as above: looks like just "setting it once" i
miu
2016/03/02 01:12:50
I removed these comments since I think they were m
|
+ DCHECK(level.get()); |
+ DCHECK(!level_); |
+ level_ = level; |
+} |
+ |
std::string WebRtcLocalAudioTrackAdapter::kind() const { |
return kAudioTrackKind; |
} |
@@ -79,8 +82,9 @@ std::string WebRtcLocalAudioTrackAdapter::kind() const { |
bool WebRtcLocalAudioTrackAdapter::set_enabled(bool enable) { |
// If we're not called on the signaling thread, we need to post a task to |
// change the state on the correct thread. |
- if (signaling_thread_.get() && !signaling_thread_->BelongsToCurrentThread()) { |
- signaling_thread_->PostTask(FROM_HERE, |
+ if (signaling_task_runner_.get() && |
o1ka
2016/02/29 14:28:04
nit: if (signaling_task_runner_ && ...)
miu
2016/03/01 09:43:54
Done.
o1ka
2016/03/01 14:18:59
Acknowledged.
|
+ !signaling_task_runner_->BelongsToCurrentThread()) { |
+ signaling_task_runner_->PostTask(FROM_HERE, |
base::Bind( |
base::IgnoreResult(&WebRtcLocalAudioTrackAdapter::set_enabled), |
this, enable)); |
@@ -93,7 +97,8 @@ bool WebRtcLocalAudioTrackAdapter::set_enabled(bool enable) { |
void WebRtcLocalAudioTrackAdapter::AddSink( |
webrtc::AudioTrackSinkInterface* sink) { |
- DCHECK(signaling_thread_checker_.CalledOnValidThread()); |
+ DCHECK(!signaling_task_runner_ || |
+ signaling_task_runner_->RunsTasksOnCurrentThread()); |
DCHECK(sink); |
#ifndef NDEBUG |
// Verify that |sink| has not been added. |
@@ -112,7 +117,8 @@ void WebRtcLocalAudioTrackAdapter::AddSink( |
void WebRtcLocalAudioTrackAdapter::RemoveSink( |
webrtc::AudioTrackSinkInterface* sink) { |
- DCHECK(signaling_thread_checker_.CalledOnValidThread()); |
+ DCHECK(!signaling_task_runner_ || |
+ signaling_task_runner_->RunsTasksOnCurrentThread()); |
DCHECK(sink); |
for (ScopedVector<WebRtcAudioSinkAdapter>::iterator it = |
sink_adapters_.begin(); |
@@ -126,28 +132,32 @@ void WebRtcLocalAudioTrackAdapter::RemoveSink( |
} |
bool WebRtcLocalAudioTrackAdapter::GetSignalLevel(int* level) { |
- DCHECK(signaling_thread_checker_.CalledOnValidThread()); |
- |
- base::AutoLock auto_lock(lock_); |
- *level = signal_level_; |
+ DCHECK(!signaling_task_runner_ || |
+ signaling_task_runner_->RunsTasksOnCurrentThread()); |
+ |
+ // |level_| is only set once, so it's safe to read without first acquiring a |
+ // mutex. |
+ if (!level_) |
o1ka
2016/02/29 14:28:04
How does it guarantee that there is no race?
miu
2016/03/01 09:43:54
Answered in comment above.
o1ka
2016/03/01 14:18:58
Same as above.
|
+ return false; |
+ const float signal_level = level_->GetCurrent(); |
+ DCHECK_GE(signal_level, 0.0f); |
+ DCHECK_LE(signal_level, 1.0f); |
+ // Convert from float in range [0.0,1.0] to an int in range [0,32767]. |
+ *level = static_cast<int>(signal_level * std::numeric_limits<int16_t>::max() + |
+ 0.5f /* rounding to nearest int */); |
return true; |
} |
rtc::scoped_refptr<webrtc::AudioProcessorInterface> |
WebRtcLocalAudioTrackAdapter::GetAudioProcessor() { |
- DCHECK(signaling_thread_checker_.CalledOnValidThread()); |
- base::AutoLock auto_lock(lock_); |
+ DCHECK(!signaling_task_runner_ || |
+ signaling_task_runner_->RunsTasksOnCurrentThread()); |
return audio_processor_.get(); |
} |
-void WebRtcLocalAudioTrackAdapter::SetSignalLevel(int signal_level) { |
- DCHECK(capture_thread_.CalledOnValidThread()); |
- base::AutoLock auto_lock(lock_); |
- signal_level_ = signal_level; |
-} |
- |
webrtc::AudioSourceInterface* WebRtcLocalAudioTrackAdapter::GetSource() const { |
- DCHECK(signaling_thread_checker_.CalledOnValidThread()); |
+ DCHECK(!signaling_task_runner_ || |
+ signaling_task_runner_->RunsTasksOnCurrentThread()); |
return track_source_; |
} |