Hi > >>> ----- Mensaje original ----- > >>>> On seamless migration "switch-over" the playback_volume_changed callback > >>>> gets > >>>> called, and at this time the channel object's nchannels property is 0, > >>>> leading to a warning like this: > >>>> > >>>> (remote-viewer:8726): GSpice-WARNING **: set_sink_input_volume() failed: > >>>> Invalid argument > >>>> > >>>> This patch fixes this. > >>> > >>> Why is that? the nchannels property is set when receiving volume. Why > >>> does > >>> the server send a volume with 0 channels? Does that make sense? Shouldn't > >>> the fix be in the server instead? > >> > >> A very valid question I tracked this down to a server bug, for which I > >> just send a patch. > >> > >> Still I would like to push the trivial spice-gtk fix to silence the > >> warning > >> upstream so, so as to silence it when using a new spice-gtk with an older > >> spice-server. > > > > I disagree, the warning shows a misbehaving server. Without it, we wouldn't > > be able to spot this kind of bug. > > I disagree with you disagree-ing: > > 1) The server sends an empty audio-volume message, this is not illegal, it > is not really useful, but not really a bug either > It's a bug. > 2) The warning shows us calling pa_context_set_sink_input_volume with invalid > parameters, which is a spice-gtk bug, not a server bug. Sure, but it also print a warning. If think you could agree it's useful to keep the warning, since you want to fix it. Replace the PA warning with a spice-gtk warning, but don't just "hide" it. > 3) My patch does not remove the warning, it avoids calling > pa_context_set_sink_input_volume with invalid parameters, which is something > we > should never deliberately do. It stops printing the warning if channels == 0, doesn't it? > ATM the server *always* sends us the wrong volume message, but we don't check > for > nchannels == 0 when processing this message. The check should probably be in channel-playback.c actually, not even in pulse backend. > I think we can agree that we should *never* deliberately call functions with > invalid parameters. So we need to *explicitly* check for nchannels == 0, > rather > then relying on pa_context_set_sink_input_volume to check this for us. I agree, but keep a warning, a return_if_fail() would do too, in channel-playback.c / channel-record.c > Now we can do 2 things when this checks triggers, we can simply ignore the > message silently (an empty volume message is a legal message protocol wise), > or we can add an explicit warning for this, but then this warning will > trigger whenever we connect to an older server! Yes, let's keep warning. Server must be fixed, even older version should be updated, this will be one more reason. _______________________________________________ Spice-devel mailing list Spice-devel@xxxxxxxxxxxxxxxxxxxxx http://lists.freedesktop.org/mailman/listinfo/spice-devel