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

[bugzilla-daemon at webkit.org]

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





--- Comment #39 from Brendan Long <b.long at cablelabs.com>  2013-07-25 11:05:49 PST ---
(In reply to comment #37)
> (From update of attachment 205340 [details])
> View in context: https://bugs.webkit.org/attachment.cgi?id=205340&action=review
> 
> Looks quite good but r- mostly because of the tests issue

Yes, makes sense. I've been meaning to get tests in but I've been distracted by a lot of stuff going on at work, and the in-band tracks.

I think this also can't be merged until input-selector works correctly. Right now, it takes a *long* time to switch streams.

> > 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.

This doesn't really simplify the code, since I still need to clear() the tagValue before I can reuse it because of GOwnPtr::outPtr():

T*& outPtr()
{
    ASSERT(!m_ptr); // <--
    return m_ptr;
}

The code would look like this:

    GOwnPtr<gchar> tagValue;
    if (gst_tag_list_get_string(tags, GST_TAG_TITLE, &tagValue.outPtr())) {
        m_label = tagValue.get();
        tagValue.clear();
        client()->audioTrackPrivateLabelChanged(this);
    }
    if (gst_tag_list_get_string(tags, GST_TAG_LANGUAGE_CODE, &tagValue.outPtr())) {
        m_language = tagValue.get();
        client()->audioTrackPrivateLanguageChanged(this);
    }

And even then we might want a clear() in the second section, so it doesn't confuse someone in the future.. I think the g_free() is simpler in this case, but it's up to you.

> > 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?

Yes that's right. Playbin refs it before returning it. I fixed it to unref it if it's an already-used pad, or adopt it when we create a track.

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