Hi, On Thu, Nov 16, 2017 at 11:57:12AM -0500, Frediano Ziglio wrote: > > > > Hi, > > > > On Thu, Nov 16, 2017 at 10:58:03AM -0500, Frediano Ziglio wrote: > > > > > > > > From: Victor Toso <me@xxxxxxxxxxxxxx> > > > > > > > > On migration, we are resending the current volume and mute state in > > > > the Guest. > > > > > > Maybe is just me but this sentence is confusing. By "we" I think you > > > mean spice-server but the confusion came from the fact that is Qemu > > > that on the new machine try to send the volume to spice-server causing > > > a potential volume/mute message to be sent to the client. > > > Another part that is not clear is the "state in the Guest" I think you > > > are meaning we are sending the "current Guest state value". > > > I'll try to rephrase: > > > "On migration, Qemu notify spice-server with the current Guest volume > > > and mute state values which currently is handled forwarding these values > > > to the client." > > > > Yes, that's correct ~ sorry for the confusion. > > > > Is the rest of commit log clear enough? Spice should mimic what happens > > in the guest but a volume/mute change on migration is not happening per > > user request which can cause odd volume/mute changes in the client. > > > > Thanks, > > Victor > > > > Was looking at these settings, maybe the fix would be better in > snd_channel_set_mute and snd_channel_set_volume (not setting the > mask) ? Agreed, I'll send a v2 > What about record_channel_client_constructed? There is a similar > check (for migration) in playback_channel_client_constructed but > not in record_channel_client_constructed. Okay, I did a quick look at it and its the same code introduced by Marc-André at f10de4bc084fcc (in this patch's commit log) but it was moved a bit during the refactor, see: Current changes are from removing "on_new_playback_channel_client" function from: f37629996c1877 sound: Remove on_new_playback_channel_client() The check was moved to that function at: 258c2234babed9 playback: Don't lose client connection after migration OT: This change actually fixed https://bugs.freedesktop.org/show_bug.cgi?id=100136 And before that, it was when the check was introduced: f10de4bc084fcc sound: do not modify client state on migration Anyway, I recall discussing this with Marc-André in the past. Both mine and his fixes worked at the time but it might be that we are dealing with different issues - It should be safe not to change audio while migration IMHO. > > > > If the client user has change its master volume in the > > > > guest it might change the client application volume too and the volume > > > > jump (increase or decrease) might happen on migration. > > > > > > > > This patch is a complement of f10de4bc084fcc - Here, volume was > > > > jumping regardless of guest's volume value. > > > > > > > > Resolves: rhbz#1425443 > > > > Signed-off-by: Victor Toso <victortoso@xxxxxxxxxx> > > > > --- > > > > server/sound.c | 12 ++++++++++++ > > > > 1 file changed, 12 insertions(+) > > > > > > > > diff --git a/server/sound.c b/server/sound.c > > > > index b1bfaaaa..c6c8bdc8 100644 > > > > --- a/server/sound.c > > > > +++ b/server/sound.c > > > > @@ -414,6 +414,12 @@ static bool snd_send_volume(SndChannelClient > > > > *client, > > > > uint32_t cap, int msg) > > > > return false; > > > > } > > > > > > > > + /* Never changes volume or mute state on migration */ > > > > + if > > > > (red_client_during_migrate_at_target(red_channel_client_get_client(rcc))) > > > > { > > > > + spice_debug("Do not change volume during migration"); > > > > + return FALSE; > > > > + } > > > > + > > > > vol = alloca(sizeof (SpiceMsgAudioVolume) + > > > > st->volume_nchannels * sizeof (uint16_t)); > > > > red_channel_client_init_send_data(rcc, msg); > > > > @@ -445,6 +451,12 @@ static bool snd_send_mute(SndChannelClient *client, > > > > uint32_t cap, int msg) > > > > return false; > > > > } > > > > > > > > + /* Never changes volume or mute state on migration */ > > > > + if > > > > (red_client_during_migrate_at_target(red_channel_client_get_client(rcc))) > > > > { > > > > + spice_debug("Do not change mute during migration"); > > > > + return FALSE; > > > > + } > > > > + > > > > red_channel_client_init_send_data(rcc, msg); > > > > mute.mute = st->mute; > > > > spice_marshall_SpiceMsgAudioMute(m, &mute); > > > > > > Frediano > >
Attachment:
signature.asc
Description: PGP signature
_______________________________________________ Spice-devel mailing list Spice-devel@xxxxxxxxxxxxxxxxxxxxx https://lists.freedesktop.org/mailman/listinfo/spice-devel