Title: [250290] trunk/Source/WebCore
- Revision
- 250290
- Author
- you...@apple.com
- Date
- 2019-09-24 01:42:03 -0700 (Tue, 24 Sep 2019)
Log Message
AudioTrackPrivateMediaStreamCocoa does not need to manipulate the audio unit in play/pause methods
https://bugs.webkit.org/show_bug.cgi?id=202097
<rdar://problem/51548144>
Reviewed by Eric Carlson.
Instead of manipulating the audio unit in play/pause methods, it is more convenient to do so in audioSamplesAvailable.
play/pause methods only update boolean values that audioSamplesAvailable will read.
In particular, m_autoPlay and m_isPlaying are no longer modified in the audio thread.
Behavior was racy so difficult to reproduce.
* platform/mediastream/mac/AudioTrackPrivateMediaStreamCocoa.cpp:
(WebCore::AudioTrackPrivateMediaStreamCocoa::playInternal):
(WebCore::AudioTrackPrivateMediaStreamCocoa::pause):
(WebCore::AudioTrackPrivateMediaStreamCocoa::audioSamplesAvailable):
* platform/mediastream/mac/AudioTrackPrivateMediaStreamCocoa.h:
Modified Paths
Diff
Modified: trunk/Source/WebCore/ChangeLog (250289 => 250290)
--- trunk/Source/WebCore/ChangeLog 2019-09-24 07:43:24 UTC (rev 250289)
+++ trunk/Source/WebCore/ChangeLog 2019-09-24 08:42:03 UTC (rev 250290)
@@ -1,3 +1,23 @@
+2019-09-24 Youenn Fablet <you...@apple.com>
+
+ AudioTrackPrivateMediaStreamCocoa does not need to manipulate the audio unit in play/pause methods
+ https://bugs.webkit.org/show_bug.cgi?id=202097
+ <rdar://problem/51548144>
+
+ Reviewed by Eric Carlson.
+
+ Instead of manipulating the audio unit in play/pause methods, it is more convenient to do so in audioSamplesAvailable.
+ play/pause methods only update boolean values that audioSamplesAvailable will read.
+ In particular, m_autoPlay and m_isPlaying are no longer modified in the audio thread.
+
+ Behavior was racy so difficult to reproduce.
+
+ * platform/mediastream/mac/AudioTrackPrivateMediaStreamCocoa.cpp:
+ (WebCore::AudioTrackPrivateMediaStreamCocoa::playInternal):
+ (WebCore::AudioTrackPrivateMediaStreamCocoa::pause):
+ (WebCore::AudioTrackPrivateMediaStreamCocoa::audioSamplesAvailable):
+ * platform/mediastream/mac/AudioTrackPrivateMediaStreamCocoa.h:
+
2019-09-23 Youenn Fablet <you...@apple.com>
Support sync-xhr feature policy
Modified: trunk/Source/WebCore/platform/mediastream/mac/AudioTrackPrivateMediaStreamCocoa.cpp (250289 => 250290)
--- trunk/Source/WebCore/platform/mediastream/mac/AudioTrackPrivateMediaStreamCocoa.cpp 2019-09-24 07:43:24 UTC (rev 250289)
+++ trunk/Source/WebCore/platform/mediastream/mac/AudioTrackPrivateMediaStreamCocoa.cpp 2019-09-24 08:42:03 UTC (rev 250290)
@@ -65,17 +65,16 @@
void AudioTrackPrivateMediaStreamCocoa::playInternal()
{
+ ASSERT(isMainThread());
+
if (m_isPlaying)
return;
- if (m_remoteIOUnit) {
- ASSERT(m_dataSource);
+ m_isPlaying = true;
+ m_autoPlay = false;
+
+ if (m_dataSource)
m_dataSource->setPaused(false);
- if (!AudioOutputUnitStart(m_remoteIOUnit))
- m_isPlaying = true;
- }
-
- m_autoPlay = !m_isPlaying;
}
void AudioTrackPrivateMediaStreamCocoa::play()
@@ -85,11 +84,14 @@
void AudioTrackPrivateMediaStreamCocoa::pause()
{
+ ASSERT(isMainThread());
+
+ if (!m_isPlaying)
+ return;
+
m_isPlaying = false;
m_autoPlay = false;
- if (m_remoteIOUnit)
- AudioOutputUnitStop(m_remoteIOUnit);
if (m_dataSource)
m_dataSource->setPaused(true);
}
@@ -170,7 +172,17 @@
{
ASSERT(description.platformDescription().type == PlatformDescription::CAAudioStreamBasicType);
+ if (!m_isPlaying) {
+ if (m_isAudioUnitStarted) {
+ if (m_remoteIOUnit)
+ AudioOutputUnitStop(m_remoteIOUnit);
+ m_isAudioUnitStarted = false;
+ }
+ return;
+ }
+
if (!m_inputDescription || *m_inputDescription != description) {
+ m_isAudioUnitStarted = false;
if (m_remoteIOUnit) {
AudioOutputUnitStop(m_remoteIOUnit);
@@ -198,12 +210,15 @@
return;
}
- if (m_isPlaying && AudioOutputUnitStart(remoteIOUnit)) {
+ if (auto error = AudioOutputUnitStart(remoteIOUnit)) {
+ ERROR_LOG(LOGIDENTIFIER, "AudioOutputUnitStart failed, error = ", error, " (", (const char*)&error, ")");
AudioComponentInstanceDispose(remoteIOUnit);
m_inputDescription = nullptr;
return;
}
+ m_isAudioUnitStarted = true;
+
m_dataSource->setVolume(m_volume);
m_remoteIOUnit = remoteIOUnit;
}
@@ -210,8 +225,21 @@
m_dataSource->pushSamples(sampleTime, audioData, sampleCount);
- if (m_autoPlay)
- playInternal();
+ if (m_autoPlay && !m_hasStartedAutoplay) {
+ m_hasStartedAutoplay = true;
+ callOnMainThread([this, protectedThis = makeRef(*this)] {
+ if (m_autoPlay)
+ playInternal();
+ });
+ }
+
+ if (!m_isAudioUnitStarted) {
+ if (auto error = AudioOutputUnitStart(m_remoteIOUnit)) {
+ ERROR_LOG(LOGIDENTIFIER, "AudioOutputUnitStart failed, error = ", error, " (", (const char*)&error, ")");
+ return;
+ }
+ m_isAudioUnitStarted = true;
+ }
}
void AudioTrackPrivateMediaStreamCocoa::sourceStopped()
Modified: trunk/Source/WebCore/platform/mediastream/mac/AudioTrackPrivateMediaStreamCocoa.h (250289 => 250290)
--- trunk/Source/WebCore/platform/mediastream/mac/AudioTrackPrivateMediaStreamCocoa.h 2019-09-24 07:43:24 UTC (rev 250289)
+++ trunk/Source/WebCore/platform/mediastream/mac/AudioTrackPrivateMediaStreamCocoa.h 2019-09-24 08:42:03 UTC (rev 250290)
@@ -75,13 +75,17 @@
const char* logClassName() const final { return "AudioTrackPrivateMediaStreamCocoa"; }
#endif
-
+ // Audio thread members
AudioComponentInstance m_remoteIOUnit { nullptr };
std::unique_ptr<CAAudioStreamDescription> m_inputDescription;
std::unique_ptr<CAAudioStreamDescription> m_outputDescription;
+ bool m_isAudioUnitStarted { false };
+ bool m_hasStartedAutoplay { false };
+ // Cross thread members
RefPtr<AudioSampleDataSource> m_dataSource;
+ // Main thread writable members
float m_volume { 1 };
bool m_isPlaying { false };
bool m_autoPlay { false };
_______________________________________________
webkit-changes mailing list
webkit-changes@lists.webkit.org
https://lists.webkit.org/mailman/listinfo/webkit-changes
