Changeset 199042 in webkit

Timestamp:

Apr 4, 2016, 10:39:18 PM (11 years ago)

Author:

jer.noble@apple.com

Message:

[iOS] Crash when playing <video> after playing Web Audio
​https://bugs.webkit.org/show_bug.cgi?id=156185
<rdar://problem/10177005>

Reviewed by Eric Carlson.

Off-by-one error in AudioDestinationIOS::render. The ivars m_firstSpareFrame and m_lastSpareFrame imply that
the sample range is inclusive, i.e. [m_firstSpareFrame .. m_lastSpareFrame], but the length of the range was
being calculated as if m_lastSpareFrame was exclusive; when the two were equal, the length was calculated as
0, rather than 1. This was caught by an ASSERT (and would have been caught by a downstream ASSERT had that one
not been present).

Fix the off-by-one by treating them as inclusive/exclusive--similar to C++ iterators--and renaming them to reflect
this: [m_startSpareFrame .. m_endSpareFrame). This corrects the "length" math which caused the crash.

• platform/audio/ios/AudioDestinationIOS.cpp:

(WebCore::AudioDestinationIOS::render):

• platform/audio/ios/AudioDestinationIOS.h:

Location:

trunk/Source/WebCore

Files:

• ChangeLog (modified) (1 diff)
• platform/audio/ios/AudioDestinationIOS.cpp (modified) (2 diffs)
• platform/audio/ios/AudioDestinationIOS.h (modified) (1 diff)

trunk/Source/WebCore/ChangeLog

@@ +1 @@
+2016-04-04  Jer Noble  <jer.noble@apple.com>
+
+        [iOS] Crash when playing <video> after playing Web Audio
+        https://bugs.webkit.org/show_bug.cgi?id=156185
+        <rdar://problem/10177005>
+
+        Reviewed by Eric Carlson.
+
+        Off-by-one error in AudioDestinationIOS::render. The ivars m_firstSpareFrame and m_lastSpareFrame imply that
+        the sample range is inclusive, i.e. [m_firstSpareFrame .. m_lastSpareFrame], but the length of the range was
+        being calculated as if m_lastSpareFrame was exclusive; when the two were equal, the length was calculated as
+        0, rather than 1. This was caught by an ASSERT (and would have been caught by a downstream ASSERT had that one
+        not been present).
+
+        Fix the off-by-one by treating them as inclusive/exclusive--similar to C++ iterators--and renaming them to reflect
+        this: [m_startSpareFrame .. m_endSpareFrame). This corrects the "length" math which caused the crash.
+
+        * platform/audio/ios/AudioDestinationIOS.cpp:
+        (WebCore::AudioDestinationIOS::render):
+        * platform/audio/ios/AudioDestinationIOS.h:
+
 2016-04-04  John Wilander  <wilander@apple.com>
 

trunk/Source/WebCore/platform/audio/ios/AudioDestinationIOS.cpp

@@ -219 +219 @@
     UInt32 frameOffset = 0;
     while (framesRemaining > 0) {
-        if (m_firstSpareFrame && m_lastSpareFrame) {
-            ASSERT(m_firstSpareFrame < m_lastSpareFrame);
-            UInt32 framesThisTime = m_lastSpareFrame - m_firstSpareFrame;
+        if (m_startSpareFrame && m_endSpareFrame) {
+            ASSERT(m_startSpareFrame < m_endSpareFrame);
+            UInt32 framesThisTime = m_endSpareFrame - m_startSpareFrame;
             assignAudioBuffersToBus(buffers, *m_renderBus, numberOfBuffers, numberOfFrames, frameOffset, framesThisTime);
-            m_renderBus->copyFromRange(*m_spareBus, m_firstSpareFrame, m_lastSpareFrame);
+            m_renderBus->copyFromRange(*m_spareBus, m_startSpareFrame, m_endSpareFrame);
             frameOffset += framesThisTime;
             framesRemaining -= framesThisTime;
-            m_firstSpareFrame = 0;
-            m_lastSpareFrame = 0;
+            m_startSpareFrame = 0;
+            m_endSpareFrame = 0;
         }
 
@@ -236 +236 @@
             m_callback.render(0, m_spareBus.get(), kRenderBufferSize);
             m_renderBus->copyFromRange(*m_spareBus, 0, framesThisTime);
-            m_firstSpareFrame = framesThisTime;
-            m_lastSpareFrame = kRenderBufferSize - 1;
+            m_startSpareFrame = framesThisTime;
+            m_endSpareFrame = kRenderBufferSize;
         } else
             m_callback.render(0, m_renderBus.get(), framesThisTime);

trunk/Source/WebCore/platform/audio/ios/AudioDestinationIOS.h

@@ -67 +67 @@
     RefPtr<AudioBus> m_renderBus;
     RefPtr<AudioBus> m_spareBus;
-    unsigned m_firstSpareFrame { 0 };
-    unsigned m_lastSpareFrame { 0 };
+    unsigned m_startSpareFrame { 0 };
+    unsigned m_endSpareFrame { 0 };
 
     double m_sampleRate;