[Webkit-unassigned] [Bug 77089] [GStreamer] 0.11 support in MediaPlayerPrivateGStreamer

[bugzilla-daemon at webkit.org]

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





--- Comment #6 from Martin Robinson <mrobinson at webkit.org>  2012-01-30 08:47:51 PST ---
(From update of attachment 124094)
View in context: https://bugs.webkit.org/attachment.cgi?id=124094&action=review

Nice stuff.

> Source/WebCore/platform/graphics/gstreamer/GRefPtrGStreamer.cpp:43
> +#ifdef GST_API_VERSION_1
> +        gst_object_ref_sink(GST_OBJECT(ptr));
> +#else
>          gst_object_ref(GST_OBJECT(ptr));
>          gst_object_sink(GST_OBJECT(ptr));
> +#endif
>      }

It's probably cleaner to just abstract this away into a helper function that does the right thing for each GStreamer. Imagine something like GtkVersioning.h.

> Source/WebCore/platform/graphics/gstreamer/MediaPlayerPrivateGStreamer.cpp:53
> +#ifdef GST_API_VERSION_1
> +#include <gst/audio/streamvolume.h>
> +#else
>  #include <gst/interfaces/streamvolume.h>
> +#endif

Headers included behind ifdefs usually go in their own block.

> Source/WebCore/platform/graphics/gstreamer/MediaPlayerPrivateGStreamer.cpp:78
> +#ifdef GST_API_VERSION_1
> +#define PLAYBIN_NAME "playbin"
> +#else
> +#define PLAYBIN_NAME "playbin2"
> +#endif

Please use a const char* instead of a #define.

static const char* gPlaybinName = "playbin"

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