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

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



Hi,

On Sun, Jan 20, 2019 at 10:57:48AM +0200, Snir Sheriber wrote:
> 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@xxxxxxxxxxxxxx>
> > > > 
> > > > 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@xxxxxxxxxx>
> > > 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.

Problem with that is we would introduce another point to control
volume. It would be great if we could keep the host messages
setting the application volume in the client OS, that seems to
work well with pulseaudio based systems and windows using
DirectSound plugins.

For alsa and wasapi, indeed it is lacking. For wasapi, there is
ongoing MR for that but not sure if there was ever
interest/support for this on alsa (I recall that they had
volume/mute controls per device, not per applications).

If this becomes really a problem with alsa users, we could indeed
consider some alternative like volume element.

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

True :)

> but for sure it's good to hush those warnings :)

Thanks,

> 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@xxxxxxxxxxxxxxxxxxxxx
> > > > https://lists.freedesktop.org/mailman/listinfo/spice-devel

> _______________________________________________
> Spice-devel mailing list
> Spice-devel@xxxxxxxxxxxxxxxxxxxxx
> https://lists.freedesktop.org/mailman/listinfo/spice-devel

Attachment: signature.asc
Description: PGP signature

_______________________________________________
Spice-devel mailing list
Spice-devel@xxxxxxxxxxxxxxxxxxxxx
https://lists.freedesktop.org/mailman/listinfo/spice-devel

[Index of Archives]     [Linux Virtualization]     [Linux Virtualization]     [Linux ARM Kernel]     [Linux ARM]     [Linux Omap]     [Fedora ARM]     [IETF Annouce]     [Security]     [Bugtraq]     [Linux OMAP]     [Linux MIPS]     [ECOS]     [Asterisk Internet PBX]     [Linux API]     [Monitors]