[Spice-devel] [spice-gtk v1] channel-main: demote log level from warning to debug

[Victor Toso]

Hi,

On Thu, Dec 01, 2016 at 04:33:36PM +0100, Christophe Fergeau wrote:
> On Thu, Dec 01, 2016 at 02:15:49PM +0100, Victor Toso wrote:
> > On Thu, Dec 01, 2016 at 11:37:32AM +0100, Christophe Fergeau wrote:
> > > On Wed, Nov 30, 2016 at 06:50:39PM +0100, Victor Toso wrote:
> > > > From: Victor Toso <me at victortoso.com>
> > > >
> > > > Failing to get playback or record volume async on startup is very
> > > > common making this warning too worrisome for users.
> > > >
> > >
> > > Looked quickly at the code, and this seems to be getting the volume info
> > > from pulseaudio. What is special at startup that makes this not work?
> >
> > As mentioned in the commit log below:
> >
> > > > If the audio back-end does not cache the last volume used or if this
> > > > is the first time the application is launched, this message will be
> > > > seen.
> >
> > With pulseaudio back-end, the playback/record volume is cached in user
> > file with the application name as reference. The volume info should
> > fail:
> >
> > 1-) For playback, in the first time a spice client is launched
> > 2-) For record, always till the first it is used and, furthermore,
> >     cached.
>
> It seems obvious to you that in this case retrieving the volume is going
> to fail, and that it's normal, but it's not obvious at all to me.
> The application is running, so it should be able to retrieve some kind
> of volume?

Ah, I thought that how to get the volume was understood and the problem
was understanding when the warning is showed, sorry.

We can retrieve the volume in two ways: from an ongoing stream
and from cache, in spice-pulse.c see the following comment on
pulse_stream_restore_info_async()

 /* If Playback/Record stream is created we use pulse API to get volume-info
  * from those streams directly. If the stream is not created, retrieve last
  * volume/mute values from Pulse database using the application name;
  * If we already have retrieved volume-info from Pulse database then it is
  * safe to return the volume-info we already have in <stream>info */

However, when we call either
[playback] pa_context_set_sink_input_volume() or
[record] pa_context_set_source_output_volume(), PulseAudio should cache
the stream volume in a local cache file, here it is $HOME/.config/pulse/

Now, the volume-sync happens when agent announce its capabilities. It is
common that we don't have a playback stream being played at this time,
and it is even harder for a record stream.

> Or is it trying to retrieve the volume that was set in the previous
> session, which I understand can fail?

Yes, I think the failure is always when retrieving the volume from cache
as it should be super simple when there is an ongoing stream.

> But in this case, why are the next calls fine with retrieving the
> volume?

Is it? I think we will keep seeing the warning unless PulseAudio caches
the stream volume.

Well, all above was true when I implemented this. I've removed the cache
file from PulseAudio and it does not seem to be creating a new one? :(

>
> Christophe
>
> >
> > For gstreamer back-end, even with pulseaudio src/sink, it might not fail
> > but it will be wrong, see:
> > https://bugzilla.gnome.org/show_bug.cgi?id=766020
> >
> > > >
> > > > Signed-off-by: Victor Toso <victortoso at redhat.com>
> > > > ---
> > > >  src/channel-main.c | 4 ++--
> > > >  1 file changed, 2 insertions(+), 2 deletions(-)
> > > > 
> > > > diff --git a/src/channel-main.c b/src/channel-main.c
> > > > index 72ca712..e632c8e 100644
> > > > --- a/src/channel-main.c
> > > > +++ b/src/channel-main.c
> > > > @@ -1171,7 +1171,7 @@ static void audio_playback_volume_info_cb(GObject *object, GAsyncResult *res, gp
> > > >                                                        &volume, &error);
> > > >      if (ret == FALSE || volume == NULL || nchannels == 0) {
> > > >          if (error != NULL) {
> > > > -            g_warning("Failed to get playback async volume info: %s", error->message);
> > > > +            spice_debug("Failed to get playback async volume info: %s", error->message);
> > > >              g_error_free(error);
> > > >          } else {
> > > >              SPICE_DEBUG("Failed to get playback async volume info");
> > > > @@ -1227,7 +1227,7 @@ static void audio_record_volume_info_cb(GObject *object, GAsyncResult *res, gpoi
> > > >      ret = spice_audio_get_record_volume_info_finish(audio, res, &mute, &nchannels, &volume, &error);
> > > >      if (ret == FALSE || volume == NULL || nchannels == 0) {
> > > >          if (error != NULL) {
> > > > -            g_warning("Failed to get record async volume info: %s", error->message);
> > > > +            spice_debug("Failed to get record async volume info: %s", error->message);
> > > >              g_error_free(error);
> > > >          } else {
> > > >              SPICE_DEBUG("Failed to get record async volume info");
> > > > -- 
> > > > 2.9.3
> > > > 
> > > > _______________________________________________
> > > > Spice-devel mailing list
> > > > Spice-devel at lists.freedesktop.org
> > > > https://lists.freedesktop.org/mailman/listinfo/spice-devel
> > 
> > 
> 
> 
> 
> > _______________________________________________
> > Spice-devel mailing list
> > Spice-devel at lists.freedesktop.org
> > https://lists.freedesktop.org/mailman/listinfo/spice-devel
> 


-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 819 bytes
Desc: not available
URL: <https://lists.freedesktop.org/archives/spice-devel/attachments/20161201/901eabff/attachment-0001.sig>