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

[Snir Sheriber]

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.

(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
>
>