Modified: trunk/Source/WebCore/ChangeLog (198653 => 198654)
--- trunk/Source/WebCore/ChangeLog 2016-03-24 23:56:27 UTC (rev 198653)
+++ trunk/Source/WebCore/ChangeLog 2016-03-25 00:52:40 UTC (rev 198654)
@@ -1,3 +1,27 @@
+2016-03-24 Jer Noble <[email protected]>
+
+ Safari Crashes if audio.src is changed while connected to AudioAnalyserNode
+ https://bugs.webkit.org/show_bug.cgi?id=153593
+ <rdar://problem/23648082>
+
+ Reviewed by Eric Carlson.
+
+ m_ringBuffer is accessed on the high-priority WebAudio thread after it has been cleared (a
+ null-deref). Protect against unsafe access on multiple threads of a non-refcounted object by
+ a simple try_lock.
+
+ Additionally, limit the use of variables in use by both the separate WebAudio thread method
+ (provideInput()) and AVAudioMix thread method (process()) where possible, and convert to
+ std::atomic<> where ivars must be acessed by both threads. m_writeCount is entirely superfluous,
+ as it is a synonym for the endTime returned by m_ringBuffer->getCurrentFrameBounds().
+
+ * platform/graphics/avfoundation/AudioSourceProviderAVFObjC.h:
+ * platform/graphics/avfoundation/AudioSourceProviderAVFObjC.mm:
+ (WebCore::AudioSourceProviderAVFObjC::provideInput):
+ (WebCore::AudioSourceProviderAVFObjC::prepare):
+ (WebCore::AudioSourceProviderAVFObjC::unprepare):
+ (WebCore::AudioSourceProviderAVFObjC::process):
+
2016-03-24 Enrica Casucci <[email protected]>
Adopt new SPI from DataDetectorsCore to decide link behavior.
Modified: trunk/Source/WebCore/platform/graphics/avfoundation/AudioSourceProviderAVFObjC.h (198653 => 198654)
--- trunk/Source/WebCore/platform/graphics/avfoundation/AudioSourceProviderAVFObjC.h 2016-03-24 23:56:27 UTC (rev 198653)
+++ trunk/Source/WebCore/platform/graphics/avfoundation/AudioSourceProviderAVFObjC.h 2016-03-25 00:52:40 UTC (rev 198654)
@@ -29,6 +29,8 @@
#if ENABLE(WEB_AUDIO) && USE(MEDIATOOLBOX)
#include "AudioSourceProvider.h"
+#include <atomic>
+#include <wtf/Lock.h>
#include <wtf/MediaTime.h>
#include <wtf/RefCounted.h>
#include <wtf/RefPtr.h>
@@ -89,13 +91,15 @@
std::unique_ptr<AudioStreamBasicDescription> m_outputDescription;
std::unique_ptr<CARingBuffer> m_ringBuffer;
+ Lock m_mutex;
MediaTime m_startTimeAtLastProcess;
MediaTime m_endTimeAtLastProcess;
- uint64_t m_writeAheadCount;
- uint64_t m_writeCount;
- uint64_t m_readCount;
- bool m_paused;
- AudioSourceProviderClient* m_client;
+ std::atomic<uint64_t> m_writeAheadCount { 0 };
+ uint64_t m_readCount { 0 };
+ enum { NoSeek = std::numeric_limits<uint64_t>::max() };
+ std::atomic<uint64_t> m_seekTo { NoSeek };
+ bool m_paused { true };
+ AudioSourceProviderClient* m_client { nullptr };
};
}
Modified: trunk/Source/WebCore/platform/graphics/avfoundation/AudioSourceProviderAVFObjC.mm (198653 => 198654)
--- trunk/Source/WebCore/platform/graphics/avfoundation/AudioSourceProviderAVFObjC.mm 2016-03-24 23:56:27 UTC (rev 198653)
+++ trunk/Source/WebCore/platform/graphics/avfoundation/AudioSourceProviderAVFObjC.mm 2016-03-25 00:52:40 UTC (rev 198654)
@@ -38,6 +38,7 @@
#import <AVFoundation/AVAudioMix.h>
#import <AVFoundation/AVMediaFormat.h>
#import <AVFoundation/AVPlayerItem.h>
+#import <mutex>
#import <objc/runtime.h>
#import <wtf/MainThread.h>
@@ -78,10 +79,6 @@
AudioSourceProviderAVFObjC::AudioSourceProviderAVFObjC(AVPlayerItem *item)
: m_avPlayerItem(item)
- , m_writeCount(0)
- , m_readCount(0)
- , m_paused(true)
- , m_client(nullptr)
{
}
@@ -92,19 +89,30 @@
void AudioSourceProviderAVFObjC::provideInput(AudioBus* bus, size_t framesToProcess)
{
- if (!m_avPlayerItem)
+ // Protect access to m_ringBuffer by try_locking the mutex. If we failed
+ // to aquire, a re-configure is underway, and m_ringBuffer is unsafe to access.
+ // Emit silence.
+ std::unique_lock<Lock> lock(m_mutex, std::try_to_lock);
+ if (!lock.owns_lock() || !m_ringBuffer) {
+ bus->zero();
return;
+ }
uint64_t startFrame = 0;
uint64_t endFrame = 0;
+ uint64_t seekTo = m_seekTo.exchange(NoSeek);
+ uint64_t writeAheadCount = m_writeAheadCount.load();
+ if (seekTo != NoSeek)
+ m_readCount = seekTo;
+
m_ringBuffer->getCurrentFrameBounds(startFrame, endFrame);
- if (m_writeCount <= m_readCount + m_writeAheadCount) {
+ size_t framesAvailable = static_cast<size_t>(endFrame - (m_readCount + writeAheadCount));
+ if (!framesAvailable) {
bus->zero();
return;
}
- size_t framesAvailable = static_cast<size_t>(endFrame - (m_readCount + m_writeAheadCount));
if (framesAvailable < framesToProcess) {
framesToProcess = framesAvailable;
bus->zero();
@@ -277,6 +285,8 @@
{
ASSERT(maxFrames >= 0);
+ std::lock_guard<Lock> lock(m_mutex);
+
m_tapDescription = std::make_unique<AudioStreamBasicDescription>(*processingFormat);
int numberOfChannels = processingFormat->mChannelsPerFrame;
size_t bytesPerFrame = processingFormat->mBytesPerFrame;
@@ -321,6 +331,8 @@
void AudioSourceProviderAVFObjC::unprepare()
{
+ std::lock_guard<Lock> lock(m_mutex);
+
m_tapDescription = nullptr;
m_outputDescription = nullptr;
m_ringBuffer = nullptr;
@@ -358,13 +370,17 @@
// Only check the write-ahead time when playback begins.
m_paused = false;
MediaTime earlyBy = rangeStart - currentTime;
- m_writeAheadCount = m_tapDescription->mSampleRate * earlyBy.toDouble();
+ m_writeAheadCount.store(m_tapDescription->mSampleRate * earlyBy.toDouble());
}
+ uint64_t startFrame = 0;
+ uint64_t endFrame = 0;
+ m_ringBuffer->getCurrentFrameBounds(startFrame, endFrame);
+
// Check to see if the underlying media has seeked, which would require us to "flush"
// our outstanding buffers.
if (rangeStart != m_endTimeAtLastProcess)
- m_readCount = m_writeCount;
+ m_seekTo.store(endFrame);
m_startTimeAtLastProcess = rangeStart;
m_endTimeAtLastProcess = rangeStart + rangeDuration;
@@ -372,10 +388,9 @@
// StartOfStream indicates a discontinuity, such as when an AVPlayerItem is re-added
// to an AVPlayer, so "flush" outstanding buffers.
if (flagsOut && *flagsOut & kMTAudioProcessingTapFlag_StartOfStream)
- m_readCount = m_writeCount;
+ m_seekTo.store(endFrame);
- m_ringBuffer->store(bufferListInOut, itemCount, m_writeCount);
- m_writeCount += itemCount;
+ m_ringBuffer->store(bufferListInOut, itemCount, endFrame);
// Mute the default audio playback by zeroing the tap-owned buffers.
for (uint32_t i = 0; i < bufferListInOut->mNumberBuffers; ++i) {
