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

Unified Diff: Source/modules/webaudio/PannerNode.cpp

Issue 196993002: Guarantee a thread safety when accessing source location info in pannerNode. (Closed) Base URL: https://chromium.googlesource.com/chromium/blink.git@master
Patch Set: Created 6 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 side-by-side diff with in-line comments
Download patch
« no previous file with comments | « Source/modules/webaudio/PannerNode.h ('k') | no next file » | no next file with comments »
Expand Comments ('e') | Collapse Comments ('c') | Show Comments Hide Comments ('s')
Index: Source/modules/webaudio/PannerNode.cpp
diff --git a/Source/modules/webaudio/PannerNode.cpp b/Source/modules/webaudio/PannerNode.cpp
index 3f2dde89def0dc6e3f5920d71ebb7663e6bd3d12..624f4bc7a499c792b2bc7d1f036c8d429e7dbeeb 100644
--- a/Source/modules/webaudio/PannerNode.cpp
+++ b/Source/modules/webaudio/PannerNode.cpp
@@ -49,6 +49,7 @@ static void fixNANs(double &x)
PannerNode::PannerNode(AudioContext* context, float sampleRate)
: AudioNode(context, sampleRate)
, m_panningModel(Panner::PanningModelHRTF)
+ , m_distanceModel(DistanceEffect::ModelInverse)
, m_position(0, 0, 0)
, m_orientation(1, 0, 0)
, m_velocity(0, 0, 0)
@@ -58,7 +59,7 @@ PannerNode::PannerNode(AudioContext* context, float sampleRate)
, m_lastGain(-1.0)
, m_cachedAzimuth(0)
, m_cachedElevation(0)
- , m_cachedDistanceConeGain(0)
+ , m_cachedDistanceConeGain(1.0f)
, m_cachedDopplerRate(1)
, m_connectionCount(0)
{
@@ -123,19 +124,19 @@ void PannerNode::process(size_t framesToProcess)
return;
}
- // HRTFDatabase should be loaded before proceeding for offline audio context when panningModel() is "HRTF".
- if (panningModel() == "HRTF" && !m_hrtfDatabaseLoader->isLoaded()) {
- if (context()->isOfflineContext()) {
- m_hrtfDatabaseLoader->waitForLoaderThreadCompletion();
- } else {
- destination->zero();
- return;
- }
- }
-
// The audio thread can't block on this lock, so we call tryLock() instead.
- MutexTryLocker tryLocker(m_pannerLock);
+ MutexTryLocker tryLocker(m_processLock);
if (tryLocker.locked()) {
+ // HRTFDatabase should be loaded before proceeding for offline audio context when panningModel() is "HRTF".
+ if (panningModel() == "HRTF" && !m_hrtfDatabaseLoader->isLoaded()) {
+ if (context()->isOfflineContext()) {
+ m_hrtfDatabaseLoader->waitForLoaderThreadCompletion();
+ } else {
+ destination->zero();
+ return;
+ }
+ }
+
// Apply the panning effect.
double azimuth;
double elevation;
@@ -158,7 +159,8 @@ void PannerNode::process(size_t framesToProcess)
// Now update the cached source location in case the source has changed.
updateCachedSourceLocationInfo();
} else {
- // Too bad - The tryLock() failed. We must be in the middle of changing the panner.
+ // Too bad - The tryLock() failed.
+ // We must be in the middle of changing the panning model, the distance model, or the source's location information.
destination->zero();
}
}
@@ -217,7 +219,7 @@ bool PannerNode::setPanningModel(unsigned model)
case HRTF:
if (!m_panner.get() || model != m_panningModel) {
// This synchronizes with process().
- MutexLocker processLocker(m_pannerLock);
+ MutexLocker processLocker(m_processLock);
OwnPtr<Panner> newPanner = Panner::create(model, sampleRate(), m_hrtfDatabaseLoader.get());
m_panner = newPanner.release();
@@ -233,37 +235,40 @@ bool PannerNode::setPanningModel(unsigned model)
void PannerNode::setPosition(float x, float y, float z)
{
- // FIXME : consider thread safety about m_position in audio thread.
- // See http://crbugs.com/350583.
FloatPoint3D position = FloatPoint3D(x, y, z);
if (m_position == position)
return;
+ // This synchronizes with process().
+ MutexLocker processLocker(m_processLock);
+
m_position = position;
}
void PannerNode::setOrientation(float x, float y, float z)
{
- // FIXME : consider thread safety about m_orientation in audio thread.
- // See http://crbugs.com/350583.
FloatPoint3D orientation = FloatPoint3D(x, y, z);
if (m_orientation == orientation)
return;
+ // This synchronizes with process().
+ MutexLocker processLocker(m_processLock);
+
m_orientation = orientation;
}
void PannerNode::setVelocity(float x, float y, float z)
{
- // FIXME : consider thread safety about m_velocity in audio thread.
- // See http://crbugs.com/350583.
FloatPoint3D velocity = FloatPoint3D(x, y, z);
if (m_velocity == velocity)
return;
+ // This synchronizes with process().
+ MutexLocker processLocker(m_processLock);
+
m_velocity = velocity;
}
@@ -300,7 +305,12 @@ bool PannerNode::setDistanceModel(unsigned model)
case DistanceEffect::ModelLinear:
case DistanceEffect::ModelInverse:
case DistanceEffect::ModelExponential:
- m_distanceEffect.setModel(static_cast<DistanceEffect::ModelType>(model), true);
+ if (model != m_distanceModel) {
+ // This synchronizes with process().
+ MutexLocker processLocker(m_processLock);
+ m_distanceEffect.setModel(static_cast<DistanceEffect::ModelType>(model), true);
+ m_distanceModel = model;
+ }
break;
default:
return false;
@@ -371,7 +381,6 @@ void PannerNode::calculateAzimuthElevation(double* outAzimuth, double* outElevat
*outElevation = elevation;
}
-
double PannerNode::calculateDopplerRate()
{
double dopplerShift = 1.0;
@@ -436,6 +445,8 @@ float PannerNode::calculateDistanceConeGain()
void PannerNode::azimuthElevation(double* outAzimuth, double* outElevation)
{
+ ASSERT(context()->isAudioThread());
+
if (isAzimuthElevationDirty())
calculateAzimuthElevation(&m_cachedAzimuth, &m_cachedElevation);
@@ -445,6 +456,8 @@ void PannerNode::azimuthElevation(double* outAzimuth, double* outElevation)
double PannerNode::dopplerRate()
{
+ ASSERT(context()->isAudioThread());
+
if (isDopplerRateDirty())
m_cachedDopplerRate = calculateDopplerRate();
@@ -453,6 +466,8 @@ double PannerNode::dopplerRate()
float PannerNode::distanceConeGain()
{
+ ASSERT(context()->isAudioThread());
+
if (isDistanceConeGainDirty())
m_cachedDistanceConeGain = calculateDistanceConeGain();
@@ -534,6 +549,8 @@ void PannerNode::notifyAudioSourcesConnectedToNode(AudioNode* node, HashMap<Audi
void PannerNode::updateCachedListener()
{
+ ASSERT(context()->isAudioThread());
+
m_cachedListener->setPosition(listener()->position());
m_cachedListener->setOrientation(listener()->orientation());
m_cachedListener->setUpVector(listener()->upVector());
@@ -544,6 +561,8 @@ void PannerNode::updateCachedListener()
void PannerNode::updateCachedSourceLocationInfo()
{
+ ASSERT(context()->isAudioThread());
+
m_cachedPosition = m_position;
m_cachedOrientation = m_orientation;
m_cachedVelocity = m_velocity;
« no previous file with comments | « Source/modules/webaudio/PannerNode.h ('k') | no next file » | no next file with comments »

Powered by Google App Engine
This is Rietveld 408576698