[Spice-devel] [spice-gtk v2] gstaudio: check before setting mute/volume properties

[Snir Sheriber]

Hi,

On 1/18/19 7:07 PM, Victor Toso wrote:
> Hi,
>
> On Fri, Jan 18, 2019 at 11:53:28AM -0500, Frediano Ziglio wrote:
>>> From: Victor Toso <me at victortoso.com>
>>>
>>> Otherwise we can get warnings such as:
>>>
>>>      GLib-GObject-WARNING **: 16:10:00.857:
>>>      g_object_set_is_valid_property: object class 'GstAutoAudioSink' has no
>>>      property named 'volume'
>>>
>>> Signed-off-by: Victor Toso <victortoso at redhat.com>
>> The patch remove the warning... but who control the volume?
> We do! This is a message that host sent to us.
>
>> Is it just constant at the maximum?
> Did not understand the question!
>
>> Maybe we should detect and add a "volume" element to the
>> pipeline?
> That's doable but different. Better is to request how is actually
> in control to set the right volume/mute for that application. If
> we use volume element, we are doing it inside the pipeline.
>
>> Does GstAutoAudioSink forward the volume settings to the chosen
>> element (if the property is available) ?
> AFAIK, yes, the sink/src elements. This was happening with
> wasapi{sink, src} elements, same should happen with alsa based on
> Snir's patch.


Actually I've experienced the same issues with alsa, I'm not sure it's 
being forwarded, i planned
to check the option to add volume element as also Frediano has suggested.
fwiw I was also not sure if there is an element that has the volume\mute 
properties and doesn't
also implement this GstStreamVolume interface, but for sure it's good to 
hush those warnings :)

Snir.


> Btw, volume/mute for wasapi is ongoing
>
>      https://gitlab.freedesktop.org/gstreamer/gst-plugins-bad/issues/715
>
> So, we should check if volume/mute property exists as they are
> not really mandatory.
>
> Cheers,
>
>>> ---
>>>   src/spice-gstaudio.c | 32 +++++++++++++++++++++++---------
>>>   1 file changed, 23 insertions(+), 9 deletions(-)
>>>
>>> diff --git a/src/spice-gstaudio.c b/src/spice-gstaudio.c
>>> index 98c1f4f..51ff028 100644
>>> --- a/src/spice-gstaudio.c
>>> +++ b/src/spice-gstaudio.c
>>> @@ -368,10 +368,13 @@ static void playback_volume_changed(GObject *object,
>>> GParamSpec *pspec, gpointer
>>>       if (!e)
>>>           e = g_object_ref(p->playback.sink);
>>>   
>>> -    if (GST_IS_STREAM_VOLUME(e))
>>> +    if (GST_IS_STREAM_VOLUME(e)) {
>>>           gst_stream_volume_set_volume(GST_STREAM_VOLUME(e),
>>>           GST_STREAM_VOLUME_FORMAT_CUBIC, vol);
>>> -    else
>>> +    } else if (g_object_class_find_property(G_OBJECT_GET_CLASS (e),
>>> "volume") != NULL) {
>>>           g_object_set(e, "volume", vol, NULL);
>>> +    } else {
>>> +        g_warning("playback: ignoring volume change on %s",
>>> gst_element_get_name(e));
>>> +    }
>>>   
>>>       g_object_unref(e);
>>>   }
>>> @@ -394,8 +397,13 @@ static void playback_mute_changed(GObject *object,
>>> GParamSpec *pspec, gpointer d
>>>       if (!e)
>>>           e = g_object_ref(p->playback.sink);
>>>   
>>> -    if (GST_IS_STREAM_VOLUME(e))
>>> +    if (GST_IS_STREAM_VOLUME(e)) {
>>>           gst_stream_volume_set_mute(GST_STREAM_VOLUME(e), mute);
>>> +    } else if (g_object_class_find_property(G_OBJECT_GET_CLASS (e), "mute")
>>> != NULL) {
>>> +        g_object_set(e, "mute", mute, NULL);
>>> +    } else {
>>> +        g_warning("playback: ignoring mute change on %s",
>>> gst_element_get_name(e));
>>> +    }
>>>   
>>>       g_object_unref(e);
>>>   }
>>> @@ -427,10 +435,13 @@ static void record_volume_changed(GObject *object,
>>> GParamSpec *pspec, gpointer d
>>>       if (!e)
>>>           e = g_object_ref(p->record.src);
>>>   
>>> -    if (GST_IS_STREAM_VOLUME(e))
>>> +    if (GST_IS_STREAM_VOLUME(e)) {
>>>           gst_stream_volume_set_volume(GST_STREAM_VOLUME(e),
>>>           GST_STREAM_VOLUME_FORMAT_CUBIC, vol);
>>> -    else
>>> -        g_warning("gst lacks volume capabilities on src");
>>> +    } else if (g_object_class_find_property(G_OBJECT_GET_CLASS (e),
>>> "volume") != NULL) {
>>> +        g_object_set(e, "volume", vol, NULL);
>>> +    } else {
>>> +        g_warning("record: ignoring volume change on %s",
>>> gst_element_get_name(e));
>>> +    }
>>>   
>>>       g_object_unref(e);
>>>   }
>>> @@ -453,10 +464,13 @@ static void record_mute_changed(GObject *object,
>>> GParamSpec *pspec, gpointer dat
>>>       if (!e)
>>>           e = g_object_ref(p->record.src);
>>>   
>>> -    if (GST_IS_STREAM_VOLUME (e))
>>> +    if (GST_IS_STREAM_VOLUME (e)) {
>>>           gst_stream_volume_set_mute(GST_STREAM_VOLUME(e), mute);
>>> -    else
>>> -        g_warning("gst lacks mute capabilities on src: %d", mute);
>>> +    } else if (g_object_class_find_property(G_OBJECT_GET_CLASS (e), "mute")
>>> != NULL) {
>>> +        g_object_set(e, "mute", mute, NULL);
>>> +    } else {
>>> +        g_warning("record: ignoring mute change on %s",
>>> gst_element_get_name(e));
>>> +    }
>>>   
>>>       g_object_unref(e);
>>>   }
>>>
>>> _______________________________________________
>>> Spice-devel mailing list
>>> Spice-devel at lists.freedesktop.org
>>> https://lists.freedesktop.org/mailman/listinfo/spice-devel
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <https://lists.freedesktop.org/archives/spice-devel/attachments/20190120/26c7c9ba/attachment.html>