[Bug 1051559]

[Cpearce-t]

Comment on attachment 763014
Enable gstreamer by default, preffed off

Review of attachment 763014:
-----------------------------------------------------------------

r=cpearce for the changes other than those in configure.in. I'll leave
that to khuey.

::: configure.in
@@ +5754,5 @@
>  dnl ========================================================
>  dnl = Enable GStreamer
>  dnl ========================================================
> +
> +if test "$OS_TARGET" = "Linux"; then

Doesn't this mean you're turning on building GStreamer on Linux, but not
providing a way to disable it since you only have an --enable-gstreamer
option? You should have way to disable gstreamer support, for Linux
systems without GStreamer, or those who choose to not have it.

::: content/media/DecoderTraits.cpp
@@ +326,5 @@
>      result = CANPLAY_YES;
>    }
>  #endif
>  #ifdef MOZ_GSTREAMER
> +  if (MediaDecoder::IsGStreamerEnabled() &&

Put the MediaDecoder::IsGStreamerEnabled() check inside
GStreamerDecoder::CanHandleMediaType(). It can't handle anything if it's
disabled right?

::: content/media/test/manifest.js
@@ +680,3 @@
>    var oldOpus = undefined;
>    try {
> +    oldGStreamer = branch.getBoolPref("gstreamer.enabled");

IIRC getBoolPref throws if a pref isn't found. Meaning that we won't
check the defaults for the other prefs, if say gstreamer.enabled isn't
defined on the platform the test is running on (i.e. Windows).

Please put each of these get*Pref calls inside their own try{}catch(){}
block.

-- 
You received this bug notification because you are a member of Mozilla
Bugs, which is subscribed to firefox in Ubuntu.
https://bugs.launchpad.net/bugs/1051559

Title:
  Build Firefox with GStreamer support

To manage notifications about this bug go to:
https://bugs.launchpad.net/firefox/+bug/1051559/+subscriptions