[Webkit-unassigned] [Bug 138562] [GStreamer] GstGL support in the video sink

[bugzilla-daemon at webkit.org]

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

--- Comment #34 from Philippe Normand <pnormand at igalia.com> ---
(In reply to comment #33)
> > Source/WebCore/platform/graphics/gstreamer/MediaPlayerPrivateGStreamerBase.cpp:354
> > +    g_return_if_fail(GST_IS_SAMPLE(sample));
> 
> Is it really possible that the client-draw signal is emitted with a NULL or
> invalid sample?
> 

No, I'll remove this g_return_if_fail.

> > Source/WebCore/platform/graphics/gstreamer/MediaPlayerPrivateGStreamerBase.cpp:360
> > +        if (m_sample)
> > +            gst_sample_unref(m_sample);
> > +        m_sample = gst_sample_ref(sample);
> 
> m_sample should probably be a GRefPtr
> 

Ok I can do that in a separate patch.

> > Source/WebCore/platform/graphics/gstreamer/MediaPlayerPrivateGStreamerBase.cpp:369
> > +            if (supportsAcceleratedRendering() && m_player->client().mediaPlayerRenderingCanBeAccelerated(m_player) && client())
> 
> I would check client() is not nullptr, before asking the m_player->client().
> 

I checked MediaPlayer, the client is never null there and no one reported a crash about this either.

> > Source/WebCore/platform/graphics/gstreamer/MediaPlayerPrivateGStreamerBase.cpp:373
> > +        }, std::chrono::milliseconds(0), G_PRIORITY_HIGH, nullptr, g_main_context_get_thread_default());
> 
> What's the point of scheduling after a 0 delay? Why not using schedule()
> instead? G_PRIORITY_HIGH is too much, it will affect any other source in the
> context.
> 

Oh yeah that's what I did first, I'll switch back to a simple schedule :)

> > Source/WebCore/platform/graphics/gstreamer/MediaPlayerPrivateGStreamerBase.cpp:374
> > +        g_cond_wait(&m_drawCondition, &m_drawMutex);
> 
> Why does this need to be blocking? what thread is this supposed to be run?
> We should probably add an ASSERT(isMainThread()) or !isMainThread.
> 

This signal is emitted from the sink streaming thread but the texture can't be used from there, hence the switch to main thread. And it needs to be blocking because the sample emitted is freed from the sink after the signal emission completed.

-- 
You are receiving this mail because:
You are the assignee for the bug.
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.webkit.org/pipermail/webkit-unassigned/attachments/20150324/e61cb4fd/attachment-0002.html>