[Bug 777409] pluginbase: Query for OpenGL app_context

[GStreamer (GNOME Bugzilla)]

https://bugzilla.gnome.org/show_bug.cgi?id=777409

--- Comment #31 from Víctor Manuel Jáquez Leal <vjaquez at igalia.com> ---
(In reply to Víctor Manuel Jáquez Leal from comment #27)
> (In reply to Matt Fischer from comment #26)
> > It works for my use cases.  However there are a couple of things that seem
> > strange about this version:
> > 
> > * gst_vaapi_plugin_base_find_gl_context() now lives in gstvaapipluginutil.c.
> > Should it be renamed?
> > * Also gst_vaapi_plugin_base_find_gl_context() calls
> > gst_vaapi_plugin_base_create_gl_context(), it seems odd for a utility
> > function to assume that it's a PLUGIN_BASE.
> 
> Nice catch!

Done.

> 
> > Maybe it would be better to just move the contents of
> > gst_vaapi_ensure_display() into gst_vaapi_plugin_base_ensure_display()? 
> > That's the only place that calls it anyway.
> 
> That makes sense.

Giving a second thought, I prefer to keeping it, just for the semantic
distinction (base plugin vs any derived future plugin).

> > Also, the IS_VIDEO_SINK || IS_VIDEO_ENCODER check causes a change in the
> > previous behavior.  Before, it would always attempt to at least query for a
> > local context, now it will not perform that query if it's a sink or encoder.
> > If we really need to avoid doing the context construction for sinks and
> > encoders, would it be better to put that check around the
> > gst_vaapi_plugin_base_create_gl_context() call instead, so that the previous
> > behavior is retained?
> 
> Good insight. We would need to split those patches in order to merge it to
> branch 1.10 without modifying the behavior in the stable branch.

I'm having doubts about merging this in 1.10, since it is a big change in
functionality.

> 
> > I'm not sure I quite understand why we need to exclude sinks and encoders in
> > the first place, though.  Now that you have the check to reject
> > default-created GL display, shouldn't it be safe to use this code for all
> > VAAPI plugins?
> 
> If vaapisink is used, the va display created for it shall be used in the
> pipeline. In the case of the encoders now I have doubts.

I have added a comment on that to explain it.

-- 
You are receiving this mail because:
You are the QA Contact for the bug.
You are the assignee for the bug.