[Spice-devel] [PATCH spice-gtk v3] gst: check pulsesrc version >= 1.14.5

[Marc-André Lureau]

Hi

On Mon, Jan 14, 2019 at 3:26 PM Snir Sheriber <ssheribe at redhat.com> wrote:
>
> Hi,
>
>
> On 1/13/19 6:31 PM, Marc-André Lureau wrote:
> > Hi
> >
> > On Sun, Jan 13, 2019 at 1:13 PM Snir Sheriber <ssheribe at redhat.com> wrote:
> >>
> >> On 1/11/19 1:03 PM, marcandre.lureau at redhat.com 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 is fixed with 1.15, and will be backported to upcoming 1.14.5.
> >>>
> >>> PulseAudio may not be picked by autoaudiosrc, but looking up the
> >>> actual source or mimicking GstAutoDetect is unnecessarily complicated.
> >>>
> >>> When pulsesrc < 1.14.5 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 | 28 ++++++++++++++++++++++++++++
> >>>    1 file changed, 28 insertions(+)
> >>>
> >>> diff --git a/src/spice-gstaudio.c b/src/spice-gstaudio.c
> >>> index d0cfbc6..49e9ae6 100644
> >>> --- a/src/spice-gstaudio.c
> >>> +++ b/src/spice-gstaudio.c
> >>> @@ -527,7 +527,35 @@ 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 major, minor, micro;
> >>> +            GstPlugin *plugin = gst_plugin_feature_get_plugin(pulsesrc);
> >>> +
> >>> +            if (sscanf(gst_plugin_get_version(plugin), "%u.%u.%u",
> >>> +                       &major, &minor, &micro) != 3) {
> >>> +                g_warn_if_reached();
> >>> +                gst_object_unref(plugin);
> >>> +                gst_object_unref(pulsesrc);
> >>> +                return NULL;
> >> In that case plugin exists but version is unclear, isn't it preferable
> >> to just setting a low rank as well?
> > I don't think we should touch it in that case, as we don't know about it.
> >
> >> Looks good to me other than that
> > ack, then?
>
>
> Actually after given it another look and a test it seems that spice
> gstaudio code is
> highly pulse-audio plugin oriented, when pulse is disabled a lot of
> warnings and
> errors are emitted, some functions and properties are not supported too.
> we may
> want to consider fallback to native-pulse when < 1.14.5 instead of
> setting it to NONE rank.

We decided to keep spice pulse as the default backend for v0.36.

GStreamer is second choice, and if pulsesrc < 1.14.5 is detected, it
is set as lower rank. But if other backends are equally bad, perhaps
it shouldn't do that.

So far, gstreamer is known to work well on Windows with directsound.
With the following release of virt-viewer, it's probably going to pick
wasapi.

On Linux, we should check that pulse and alsa work well enough. In all
cases, I don't think spice-gtk should try to make too much guesses
about gstreamer support.


>
> (for example see "[PATCH spice-gtk] gstaudio: set object also when
> GstStreamVolume is not implemented" )
>
>
> Snir.
>
>
> >
> > thanks
> >
> >>
> >> Snir.
> >>
> >>> +            }
> >>> +
> >>> +            if (major < 1 ||
> >>> +                (major == 1 && minor < 14) ||
> >>> +                (major == 1 && minor == 14 && micro < 5)) {
> >>> +                g_warning("Bad pulsesrc version %s, lowering its rank",
> >>> +                          gst_plugin_get_version(plugin));
> >>> +                gst_plugin_feature_set_rank(pulsesrc, GST_RANK_NONE);
> >>> +            }
> >>> +
> >>> +            gst_object_unref(plugin);
> >>> +            gst_object_unref(pulsesrc);
> >>> +        }
> >>> +
> >>>            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