Hi, On Sun, Jan 27, 2019 at 06:25:18AM -0500, Frediano Ziglio wrote: > > > > From: Victor Toso <[email protected]> > > > > Runtime check is better fit to enable/disable vaapisink. For that, use > > spice_check_gst_plugin_version() from previous commit. > > > > Signed-off-by: Victor Toso <[email protected]> > > --- > > src/channel-display-gst.c | 25 +++++++++++-------------- > > 1 file changed, 11 insertions(+), 14 deletions(-) > > > > diff --git a/src/channel-display-gst.c b/src/channel-display-gst.c > > index 4272ade..ef578db 100644 > > --- a/src/channel-display-gst.c > > +++ b/src/channel-display-gst.c > > @@ -21,6 +21,7 @@ > > #include "spice-common.h" > > #include "spice-channel-priv.h" > > > > +#include "spice-session-priv.h" > > #include "channel-display-priv.h" > > > > #include <gst/gst.h> > > @@ -431,6 +432,7 @@ static gboolean create_pipeline(SpiceGstDecoder > > *decoder) > > GstElement *playbin, *sink; > > SpiceGstPlayFlags flags; > > GstCaps *caps; > > + GError *err = NULL; > > > > playbin = gst_element_factory_make("playbin", "playbin"); > > if (playbin == NULL) { > > @@ -461,29 +463,24 @@ static gboolean create_pipeline(SpiceGstDecoder > > *decoder) > > NULL); > > > > decoder->appsink = GST_APP_SINK(sink); > > - } else { > > + } else if (!spice_check_gst_plugin_version("vaapisink", 1, 14, 0, > > &err)) > > { > > + GstPluginFeature *vaapisink; > > I would update the style and declare and initialise on the same > statement. > Time to move to a more C99 style, no reason to stick to C89.
Right, I don't mind.
> > +
> > /* handle has received, it means playbin will render directly into
> > * widget using the gstvideooverlay interface instead of app-sink.
> > */
> > SPICE_DEBUG("Video is presented using gstreamer's GstVideoOverlay
> > interface");
> >
> > -#if !GST_CHECK_VERSION(1,14,0)
> > /* Avoid using vaapisink if exist since vaapisink could be
> > * buggy when it is combined with playbin. changing its rank to
> > * none will make playbin to avoid of using it.
> > */
> > - GstRegistry *registry = NULL;
> > - GstPluginFeature *vaapisink = NULL;
> > -
> > - registry = gst_registry_get();
> > - if (registry) {
> > - vaapisink = gst_registry_lookup_feature(registry, "vaapisink");
> > - }
> > - if (vaapisink) {
> > - gst_plugin_feature_set_rank(vaapisink, GST_RANK_NONE);
> > - gst_object_unref(vaapisink);
> > - }
> > -#endif
> > + vaapisink = gst_registry_lookup_feature(gst_registry_get(),
> > "vaapisink");
>
> It seems every time you call spice_check_gst_plugin_version you
> are going to use the plugin feature pointer, maybe
> spice_check_gst_plugin_version should return the pointer if
> successful.
Sure, i'll send a v2 series soon.
> Note that assuming that gst_registry_lookup_feature will return
> not NULL is racy and can lead to crashes, this would prevent
> this bug (although rare).
> > + gst_plugin_feature_set_rank(vaapisink, GST_RANK_NONE);
> > + g_clear_object(&vaapisink);
> > + } else if (err != NULL) {
> > + g_warning("Failure while checking vaapisink version: %s",
> > err->message);
> > + g_clear_error(&err);
> > }
> >
> > g_signal_connect(playbin, "deep-element-added",
> > G_CALLBACK(deep_element_added_cb), decoder);
>
> Frediano
Cheers,
signature.asc
Description: PGP signature
_______________________________________________ Spice-devel mailing list [email protected] https://lists.freedesktop.org/mailman/listinfo/spice-devel
