>
> From: Victor Toso <me@xxxxxxxxxxxxxx>
>
> 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 <victortoso@xxxxxxxxxx>
> ---
> 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.
> +
> /* 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.
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
_______________________________________________
Spice-devel mailing list
Spice-devel@xxxxxxxxxxxxxxxxxxxxx
https://lists.freedesktop.org/mailman/listinfo/spice-devel