[Webkit-unassigned] [Bug 103771] [GStreamer] support in-band text tracks

[bugzilla-daemon at webkit.org]

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





--- Comment #151 from Eric Carlson <eric.carlson at apple.com>  2013-08-30 11:05:42 PST ---
(In reply to comment #150)
> I made most of the changes suggested, except:
> 
> (In reply to comment #149)
> > > Source/WebCore/platform/graphics/gstreamer/InbandTextTrackPrivateGStreamer.cpp:201
> > > +        m_label = emptyAtom;
> > > +        m_language = emptyAtom;
> > 
> > If the track previously had a label/language, InbandTextTrack will now be out of sync.
> 
> I fixed this so it sends events when the label and language change, instead of every time we see non-empty tags:
> 
>     String label;
>     String language;
>     if (tags) {
>         gchar* tagValue;
>         if (gst_tag_list_get_string(tags, GST_TAG_TITLE, &tagValue)) {
>             INFO_MEDIA_MESSAGE("Track %d got title %s.", m_index, tagValue);
>             label = tagValue;
>             g_free(tagValue);
>         }
> 
>         if (gst_tag_list_get_string(tags, GST_TAG_LANGUAGE_CODE, &tagValue)) {
>             INFO_MEDIA_MESSAGE("Track %d got language %s.", m_index, tagValue);
>             language = tagValue;
>             g_free(tagValue);
>         }
>     }
> 
>     if (m_label != label) {
>         m_label = label;
>         client()->labelChanged(this, m_label);
>     }
> 
>     if (m_language != language) {
>         m_language = language;
>         client()->languageChanged(this, m_language);
>     }
> 
Much cleaner, thanks.

> > > Source/WebCore/platform/graphics/gstreamer/MediaPlayerPrivateGStreamer.cpp:645
> > > +    m_textTimerHandler = 0;
> > 
> > Do you not need to call g_source_remove() here as well?
> 
> No, `mediaPlayerPrivateTextChangeTimeoutCallback` returns FALSE. From the documentation for g_timeout_add:
> 
> > The function is called repeatedly until it returns FALSE, at which point the timeout is automatically destroyed and the function will not be called again. 
> 
> For these functions, I followed what was already being done in MediaPlayerPrivateGStreamer, but it's possible that this:
> 
>     void MediaPlayerPrivateGStreamer::textChanged()
>     {
>         if (m_textTimerHandler)
>             g_source_remove(m_textTimerHandler);
>         m_textTimerHandler = g_timeout_add(0, reinterpret_cast<GSourceFunc>(mediaPlayerPrivateTextChangeTimeoutCallback), this);
>     }
> 
> Could be this:
> 
>     void MediaPlayerPrivateGStreamer::textChanged()
>     {
>         if (!m_textTimerHandler)
>             m_textTimerHandler = g_timeout_add(0, reinterpret_cast<GSourceFunc>(mediaPlayerPrivateTextChangeTimeoutCallback), this);
>     }
> 
> I *think* rescheduling the event is useful though, since we tend to get all of the tracks at once, and pushing the event back every time we see one lets us consolidate all of those into one event.

I know essentially nothing about the GTK API, so it was just an idle question. Thanks for the explanation!

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