> > Hi > > On Wed, Jan 16, 2019 at 3:38 PM Frediano Ziglio <fziglio@xxxxxxxxxx> wrote: > > > > > > > > Hi > > > > > > On Wed, Jan 16, 2019 at 3:14 PM Frediano Ziglio <fziglio@xxxxxxxxxx> > > > wrote: > > > > > > > > > > > > > > Hi, > > > > > > > > > > On Wed, Jan 16, 2019 at 12:43:06PM +0400, Marc-André Lureau wrote: > > > > > > Hi > > > > > > > > > > > > > > > > > > On Fri, Jan 11, 2019 at 3:03 PM <marcandre.lureau@xxxxxxxxxx> > > > > > > wrote: > > > > > > > > > > > > > > From: Marc-André Lureau <marcandre.lureau@xxxxxxxxxx> > > > > > > > > > > > > > > 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@xxxxxxxxxx> > > > > > > > > > > > > I think we agreed that this is the way to go. > > > > > > > > > > > > ack for v0.36, or should we continue the discussion for v0.37? > > > > > > > > > > ack for v0.36 > > > > > > > > > > > > > > > > > > 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, µ) != 3) { > > > > I think this test will fail with versions like 1.15 too. > > Maybe a > > > > unsigned major, minor = 0, micro = 0; > > > > if (sscanf(gst_plugin_get_version(plugin), "%u.%u.%u", &major, &minor, > > µ) < 2) { > > .... > > GStreamer versions are always at least 3 digits afaik. Other versions, > we can just ignore. > Not much documentation at https://gstreamer.freedesktop.org/data/doc/gstreamer/head/gstreamer/html/GstPlugin.html#GST-PLUGIN-DEFINE:CAPS The current code is not actually ignoring, it would return NULL so disabling GstAudio. > > > > OT: is it worth this code on Windows ? > > PA used to build on windows. I don't know today. > > > > > > > > > > + g_warn_if_reached(); > > > > > > > + gst_object_unref(plugin); > > > > > > > + gst_object_unref(pulsesrc); > > > > > > > + return NULL; > > > > > > > + } > > > > > > > + > > > > > > > + 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, > > > > > > > > Looks like the version merged in master is quite different from this > > > > version, > > > > in particular: > > > > > > > > if (maj < 1 || min < 15) { > > > > g_warning("Bad pulsesrc version, lowering its rank"); > > > > gst_plugin_feature_set_rank(pulsesrc, GST_RANK_NONE); > > > > } > > > > > > > > will lower the rank even if the version is 2.0 which probably is higher > > > > than 1.15. > > > > > > > > By the way, it looks like you pushed the wrong patch, like this old one > > > > https://lists.freedesktop.org/archives/spice-devel/2019-January/047138.html > > > > which was not acked. > > > > > > yes, wrong version, my bad! > > > > > > Thanks for noticing!! > > > > > > > Maybe more clean to revert the pushed one and merge an updated one? > > > > Frediano > > > > -- > Marc-André Lureau > _______________________________________________ Spice-devel mailing list Spice-devel@xxxxxxxxxxxxxxxxxxxxx https://lists.freedesktop.org/mailman/listinfo/spice-devel