Commit 7ad07b8c authored by sigbjornf's avatar sigbjornf Committed by Commit Bot

Avoid AudioBufferSourceHandler data race.

Following r478084, the main thread may contend with the audio thread
on accessing AudioBufferSourceNodeHandler's mutable state. Coordinate
such access by introducing a Mutex over |min_playback_rate_|.

Using atomic ops would be the natural choice for handling this, but
steering clear of those over doubles (cf. https://crrev.com/1256053006)
until std::atomic<> is allowed.

R=haraken,hongchan
BUG=731568

Review-Url: https://codereview.chromium.org/2929283002
Cr-Commit-Position: refs/heads/master@{#478679}
parent 775f2fb6
...@@ -590,12 +590,21 @@ double AudioBufferSourceHandler::ComputePlaybackRate() { ...@@ -590,12 +590,21 @@ double AudioBufferSourceHandler::ComputePlaybackRate() {
if (!is_playback_rate_valid) if (!is_playback_rate_valid)
final_playback_rate = 1.0; final_playback_rate = 1.0;
// Record the minimum playback rate for use by handleStoppableSourceNode. // Record the minimum playback rate for use by HandleStoppableSourceNode.
min_playback_rate_ = std::min(final_playback_rate, min_playback_rate_); if (final_playback_rate < min_playback_rate_) {
MutexLocker locker(min_playback_rate_mutex_);
min_playback_rate_ = final_playback_rate;
}
return final_playback_rate; return final_playback_rate;
} }
double AudioBufferSourceHandler::GetMinPlaybackRate() {
DCHECK(IsMainThread());
MutexLocker locker(min_playback_rate_mutex_);
return min_playback_rate_;
}
bool AudioBufferSourceHandler::PropagatesSilence() const { bool AudioBufferSourceHandler::PropagatesSilence() const {
return !IsPlayingOrScheduled() || HasFinished() || !buffer_; return !IsPlayingOrScheduled() || HasFinished() || !buffer_;
} }
...@@ -611,12 +620,13 @@ void AudioBufferSourceHandler::HandleStoppableSourceNode() { ...@@ -611,12 +620,13 @@ void AudioBufferSourceHandler::HandleStoppableSourceNode() {
// If looping was ever done (m_didSetLooping = true), give up. We can't // If looping was ever done (m_didSetLooping = true), give up. We can't
// easily determine how long we looped so we don't know the actual duration // easily determine how long we looped so we don't know the actual duration
// thus far, so don't try to do anything fancy. // thus far, so don't try to do anything fancy.
double min_playback_rate = GetMinPlaybackRate();
if (!DidSetLooping() && Buffer() && IsPlayingOrScheduled() && if (!DidSetLooping() && Buffer() && IsPlayingOrScheduled() &&
min_playback_rate_ > 0) { min_playback_rate > 0) {
// Adjust the duration to include the playback rate. Only need to account // Adjust the duration to include the playback rate. Only need to account
// for rate < 1 which makes the sound last longer. For rate >= 1, the // for rate < 1 which makes the sound last longer. For rate >= 1, the
// source stops sooner, but that's ok. // source stops sooner, but that's ok.
double actual_duration = Buffer()->duration() / min_playback_rate_; double actual_duration = Buffer()->duration() / min_playback_rate;
double stop_time = start_time_ + actual_duration; double stop_time = start_time_ + actual_duration;
......
...@@ -168,8 +168,18 @@ class AudioBufferSourceHandler final : public AudioScheduledSourceHandler { ...@@ -168,8 +168,18 @@ class AudioBufferSourceHandler final : public AudioScheduledSourceHandler {
// conversion factor, and the value of playbackRate and detune AudioParams. // conversion factor, and the value of playbackRate and detune AudioParams.
double ComputePlaybackRate(); double ComputePlaybackRate();
double GetMinPlaybackRate();
// The minimum playbackRate value ever used for this source. // The minimum playbackRate value ever used for this source.
double min_playback_rate_; double min_playback_rate_;
// |min_playback_rate_| may be updated by the audio thread
// while the main thread checks if the node is in a stoppable
// state, hence access needs to be atomic.
//
// TODO: when the codebase adopts std::atomic<>, use it for
// |min_playback_rate_|.
Mutex min_playback_rate_mutex_;
}; };
class AudioBufferSourceNode final : public AudioScheduledSourceNode { class AudioBufferSourceNode final : public AudioScheduledSourceNode {
......
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment