<!DOCTYPE html PUBLIC "-//W3C//DTD XHTML 1.1//EN"
"http://www.w3.org/TR/xhtml11/DTD/xhtml11.dtd">
<html xmlns="http://www.w3.org/1999/xhtml">
<head><meta http-equiv="content-type" content="text/html; charset=utf-8" />
<title>[213072] trunk/Source/WebCore</title>
</head>
<body>
<style type="text/css"><!--
#msg dl.meta { border: 1px #006 solid; background: #369; padding: 6px; color: #fff; }
#msg dl.meta dt { float: left; width: 6em; font-weight: bold; }
#msg dt:after { content:':';}
#msg dl, #msg dt, #msg ul, #msg li, #header, #footer, #logmsg { font-family: verdana,arial,helvetica,sans-serif; font-size: 10pt; }
#msg dl a { font-weight: bold}
#msg dl a:link { color:#fc3; }
#msg dl a:active { color:#ff0; }
#msg dl a:visited { color:#cc6; }
h3 { font-family: verdana,arial,helvetica,sans-serif; font-size: 10pt; font-weight: bold; }
#msg pre { overflow: auto; background: #ffc; border: 1px #fa0 solid; padding: 6px; }
#logmsg { background: #ffc; border: 1px #fa0 solid; padding: 1em 1em 0 1em; }
#logmsg p, #logmsg pre, #logmsg blockquote { margin: 0 0 1em 0; }
#logmsg p, #logmsg li, #logmsg dt, #logmsg dd { line-height: 14pt; }
#logmsg h1, #logmsg h2, #logmsg h3, #logmsg h4, #logmsg h5, #logmsg h6 { margin: .5em 0; }
#logmsg h1:first-child, #logmsg h2:first-child, #logmsg h3:first-child, #logmsg h4:first-child, #logmsg h5:first-child, #logmsg h6:first-child { margin-top: 0; }
#logmsg ul, #logmsg ol { padding: 0; list-style-position: inside; margin: 0 0 0 1em; }
#logmsg ul { text-indent: -1em; padding-left: 1em; }#logmsg ol { text-indent: -1.5em; padding-left: 1.5em; }
#logmsg > ul, #logmsg > ol { margin: 0 0 1em 0; }
#logmsg pre { background: #eee; padding: 1em; }
#logmsg blockquote { border: 1px solid #fa0; border-left-width: 10px; padding: 1em 1em 0 1em; background: white;}
#logmsg dl { margin: 0; }
#logmsg dt { font-weight: bold; }
#logmsg dd { margin: 0; padding: 0 0 0.5em 0; }
#logmsg dd:before { content:'\00bb';}
#logmsg table { border-spacing: 0px; border-collapse: collapse; border-top: 4px solid #fa0; border-bottom: 1px solid #fa0; background: #fff; }
#logmsg table th { text-align: left; font-weight: normal; padding: 0.2em 0.5em; border-top: 1px dotted #fa0; }
#logmsg table td { text-align: right; border-top: 1px dotted #fa0; padding: 0.2em 0.5em; }
#logmsg table thead th { text-align: center; border-bottom: 1px solid #fa0; }
#logmsg table th.Corner { text-align: left; }
#logmsg hr { border: none 0; border-top: 2px dashed #fa0; height: 1px; }
#header, #footer { color: #fff; background: #636; border: 1px #300 solid; padding: 6px; }
#patch { width: 100%; }
#patch h4 {font-family: verdana,arial,helvetica,sans-serif;font-size:10pt;padding:8px;background:#369;color:#fff;margin:0;}
#patch .propset h4, #patch .binary h4 {margin:0;}
#patch pre {padding:0;line-height:1.2em;margin:0;}
#patch .diff {width:100%;background:#eee;padding: 0 0 10px 0;overflow:auto;}
#patch .propset .diff, #patch .binary .diff {padding:10px 0;}
#patch span {display:block;padding:0 10px;}
#patch .modfile, #patch .addfile, #patch .delfile, #patch .propset, #patch .binary, #patch .copfile {border:1px solid #ccc;margin:10px 0;}
#patch ins {background:#dfd;text-decoration:none;display:block;padding:0 10px;}
#patch del {background:#fdd;text-decoration:none;display:block;padding:0 10px;}
#patch .lines, .info {color:#888;background:#fff;}
--></style>
<div id="msg">
<dl class="meta">
<dt>Revision</dt> <dd><a href="http://trac.webkit.org/projects/webkit/changeset/213072">213072</a></dd>
<dt>Author</dt> <dd>jer.noble@apple.com</dd>
<dt>Date</dt> <dd>2017-02-27 09:04:53 -0800 (Mon, 27 Feb 2017)</dd>
</dl>
<h3>Log Message</h3>
<pre>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:</pre>
<h3>Modified Paths</h3>
<ul>
<li><a href="#trunkSourceWebCoreChangeLog">trunk/Source/WebCore/ChangeLog</a></li>
<li><a href="#trunkSourceWebCoreplatformaudiomacAudioSampleDataSourceh">trunk/Source/WebCore/platform/audio/mac/AudioSampleDataSource.h</a></li>
<li><a href="#trunkSourceWebCoreplatformaudiomacAudioSampleDataSourcemm">trunk/Source/WebCore/platform/audio/mac/AudioSampleDataSource.mm</a></li>
</ul>
</div>
<div id="patch">
<h3>Diff</h3>
<a id="trunkSourceWebCoreChangeLog"></a>
<div class="modfile"><h4>Modified: trunk/Source/WebCore/ChangeLog (213071 => 213072)</h4>
<pre class="diff"><span>
<span class="info">--- 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)
</span><span class="lines">@@ -1,5 +1,26 @@
</span><span class="cx"> 2017-02-21 Jer Noble <jer.noble@apple.com>
</span><span class="cx">
</span><ins>+ 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.noble@apple.com>
+
</ins><span class="cx"> AudioTrackPrivateMediaStreamCocoa should not exclusively lock its read and write threads.
</span><span class="cx"> https://bugs.webkit.org/show_bug.cgi?id=168643
</span><span class="cx">
</span></span></pre></div>
<a id="trunkSourceWebCoreplatformaudiomacAudioSampleDataSourceh"></a>
<div class="modfile"><h4>Modified: trunk/Source/WebCore/platform/audio/mac/AudioSampleDataSource.h (213071 => 213072)</h4>
<pre class="diff"><span>
<span class="info">--- 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)
</span><span class="lines">@@ -29,7 +29,6 @@
</span><span class="cx">
</span><span class="cx"> #include "AudioSampleBufferList.h"
</span><span class="cx"> #include <CoreAudio/CoreAudioTypes.h>
</span><del>-#include <wtf/Lock.h>
</del><span class="cx"> #include <wtf/MediaTime.h>
</span><span class="cx"> #include <wtf/RefCounted.h>
</span><span class="cx"> #include <wtf/RefPtr.h>
</span><span class="lines">@@ -94,7 +93,6 @@
</span><span class="cx"> std::unique_ptr<CARingBuffer> m_ringBuffer;
</span><span class="cx"> size_t m_maximumSampleCount { 0 };
</span><span class="cx">
</span><del>- Lock m_lock;
</del><span class="cx"> float m_volume { 1.0 };
</span><span class="cx"> bool m_muted { false };
</span><span class="cx"> bool m_paused { true };
</span></span></pre></div>
<a id="trunkSourceWebCoreplatformaudiomacAudioSampleDataSourcemm"></a>
<div class="modfile"><h4>Modified: trunk/Source/WebCore/platform/audio/mac/AudioSampleDataSource.mm (213071 => 213072)</h4>
<pre class="diff"><span>
<span class="info">--- 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)
</span><span class="lines">@@ -70,8 +70,6 @@
</span><span class="cx">
</span><span class="cx"> void AudioSampleDataSource::setPaused(bool paused)
</span><span class="cx"> {
</span><del>- std::lock_guard<Lock> lock(m_lock);
-
</del><span class="cx"> if (paused == m_paused)
</span><span class="cx"> return;
</span><span class="cx">
</span><span class="lines">@@ -143,8 +141,6 @@
</span><span class="cx">
</span><span class="cx"> void AudioSampleDataSource::pushSamplesInternal(const AudioBufferList& bufferList, const MediaTime& presentationTime, size_t sampleCount)
</span><span class="cx"> {
</span><del>- ASSERT(m_lock.isHeld());
-
</del><span class="cx"> const AudioBufferList* sampleBufferList;
</span><span class="cx"> if (m_converter) {
</span><span class="cx"> m_scratchBuffer->reset();
</span><span class="lines">@@ -190,8 +186,6 @@
</span><span class="cx">
</span><span class="cx"> void AudioSampleDataSource::pushSamples(const AudioStreamBasicDescription& sampleDescription, CMSampleBufferRef sampleBuffer)
</span><span class="cx"> {
</span><del>- std::lock_guard<Lock> lock(m_lock);
-
</del><span class="cx"> ASSERT_UNUSED(sampleDescription, *m_inputDescription == sampleDescription);
</span><span class="cx"> ASSERT(m_ringBuffer);
</span><span class="cx">
</span><span class="lines">@@ -201,7 +195,6 @@
</span><span class="cx">
</span><span class="cx"> void AudioSampleDataSource::pushSamples(const MediaTime& sampleTime, const PlatformAudioData& audioData, size_t sampleCount)
</span><span class="cx"> {
</span><del>- std::unique_lock<Lock> lock(m_lock, std::try_to_lock);
</del><span class="cx"> ASSERT(is<WebAudioBufferList>(audioData));
</span><span class="cx"> pushSamplesInternal(*downcast<WebAudioBufferList>(audioData).list(), sampleTime, sampleCount);
</span><span class="cx"> }
</span><span class="lines">@@ -208,7 +201,6 @@
</span><span class="cx">
</span><span class="cx"> bool AudioSampleDataSource::pullSamplesInternal(AudioBufferList& buffer, size_t& sampleCount, uint64_t timeStamp, double /*hostTime*/, PullMode mode)
</span><span class="cx"> {
</span><del>- ASSERT(m_lock.isHeld());
</del><span class="cx"> size_t byteCount = sampleCount * m_outputDescription->bytesPerFrame();
</span><span class="cx">
</span><span class="cx"> ASSERT(buffer.mNumberBuffers == m_ringBuffer->channelCount());
</span><span class="lines">@@ -298,8 +290,7 @@
</span><span class="cx">
</span><span class="cx"> bool AudioSampleDataSource::pullAvalaibleSamplesAsChunks(AudioBufferList& buffer, size_t sampleCountPerChunk, uint64_t timeStamp, Function<void()>&& consumeFilledBuffer)
</span><span class="cx"> {
</span><del>- std::unique_lock<Lock> lock(m_lock, std::try_to_lock);
- if (!lock.owns_lock() || !m_ringBuffer)
</del><ins>+ if (!m_ringBuffer)
</ins><span class="cx"> return false;
</span><span class="cx">
</span><span class="cx"> ASSERT(buffer.mNumberBuffers == m_ringBuffer->channelCount());
</span><span class="lines">@@ -324,8 +315,7 @@
</span><span class="cx">
</span><span class="cx"> bool AudioSampleDataSource::pullSamples(AudioBufferList& buffer, size_t sampleCount, uint64_t timeStamp, double hostTime, PullMode mode)
</span><span class="cx"> {
</span><del>- std::unique_lock<Lock> lock(m_lock, std::try_to_lock);
- if (!lock.owns_lock() || !m_ringBuffer) {
</del><ins>+ if (!m_ringBuffer) {
</ins><span class="cx"> size_t byteCount = sampleCount * m_outputDescription->bytesPerFrame();
</span><span class="cx"> AudioSampleBufferList::zeroABL(buffer, byteCount);
</span><span class="cx"> return false;
</span><span class="lines">@@ -336,8 +326,7 @@
</span><span class="cx">
</span><span class="cx"> bool AudioSampleDataSource::pullSamples(AudioSampleBufferList& buffer, size_t sampleCount, uint64_t timeStamp, double hostTime, PullMode mode)
</span><span class="cx"> {
</span><del>- std::unique_lock<Lock> lock(m_lock, std::try_to_lock);
- if (!lock.owns_lock() || !m_ringBuffer) {
</del><ins>+ if (!m_ringBuffer) {
</ins><span class="cx"> buffer.zero();
</span><span class="cx"> return false;
</span><span class="cx"> }
</span></span></pre>
</div>
</div>
</body>
</html>