[webkit-reviews] review granted: [Bug 70155] Add AudioSourceProviderClient and setFormat() method so we can know audio stream format : [Attachment 112278] Patch

[bugzilla-daemon at webkit.org]

Eric Carlson <eric.carlson at apple.com> has granted Chris Rogers
<crogers at google.com>'s request for review:
Bug 70155: Add AudioSourceProviderClient and setFormat() method so we can know
audio stream format
https://bugs.webkit.org/show_bug.cgi?id=70155

Attachment 112278: Patch
https://bugs.webkit.org/attachment.cgi?id=112278&action=review

------- Additional Comments from Eric Carlson <eric.carlson at apple.com>
View in context: https://bugs.webkit.org/attachment.cgi?id=112278&action=review


Looks like HTMLMediaElement.cpp needs to include AudioSourceProviderClient.h.


> Source/WebCore/html/HTMLMediaElement.cpp:3008
> +	   // If we're creating the player from scratch, make sure its
AudioSourceProvider knows about the MediaElementAudioSourceNode.
> +	   if (audioSourceProvider())
> +	       audioSourceProvider()->setClient(m_audioSourceNode);

Nit: this comment doesn't make sense, how does "if (audioSourceProvider())"
tell you if the player was created from scratch?


> Source/WebCore/platform/audio/AudioSourceProvider.h:43
> +    // If a client is set, we call it back when the audio format is
available.

Nit: "when the audio format is available *or changes*"?


> Source/WebCore/webaudio/MediaElementAudioSourceNode.cpp:63
> +void MediaElementAudioSourceNode::setFormat(size_t, float)
> +{
> +    // FIXME: setup a sample-rate converter if necessary to convert to the
AudioContext sample-rate.
> +}

It is probably worth having an ASSERT here as long as the sample-rate converter
setup is missing.