- Revision
- 261063
- Author
- [email protected]
- Date
- 2020-05-03 11:16:31 -0700 (Sun, 03 May 2020)
Log Message
AudioMediaStreamTrackRendererCocoa should create/start/stop its remote unit on the main thread
https://bugs.webkit.org/show_bug.cgi?id=211287
Reviewed by Eric Carlson.
Creating/starting/stopping audio units in different threads is error prone.
Now that we have an observer model where we have observers for when to play in the main thread and
based on that, we decide to receive audio samples in a background thread, we can simplify the logic of AudioMediaStreamTrackRendererCocoa.
We do this by creating/starting the unit in AudioMediaStreamTrackRendererCocoa::start.
At that point, AudioMediaStreamTrackRendererCocoa is not expected to receive any sample.
Just after starting, AudioTrackPrivateMediaStream will receive audio samples and forward them to AudioMediaStreamTrackRendererCocoa.
AudioMediaStreamTrackRendererCocoa will then create in a background thread the AudioSampleDataSource that is responsible to adapt the received audio samples to the unit.
Manually tested.
* platform/audio/mac/AudioSampleDataSource.h:
(WebCore::AudioSampleDataSource::inputDescription const):
* platform/audio/mac/CAAudioStreamDescription.h:
* platform/mediastream/AudioTrackPrivateMediaStream.cpp:
(WebCore::AudioTrackPrivateMediaStream::startRenderer):
Ensure to start the unit and then start gettting audio samples.
* platform/mediastream/mac/AudioMediaStreamTrackRendererCocoa.cpp:
(WebCore::AudioMediaStreamTrackRendererCocoa::start):
(WebCore::AudioMediaStreamTrackRendererCocoa::stop):
(WebCore::AudioMediaStreamTrackRendererCocoa::clear):
(WebCore::AudioMediaStreamTrackRendererCocoa::pushSamples):
(WebCore::AudioMediaStreamTrackRendererCocoa::render):
* platform/mediastream/mac/AudioMediaStreamTrackRendererCocoa.h:
Modified Paths
Diff
Modified: trunk/Source/WebCore/ChangeLog (261062 => 261063)
--- trunk/Source/WebCore/ChangeLog 2020-05-03 15:59:09 UTC (rev 261062)
+++ trunk/Source/WebCore/ChangeLog 2020-05-03 18:16:31 UTC (rev 261063)
@@ -1,3 +1,34 @@
+2020-05-03 Youenn Fablet <[email protected]>
+
+ AudioMediaStreamTrackRendererCocoa should create/start/stop its remote unit on the main thread
+ https://bugs.webkit.org/show_bug.cgi?id=211287
+
+ Reviewed by Eric Carlson.
+
+ Creating/starting/stopping audio units in different threads is error prone.
+ Now that we have an observer model where we have observers for when to play in the main thread and
+ based on that, we decide to receive audio samples in a background thread, we can simplify the logic of AudioMediaStreamTrackRendererCocoa.
+ We do this by creating/starting the unit in AudioMediaStreamTrackRendererCocoa::start.
+ At that point, AudioMediaStreamTrackRendererCocoa is not expected to receive any sample.
+ Just after starting, AudioTrackPrivateMediaStream will receive audio samples and forward them to AudioMediaStreamTrackRendererCocoa.
+ AudioMediaStreamTrackRendererCocoa will then create in a background thread the AudioSampleDataSource that is responsible to adapt the received audio samples to the unit.
+
+ Manually tested.
+
+ * platform/audio/mac/AudioSampleDataSource.h:
+ (WebCore::AudioSampleDataSource::inputDescription const):
+ * platform/audio/mac/CAAudioStreamDescription.h:
+ * platform/mediastream/AudioTrackPrivateMediaStream.cpp:
+ (WebCore::AudioTrackPrivateMediaStream::startRenderer):
+ Ensure to start the unit and then start gettting audio samples.
+ * platform/mediastream/mac/AudioMediaStreamTrackRendererCocoa.cpp:
+ (WebCore::AudioMediaStreamTrackRendererCocoa::start):
+ (WebCore::AudioMediaStreamTrackRendererCocoa::stop):
+ (WebCore::AudioMediaStreamTrackRendererCocoa::clear):
+ (WebCore::AudioMediaStreamTrackRendererCocoa::pushSamples):
+ (WebCore::AudioMediaStreamTrackRendererCocoa::render):
+ * platform/mediastream/mac/AudioMediaStreamTrackRendererCocoa.h:
+
2020-05-03 Rob Buis <[email protected]>
atob() should not accept a vertical tab
Modified: trunk/Source/WebCore/platform/audio/mac/AudioSampleDataSource.h (261062 => 261063)
--- trunk/Source/WebCore/platform/audio/mac/AudioSampleDataSource.h 2020-05-03 15:59:09 UTC (rev 261062)
+++ trunk/Source/WebCore/platform/audio/mac/AudioSampleDataSource.h 2020-05-03 18:16:31 UTC (rev 261063)
@@ -73,6 +73,8 @@
void setMuted(bool muted) { m_muted = muted; }
bool muted() const { return m_muted; }
+ const CAAudioStreamDescription* inputDescription() const { return m_inputDescription.get(); }
+
#if !RELEASE_LOG_DISABLED
const Logger& logger() const final { return m_logger; }
const void* logIdentifier() const final { return m_logIdentifier; }
Modified: trunk/Source/WebCore/platform/audio/mac/CAAudioStreamDescription.h (261062 => 261063)
--- trunk/Source/WebCore/platform/audio/mac/CAAudioStreamDescription.h 2020-05-03 15:59:09 UTC (rev 261062)
+++ trunk/Source/WebCore/platform/audio/mac/CAAudioStreamDescription.h 2020-05-03 18:16:31 UTC (rev 261063)
@@ -64,9 +64,9 @@
uint32_t bytesPerPacket() const { return m_streamDescription.mBytesPerPacket; }
uint32_t formatFlags() const { return m_streamDescription.mFormatFlags; }
- bool operator==(const AudioStreamBasicDescription& other) { return m_streamDescription == other; }
- bool operator!=(const AudioStreamBasicDescription& other) { return !operator == (other); }
- bool operator==(const AudioStreamDescription& other)
+ bool operator==(const AudioStreamBasicDescription& other) const { return m_streamDescription == other; }
+ bool operator!=(const AudioStreamBasicDescription& other) const { return !operator == (other); }
+ bool operator==(const AudioStreamDescription& other) const
{
if (other.platformDescription().type != PlatformDescription::CAAudioStreamBasicType)
return false;
@@ -73,7 +73,7 @@
return operator==(*WTF::get<const AudioStreamBasicDescription*>(other.platformDescription().description));
}
- bool operator!=(const AudioStreamDescription& other) { return !operator == (other); }
+ bool operator!=(const AudioStreamDescription& other) const { return !operator == (other); }
const AudioStreamBasicDescription& streamDescription() const { return m_streamDescription; }
AudioStreamBasicDescription& streamDescription() { return m_streamDescription; }
Modified: trunk/Source/WebCore/platform/mediastream/AudioMediaStreamTrackRenderer.h (261062 => 261063)
--- trunk/Source/WebCore/platform/mediastream/AudioMediaStreamTrackRenderer.h 2020-05-03 15:59:09 UTC (rev 261062)
+++ trunk/Source/WebCore/platform/mediastream/AudioMediaStreamTrackRenderer.h 2020-05-03 18:16:31 UTC (rev 261063)
@@ -47,7 +47,7 @@
virtual void start() = 0;
virtual void stop() = 0;
virtual void clear() = 0;
- // May be called on a background thread.
+ // May be called on a background thread. It should only be called after start/before stop is called.
virtual void pushSamples(const WTF::MediaTime&, const PlatformAudioData&, const AudioStreamDescription&, size_t) = 0;
virtual void setMuted(bool);
Modified: trunk/Source/WebCore/platform/mediastream/AudioTrackPrivateMediaStream.cpp (261062 => 261063)
--- trunk/Source/WebCore/platform/mediastream/AudioTrackPrivateMediaStream.cpp 2020-05-03 15:59:09 UTC (rev 261062)
+++ trunk/Source/WebCore/platform/mediastream/AudioTrackPrivateMediaStream.cpp 2020-05-03 18:16:31 UTC (rev 261063)
@@ -139,8 +139,8 @@
return;
m_isPlaying = true;
+ m_renderer->start();
m_audioSource->addAudioSampleObserver(*this);
- m_renderer->start();
}
void AudioTrackPrivateMediaStream::stopRenderer()
Modified: trunk/Source/WebCore/platform/mediastream/mac/AudioMediaStreamTrackRendererCocoa.cpp (261062 => 261063)
--- trunk/Source/WebCore/platform/mediastream/mac/AudioMediaStreamTrackRendererCocoa.cpp 2020-05-03 15:59:09 UTC (rev 261062)
+++ trunk/Source/WebCore/platform/mediastream/mac/AudioMediaStreamTrackRendererCocoa.cpp 2020-05-03 18:16:31 UTC (rev 261063)
@@ -47,37 +47,36 @@
{
clear();
- m_shouldPlay = true;
+ CAAudioStreamDescription outputDescription;
+ auto remoteIOUnit = createAudioUnit(outputDescription);
+ if (!remoteIOUnit)
+ return;
- if (m_dataSource)
- m_dataSource->setPaused(false);
+ if (auto error = AudioOutputUnitStart(remoteIOUnit)) {
+ ERROR_LOG(LOGIDENTIFIER, "AudioOutputUnitStart failed, error = ", error, " (", (const char*)&error, ")");
+ AudioComponentInstanceDispose(remoteIOUnit);
+ return;
+ }
+ m_outputDescription = makeUnique<CAAudioStreamDescription>(outputDescription);
+ m_remoteIOUnit = remoteIOUnit;
}
void AudioMediaStreamTrackRendererCocoa::stop()
{
- m_shouldPlay = false;
+ if (!m_remoteIOUnit)
+ return;
- if (m_dataSource)
- m_dataSource->setPaused(true);
+ AudioOutputUnitStop(m_remoteIOUnit);
+ AudioComponentInstanceDispose(m_remoteIOUnit);
+ m_remoteIOUnit = nullptr;
}
void AudioMediaStreamTrackRendererCocoa::clear()
{
- m_shouldPlay = false;
+ stop();
- if (m_dataSource)
- m_dataSource->setPaused(true);
-
- if (m_remoteIOUnit) {
- AudioOutputUnitStop(m_remoteIOUnit);
- AudioComponentInstanceDispose(m_remoteIOUnit);
- m_remoteIOUnit = nullptr;
- }
-
m_dataSource = nullptr;
- m_inputDescription = nullptr;
m_outputDescription = nullptr;
- m_isAudioUnitStarted = false;
}
AudioComponentInstance AudioMediaStreamTrackRendererCocoa::createAudioUnit(CAAudioStreamDescription& outputDescription)
@@ -148,78 +147,42 @@
void AudioMediaStreamTrackRendererCocoa::pushSamples(const MediaTime& sampleTime, const PlatformAudioData& audioData, const AudioStreamDescription& description, size_t sampleCount)
{
ASSERT(description.platformDescription().type == PlatformDescription::CAAudioStreamBasicType);
- if (!m_shouldPlay) {
- if (m_isAudioUnitStarted) {
- if (m_remoteIOUnit)
- AudioOutputUnitStop(m_remoteIOUnit);
- m_isAudioUnitStarted = false;
- }
+ if (!m_remoteIOUnit)
return;
- }
- if (!m_inputDescription || *m_inputDescription != description) {
- m_isAudioUnitStarted = false;
+ if (!m_dataSource || !m_dataSource->inputDescription() || *m_dataSource->inputDescription() != description) {
+ auto dataSource = AudioSampleDataSource::create(description.sampleRate() * 2, *this);
- if (m_remoteIOUnit) {
- AudioOutputUnitStop(m_remoteIOUnit);
- AudioComponentInstanceDispose(m_remoteIOUnit);
- m_remoteIOUnit = nullptr;
- }
-
- m_inputDescription = nullptr;
- m_outputDescription = nullptr;
-
- CAAudioStreamDescription inputDescription = toCAAudioStreamDescription(description);
- CAAudioStreamDescription outputDescription;
-
- auto remoteIOUnit = createAudioUnit(outputDescription);
- if (!remoteIOUnit)
+ if (dataSource->setInputFormat(toCAAudioStreamDescription(description))) {
+ ERROR_LOG(LOGIDENTIFIER, "Unable to set the input format of data source");
return;
-
- m_inputDescription = makeUnique<CAAudioStreamDescription>(inputDescription);
- m_outputDescription = makeUnique<CAAudioStreamDescription>(outputDescription);
-
- m_dataSource = AudioSampleDataSource::create(description.sampleRate() * 2, *this);
-
- if (m_dataSource->setInputFormat(inputDescription) || m_dataSource->setOutputFormat(outputDescription)) {
- AudioComponentInstanceDispose(remoteIOUnit);
- return;
}
- if (auto error = AudioOutputUnitStart(remoteIOUnit)) {
- ERROR_LOG(LOGIDENTIFIER, "AudioOutputUnitStart failed, error = ", error, " (", (const char*)&error, ")");
- AudioComponentInstanceDispose(remoteIOUnit);
- m_inputDescription = nullptr;
+ if (dataSource->setOutputFormat(*m_outputDescription)) {
+ ERROR_LOG(LOGIDENTIFIER, "Unable to set the output format of data source");
return;
}
- m_isAudioUnitStarted = true;
+ dataSource->setPaused(false);
+ dataSource->setVolume(volume());
- m_dataSource->setVolume(volume());
- m_remoteIOUnit = remoteIOUnit;
+ m_dataSource = WTFMove(dataSource);
}
m_dataSource->pushSamples(sampleTime, audioData, sampleCount);
-
- if (!m_isAudioUnitStarted) {
- if (auto error = AudioOutputUnitStart(m_remoteIOUnit)) {
- ERROR_LOG(LOGIDENTIFIER, "AudioOutputUnitStart failed, error = ", error, " (", (const char*)&error, ")");
- return;
- }
- m_isAudioUnitStarted = true;
- }
}
+// May get called on a background thread.
OSStatus AudioMediaStreamTrackRendererCocoa::render(UInt32 sampleCount, AudioBufferList& ioData, UInt32 /*inBusNumber*/, const AudioTimeStamp& timeStamp, AudioUnitRenderActionFlags& actionFlags)
{
- if (isMuted() || !m_shouldPlay || !m_dataSource) {
+ auto dataSource = m_dataSource;
+ if (!dataSource) {
AudioSampleBufferList::zeroABL(ioData, static_cast<size_t>(sampleCount * m_outputDescription->bytesPerFrame()));
actionFlags = kAudioUnitRenderAction_OutputIsSilence;
return 0;
}
- m_dataSource->pullSamples(ioData, static_cast<size_t>(sampleCount), timeStamp.mSampleTime, timeStamp.mHostTime, AudioSampleDataSource::Copy);
-
+ dataSource->pullSamples(ioData, static_cast<size_t>(sampleCount), timeStamp.mSampleTime, timeStamp.mHostTime, AudioSampleDataSource::Copy);
return 0;
}
Modified: trunk/Source/WebCore/platform/mediastream/mac/AudioMediaStreamTrackRendererCocoa.h (261062 => 261063)
--- trunk/Source/WebCore/platform/mediastream/mac/AudioMediaStreamTrackRendererCocoa.h 2020-05-03 15:59:09 UTC (rev 261062)
+++ trunk/Source/WebCore/platform/mediastream/mac/AudioMediaStreamTrackRendererCocoa.h 2020-05-03 18:16:31 UTC (rev 261063)
@@ -57,15 +57,11 @@
AudioComponentInstance createAudioUnit(CAAudioStreamDescription&);
- // Audio thread members
AudioComponentInstance m_remoteIOUnit { nullptr };
- std::unique_ptr<CAAudioStreamDescription> m_inputDescription;
std::unique_ptr<CAAudioStreamDescription> m_outputDescription;
- // Cross thread members
+ // Audio threads member
RefPtr<AudioSampleDataSource> m_dataSource;
- bool m_isAudioUnitStarted { false };
- bool m_shouldPlay { false };
};
}
