<html><head><meta http-equiv="Content-Type" content="text/html charset=utf-8"></head><body style="word-wrap: break-word; -webkit-nbsp-mode: space; -webkit-line-break: after-white-space;" class=""><meta http-equiv="Content-Type" content="text/html charset=utf-8" class=""><div style="word-wrap: break-word; -webkit-nbsp-mode: space; -webkit-line-break: after-white-space;" class=""> <div class=""><blockquote type="cite" class=""><div class="">On 17 Jul 2017, at 13:49, Christophe Fergeau <<a href="mailto:cfergeau@redhat.com" class="">cfergeau@redhat.com</a>> wrote:</div> <div class=""><span style="font-family: Helvetica; font-size: 12px; font-style: normal; font-variant-caps: normal; font-weight: normal; letter-spacing: normal; text-align: start; text-indent: 0px; text-transform: none; white-space: normal; word-spacing: 0px; -webkit-text-stroke-width: 0px; float: none; display: inline !important;" class="">On Mon, Jul 17, 2017 at 11:44:04AM +0200, Christophe de Dinechin wrote:<br style="font-family: Helvetica; font-size: 12px; font-style: normal; font-variant-caps: normal; font-weight: normal; letter-spacing: normal; text-align: start; text-indent: 0px; text-transform: none; white-space: normal; word-spacing: 0px; -webkit-text-stroke-width: 0px;" class=""><blockquote type="cite" style="font-family: Helvetica; font-size: 12px; font-style: normal; font-variant-caps: normal; font-weight: normal; letter-spacing: normal; orphans: auto; text-align: start; text-indent: 0px; text-transform: none; white-space: normal; widows: auto; word-spacing: 0px; -webkit-text-size-adjust: auto; -webkit-text-stroke-width: 0px;" class="">From: Christophe de Dinechin <<a href="mailto:dinechin@redhat.com" class="">dinechin@redhat.com</a>> The mm timer is a millisecond timer that wraps around after ~49 days. All comparisons that look like aInstead, use signed ((int)(a-b)<0), which may fail if there is more than 25 days between a and b (should not be happening under normal conditions), but is robust to the timer wrapping around. Signed-off-by: Christophe de Dinechin <<a href="mailto:dinechin@redhat.com" class="">dinechin@redhat.com</a>> --- src/channel-display-gst.c | 4 ++-- src/channel-display-mjpeg.c | 4 ++-- src/channel-display.c | 2 +- src/channel-playback.c | 2 +- src/spice-channel-priv.h | 2 ++ src/spice-session.c | 4 ++-- 6 files changed, 10 insertions(+), 8 deletions(-) diff --git a/src/channel-display-gst.c b/src/channel-display-gst.c index adcd6ef..3f51361 100644 --- a/src/channel-display-gst.c +++ b/src/channel-display-gst.c @@ -166,7 +166,7 @@ static void schedule_frame(SpiceGstDecoder *decoder) break; } - if (now < gstframe->frame->mm_time) { + if (spice_mmtime_diff(now, gstframe->frame->mm_time) < 0) { decoder->timer_id = g_timeout_add(gstframe->frame->mm_time - now, display_frame, decoder); } else if (g_queue_get_length(decoder->display_queue) == 1) { @@ -511,7 +511,7 @@ static gboolean spice_gst_decoder_queue_frame(VideoDecoder *video_decoder, return TRUE; } - if (frame->mm_time < decoder->last_mm_time) { + if (spice_mmtime_diff(frame->mm_time, decoder->last_mm_time) < 0) { SPICE_DEBUG("new-frame-time < last-frame-time (%u < %u):" " resetting stream", frame->mm_time, decoder->last_mm_time); diff --git a/src/channel-display-mjpeg.c b/src/channel-display-mjpeg.c index ee33b01..3ba098c 100644 --- a/src/channel-display-mjpeg.c +++ b/src/channel-display-mjpeg.c @@ -201,7 +201,7 @@ static void mjpeg_decoder_schedule(MJpegDecoder *decoder) decoder->cur_frame = NULL; do { if (frame) { - if (time <= frame->mm_time) { + if (spice_mmtime_diff(time, frame->mm_time) <= 0) { guint32 d = frame->mm_time - time; decoder->cur_frame = frame; decoder->timer_id = g_timeout_add(d, mjpeg_decoder_decode_frame, decoder); @@ -251,7 +251,7 @@ static gboolean mjpeg_decoder_queue_frame(VideoDecoder *video_decoder, last_frame = g_queue_peek_tail(decoder->msgq); if (last_frame) { - if (frame->mm_time < last_frame->mm_time) { + if (spice_mmtime_diff(frame->mm_time, last_frame->mm_time) < 0) { /* This should really not happen */ SPICE_DEBUG("new-frame-time < last-frame-time (%u < %u):" " resetting stream", diff --git a/src/channel-display.c b/src/channel-display.c index 3c98d79..06ed18a 100644 --- a/src/channel-display.c +++ b/src/channel-display.c @@ -1366,7 +1366,7 @@ static void display_update_stream_report(SpiceDisplayChannel *channel, uint32_t } if (st->report_num_frames >= st->report_max_window || - now - st->report_start_time >= st->report_timeout || + spice_mmtime_diff(now - st->report_start_time, st->report_timeout) >= 0 || st->report_drops_seq_len >= STREAM_REPORT_DROP_SEQ_LEN_LIMIT) { SpiceMsgcDisplayStreamReport report; SpiceSession *session = spice_channel_get_session(SPICE_CHANNEL(channel)); diff --git a/src/channel-playback.c b/src/channel-playback.c index ca14b96..afc9059 100644 --- a/src/channel-playback.c +++ b/src/channel-playback.c @@ -313,7 +313,7 @@ static void playback_handle_data(SpiceChannel *channel, SpiceMsgIn *in) packet->time, packet->data, packet->data_size); #endif - if (c->last_time > packet->time) + if (spice_mmtime_diff(c->last_time, packet->time) > 0) g_warn_if_reached(); c->last_time = packet->time; diff --git a/src/spice-channel-priv.h b/src/spice-channel-priv.h index 7288920..20d3495 100644 --- a/src/spice-channel-priv.h +++ b/src/spice-channel-priv.h @@ -43,6 +43,8 @@ G_BEGIN_DECLS #define CHANNEL_DEBUG(channel, fmt, ...) \ SPICE_DEBUG("%s: " fmt, SPICE_CHANNEL(channel)->priv->name, ## __VA_ARGS__) +#define spice_mmtime_diff(now, time) ((int32_t) ((now)-(time))) </blockquote><br style="font-family: Helvetica; font-size: 12px; font-style: normal; font-variant-caps: normal; font-weight: normal; letter-spacing: normal; text-align: start; text-indent: 0px; text-transform: none; white-space: normal; word-spacing: 0px; -webkit-text-stroke-width: 0px;" class=""><span style="font-family: Helvetica; font-size: 12px; font-style: normal; font-variant-caps: normal; font-weight: normal; letter-spacing: normal; text-align: start; text-indent: 0px; text-transform: none; white-space: normal; word-spacing: 0px; -webkit-text-stroke-width: 0px; float: none; display: inline !important;" class="">Nit: I think the 'now' and 'time' naming here could be misleading, what<br style="font-family: Helvetica; font-size: 12px; font-style: normal; font-variant-caps: normal; font-weight: normal; letter-spacing: normal; text-align: start; text-indent: 0px; text-transform: none; white-space: normal; word-spacing: 0px; -webkit-text-stroke-width: 0px;" class=""><span style="font-family: Helvetica; font-size: 12px; font-style: normal; font-variant-caps: normal; font-weight: normal; letter-spacing: normal; text-align: start; text-indent: 0px; text-transform: none; white-space: normal; word-spacing: 0px; -webkit-text-stroke-width: 0px; float: none; display: inline !important;" class="">about just "mmtime0" and "mmtime1" for example?<br style="font-family: Helvetica; font-size: 12px; font-style: normal; font-variant-caps: normal; font-weight: normal; letter-spacing: normal; text-align: start; text-indent: 0px; text-transform: none; white-space: normal; word-spacing: 0px; -webkit-text-stroke-width: 0px;" class=""></div></blockquote><div class=""> </div><div class="">Well, that was to match the typical usage where ‘now’ would be the current time, and ‘time’ would be some other reference time. To me, that’s more readable that way. But YMMV…</div><div class=""> </div> <blockquote type="cite" class=""><div class=""><br style="font-family: Helvetica; font-size: 12px; font-style: normal; font-variant-caps: normal; font-weight: normal; letter-spacing: normal; text-align: start; text-indent: 0px; text-transform: none; white-space: normal; word-spacing: 0px; -webkit-text-stroke-width: 0px;" class=""><span style="font-family: Helvetica; font-size: 12px; font-style: normal; font-variant-caps: normal; font-weight: normal; letter-spacing: normal; text-align: start; text-indent: 0px; text-transform: none; white-space: normal; word-spacing: 0px; -webkit-text-stroke-width: 0px; float: none; display: inline !important;" class="">Acked-by: Christophe Fergeau <<a href="mailto:cfergeau@redhat.com" style="font-family: Helvetica; font-size: 12px; font-style: normal; font-variant-caps: normal; font-weight: normal; letter-spacing: normal; orphans: auto; text-align: start; text-indent: 0px; text-transform: none; white-space: normal; widows: auto; word-spacing: 0px; -webkit-text-size-adjust: auto; -webkit-text-stroke-width: 0px;" class="">cfergeau@redhat.com</a><span style="font-family: Helvetica; font-size: 12px; font-style: normal; font-variant-caps: normal; font-weight: normal; letter-spacing: normal; text-align: start; text-indent: 0px; text-transform: none; white-space: normal; word-spacing: 0px; -webkit-text-stroke-width: 0px; float: none; display: inline !important;" class="">></div></blockquote></div> </div></body></html>