Chromium Code Reviews
chromiumcodereview-hr@appspot.gserviceaccount.com (chromiumcodereview-hr) | Please choose your nickname with Settings | Help | Chromium Project | Gerrit Changes | Sign out
(783)

Unified Diff: third_party/WebKit/Source/platform/audio/AudioDestination.cpp

Issue 2740103005: Fix premature access on m_fifo in AudioDestination. (Closed)
Patch Set: Initial commit Created 3 years, 9 months ago
Use n/p to move between diff chunks; N/P to move between comments. Draft comments are only viewable by you.
Jump to:
View side-by-side diff with in-line comments
Download patch
Index: third_party/WebKit/Source/platform/audio/AudioDestination.cpp
diff --git a/third_party/WebKit/Source/platform/audio/AudioDestination.cpp b/third_party/WebKit/Source/platform/audio/AudioDestination.cpp
index 81e007261640b149d504feb6e3cea39804c10d15..5b188097e8aa0194301db1121ddfb14873408cd6 100644
--- a/third_party/WebKit/Source/platform/audio/AudioDestination.cpp
+++ b/third_party/WebKit/Source/platform/audio/AudioDestination.cpp
@@ -45,8 +45,12 @@ namespace blink {
// TODO(hongchan): This was estimated based on the largest callback buffer size
// that we would ever need. The current UMA stats indicates that this is, in
// fact, probably too small. There are Android devices out there with a size of
-// 8000 or so. We might need to make this larger. See: crbug.com/670747
+// ~80K or so. We might need to make this larger. See: crbug.com/670747
+#if OS(ANDROID)
+const size_t kFIFOSize = 16384;
+#else
Raymond Toy 2017/03/10 19:34:55 Why change this now? No one has ever complained o
hongchan 2017/03/10 20:57:36 Okay. I am reverting this.
const size_t kFIFOSize = 8192;
+#endif
std::unique_ptr<AudioDestination> AudioDestination::create(
AudioIOCallback& callback,
@@ -70,7 +74,16 @@ AudioDestination::AudioDestination(AudioIOCallback& callback,
false)),
m_renderBus(AudioBus::create(numberOfOutputChannels,
AudioUtilities::kRenderQuantumFrames)),
+ m_fifo(
+ WTF::wrapUnique(new PushPullFIFO(numberOfOutputChannels, kFIFOSize))),
m_framesElapsed(0) {
+ CHECK(m_fifo);
Raymond Toy 2017/03/10 19:34:55 How can m_fifo not be null?
hongchan 2017/03/10 20:57:36 Do you want me to remove this check then? I can ce
Raymond Toy 2017/03/10 21:11:09 AFAICT, the only way this can happen is if you run
hongchan 2017/03/10 21:13:55 Removed.
+
+ m_callbackBufferSize = hardwareBufferSize();
+ if (!checkBufferSize()) {
+ NOTREACHED();
+ }
+
// Create WebAudioDevice. blink::WebAudioDevice is designed to support the
// local input (e.g. loopback from OS audio system), but Chromium's media
// renderer does not support it currently. Thus, we use zero for the number
@@ -79,15 +92,6 @@ AudioDestination::AudioDestination(AudioIOCallback& callback,
0, numberOfOutputChannels, latencyHint, this, String(),
std::move(securityOrigin)));
DCHECK(m_webAudioDevice);
-
- m_callbackBufferSize = m_webAudioDevice->framesPerBuffer();
-
- if (!checkBufferSize()) {
- NOTREACHED();
- }
-
- // Create a FIFO.
- m_fifo = WTF::wrapUnique(new PushPullFIFO(numberOfOutputChannels, kFIFOSize));
}
AudioDestination::~AudioDestination() {
@@ -102,6 +106,12 @@ void AudioDestination::render(const WebVector<float*>& destinationData,
CHECK_EQ(destinationData.size(), m_numberOfOutputChannels);
CHECK_EQ(numberOfFrames, m_callbackBufferSize);
+ // Note that this method is called by AudioDeviceThread. If FIFO is not ready,
+ // or the requested render size is greater than FIFO size return here.
+ // (crbug.com/692423)
+ if (!m_fifo || m_fifo->length() < numberOfFrames)
+ return;
+
m_framesElapsed -= std::min(m_framesElapsed, priorFramesSkipped);
double outputPosition =
m_framesElapsed / static_cast<double>(m_webAudioDevice->sampleRate()) -

Powered by Google App Engine
This is Rietveld 408576698