Bug 66893 – [WebAudio] Undeclared dependency to VIDEO

• Status: RESOLVED
• Resolution: FIXED
• Component: Web Audio

[Philippe Normand]

The createMediaElementSource() of AudioContext introduces a dependency on the VIDEO feature but it's not explicit, so this breaks:

build-webkit --no-video --web-audio

I tried to use [Conditional=VIDEO] as a prefix to the new method in the idl file but it seems this works only for attributes.

[Philippe Normand]

Created attachment 105138 [details]
proposed patch

[WebKit Review Bot]

Comment on attachment 105138 [details]
proposed patch

Attachment 105138 [details] did not pass chromium-ews (chromium-xvfb):
Output: http://queues.webkit.org/results/9511260

[WebKit Review Bot]

Comment on attachment 105138 [details]
proposed patch

Attachment 105138 [details] did not pass mac-ews (mac):
Output: http://queues.webkit.org/results/9511284

[WebKit Review Bot]

Comment on attachment 105138 [details]
proposed patch

Attachment 105138 [details] did not pass mac-ews (mac):
Output: http://queues.webkit.org/results/9510309

[Philippe Normand]

Created attachment 105160 [details]
proposed patch

[Philippe Normand]

(In reply to comment #5)
> Created an attachment (id=105160) [details]
> proposed patch

This one should make the Chromium EWS happy.
About the mac ews, not sure, it might need a clobber build.

[WebKit Review Bot]

Comment on attachment 105160 [details]
proposed patch

Attachment 105160 [details] did not pass mac-ews (mac):
Output: http://queues.webkit.org/results/9510377

[Philippe Normand]

Created attachment 105475 [details]
proposed patch

Update after http://trac.webkit.org/changeset/93903

[WebKit Review Bot]

Comment on attachment 105475 [details]
proposed patch

Attachment 105475 [details] did not pass chromium-ews (chromium-xvfb):
Output: http://queues.webkit.org/results/9567016

[WebKit Review Bot]

Comment on attachment 105475 [details]
proposed patch

Attachment 105475 [details] did not pass mac-ews (mac):
Output: http://queues.webkit.org/results/9563114

[Eric Seidel (no email)]

Comment on attachment 105475 [details]
proposed patch

Attachment 105475 [details] did not pass cr-mac-ews (chromium):
Output: http://queues.webkit.org/results/9564151

[Philippe Normand]

Created attachment 105489 [details]
proposed patch

[WebKit Review Bot]

Comment on attachment 105489 [details]
proposed patch

Attachment 105489 [details] did not pass chromium-ews (chromium-xvfb):
Output: http://queues.webkit.org/results/9558318

[WebKit Review Bot]

Comment on attachment 105489 [details]
proposed patch

Attachment 105489 [details] did not pass mac-ews (mac):
Output: http://queues.webkit.org/results/9564176

[Eric Seidel (no email)]

Comment on attachment 105489 [details]
proposed patch

Attachment 105489 [details] did not pass cr-mac-ews (chromium):
Output: http://queues.webkit.org/results/9565154

[Philippe Normand]

Created attachment 105492 [details]
proposed patch

[WebKit Review Bot]

Comment on attachment 105492 [details]
proposed patch

Attachment 105492 [details] did not pass mac-ews (mac):
Output: http://queues.webkit.org/results/9565160

[Chris Rogers]

Comment on attachment 105492 [details]
proposed patch

View in context: https://bugs.webkit.org/attachment.cgi?id=105492&action=review

Hi Philippe, can you try my suggestion to avoid custom bindings?

Thanks,
Chris

> Source/WebCore/webaudio/AudioContext.idl:62
> +        [Custom] MediaElementAudioSourceNode createMediaElementSource(in HTMLMediaElement mediaElement)

Can't we avoid using the Custom bindings and instead have something like this:

#if defined(ENABLE_VIDEO) && ENABLE_VIDEO
MediaElementAudioSourceNode createMediaElementSource(in HTMLMediaElement mediaElement)
            raises(DOMException);
#endif

[Philippe Normand]

Created attachment 105591 [details]
updated patch

[Philippe Normand]

(In reply to comment #18)
> (From update of attachment 105492 [details])
> View in context: https://bugs.webkit.org/attachment.cgi?id=105492&action=review
> 
> Hi Philippe, can you try my suggestion to avoid custom bindings?
> 

Good idea indeed!
Thanks for the feedback.

[WebKit Review Bot]

Comment on attachment 105591 [details]
updated patch

Attachment 105591 [details] did not pass mac-ews (mac):
Output: http://queues.webkit.org/results/9565412

[Philippe Normand]

Would you mind review this Kenneth? Chris did a first pass.

[Kenneth Russell]

Comment on attachment 105591 [details]
updated patch

Looks fine to me.

[Philippe Normand]

Committed r94389: <http://trac.webkit.org/changeset/94389>

[Philippe Normand]

I'll have a look at the mac build

[Philippe Normand]

Build fix is to use Conditional=WEB_AUDIO&VIDEO in MediaElementAudioSourceNode.idl instead of 2 separate Conditional lines.
I'll reland the patch, I verified the fix on my mac :)

[Philippe Normand]

Committed r94397: <http://trac.webkit.org/changeset/94397>