Title: [213072] trunk/Source/WebCore
Revision
213072
Author
jer.no...@apple.com
Date
2017-02-27 09:04:53 -0800 (Mon, 27 Feb 2017)

Log Message

AudioSampleDataSource should not exclusively lock its read and write threads.
https://bugs.webkit.org/show_bug.cgi?id=168646

Reviewed by Eric Carlson.

Locking the write thread causes the read thread to drop audio samples and generates audible
glitches, and the realtime audio thread backing the read thread should never block. There's
no real reason to lock these threads against one another here; they both rely on the
CARingBuffer to safely and simultaneously read and write data.

* platform/audio/mac/AudioSampleDataSource.cpp:
(WebCore::AudioSampleDataSource::setPaused):
(WebCore::AudioSampleDataSource::pushSamplesInternal):
(WebCore::AudioSampleDataSource::pushSamples):
(WebCore::AudioSampleDataSource::pullSamplesInternal):
(WebCore::AudioSampleDataSource::pullAvalaibleSamplesAsChunks):
(WebCore::AudioSampleDataSource::pullSamples):
* platform/audio/mac/AudioSampleDataSource.h:

Modified Paths

Diff

Modified: trunk/Source/WebCore/ChangeLog (213071 => 213072)


--- trunk/Source/WebCore/ChangeLog	2017-02-27 17:03:29 UTC (rev 213071)
+++ trunk/Source/WebCore/ChangeLog	2017-02-27 17:04:53 UTC (rev 213072)
@@ -1,5 +1,26 @@
 2017-02-21  Jer Noble  <jer.no...@apple.com>
 
+        AudioSampleDataSource should not exclusively lock its read and write threads.
+        https://bugs.webkit.org/show_bug.cgi?id=168646
+
+        Reviewed by Eric Carlson.
+
+        Locking the write thread causes the read thread to drop audio samples and generates audible
+        glitches, and the realtime audio thread backing the read thread should never block. There's
+        no real reason to lock these threads against one another here; they both rely on the
+        CARingBuffer to safely and simultaneously read and write data.
+
+        * platform/audio/mac/AudioSampleDataSource.cpp:
+        (WebCore::AudioSampleDataSource::setPaused):
+        (WebCore::AudioSampleDataSource::pushSamplesInternal):
+        (WebCore::AudioSampleDataSource::pushSamples):
+        (WebCore::AudioSampleDataSource::pullSamplesInternal):
+        (WebCore::AudioSampleDataSource::pullAvalaibleSamplesAsChunks):
+        (WebCore::AudioSampleDataSource::pullSamples):
+        * platform/audio/mac/AudioSampleDataSource.h:
+
+2017-02-21  Jer Noble  <jer.no...@apple.com>
+
         AudioTrackPrivateMediaStreamCocoa should not exclusively lock its read and write threads.
         https://bugs.webkit.org/show_bug.cgi?id=168643
 

Modified: trunk/Source/WebCore/platform/audio/mac/AudioSampleDataSource.h (213071 => 213072)


--- trunk/Source/WebCore/platform/audio/mac/AudioSampleDataSource.h	2017-02-27 17:03:29 UTC (rev 213071)
+++ trunk/Source/WebCore/platform/audio/mac/AudioSampleDataSource.h	2017-02-27 17:04:53 UTC (rev 213072)
@@ -29,7 +29,6 @@
 
 #include "AudioSampleBufferList.h"
 #include <CoreAudio/CoreAudioTypes.h>
-#include <wtf/Lock.h>
 #include <wtf/MediaTime.h>
 #include <wtf/RefCounted.h>
 #include <wtf/RefPtr.h>
@@ -94,7 +93,6 @@
     std::unique_ptr<CARingBuffer> m_ringBuffer;
     size_t m_maximumSampleCount { 0 };
 
-    Lock m_lock;
     float m_volume { 1.0 };
     bool m_muted { false };
     bool m_paused { true };

Modified: trunk/Source/WebCore/platform/audio/mac/AudioSampleDataSource.mm (213071 => 213072)


