[Spice-devel] [spice-gtk] RFC: Allow to limit the latency introduced by lip sync

[Snir Sheriber]

Hi,


On 08/15/2017 05:56 PM, Frediano Ziglio wrote:
> This patch allows to reduce the time the client wait to display frames.
> It can make the lip sync not working but it allows to see the
> video latency introduced by this code.
> This patch is meant to be used mainly for debugging:
> - recently we are seeing some artifacts due to this delay and
>    overlaying, removing the delay make easier to understand if the
>    problem is related to this or other issues;
> - we are testing guest encoding and we are experiencing a big lag.
>    This avoid this part of the lag helping understand where the other
>    part of the delay happens.

Actually IMHO this could be also good enough solution for the building 
latency issue
for now.
I mentioned it also in Victor's mmtime-adjustment patch thread (at least 
I thought I
did- just noticed i accidentally replayed to off-list :/), if we'll let 
the user the option
to enable\disable this frames-scheduling-limitation (as presented here) 
on-the-fly
(e.g as menu checkbox -as in victor's suggestion) he (the user) would be 
able
to enable it in case he's feeling latency was built, and latency will 
disappear
immediately (and audio will become out of sync). Adjusting mm_time alone
doesn't seem to fix already built latency...

Snir.
> Signed-off-by: Frediano Ziglio <fziglio at redhat.com>
> ---
> This patch is not intended to be merged as is but to be used to fix
> above problems or as a start for some better settings.
> ---
>   src/channel-display-gst.c   | 10 ++++++----
>   src/channel-display-mjpeg.c | 24 +++++++++++++++++++-----
>   src/channel-display-priv.h  |  3 +++
>   3 files changed, 28 insertions(+), 9 deletions(-)
>
> Changes since last version:
> - rebased on master.
>
> It seems is still useful, still not clear all sync process.
>
> diff --git a/src/channel-display-gst.c b/src/channel-display-gst.c
> index f978602..9ee6229 100644
> --- a/src/channel-display-gst.c
> +++ b/src/channel-display-gst.c
> @@ -173,8 +173,10 @@ static void schedule_frame(SpiceGstDecoder *decoder)
>               break;
>           }
>   
> -        if (spice_mmtime_diff(now, gstframe->frame->mm_time) < 0) {
> -            decoder->timer_id = g_timeout_add(gstframe->frame->mm_time - now,
> +        gint32 time_diff = spice_mmtime_diff(gstframe->frame->mm_time, now);
> +        time_diff = MIN(time_diff, max_frame_delay_ms);
> +        if (time_diff > 0) {
> +            decoder->timer_id = g_timeout_add(time_diff,
>                                                 display_frame, decoder);
>           } else if (g_queue_get_length(decoder->display_queue) == 1) {
>               /* Still attempt to display the least out of date frame so the
> @@ -182,8 +184,8 @@ static void schedule_frame(SpiceGstDecoder *decoder)
>                */
>               decoder->timer_id = g_timeout_add(0, display_frame, decoder);
>           } else {
> -            SPICE_DEBUG("%s: rendering too late by %u ms (ts: %u, mmtime: %u), dropping",
> -                        __FUNCTION__, now - gstframe->frame->mm_time,
> +            SPICE_DEBUG("%s: rendering too late by %d ms (ts: %u, mmtime: %u), dropping",
> +                        __FUNCTION__, -time_diff,
>                           gstframe->frame->mm_time, now);
>               stream_dropped_frame_on_playback(decoder->base.stream);
>               g_queue_pop_head(decoder->display_queue);
> diff --git a/src/channel-display-mjpeg.c b/src/channel-display-mjpeg.c
> index f0d55f6..cc9ddf1 100644
> --- a/src/channel-display-mjpeg.c
> +++ b/src/channel-display-mjpeg.c
> @@ -189,6 +189,19 @@ static gboolean mjpeg_decoder_decode_frame(gpointer video_decoder)
>   
>   /* ---------- VideoDecoder's queue scheduling ---------- */
>   
> +enum { MAX_DELAY_MS = 2000 };
> +gint32 max_frame_delay_ms = MAX_DELAY_MS;
> +
> +SPICE_CONSTRUCTOR_FUNC(max_delay_init)
> +{
> +    const char *str_delay = g_getenv("SPICE_MAX_FRAME_DELAY");
> +    if (str_delay) {
> +        int delay = atoi(str_delay);
> +        if (delay >= 0 && delay <= MAX_DELAY_MS)
> +            max_frame_delay_ms = delay;
> +    }
> +}
> +
>   static void mjpeg_decoder_schedule(MJpegDecoder *decoder)
>   {
>       SPICE_DEBUG("%s", __FUNCTION__);
> @@ -201,15 +214,16 @@ static void mjpeg_decoder_schedule(MJpegDecoder *decoder)
>       decoder->cur_frame = NULL;
>       do {
>           if (frame) {
> -            if (spice_mmtime_diff(time, frame->mm_time) <= 0) {
> -                guint32 d = frame->mm_time - time;
> +            gint32 time_diff = spice_mmtime_diff(frame->mm_time, time);
> +            time_diff = MIN(time_diff, max_frame_delay_ms);
> +            if (time_diff >= 0) {
>                   decoder->cur_frame = frame;
> -                decoder->timer_id = g_timeout_add(d, mjpeg_decoder_decode_frame, decoder);
> +                decoder->timer_id = g_timeout_add(time_diff, mjpeg_decoder_decode_frame, decoder);
>                   break;
>               }
>   
> -            SPICE_DEBUG("%s: rendering too late by %u ms (ts: %u, mmtime: %u), dropping ",
> -                        __FUNCTION__, time - frame->mm_time,
> +            SPICE_DEBUG("%s: rendering too late by %d ms (ts: %u, mmtime: %u), dropping ",
> +                        __FUNCTION__, -time_diff,
>                           frame->mm_time, time);
>               stream_dropped_frame_on_playback(decoder->base.stream);
>               free_spice_frame(frame);
> diff --git a/src/channel-display-priv.h b/src/channel-display-priv.h
> index 1389868..92bd85a 100644
> --- a/src/channel-display-priv.h
> +++ b/src/channel-display-priv.h
> @@ -197,6 +197,9 @@ void stream_display_frame(display_stream *st, SpiceFrame *frame, uint32_t width,
>   gint64 get_stream_id_by_stream(SpiceChannel *channel, display_stream *st);
>   
>   
> +/* maximum delay for frames */
> +extern gint32 max_frame_delay_ms;
> +
>   G_END_DECLS
>   
>   #endif // CHANNEL_DISPLAY_PRIV_H_