[Spice-devel] [RFC PATCH spice-server] Send real time to client

[Victor Toso]

Hi,

Happy to see this patch and the discussion, sorry if I'm a bit late.

On Tue, Oct 03, 2017 at 04:18:57PM +0000, Frediano Ziglio wrote:
> > > On 3 Oct 2017, at 15:49, Frediano Ziglio < fziglio at redhat.com > wrote:
> > > The current design is "we don't care about the delay".
> > > If we don't agree on the design change the current behaviour is
> > > fine.
>
> > Mmmmh, sorry, without facial expressions, it’s hard for me to tell
> > if you are serious or sarcastic here. I thought I understood you
> > wanted to address the latency problem. Doesn’t that imply a change
> > in design?
>
> Well, I'm not the king of the castle. Considering that there are
> plenty of code to optimize the "movie night" use case and that people
> in the team in months never suggested to remove that code I can't see
> a strong design change. Not that the current code is easy to
> understand and change.

Not suggesting to remove is different to taking it in consideration with
a patch, like yours. IMHO, the proposal makes sense.

> Personally from this patch and my videos (playing games) you can
> easily guess my opinion. I would personally merge this patch and work
> on reducing the latency on the client to a minimum.

I'll test it today to provide some feedback.

> > > > I think that dropping audio packets makes sense. But then,
> > > > should we send audio and video over UDP instead of TCP?

> > > TCP will catch up. No reasons for UDP. Yes, UDP would do the drop
> > > on its own avoiding retransmissions but you have to change all the
> > > entire protocol, design and implementation (not saying is not
> > > worth investigating).
>
> > OK. I was only observing that TCP only makes sense if we prefer
> > packets not to be dropped. By having TCP retried (and associated
> > delays) only to drop the packets in the end makes little sense to
> > me.
>
> In this case would be audio packets... messages if you prefer. There
> must be some reasons why YouTube, Facebook and other streaming
> websites uses tcp.

My first though was "hey, their intent is not live streaming" but then I
recall that they actually do live streaming. That's actually a good
point worth research. Quickly looking at [0], one use case that differs
a lot from us is the high amount of clients per live-stream that is
ongoing.

[0] https://code.facebook.com/posts/1653074404941839/under-the-hood-broadcasting-live-video-to-millions/

> Replacing tcp with udp means you have to deal with a lot of problems.
> If you want a reliable connection then you'll end up reimplementing
> tcp. If you accepts a not reliable connection it means you have to
> deal with errors, changing protocol, compression, inserting redundancy
> (so spending more bandwidth) and data synchronization points.

(...)

> > So maybe I got confused and you were not suggesting we drop packets.
> > Then what were you proposing? Or were you not proposing anything and
> > just asking around to get ideas?
>
> Was not referring at network packets. Audio is transmitted in chunks
> compressed and sent inside spice messages. There messages are queued
> and synchronization is based on them. I was suggesting to discard them
> if the queue reach some threshold.

(...)

> > So maybe I got confused and you were not suggesting we drop packets.
> > Yes. The 500ms below were that limit in my mind.
>
> Ok, so 500ms as an upper limit, not a lower one (like happens during
> normal streaming buffering).

(...)

> > > This plus the video sync means that you click a button (or
> > > whatever action) and you'll see the results after 1/2 seconds. Can
> > > be acceptable for a office
> > > usage, surely not for animations, games or any possible realtime
> > > simulation would be impossible.
>
> > I was not suggesting that we accept a 500ms delay, but rather that
> > this be the maximum acceptable delay before we take harder measures,
> > like a restart. I think 0.5s is acceptable if it only happens a
> > couple of times a day when you lost Wi Fi.
>
> I won't personally live with that for long. I would try to catch up.

> Some notes about this patch that are not easy to see.
> The removed code basically introduces a minimum 400ms delay on
> multimedia (audio/video) data.

> Note that part of graphics is video and part graphic commands which
> potentially create different artifacts in the client.

> But the removal of this code cause some changes on streaming reports
> used for video bandwidth adjustment. The reason is that the num_drops
> and last_frame_delay depends on the difference between message
> multimedia time and the client concept of the multimedia time.

> So potentially we'll have a smaller delay (last_frame_delay) and more
> "dropped" frames. I quoted "dropped" as the computation of this value
> is more dependent on the network delay (network latency and time spent
> sending the video/audio messages).

Should be fine AFAICT.

> Is true that for mjpeg the frame is dropped (not displayed). On this
> not displaying it's quite odd. We have some video information but if
> it arrives too late is dropped but basically we are dropping the last
> frame without knowing if we'll have other data to display soon.

Yes, probably it should only be dropped if it has more data in the
queue already...

> Also if network conditions changed and the latency is bigger this can
> cause lot of frames to be dropped.

AFAIK, client does not know about network conditions, only server does.
If we need the client to consider changes in network conditions, server
must send these information periodically to the client (that would be a
new message). Not only in the streaming front but this data could be
used in better statistics with file-transfers for instance.

> To reduce CPU usage would be better to queue compressed frames and
> drop old ones instead of dropping new ones.

Right

> About the client concept of multimedia time. The server sends some
> messages to tell its time (with a -400ms skew) and client try to use
> this however with audio every seconds the client set the time based on
> the audio time (which has not the skew). So this patch avoids the
> client code to have 2 different concepts of time (skewed or not based
> on audio) which seems a good reason to have this patch.

I agree.

> Back to the delay, frames dropped and stream reports not sure how the
> bandwidth adjustment is affected (I'm actually trying this patch with
> the streaming agent which does not do any adjustment). On the other
> hand if audio is on the behaviour should be more similar.

I need to check the code but AFAIR, the stream report was only useful
in case we drop 100% of frames which means some issue in the client and
not worth to keep the streaming.

Cheers,
    Victor
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 833 bytes
Desc: not available
URL: <https://lists.freedesktop.org/archives/spice-devel/attachments/20171005/0770e152/attachment.sig>