Bug 117383 – [GStreamer] Add handling for GstContext

• Status: RESOLVED
• Resolution: INVALID
• Component: Media

[Sebastian Dröge (slomo)]

The GStreamer backend should add generic handling for GstContext, which is new API in 1.1/1.2 that allows elements inside the pipeline to share contexts (e.g. display handles). The application needs to merge and propagate element contexts and set them on the pipeline.

Patch follows later.

[Víctor M. Jáquez L.]

yupi!!!

you may like to see this bug/patch too

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

[Víctor M. Jáquez L.]

Created attachment 206898 [details]
Patch

[Martin Robinson]

Comment on attachment 206898 [details]
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=206898&action=review

Seems reasonable, but I think a smart pointer is the way to go here.

> Source/WebCore/platform/graphics/gstreamer/MediaPlayerPrivateGStreamer.cpp:156
> +    GstStructure* s2 = reinterpret_cast<GstStructure*>(userData);

Please don't use obscure abbreviations for variable names.

> Source/WebCore/platform/graphics/gstreamer/MediaPlayerPrivateGStreamer.cpp:269
> +        gst_context_unref(m_context);

Looks like this could be a RefPtr specialization which would allow you to avoid the initialization and destruction of this member.

> Source/WebCore/platform/graphics/gstreamer/MediaPlayerPrivateGStreamer.cpp:858
> +            s1 = gst_context_get_structure(newContext);
> +            s2 = gst_context_writable_structure(m_context);

Please make these variable names real words and declare them when you first define them.

[Víctor M. Jáquez L.]

Comment on attachment 206898 [details]
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=206898&action=review

>> Source/WebCore/platform/graphics/gstreamer/MediaPlayerPrivateGStreamer.cpp:269
>> +        gst_context_unref(m_context);
> 
> Looks like this could be a RefPtr specialization which would allow you to avoid the initialization and destruction of this member.

That was my first approach, but I couldn't find a way to handle the gst_context_relace() used below :/ I'll dig on it

[Sebastian Dröge (slomo)]

For all this note that the GstContext API and expected usage is still WIP, I'm not 100% happy with it yet. Should stabilize over the next weeks though, now that I'm porting gst-plugins-gl to it.

[Martin Robinson]

(In reply to comment #5)
> For all this note that the GstContext API and expected usage is still WIP, I'm not 100% happy with it yet. Should stabilize over the next weeks though, now that I'm porting gst-plugins-gl to it.

Maybe we should wait until the API stabilizes, to avoid shipping something that doesn't work with certain GStreamer versions then.

[Víctor M. Jáquez L.]

(In reply to comment #6)
> (In reply to comment #5)
> > For all this note that the GstContext API and expected usage is still WIP, I'm not 100% happy with it yet. Should stabilize over the next weeks though, now that I'm porting gst-plugins-gl to it.
> 
> Maybe we should wait until the API stabilizes, to avoid shipping something that doesn't work with certain GStreamer versions then.

Agreed.

[Sebastian Dröge (slomo)]

No application-level handling of GstContext is necessary anymore in 1.1.90 or later. Applications can override the default handling though, but WebKit does not need that.