Add support for GPUProcess WebAudio media element providers
Created attachment 411313 [details] Patch
Created attachment 411422 [details] Patch
Created attachment 411428 [details] Patch
Created attachment 411430 [details] Patch
Created attachment 411431 [details] Patch
Created attachment 411432 [details] Patch
Comment on attachment 411432 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=411432&action=review > Source/WebCore/platform/graphics/avfoundation/AudioSourceProviderAVFObjC.mm:345 > + m_ringBuffer = m_ringBufferCallback ? m_ringBufferCallback(description, capacity) : nullptr; > + if (!m_ringBuffer) { > + m_ringBuffer = makeUnique<CARingBuffer>(); > + m_ringBuffer->allocate(description, capacity); > + } Is it possible for m_ringBufferCallback() to return NULL? If not, I would change this to something like if (m_ringBufferCallback) m_ringBuffer = m_ringBufferCallback(...) else { m_ringBuffer = makeUnique<...> ... } > Source/WebCore/platform/mediastream/mac/WebAudioSourceProviderCocoa.mm:144 > + m_dataSource->pushSamples(MediaTime(m_writeCount, m_inputDescription->sampleRate()), data, frameCount); Good catch! > Source/WebKit/GPUProcess/media/RemoteAudioSourceProviderProxy.cpp:29 > +#if ENABLE(GPU_PROCESS) && ENABLE(WEB_AUDIO) && PLATFORM(COCOA) `ENABLE_WEB_AUDIO` is defined in PlatformEnableCocoa.h, so I think `ENABLE(WEB_AUDIO) && PLATFORM(COCOA)` can be just `PLATFORM(COCOA)` > Source/WebKit/GPUProcess/media/RemoteAudioSourceProviderProxy.h:28 > +#if ENABLE(GPU_PROCESS) && ENABLE(WEB_AUDIO) && PLATFORM(COCOA) Ditto. > Source/WebKit/GPUProcess/media/RemoteMediaPlayerProxy.cpp:889 > +#if ENABLE(WEB_AUDIO) && PLATFORM(COCOA) Ditto. > Source/WebKit/GPUProcess/media/RemoteMediaPlayerProxy.cpp:903 > +#if ENABLE(WEB_AUDIO) && PLATFORM(COCOA) Ditto. > Source/WebKit/GPUProcess/media/RemoteMediaPlayerProxy.h:314 > +#if ENABLE(WEB_AUDIO) && PLATFORM(COCOA) Ditto. > Source/WebKit/WebProcess/GPU/GPUProcessConnection.cpp:105 > +#if PLATFORM(COCOA) && ENABLE(WEB_AUDIO) Ditto. > Source/WebKit/WebProcess/GPU/GPUProcessConnection.h:70 > +#if PLATFORM(COCOA) && ENABLE(WEB_AUDIO) Ditto. > Source/WebKit/WebProcess/GPU/GPUProcessConnection.h:108 > +#if PLATFORM(COCOA) && ENABLE(WEB_AUDIO) Ditto. > Source/WebKit/WebProcess/GPU/media/RemoteAudioSourceProvider.cpp:29 > #if ENABLE(GPU_PROCESS) && ENABLE(WEB_AUDIO) && PLATFORM(COCOA) Ditto. > Source/WebKit/WebProcess/cocoa/RemoteCaptureSampleManager.cpp:29 > +#include "Logging.h" Is this needed?
(In reply to Eric Carlson from comment #7) > Comment on attachment 411432 [details] > Patch > > View in context: > https://bugs.webkit.org/attachment.cgi?id=411432&action=review > > > Source/WebCore/platform/graphics/avfoundation/AudioSourceProviderAVFObjC.mm:345 > > + m_ringBuffer = m_ringBufferCallback ? m_ringBufferCallback(description, capacity) : nullptr; > > + if (!m_ringBuffer) { > > + m_ringBuffer = makeUnique<CARingBuffer>(); > > + m_ringBuffer->allocate(description, capacity); > > + } > > Is it possible for m_ringBufferCallback() to return NULL? If not, I would > change this to something like > > if (m_ringBufferCallback) > m_ringBuffer = m_ringBufferCallback(...) > else { > m_ringBuffer = makeUnique<...> > ... > } I did that initially but thought it was as good this way. I'll change the ring buffer callback to pass a UniqueRef. > > Source/WebCore/platform/mediastream/mac/WebAudioSourceProviderCocoa.mm:144 > > + m_dataSource->pushSamples(MediaTime(m_writeCount, m_inputDescription->sampleRate()), data, frameCount); > > Good catch! > > > Source/WebKit/GPUProcess/media/RemoteAudioSourceProviderProxy.cpp:29 > > +#if ENABLE(GPU_PROCESS) && ENABLE(WEB_AUDIO) && PLATFORM(COCOA) > > `ENABLE_WEB_AUDIO` is defined in PlatformEnableCocoa.h, so I think > `ENABLE(WEB_AUDIO) && PLATFORM(COCOA)` can be just `PLATFORM(COCOA)` CMake build is defining these macros in Source/cmake/OptionsXXX.cmake so it might be safer to keep these. > > Source/WebKit/WebProcess/cocoa/RemoteCaptureSampleManager.cpp:29 > > +#include "Logging.h" > > Is this needed? Unified build issue.
Created attachment 411547 [details] Patch for landing
Committed r268577: <https://trac.webkit.org/changeset/268577> All reviewed patches have been landed. Closing bug and clearing flags on attachment 411547 [details].
<rdar://problem/70371753>