Implement WebMediaPlayerClientImpl::audioSourceProvider() and interface into chromium
Created attachment 104287 [details] Patch
This is one patch out of several to implement MediaElementAudioSourceNode (see approach #2): https://dvcs.w3.org/hg/audio/raw-file/tip/webaudio/specification.html#AudioElementIntegration-section It's a simple chromium implementation for WebMediaPlayerClientImpl::audioSourceProvider(). which is meant to override the default empty implementation in MediaPlayerPrivateInterface::audioSourceProvider() Please see this patch for more details of how this grows out of WebCore: https://bugs.webkit.org/show_bug.cgi?id=66398 It reaches out into chromium code via the WebAudioSourceProvider abstract interface For the chromium-side, here's a patch for a very early prototype implementation (incomplete and rough edges): http://codereview.chromium.org/7631033/
Comment on attachment 104287 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=104287&action=review > Source/WebKit/chromium/public/WebMediaPlayer.h:158 > + virtual WebAudioSourceProvider* webAudioSourceProvider() { return 0; } nit: we drop the "web" prefix on methods like this. see for example WebFrame::dataSource(). that returns a WebDataSource. > Source/WebKit/chromium/src/WebMediaPlayerClientImpl.cpp:626 > +void WebMediaPlayerClientImpl::AudioSourceProviderImpl::provideInput(WebCore::AudioBus* bus, size_t framesToProcess) it seems like this function should be listed after we are done defining all of the WebMediaPlayerClientImpl functions, and methods in the .cpp file should appear in the same order as they are declared in the .h file. > Source/WebKit/chromium/src/WebMediaPlayerClientImpl.h:173 > + void set(WebAudioSourceProvider* provider) { m_webAudioSourceProvider = provider; } nit: set -> initialize? (it seems like "set" is a bit too vague, and setProvider would be funny too since your implementing a provider. initialize seems to capture what you are doing.)
Created attachment 104382 [details] Patch
Comment on attachment 104287 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=104287&action=review >> Source/WebKit/chromium/public/WebMediaPlayer.h:158 >> + virtual WebAudioSourceProvider* webAudioSourceProvider() { return 0; } > > nit: we drop the "web" prefix on methods like this. see for example WebFrame::dataSource(). that returns a WebDataSource. FIXED >> Source/WebKit/chromium/src/WebMediaPlayerClientImpl.cpp:626 >> +void WebMediaPlayerClientImpl::AudioSourceProviderImpl::provideInput(WebCore::AudioBus* bus, size_t framesToProcess) > > it seems like this function should be listed after we are done defining > all of the WebMediaPlayerClientImpl functions, and methods in the .cpp > file should appear in the same order as they are declared in the .h file. I've moved WebMediaPlayerClientImpl::AudioSourceProviderImpl::provideInput() to the end of the .cpp (after all the WebMediaPlayerClientImpl methods). Also, I've put the audioSourceProvider() immediately following videoDecodedByteCount() following the order from MediaPlayerPrivateInterface >> Source/WebKit/chromium/src/WebMediaPlayerClientImpl.h:173 >> + void set(WebAudioSourceProvider* provider) { m_webAudioSourceProvider = provider; } > > nit: set -> initialize? (it seems like "set" is a bit too vague, and setProvider would be funny too since your implementing a provider. initialize seems to capture what you are doing.) Seems reasonable - FIXED
Comment on attachment 104382 [details] Patch Clearing flags on attachment: 104382 Committed r93367: <http://trac.webkit.org/changeset/93367>
All reviewed patches have been landed. Closing bug.