Update class to be able to copy or mix input, and to allocate storage from the information in a stream description.
Created attachment 300248 [details] Proposed patch
Attachment 300248 [details] did not pass style-queue: ERROR: Source/WebCore/platform/audio/mac/CARingBuffer.h:58: The parameter name "mode" adds no information, so it should be removed. [readability/parameter_name] [5] Total errors found: 1 in 3 files If any of these errors are false positives, please file a bug against check-webkit-style.
Comment on attachment 300248 [details] Proposed patch View in context: https://bugs.webkit.org/attachment.cgi?id=300248&action=review > Source/WebCore/platform/audio/mac/CARingBuffer.cpp:140 > + case CARingBuffer::Mix: > + float* destination = static_cast<float*>(dest->mData); > + vadd(destination, 1, reinterpret_cast<float*>(*buffers + srcOffset), 1, destination, 1, std::min<size_t>(nbytes, dest->mDataByteSize - destOffset)); > + break; This doesn't feel right. A float is not the same size as a byte, so I feel like this is going to overrun outside the bounds of the passed in list. (Well, the std::min will keep that from happening, but it'll still not behave correctly.) > Source/WebCore/platform/audio/mac/CARingBuffer.cpp:163 > + ZeroRange(static_cast<Byte**>(m_buffers->data()), m_channelCount, 0, m_capacityBytes); You shouldn't have to zero anything here. Rather, just update the bounds below. > Source/WebCore/platform/audio/mac/CARingBuffer.cpp:184 > + uint64_t start, end; > + getCurrentFrameBounds(start, end); > + if (start == end) > + setCurrentFrameBounds(startFrame, startFrame); > + What's this for?
Comment on attachment 300248 [details] Proposed patch View in context: https://bugs.webkit.org/attachment.cgi?id=300248&action=review >> Source/WebCore/platform/audio/mac/CARingBuffer.cpp:140 >> + break; > > This doesn't feel right. A float is not the same size as a byte, so I feel like this is going to overrun outside the bounds of the passed in list. (Well, the std::min will keep that from happening, but it'll still not behave correctly.) Good point. While we the upcoming patches only use it with floats, we might as well support other the formats. >> Source/WebCore/platform/audio/mac/CARingBuffer.cpp:163 >> + ZeroRange(static_cast<Byte**>(m_buffers->data()), m_channelCount, 0, m_capacityBytes); > > You shouldn't have to zero anything here. Rather, just update the bounds below. Fixed. >> Source/WebCore/platform/audio/mac/CARingBuffer.cpp:184 >> + > > What's this for? Oops, leftover testing.
Created attachment 300359 [details] Updated patch, with API test.
Attachment 300359 [details] did not pass style-queue: ERROR: Source/WebCore/platform/audio/mac/CARingBuffer.h:59: The parameter name "mode" adds no information, so it should be removed. [readability/parameter_name] [5] Total errors found: 1 in 14 files If any of these errors are false positives, please file a bug against check-webkit-style.
Comment on attachment 300359 [details] Updated patch, with API test. View in context: https://bugs.webkit.org/attachment.cgi?id=300359&action=review r=me, with nit > LayoutTests/webaudio/mediastreamaudiosourcenode.html:32 > - stream.getAudioTracks()[0].stop(); > +// stream.getAudioTracks()[0].stop(); We probably still need this. :)
Committed r211539 and r211540: https://trac.webkit.org/r211539, https://trac.webkit.org/r211540
<rdar://problem/30318329>
Plus https://trac.webkit.org/r211545 to fix the 32-bit build.