[Webkit-unassigned] [Bug 117039] [GStreamer] Support audio and video tracks

[bugzilla-daemon at webkit.org]

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


Philippe Normand <pnormand at igalia.com> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
 Attachment #205340|review?                     |review-
               Flag|                            |




--- Comment #37 from Philippe Normand <pnormand at igalia.com>  2013-07-25 03:25:48 PST ---
(From update of attachment 205340)
View in context: https://bugs.webkit.org/attachment.cgi?id=205340&action=review

Looks quite good but r- mostly because of the tests issue

> Source/WebCore/ChangeLog:8
> +        No new tests (OOPS!).

Can we land this along with the new tests? It'd be really good to do so.

> Source/WebCore/platform/graphics/gstreamer/AudioTrackPrivateGStreamer.cpp:130
> +    gchar* tagValue;

I think a GOwnPtr<gchar> can be used here. So no g_free call needed below.

> Source/WebCore/platform/graphics/gstreamer/MediaPlayerPrivateGStreamer.cpp:606
> +        g_signal_emit_by_name(m_playBin.get(), "get-video-pad", i, &pad, NULL);

I think the pad needs to be unreffed after usage. Can you please double-check it's not leaked?

> Source/WebCore/platform/graphics/gstreamer/MediaPlayerPrivateGStreamer.cpp:609
> +        if (i < (gint)m_videoTracks.size()) {

I'm ok with a cast but not C-style please ;)

> Source/WebCore/platform/graphics/gstreamer/MediaPlayerPrivateGStreamer.cpp:621
> +    while ((gint)m_videoTracks.size() > numTracks) {

Ditto

> Source/WebCore/platform/graphics/gstreamer/MediaPlayerPrivateGStreamer.cpp:659
> +        g_signal_emit_by_name(m_playBin.get(), "get-audio-pad", i, &pad, NULL);

Same remarks about pad unref and casts

> Source/WebCore/platform/graphics/gstreamer/VideoTrackPrivateGStreamer.cpp:133
> +    gchar* tagValue;

Same here, if a GOwnPtr<gchar> can be used it'd be better.

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