[Bug 777409] pluginbase: Query for OpenGL app_context

[GStreamer (GNOME Bugzilla)]

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

--- Comment #27 from Víctor Manuel Jáquez Leal <vjaquez at igalia.com> ---
(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!

> 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.

> 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 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.

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