--- trunk/Source/WebCore/platform/audio/mac/AudioSampleDataSource.mm	2017-02-27 17:03:29 UTC (rev 213071)
+++ trunk/Source/WebCore/platform/audio/mac/AudioSampleDataSource.mm	2017-02-27 17:04:53 UTC (rev 213072)
@@ -70,8 +70,6 @@
 
 void AudioSampleDataSource::setPaused(bool paused)
 {
-    std::lock_guard<Lock> lock(m_lock);
-
     if (paused == m_paused)
         return;
 
@@ -143,8 +141,6 @@
 
 void AudioSampleDataSource::pushSamplesInternal(const AudioBufferList& bufferList, const MediaTime& presentationTime, size_t sampleCount)
 {
-    ASSERT(m_lock.isHeld());
-
     const AudioBufferList* sampleBufferList;
     if (m_converter) {
         m_scratchBuffer->reset();
@@ -190,8 +186,6 @@
 
 void AudioSampleDataSource::pushSamples(const AudioStreamBasicDescription& sampleDescription, CMSampleBufferRef sampleBuffer)
 {
-    std::lock_guard<Lock> lock(m_lock);
-
     ASSERT_UNUSED(sampleDescription, *m_inputDescription == sampleDescription);
     ASSERT(m_ringBuffer);
     
@@ -201,7 +195,6 @@
 
 void AudioSampleDataSource::pushSamples(const MediaTime& sampleTime, const PlatformAudioData& audioData, size_t sampleCount)
 {
-    std::unique_lock<Lock> lock(m_lock, std::try_to_lock);
     ASSERT(is<WebAudioBufferList>(audioData));
     pushSamplesInternal(*downcast<WebAudioBufferList>(audioData).list(), sampleTime, sampleCount);
 }
@@ -208,7 +201,6 @@
 
 bool AudioSampleDataSource::pullSamplesInternal(AudioBufferList& buffer, size_t& sampleCount, uint64_t timeStamp, double /*hostTime*/, PullMode mode)
 {
-    ASSERT(m_lock.isHeld());
     size_t byteCount = sampleCount * m_outputDescription->bytesPerFrame();
 
     ASSERT(buffer.mNumberBuffers == m_ringBuffer->channelCount());
@@ -298,8 +290,7 @@
 
 bool AudioSampleDataSource::pullAvalaibleSamplesAsChunks(AudioBufferList& buffer, size_t sampleCountPerChunk, uint64_t timeStamp, Function<void()>&& consumeFilledBuffer)
 {
-    std::unique_lock<Lock> lock(m_lock, std::try_to_lock);
-    if (!lock.owns_lock() || !m_ringBuffer)
+    if (!m_ringBuffer)
         return false;
 
     ASSERT(buffer.mNumberBuffers == m_ringBuffer->channelCount());
@@ -324,8 +315,7 @@
 
 bool AudioSampleDataSource::pullSamples(AudioBufferList& buffer, size_t sampleCount, uint64_t timeStamp, double hostTime, PullMode mode)
 {
-    std::unique_lock<Lock> lock(m_lock, std::try_to_lock);
-    if (!lock.owns_lock() || !m_ringBuffer) {
+    if (!m_ringBuffer) {
         size_t byteCount = sampleCount * m_outputDescription->bytesPerFrame();
         AudioSampleBufferList::zeroABL(buffer, byteCount);
         return false;
@@ -336,8 +326,7 @@
 
 bool AudioSampleDataSource::pullSamples(AudioSampleBufferList& buffer, size_t sampleCount, uint64_t timeStamp, double hostTime, PullMode mode)
 {
-    std::unique_lock<Lock> lock(m_lock, std::try_to_lock);
-    if (!lock.owns_lock() || !m_ringBuffer) {
+    if (!m_ringBuffer) {
         buffer.zero();
         return false;
     }
_______________________________________________
webkit-changes mailing list
webkit-changes@lists.webkit.org
https://lists.webkit.org/mailman/listinfo/webkit-changes

Reply via email to