Bug 63472 - [Qt] Change default backend to use GStreamer on Linux and QuickTime on Mac.
Summary: [Qt] Change default backend to use GStreamer on Linux and QuickTime on Mac.
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: WebKit Qt (show other bugs)
Version: 528+ (Nightly build)
Hardware: All All
: P2 Normal
Assignee: Alexis Menard (darktears)
URL:
Keywords: Qt, QtTriaged
Depends on: 58548 65369
Blocks:
  Show dependency treegraph
 
Reported: 2011-06-27 12:27 PDT by Alexis Menard (darktears)
Modified: 2011-07-29 08:41 PDT (History)
5 users (show)

See Also:


Attachments
Patch (7.93 KB, patch)
2011-06-27 12:47 PDT, Alexis Menard (darktears)
no flags Details | Formatted Diff | Diff
Patch (7.60 KB, patch)
2011-07-25 12:04 PDT, Alexis Menard (darktears)
akling: review+
ossy: commit-queue-
Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Alexis Menard (darktears) 2011-06-27 12:27:09 PDT
As discussed on the mailing list https://lists.webkit.org/pipermail/webkit-qt/2011-June/001633.html, it is better for the Qt port to move away from Qt Multimedia as the default backends on Linux and Mac. We can rely on QuickTime and GStreamer backends inside WebKit as they are more easy to change, more reliable to use and we share the maintenance with the WebKit community. USE_QTMULTIMEDIA=1 is now the option to force the usage of Qt Multimedia.

I'll commit later this patch when we have a public build bot on Mac and the 58548 will be fixed.
Comment 1 Alexis Menard (darktears) 2011-06-27 12:47:06 PDT
Created attachment 98768 [details]
Patch
Comment 2 Laszlo Gombos 2011-06-27 14:21:44 PDT
Comment on attachment 98768 [details]
Patch

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

> Source/WebCore/WebCore.pri:-257
> -        DEFINES += WTF_USE_QTKIT=1

I do not see where QuickTime backend is set as a default media player. Where is WTF_USE_QTKIT set to 1 ?

> Source/WebCore/WebCore.pri:-266
> -        DEFINES += WTF_USE_GSTREAMER=1

Same as above; where is WTF_USE_GSTREAMER set to 1 ?
Comment 3 Alexis Menard (darktears) 2011-06-27 14:34:05 PDT
Comment on attachment 98768 [details]
Patch

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

>> Source/WebCore/WebCore.pri:-257
>> -        DEFINES += WTF_USE_QTKIT=1
> 
> I do not see where QuickTime backend is set as a default media player. Where is WTF_USE_QTKIT set to 1 ?

feature.pri which is include everywhere first.

>> Source/WebCore/WebCore.pri:-266
>> -        DEFINES += WTF_USE_GSTREAMER=1
> 
> Same as above; where is WTF_USE_GSTREAMER set to 1 ?

Ditto.
Comment 4 Laszlo Gombos 2011-06-28 21:42:54 PDT
Comment on attachment 98768 [details]
Patch

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

Overall the direction looks good to me.

> Source/WebCore/features.pri:213
>          DEFINES -= ENABLE_VIDEO=0
>          DEFINES += ENABLE_VIDEO=1
>          DEFINES -= WTF_USE_QT_MULTIMEDIA=0
>          DEFINES += WTF_USE_QT_MULTIMEDIA=1
> +        DEFINES -= WTF_USE_QTKIT=1
> +        DEFINES += WTF_USE_QTKIT=0
> +        DEFINES -= WTF_USE_GSTREAMER=1
> +        DEFINES += WTF_USE_GSTREAMER=0

Are the "DEFINES += XXX=0" statements are really necessary? It seems that the corresponding tests are all testing for "contains(DEFINES, XXX=1)"  I also find it hard to maintain and read. Would the following work instead ? 

# HTML5 Media Support
!contains(DEFINES, ENABLE_VIDEO=.) {
    linux-*|mac|contains(MOBILITY_CONFIG, multimedia) {

        DEFINES += ENABLE_VIDEO=1

        mac:!contains(DEFINES, USE_QTMULTIMEDIA=1) {
            DEFINES += WTF_USE_QTKIT=1
        } else: linux-*:!contains(DEFINES, USE_QTMULTIMEDIA=1) {
            DEFINES += WTF_USE_GSTREAMER=1
        } else: contains(MOBILITY_CONFIG, multimedia) {
            DEFINES += WTF_USE_QT_MULTIMEDIA=1
        }
    }
}
Comment 5 Yael 2011-07-07 14:48:38 PDT
Comment on attachment 98768 [details]
Patch

Since you are turning video on by default now, you should update http://trac.webkit.org/wiki/BuildingQtOnLinux with the new build dependencies.
Comment 6 Alexis Menard (darktears) 2011-07-25 12:04:28 PDT
Created attachment 101890 [details]
Patch
Comment 7 Csaba Osztrogonác 2011-07-26 05:37:00 PDT
Comment on attachment 101890 [details]
Patch

r- because it broke incremental build. And it killed the Qt EWS too.
Check https://bugs.webkit.org/show_bug.cgi?id=38054 for details.
Comment 8 Andreas Kling 2011-07-26 06:20:09 PDT
Comment on attachment 101890 [details]
Patch

r=me, but you need to find a way to land this without making our bot masters cry.
Comment 9 Andras Becsi 2011-07-26 06:34:53 PDT
Committed r91752: <http://trac.webkit.org/changeset/91752>
Comment 10 Ademar Reis 2011-07-29 08:41:36 PDT
Revision r91752 cherry-picked into qtwebkit-2.2 with commit 7a53b15 <http://gitorious.org/webkit/qtwebkit/commit/7a53b15>