[Webkit-unassigned] [Bug 34631] [Qt] Switching from Phonon to QtMultimedia Backend for Qt 4.7

[bugzilla-daemon at webkit.org]

https://bugs.webkit.org/show_bug.cgi?id=34631





--- Comment #32 from Nick Young <nicholas.young at nokia.com>  2010-02-21 17:27:20 PST ---
(In reply to comment #30)
> LGTM, a few nitpicks below. I also had to change QtMedia to QtMultimedia a few
> places to make it compile against latest qt-multimedia-team's master, so r- for
> now, but should be able to land this soon.

A namespace change was made between me posting that patch and you pulling the
repo ;) Nevermind, fixed now.

> You can use the short-form !contains(DEFINES, ENABLE_VIDEO=.)

Sure.

> Is this not implemented yet in QtMultimedia?

At one stage it was causing some problems, because the gstreamer backend only
returns true to this after you've begun playback. However, I've done a quick
test and there does not seem to be any problems now.

> Coding-style: bytesLoadedKey

Done.

> Minor visual style nitpick: Is this to contrast the dark red? The gray outline
> does not match the other controls, such as the play buttons etc. I'd say skip
> this for now, and we can revisit the styling of the whole default control style
> if neccecary.
> 
> Same with this, I'd rather we keep the no-pen look for now :)

OK, that seems reasonable. I've removed those, and changed the volume control
slightly so it's consistent with the no-pen style.

Patch Soon :)

-- 
Configure bugmail: https://bugs.webkit.org/userprefs.cgi?tab=email
------- You are receiving this mail because: -------
You are the assignee for the bug.