Bug 66893 - [WebAudio] Undeclared dependency to VIDEO
Summary: [WebAudio] Undeclared dependency to VIDEO
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: Web Audio (show other bugs)
Version: 528+ (Nightly build)
Hardware: Unspecified Unspecified
: P2 Normal
Assignee: Nobody
URL:
Keywords:
Depends on: 67468
Blocks:
  Show dependency treegraph
 
Reported: 2011-08-24 14:40 PDT by Philippe Normand
Modified: 2011-09-02 02:58 PDT (History)
6 users (show)

See Also:


Attachments
proposed patch (7.15 KB, patch)
2011-08-25 00:18 PDT, Philippe Normand
webkit.review.bot: commit-queue-
Details | Formatted Diff | Diff
proposed patch (8.94 KB, patch)
2011-08-25 04:38 PDT, Philippe Normand
webkit.review.bot: commit-queue-
Details | Formatted Diff | Diff
proposed patch (9.00 KB, patch)
2011-08-29 01:36 PDT, Philippe Normand
webkit.review.bot: commit-queue-
Details | Formatted Diff | Diff
proposed patch (8.98 KB, patch)
2011-08-29 07:42 PDT, Philippe Normand
webkit.review.bot: commit-queue-
Details | Formatted Diff | Diff
proposed patch (9.10 KB, patch)
2011-08-29 07:59 PDT, Philippe Normand
webkit.review.bot: commit-queue-
Details | Formatted Diff | Diff
updated patch (5.71 KB, patch)
2011-08-30 01:03 PDT, Philippe Normand
no flags Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Philippe Normand 2011-08-24 14:40:28 PDT
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.
Comment 1 Philippe Normand 2011-08-25 00:18:27 PDT
Created attachment 105138 [details]
proposed patch
Comment 2 WebKit Review Bot 2011-08-25 00:30:18 PDT
Comment on attachment 105138 [details]
proposed patch

Attachment 105138 [details] did not pass chromium-ews (chromium-xvfb):
Output: http://queues.webkit.org/results/9511260
Comment 3 WebKit Review Bot 2011-08-25 01:21:20 PDT
Comment on attachment 105138 [details]
proposed patch

Attachment 105138 [details] did not pass mac-ews (mac):
Output: http://queues.webkit.org/results/9511284
Comment 4 WebKit Review Bot 2011-08-25 01:51:26 PDT
Comment on attachment 105138 [details]
proposed patch

Attachment 105138 [details] did not pass mac-ews (mac):
Output: http://queues.webkit.org/results/9510309
Comment 5 Philippe Normand 2011-08-25 04:38:56 PDT
Created attachment 105160 [details]
proposed patch
Comment 6 Philippe Normand 2011-08-25 04:39:56 PDT
(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.
Comment 7 WebKit Review Bot 2011-08-25 05:46:06 PDT
Comment on attachment 105160 [details]
proposed patch

Attachment 105160 [details] did not pass mac-ews (mac):
Output: http://queues.webkit.org/results/9510377
Comment 8 Philippe Normand 2011-08-29 01:36:11 PDT
Created attachment 105475 [details]
proposed patch

Update after http://trac.webkit.org/changeset/93903
Comment 9 WebKit Review Bot 2011-08-29 01:41:39 PDT
Comment on attachment 105475 [details]
proposed patch

Attachment 105475 [details] did not pass chromium-ews (chromium-xvfb):
Output: http://queues.webkit.org/results/9567016
Comment 10 WebKit Review Bot 2011-08-29 01:44:01 PDT
Comment on attachment 105475 [details]
proposed patch

Attachment 105475 [details] did not pass mac-ews (mac):
Output: http://queues.webkit.org/results/9563114
Comment 11 Eric Seidel (no email) 2011-08-29 05:59:59 PDT
Comment on attachment 105475 [details]
proposed patch

Attachment 105475 [details] did not pass cr-mac-ews (chromium):
Output: http://queues.webkit.org/results/9564151
Comment 12 Philippe Normand 2011-08-29 07:42:07 PDT
Created attachment 105489 [details]
proposed patch
Comment 13 WebKit Review Bot 2011-08-29 07:48:48 PDT
Comment on attachment 105489 [details]
proposed patch

Attachment 105489 [details] did not pass chromium-ews (chromium-xvfb):
Output: http://queues.webkit.org/results/9558318
Comment 14 WebKit Review Bot 2011-08-29 07:50:01 PDT
Comment on attachment 105489 [details]
proposed patch

Attachment 105489 [details] did not pass mac-ews (mac):
Output: http://queues.webkit.org/results/9564176
Comment 15 Eric Seidel (no email) 2011-08-29 07:50:53 PDT
Comment on attachment 105489 [details]
proposed patch

Attachment 105489 [details] did not pass cr-mac-ews (chromium):
Output: http://queues.webkit.org/results/9565154
Comment 16 Philippe Normand 2011-08-29 07:59:33 PDT
Created attachment 105492 [details]
proposed patch
Comment 17 WebKit Review Bot 2011-08-29 08:11:33 PDT
Comment on attachment 105492 [details]
proposed patch

Attachment 105492 [details] did not pass mac-ews (mac):
Output: http://queues.webkit.org/results/9565160
Comment 18 Chris Rogers 2011-08-29 14:56:32 PDT
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
Comment 19 Philippe Normand 2011-08-30 01:03:55 PDT
Created attachment 105591 [details]
updated patch
Comment 20 Philippe Normand 2011-08-30 01:04:54 PDT
(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.
Comment 21 WebKit Review Bot 2011-08-30 04:47:25 PDT
Comment on attachment 105591 [details]
updated patch

Attachment 105591 [details] did not pass mac-ews (mac):
Output: http://queues.webkit.org/results/9565412
Comment 22 Philippe Normand 2011-09-01 08:20:05 PDT
Would you mind review this Kenneth? Chris did a first pass.
Comment 23 Kenneth Russell 2011-09-01 12:04:23 PDT
Comment on attachment 105591 [details]
updated patch

Looks fine to me.
Comment 24 Philippe Normand 2011-09-02 00:34:42 PDT
Committed r94389: <http://trac.webkit.org/changeset/94389>
Comment 25 Philippe Normand 2011-09-02 00:58:22 PDT
I'll have a look at the mac build
Comment 26 Philippe Normand 2011-09-02 02:53:45 PDT
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 :)
Comment 27 Philippe Normand 2011-09-02 02:58:19 PDT
Committed r94397: <http://trac.webkit.org/changeset/94397>