Title: [278755] trunk/Source/WebCore
- Revision
- 278755
- Author
- [email protected]
- Date
- 2021-06-10 22:29:30 -0700 (Thu, 10 Jun 2021)
Log Message
CARingBuffer::frameOffset() makes incorrect assumptions about the frame count
https://bugs.webkit.org/show_bug.cgi?id=226253
<rdar://problem/78463453>
Reviewed by Eric Carlson.
CARingBuffer::frameOffset() was trying to avoid doing a `frameNumber % m_frameCount`
modulo operation by doing a `frameNumber & (m_frameCount - 1)`. However, this bitwise
operation is only equivalent if m_frameCount is a power of 2. It isn't enforced
anywhere that the frameCount is a power of 2. As a matter of fact, we frequently use
2*sampleRate which is often 2*44100=88200, which is NOT a power of 2.
When adding logging, I saw frameOffset(512) returning 0 for a frameCount of 88200, which
made no sense. It was causing offset0 and offset1 in CARingBuffer::fetchInternal() to
be both 0 (even though startRead was 0 and endRead was 512) and it was leading the
function to make bad computations.
To address the issue, I updated CARingBuffer::frameOffset() to use a simple modulo
operation. It is safer as it makes no assumption on the frame count and it is more
readable. If we're worried about the performance, we could alternatively round up the
frameCount to the next power of 2 and keep using the bitwise operation, but I am
personally do not think it is worth it.
* platform/audio/cocoa/CARingBuffer.cpp:
(WebCore::CARingBuffer::initializeAfterAllocation):
* platform/audio/cocoa/CARingBuffer.h:
(WebCore::CARingBuffer::frameOffset):
Modified Paths
Diff
Modified: trunk/Source/WebCore/ChangeLog (278754 => 278755)
--- trunk/Source/WebCore/ChangeLog 2021-06-11 05:03:17 UTC (rev 278754)
+++ trunk/Source/WebCore/ChangeLog 2021-06-11 05:29:30 UTC (rev 278755)
@@ -1,3 +1,33 @@
+2021-06-10 Chris Dumez <[email protected]>
+
+ CARingBuffer::frameOffset() makes incorrect assumptions about the frame count
+ https://bugs.webkit.org/show_bug.cgi?id=226253
+ <rdar://problem/78463453>
+
+ Reviewed by Eric Carlson.
+
+ CARingBuffer::frameOffset() was trying to avoid doing a `frameNumber % m_frameCount`
+ modulo operation by doing a `frameNumber & (m_frameCount - 1)`. However, this bitwise
+ operation is only equivalent if m_frameCount is a power of 2. It isn't enforced
+ anywhere that the frameCount is a power of 2. As a matter of fact, we frequently use
+ 2*sampleRate which is often 2*44100=88200, which is NOT a power of 2.
+
+ When adding logging, I saw frameOffset(512) returning 0 for a frameCount of 88200, which
+ made no sense. It was causing offset0 and offset1 in CARingBuffer::fetchInternal() to
+ be both 0 (even though startRead was 0 and endRead was 512) and it was leading the
+ function to make bad computations.
+
+ To address the issue, I updated CARingBuffer::frameOffset() to use a simple modulo
+ operation. It is safer as it makes no assumption on the frame count and it is more
+ readable. If we're worried about the performance, we could alternatively round up the
+ frameCount to the next power of 2 and keep using the bitwise operation, but I am
+ personally do not think it is worth it.
+
+ * platform/audio/cocoa/CARingBuffer.cpp:
+ (WebCore::CARingBuffer::initializeAfterAllocation):
+ * platform/audio/cocoa/CARingBuffer.h:
+ (WebCore::CARingBuffer::frameOffset):
+
2021-06-10 Yury Semikhatsky <[email protected]>
[REGRESSION][Curl] Network::ResourceTiming are broken after r278391
Modified: trunk/Source/WebCore/platform/audio/cocoa/CARingBuffer.cpp (278754 => 278755)
--- trunk/Source/WebCore/platform/audio/cocoa/CARingBuffer.cpp 2021-06-11 05:03:17 UTC (rev 278754)
+++ trunk/Source/WebCore/platform/audio/cocoa/CARingBuffer.cpp 2021-06-11 05:29:30 UTC (rev 278755)
@@ -98,7 +98,6 @@
m_channelCount = format.numberOfChannelStreams();
m_bytesPerFrame = format.bytesPerFrame();
m_frameCount = frameCount;
- m_frameCountMask = frameCount - 1;
m_capacityBytes = computeCapacityBytes(format, frameCount);
m_pointers.resize(m_channelCount);
Modified: trunk/Source/WebCore/platform/audio/cocoa/CARingBuffer.h (278754 => 278755)
--- trunk/Source/WebCore/platform/audio/cocoa/CARingBuffer.h 2021-06-11 05:03:17 UTC (rev 278754)
+++ trunk/Source/WebCore/platform/audio/cocoa/CARingBuffer.h 2021-06-11 05:29:30 UTC (rev 278755)
@@ -122,7 +122,7 @@
private:
void updateFrameBounds();
- size_t frameOffset(uint64_t frameNumber) { return (frameNumber & m_frameCountMask) * m_bytesPerFrame; }
+ size_t frameOffset(uint64_t frameNumber) const { return (frameNumber % m_frameCount) * m_bytesPerFrame; }
void clipTimeBounds(uint64_t& startRead, uint64_t& endRead);
void setCurrentFrameBounds(uint64_t startFrame, uint64_t endFrame);
@@ -140,7 +140,6 @@
uint32_t m_channelCount { 0 };
size_t m_bytesPerFrame { 0 };
uint32_t m_frameCount { 0 };
- uint32_t m_frameCountMask { 0 };
size_t m_capacityBytes { 0 };
CAAudioStreamDescription m_description;
_______________________________________________
webkit-changes mailing list
[email protected]
https://lists.webkit.org/mailman/listinfo/webkit-changes
