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

[Marc-André Lureau]

----- Original Message -----
> > 
> > Hi
> > 
> > ----- Original Message -----
> > > 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.
> > 
> > How does it help with debugging? I can't find a good reason to have a
> > maximized delay for debugging reasons, or for user-experience, however a
> > warning for delay > X seconds could be useful.
> > 
> 
> - 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;

If this is temporarily there, I would suggest to keep it out of tree for now.

Shouldn't there be an option to ignore sync all together instead then?

> - 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.
> 
> I'll add these to comment

Wouldn't a warning for long delay give you the information you need?
 
> 
> Frediano
> 
> > > 
> > > Signed-off-by: Frediano Ziglio <fziglio at redhat.com>
> > > ---
> > >  src/channel-display-gst.c   |  1 +
> > >  src/channel-display-mjpeg.c | 14 ++++++++++++++
> > >  src/channel-display-priv.h  |  3 +++
> > >  3 files changed, 18 insertions(+)
> > > 
> > > diff --git a/src/channel-display-gst.c b/src/channel-display-gst.c
> > > index 9c62e67..379c935 100644
> > > --- a/src/channel-display-gst.c
> > > +++ b/src/channel-display-gst.c
> > > @@ -184,6 +184,7 @@ static void schedule_frame(SpiceGstDecoder *decoder)
> > >  
> > >          SpiceStreamDataHeader *op = spice_msg_in_parsed(frame->msg);
> > >          gint32 time_diff = compute_mm_time_diff(op->multi_media_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);
> > > diff --git a/src/channel-display-mjpeg.c b/src/channel-display-mjpeg.c
> > > index 1b7373b..5c778a9 100644
> > > --- a/src/channel-display-mjpeg.c
> > > +++ b/src/channel-display-mjpeg.c
> > > @@ -182,6 +182,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__);
> > > @@ -196,6 +209,7 @@ static void mjpeg_decoder_schedule(MJpegDecoder
> > > *decoder)
> > >          if (frame_msg) {
> > >              SpiceStreamDataHeader *op = spice_msg_in_parsed(frame_msg);
> > >              gint32 time_diff =
> > >              compute_mm_time_diff(op->multi_media_time,
> > >              time);
> > > +            time_diff = MIN(time_diff, max_frame_delay_ms);
> > >              if (time_diff >= 0) {
> > >                  decoder->cur_frame_msg = frame_msg;
> > >                  decoder->timer_id = g_timeout_add(time_diff,
> > >                  mjpeg_decoder_decode_frame, decoder);
> > > diff --git a/src/channel-display-priv.h b/src/channel-display-priv.h
> > > index 3cd0727..1254f9d 100644
> > > --- a/src/channel-display-priv.h
> > > +++ b/src/channel-display-priv.h
> > > @@ -157,6 +157,9 @@ static inline gint32 compute_mm_time_diff(guint32
> > > time1,
> > > guint32 time2)
> > >      return (gint32) (guint32) (time1 - time2);
> > >  }
> > >  
> > > +/* maximum delay for frames */
> > > +extern gint32 max_frame_delay_ms;
> > > +
> > >  G_END_DECLS
> > >  
> > >  #endif // CHANNEL_DISPLAY_PRIV_H_
> 
> _______________________________________________
> Spice-devel mailing list
> Spice-devel at lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/spice-devel
>