[Spice-devel] [PATCH spice-gtk v2 03/15] gst: check pulsesrc version >= 1.15

[Marc-André Lureau]

On Thu, Jan 10, 2019 at 12:58 PM Snir Sheriber <ssheribe at redhat.com> wrote:
>
> Hi,
>
> On 1/9/19 5:59 PM, Marc-André Lureau wrote:
> > Hi
> >
> > On Wed, Jan 9, 2019 at 5:31 PM Snir Sheriber <ssheribe at redhat.com> wrote:
> >>
> >> On 1/9/19 12:09 PM, Frediano Ziglio wrote:
> >>> From: Marc-André Lureau <marcandre.lureau at redhat.com>
> >>>
> >>> There is a racy bug in pulsesrc that we can't easily workaround:
> >>> https://gitlab.freedesktop.org/gstreamer/gst-plugins-good/merge_requests/69
> >>>
> >>> It will hopefully be fixed with 1.15.
> >>>
> >>> PulseAudio may not be picked by autoaudiosrc, but looking up the
> >>> actual source or mimicking GstAutoDetect is unnecessarily complicated.
> >>>
> >>> When pulsesrc < 1.15 is detected, let's drop its rank, so it won't be picked.
> >>>
> >>> Signed-off-by: Marc-André Lureau <marcandre.lureau at redhat.com>
> >>> ---
> >>>    src/spice-gstaudio.c | 23 +++++++++++++++++++++++
> >>>    1 file changed, 23 insertions(+)
> >>>
> >>> diff --git a/src/spice-gstaudio.c b/src/spice-gstaudio.c
> >>> index aff69d27..40050007 100644
> >>> --- a/src/spice-gstaudio.c
> >>> +++ b/src/spice-gstaudio.c
> >>> @@ -527,7 +527,30 @@ SpiceGstaudio *spice_gstaudio_new(SpiceSession *session, GMainContext *context,
> >>>                                      const char *name)
> >>>    {
> >>>        GError *err = NULL;
> >>> +
> >>>        if (gst_init_check(NULL, NULL, &err)) {
> >>> +        GstPluginFeature *pulsesrc;
> >>> +
> >>> +        pulsesrc = gst_registry_lookup_feature(gst_registry_get(), "pulsesrc");
> >>> +        if (pulsesrc) {
> >>> +            unsigned maj, min;
> >>> +            GstPlugin *plugin = gst_plugin_feature_get_plugin(pulsesrc);
> >>> +
> >>> +            if (sscanf(gst_plugin_get_version(plugin), "%u.%u", &maj, &min) != 2) {
> >> Hi,
> >>
> >>
> >> IIUC !GST_CHECK_VERSION could be used here, since plugin version must
> >> match gstreamer core version,
> >> If it doesn't this plugin won't be loaded anyway
> > Really? I didn't know that, do you have a pointer? Or I will look at
> > gstreamer code.
>
>
> I saw it mentioned somewhere but it also kind of being said here:
>
> https://gstreamer.freedesktop.org/data/doc/gstreamer/head/gstreamer/html/gstreamer-GstVersion.html
>
> "The GStreamer runtime checks if these plugin and core version match and
> refuses to use a plugin compiled against a different version of GStreamer"

fwiw Here is the function:

 464 gst_plugin_check_version (gint major, gint minor)
 465 {
 466   /* return NULL if the major and minor version numbers are not
compatible */
 467   /* with ours. */
 468   if (major != GST_VERSION_MAJOR || minor > GST_VERSION_MINOR)
 469     return FALSE;
 470
 471   return TRUE;
 472 }

>
>
> > I was thinking I would augment the runtime check for upcoming 1.14.X
> > when it has the fix. GST_CHECK_VERSION wouldn't be enough then.
>
>
> GST_CHECK_VERSION checks for micro version as well
>
>
> Snir.
>
> >
> >> Snir.
> >>
> >>
> >>> +                g_warn_if_reached();
> >>> +                gst_object_unref(plugin);
> >>> +                gst_object_unref(pulsesrc);
> >>> +                return NULL;
> >>> +            }
> >>> +
> >>> +            gst_object_unref(plugin);
> >>> +            gst_object_unref(pulsesrc);
> >>> +            if (maj < 1 || min < 15) {
> >>> +                g_warning("Bad pulsesrc version, lowering its rank");
> >>> +                gst_plugin_feature_set_rank(pulsesrc, GST_RANK_NONE);
> >>> +            }
> >>> +        }
> >>> +
> >>>            return g_object_new(SPICE_TYPE_GSTAUDIO,
> >>>                                "session", session,
> >>>                                "main-context", context,
> >> _______________________________________________
> >> Spice-devel mailing list
> >> Spice-devel at lists.freedesktop.org
> >> https://lists.freedesktop.org/mailman/listinfo/spice-devel
> >
> >



-- 
Marc-André Lureau