[Bug 704321] Add complete support for GstContext

[GStreamer (bugzilla.gnome.org)]

https://bugzilla.gnome.org/show_bug.cgi?id=704321
  GStreamer | gst-plugins-gl | git

--- Comment #12 from Matthew Waters <ystreet00 at gmail.com> 2013-11-06 14:22:48 UTC ---
(In reply to comment #10)
> Review of attachment 257937 [details]:
> 
> ::: gst-libs/gst/gl/gstgldisplay.c
> @@ +156,2 @@
>    s = gst_context_writable_structure (context);
>    gst_structure_set (s, GST_GL_DISPLAY_CONTEXT_TYPE, GST_TYPE_GL_DISPLAY,
> 
> You might want to use "display" or a shorter string here. The long complex type
> string is used as the name of the context now.

Ok

> ::: gst-libs/gst/gl/gstgldisplay.h
> @@ +77,3 @@
>  gboolean gst_context_get_gl_display (GstContext * context, GstGLDisplay **
> display);
> 
> +gboolean gst_gl_display_replace (GstGLDisplay **oldobje, GstGLDisplay
> *newobj);
> 
> We don't do such things usually, just cast in the code that uses it to
> GstObject** and GstObject*

Ok

> ::: gst-libs/gst/gl/gstglfilter.c
> @@ +219,3 @@
> +          gst_structure_has_name (structure, "gstglcontext")) {
> +        gst_structure_set (structure, "gstglcontext", G_TYPE_POINTER,
> +            filter->context, NULL);
> 
> This custom query is not needed anymore?

Soon :). That's to be moved to the ALLOCATION query.  Still needed currently
because GstGLDisplay does not hold onto the GstGLContext.

> @@ +289,3 @@
> +
> +  structure = gst_structure_new_empty ("gstglcontext");
> +  context_query = gst_query_new_custom (GST_QUERY_CUSTOM, structure);
> 
> This should be done with the context query

See above

> ::: gst-libs/gst/gl/gstglmixer.c
> @@ +379,3 @@
> +      if (gst_structure_has_name (structure, "gstglcontext")) {
> +        gst_structure_set (structure, "gstglcontext", G_TYPE_POINTER,
> +            mix->context, NULL);
> 
> Not needed anymore
> 
> @@ +952,3 @@
> +
> +    structure = gst_structure_new_empty ("gstglcontext");
> +    context_query = gst_query_new_custom (GST_QUERY_CUSTOM, structure);
> 
> Use context query
> 
> ::: gst-libs/gst/gl/gstglutils.c
> @@ +474,3 @@
> +  }
> +
> +  if (ctxt) {
> 
> If the context is set as result of the need-context message, nothing will put
> it into ctxt and this function won't return the display

Hmm, this will probably require intervention from the specific element now that
gst_element_get_context has disappeared.  Just to make sure this is the correct
order of proceedings for the need-context message.

1. Element posts need-context with a type
2. Some bin/pipeline containing the element can respond if it already has that
context
3. Else the application is given a chance (via the sync bus) to respond
4. If not successful create our own and propogate via have-context

The only way that I can find that the app/bin/pipeline can respond with is via
gst_element_set_context (which we currently do not implement).

However, we do not really care that ctxt has been set or not, only that
*display_ptr has been set.  Which, assuming that the element updates the
pointer display_ptr points to in a gst_element_set_context, everything's fine.

> ::: gst/gl/gstglimagesink.c
> @@ +408,3 @@
> +      if (gst_structure_has_name (structure, "gstglcontext")) {
> +        gst_structure_set (structure, "gstglcontext", G_TYPE_POINTER,
> +            glimage_sink->context, NULL);
> 
> Not needed anymore
> 
> ::: gst/gl/gstgltestsrc.c
> @@ +552,3 @@
> +
> +  structure = gst_structure_new_empty ("gstglcontext");
> +  context_query = gst_query_new_custom (GST_QUERY_CUSTOM, structure);
> 
> Use context query

P.S we don't handle the context query in elements correctly.

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