Bug 106651 - [Qt][WK1] Web Audio support
Summary: [Qt][WK1] Web Audio support
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: Allan Sandfeld Jensen
URL:
Keywords:
Depends on:
Blocks: 110211
  Show dependency treegraph
 
Reported: 2013-01-11 03:18 PST by Allan Sandfeld Jensen
Modified: 2013-02-19 05:11 PST (History)
6 users (show)

See Also:


Attachments
Patch (11.62 KB, patch)
2013-01-11 03:25 PST, Allan Sandfeld Jensen
no flags Details | Formatted Diff | Diff
Patch (16.72 KB, patch)
2013-01-11 04:40 PST, Allan Sandfeld Jensen
no flags Details | Formatted Diff | Diff
Patch (16.59 KB, patch)
2013-01-11 05:46 PST, Allan Sandfeld Jensen
jturcotte: review+
Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Allan Sandfeld Jensen 2013-01-11 03:18:44 PST
If WebAudio is compiled in, it can currently only be used by the WebKit2 API. For WebKit1 support, we need to be able to enable it at runtime, and support audio-testing in DumpRenderTree.
Comment 1 Allan Sandfeld Jensen 2013-01-11 03:25:16 PST
Created attachment 182315 [details]
Patch
Comment 2 Allan Sandfeld Jensen 2013-01-11 04:40:27 PST
Created attachment 182321 [details]
Patch

Make WebAudio configurable from QtTestBrowser, and enable LEGACY_WEB_AUDIO which is needed for the majority of Web Audio tests.
Comment 3 Jocelyn Turcotte 2013-01-11 05:09:15 PST
Comment on attachment 182321 [details]
Patch

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

> Source/WebCore/bridge/qt/qt_runtime.cpp:509
> +            } else if (type == RTUint8Array) {
> +                WTF::Uint8Array* arr = toUint8Array(toJS(toJS(context), value));
> +                ret = QVariant(QByteArray(reinterpret_cast<const char*>(arr->data()), arr->length()));
> +                dist = 0;

Uint8ClampedArray is a child class of Uint8Array, could you merge the two cases?

> Source/WebKit/qt/Api/qwebsettings.cpp:541
> +    d->attributes.insert(QWebSettings::WebAudioEnabled, false);

Should this be enabled by default like other HTML5 features?
On one side I'm not sure if it's good to change the default value afterward.
On the other side we can't enable it by default if we only have it working on Linux.

> Tools/DumpRenderTree/qt/DumpRenderTreeQt.cpp:931
> +        fprintf(stdout, "Content-Type: %s\n", resultContentType.toUtf8().constData());

This could be a good use of qPrintable.

> Tools/DumpRenderTree/qt/DumpRenderTreeQt.cpp:942
> +        fprintf(stdout, "Content-Type: %s\n", resultContentType.toUtf8().constData());

ditto

> Tools/QtTestBrowser/launcherwindow.cpp:219
> -    settings->setAttribute(QWebSettings::WebGLEnabled, m_windowOptions.useWebGL);
> +    m_windowOptions.useWebGL = settings->testAttribute(QWebSettings::WebGLEnabled);

I think that this will break the -webgl command line option.
Comment 4 Allan Sandfeld Jensen 2013-01-11 05:34:19 PST
(In reply to comment #3)
> (From update of attachment 182321 [details])
> View in context: https://bugs.webkit.org/attachment.cgi?id=182321&action=review
> 
> > Source/WebCore/bridge/qt/qt_runtime.cpp:509
> > +            } else if (type == RTUint8Array) {
> > +                WTF::Uint8Array* arr = toUint8Array(toJS(toJS(context), value));
> > +                ret = QVariant(QByteArray(reinterpret_cast<const char*>(arr->data()), arr->length()));
> > +                dist = 0;
> 
> Uint8ClampedArray is a child class of Uint8Array, could you merge the two cases?
> 
It is possible, at least from JS -> Qt it should be safe. The other way we still want to get the clampedarray.

> > Source/WebKit/qt/Api/qwebsettings.cpp:541
> > +    d->attributes.insert(QWebSettings::WebAudioEnabled, false);
> 
> Should this be enabled by default like other HTML5 features?
> On one side I'm not sure if it's good to change the default value afterward.
> On the other side we can't enable it by default if we only have it working on Linux.
> 
True. I want to start by having it off by default until we know if it works on more than linux. Note that this patch still doesn't enable the feature at compile time.

> > Tools/QtTestBrowser/launcherwindow.cpp:219
> > -    settings->setAttribute(QWebSettings::WebGLEnabled, m_windowOptions.useWebGL);
> > +    m_windowOptions.useWebGL = settings->testAttribute(QWebSettings::WebGLEnabled);
> 
> I think that this will break the -webgl command line option.

I hadn't thought of that, but I guess it still works. WebGL is enabled by default these days.
Comment 5 Allan Sandfeld Jensen 2013-01-11 05:46:43 PST
Created attachment 182327 [details]
Patch
Comment 6 Allan Sandfeld Jensen 2013-01-11 06:39:08 PST
Committed r139437: <http://trac.webkit.org/changeset/139437>