Hi,
On 04/01/2013 10:38 PM, Marc-André Lureau wrote:
----- Mensaje original -----
Hi,
On 04/01/2013 07:58 PM, Marc-André Lureau wrote:
----- 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
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.
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.
ATM the server *always* sends us the wrong volume message, but we don't check for
nchannels == 0 when processing this message.
Normally when we get the initial audio-volume-msg we don't have a playback
stream yet, so we don't call pa_context_set_sink_input_volume.
But on live migration we do have a playback stream so we end up calling
pa_context_set_sink_input_volume with invalid parameters.
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.
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!
I suggest we compromise and add an explicit check for nchannels == 0 and
which logs a debug message when triggered.
Regards,
Hans
_______________________________________________
Spice-devel mailing list
Spice-devel@xxxxxxxxxxxxxxxxxxxxx
http://lists.freedesktop.org/mailman/listinfo/spice-devel