[Spice-devel] Very poor video performance using Virt Viewer compared to RDP

Sat Oct 15 15:26:02 UTC 2016

Hi!

On Fri, Oct 14, 2016 at 04:55:35PM +0200, Francois Gouget wrote:
> On Thu, 13 Oct 2016, Frediano Ziglio wrote:
> [...]
> > I discovered where the main lag came quite by "accident". To test
> > I used one small utility and this utility don't have a way to change
> > parameters (like bandwidth) so knowing how the tcp work I stopped
> > the utility and started again as fast as I can with different
> > parameters. This of course create packets loss and some delay but
> > the connection works fine as tcp retransmit the packets lost.
> > But surprisingly the delay introduced by this accident was maintained
> > in the video! I then removed all Virgl stuff, tried again with a video
> > and the lag is still present. Also interrupting the utility and
> > starting again was increasing the lag (I managed to have about
> > 3 seconds lag!).
> >
> > Now... I have a patch for the client but as I said I'm not familiar
> > with the client too much and it's really terrible. It mainly remove
> > the synchronization with the audio (with still lags) and play the
> > video as fast as possible. By the way, I think is time to send as
> > someone could be able to make a sensible version of this patch
> > (perhaps fixing the sound too)
>
> This could be caused by what I consider to be a bug in spice-gtk's
> audio handling. This bug is triggered by the PulseAudio backend. I
> don't know about the others as I have never used them.
>
> So what happens is that PulseAudio sends regular notifications about
> changes in the latency of the audio device. I suppose this can be
> useful in case the audio goes to a network device, or if it gets
> rerouted from the builtin audio card to a USB headset or other such
> scenarios.
>
> spice-pulse.c handles these notifications in
> stream_update_latency_callback() and forwards the information to
> spice_playback_channel_set_delay() (in channel-playback.c). There the
> delay gets added to the timestamp of the **last received audio
> packet** and set as the current mmtime.

In spice-pulse, this was introduced in:
https://cgit.freedesktop.org/spice/spice-gtk/commit/?id=54bd520f342f83bbc3bef

Not sure if this was intentionally bound with video stream. IMHO, it
seems error prone to do as much as sending the audio samples to
pulseaudio to later recover latency information and then trying to fix
it with mmtime.

Still, this is very interesting. I played a lot with audio and I don't
recall any sync issues so far.

> This means that if you stop receiving audio packets the mmtime gets
> stuck. Essentially time stands still as far as spice-gtk is concerned!
> So anything that deals with mmtimes breaks down, like the video
> playback.

Using only audio playback, I'm not able to easily make mmtime to
increase greatly. I'll try to setup my environment with the latency
utility [0] from Frediano to investigate it further.

[0] https://github.com/freddy77/latency

>
> Why would one stop receiving audio packets?
> * This happens whenever an application opens the audio device but then
>   does not write anything to it. For instance if you start mplayer and
>   then pause playback it will keep the audio device open but stop
>   writing audio data to it.
>   Fortunately QEMU has a workaround for this: it closes the playback
>   channel if there is nothing to play for 300 ms or more. 300 ms is
>   short enough that you will not really notice the short pause in the
>   video playback.

Ah, so, that's the reason I'm not able to easily reproduce this. I read
your email twice but missed this info. Do you know the hash for this in
QEMU?

>   The Xspice server was missing this workaround which
>   is how I discovered this bug. But I updated it to match QEMU in
>   commit 76fd0a37.

(just for reference)
https://cgit.freedesktop.org/xorg/driver/xf86-video-qxl/commit/?id=76fd0a37

>
> * This can also happen if you start losing audio packets. I suspect
>   this is what you're seeing. The video streaming code tries hard to
>   leave enough bandwidth for the audio streams, mostly because it's
>   better to degrade video quality a bit than have choppy audio. But
>   this also helps avoid this isue.
>
> But even in the normal case this bug can disrupt video playback a bit.
>
> Audio packets are sent at regular intervals, typically every 10ms in
> Xspice's case. Meanwhile stream_update_latency_callback() is called on a
> totally independent schedule and when that happens the last audio packet
> will be somehwere between 0 and 10 ms old. This means this bug
> introduces a 10 ms jitter in mmtime.

The synchronization between video/audio should come from source
(spice-server or qemu) and not from pulseaudio. Actually, you can see
that spice-server does send mmtime with SPICE_MSG_MAIN_MULTI_MEDIA_TIME
and it seems to be used in a few places:

server/sound.c:1080: reds_enable_mm_time(snd_channel_get_server(channel));
server/sound.c:1205: reds_enable_mm_time(snd_channel_get_server(channel));
server/main-dispatcher.c:219: reds_set_client_mm_time_latency(self->priv->reds, msg->client, msg->latency);
server/main-dispatcher.c:254: reds_set_client_mm_time_latency(self->priv->reds, client, latency);

I would hope that any mmtime issue would come from spice-server instead
of pulseaudio but as I did not reproduce the issue so far, is hard to
tell.

>
> The issue is that the mmtime is pretty important for streaming,
> particularly video streaming. It's used by the client to measure how
> much margin it has between reception of a frame and when that frame
> should be displayed. Based onthat margin the video streaming code
> decides whether it should lower the bandwidth or keep it as is. When
> bandwidth is plentiful the margin would typically be around 300 ms and
> if you divide that into four that gives you 75 ms buckets. So a 10 ms
> error could relatively easily move you from one bucket to another. A
> 10 ms error is also likely to exceed the rtt jitter in many cases.
>
> I'm not sure what the right fix is. It most likely involves ignoring
> the last audio packet mmtime. Maybe keep track of the last delay value
> and adjust mmtime by the difference between the old and new value?

I'll try to study this part of the code a little bit more. IMHO, the
right fix would imply some synchronization method for audio+video from
spice-server that can be used in the client (so they can be played
synced).

The mmtime being set from pulseaudio latency (at any given time) seems
wrong to me.

  toso

>
> --
> Francois Gouget <fgouget at codeweavers.com>
> _______________________________________________
> 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/20161015/4efd555e/attachment-0001.sig>