[Spice-devel] [spice] Enable mm_time adjustments on startup

[Frediano Ziglio]

> On Wed, 17 Apr 2019, Snir Sheriber wrote:
> [...]
> > > The reason for that is that while a minimum 400 ms latency is fine when
> > > playing a YouTube video [1], it's very annoying when the whole desktop
> > > is being streamed, either through the streaming agent or through the
> > > future Virgl remote access, because then it translates into a 400 ms
> > 
> > Are you working on something like that (remote virgl)?
> 
> Yes. See (though I need to update refresh these GitHub branches):
> 
> https://lists.freedesktop.org/archives/spice-devel/2019-January/047329.html
> 
> 
> > Notice that currently there's small hacky patch on client to ignore latency
> > when it's full
> > screen streaming and there is no audio playback.
> > 
> > (d047b2fb7f5d492d6c49f589ba5ff862c6b115da)
> 
> Right. It never actually had any effect in my tests.
> * When testing the fullscreen streaming in QEmu (i.e. when
>   streaming_mode is true) I had to use the mjpeg encoding because there
>   seems to be some conflict between QEmu and GStreamer on Debian. But
>   the patch has no effect on the builtin mjpeg decoder because the
>   latency in mjpeg_decoder_queue_frame() is to drop frames if it is
>   negative. To determine when to display the frame it uses the frame's
>   timestamp.
> * The rest of the time I'm testing by running an OpenGL application in a
>   regular session and so streaming_mode is false so the GstVideoOverlay
>   is not used and thus the patch has no effect.
> 
>   That's because spice_gst_decoder_queue_frame() uses latency to drop
>   frames if it is negative; and to determine the deadline for decoding
>   the frame. When not using the video overlay the decoded frames then
>   get queued until it is time to display them, according to their
>   mm_time timestamp which is not impacted by latency.
> 
> That said I'm not sure ignoring the frames mm_time timestamp is a good
> idea as it means the network jitter will translate directly into
> framerate jitter (e.g. in OpenGL applications or in silent movies).
> 
> 
> > > Other steps are:
> > > * Reducing the default latency.
> > 
> > What will be the default? what will happen to late video frames?
> 
> Late frames will be dropped as has always been the case (at least in
> the mjpeg case otherwise it's a bit more complex).
> 

I think the terminology should really be changed.
"Late frame": what does it mean? Ignore that we know the protocol and
implementation it does not make sense, all frames are late, this is
live streaming, there can be a "too late", not a "late", they are all late.
We have an expectation and we define the late or not based on that
expectation.

> As before the latency must be adjusted, and particularly, increased, as
> required to avoid dropping frames. Of course when starting with a

Honestly, we receive a frame, this is the best and more updated screen
we have. What do you want to do? Drop it, that's obvious. Are we really
sure about that? Is that the reason why we "must" increase this "latency"?

"latency": this term is really confusing. Taking into account that we
need to deal also with network latency but this is not it I find always
really confusing. Can't we start using "delay" or something else?

What's also confusing is the computation. Usually when you have
latency issues the more latency you have the worst it is.
In client you compute a value and the _less_ it is the worst it is.
Usually a negative latency is an indication that or you craft a
time machine or something went wrong.

> huge latency like 400 ms handling the latency correctly is much less
> important as it only become important when you have really a bad network
> connection or a very long encoding times.
> 
> It's important to distinguish increasing and decreasing the latency.
> 
> Increasing the latency means a frame queued on the client will be
> displayed even later. So the latency can be increased freely.
> 

If I'm drawing with the mouse something on the screen and the
latency is 10 minutes I don't really think that increase "freely"
is really good.
Also this potentially can lead to a queue of 10 minutes in the
client.

> Decreasing the latency means a bunch of frames queued on the client may
> suddenly become late, causing them to be dropped. So more care must be
> taken.
> 

Is the problem of "care to take" something not avoidable or is it
something we create with our hands? I mean, I know that current
implementation will drop frames which we don't want but is it
not a workaround of a bad implementation?

I personally would propose to avoid all that complicated computations
between server and client.
If every frame and audio piece is timed with a consistent monotonic time
(and current they ARE) they can be consumed after ENCODING + TRANSMISSION
+ DECODING time. Now, all the parts have skews so to avoid scattering
usually you attempt to have a minimum (that is the 400ms).
Now it could be pretty simple for the client to compute the ENCODING +
TRANSMISSION + DECODING values, just piece server_mm_time - piece_mm_time!
Now: currently the server_mm_time we have is skewed and fluctuating but
won't be hard to get a more consistent one (and easier to update!).
The easy formula to compute the "minimum" is obviously the maximum of
the previous TIMINGs (where TIMING == ENCODING + TRANSMISSION + DECODING)
however this does not take into account that you can have peaks that
will slow down everything forever. You usually wants to ignore peaks and
look at more local average. Also you should take into account that we are
human and you don't need a precision or milliseconds (well, this is more
true for video than for audio... but simply consider that if you want a
precision in ms in video you need 1000 FPS!).

That said, back to the patch. Instead of just setting mm_time_enabled
directly I would call reds_enable_mm_time, it's not in a hot path so
the performances are not important, the main channel is just created
so reds_send_mm_time will do nothing but you'll have a more consistent
setting.

Frediano