[Spice-devel] [PATCH] reuse red_get_monotonic_time function

[Frediano Ziglio]

> 
> Hi Frediano,
> 
> I noticed that red_get_monotonic_time() returns int64_t. Should it be
> uint64_t?
> 

Well... the 63 bit precision allow dates till 2192 which is enough.
However some computation on unsigned values could lead to overflow/underflow
while with signed values is much harder to have.
Considering that some functions uses signed some other unsigned, that range is
enough for all values returned from this function, and that C converts signed
to unsigned by default when values are mixed signed is currently IMHO the
best thing.

Frediano

> The patch looks good.
> 
> Acked-by: Pavel Grunt <pgrunt at redhat.com>
> 
> On Fri, 2015-11-20 at 12:13 +0000, Frediano Ziglio wrote:
> > Signed-off-by: Frediano Ziglio <fziglio at redhat.com>
> > ---
> >  server/main_channel.c  | 10 ++--------
> >  server/mjpeg_encoder.c |  9 +++------
> >  server/red_channel.c   | 13 +++----------
> >  3 files changed, 8 insertions(+), 24 deletions(-)
> > 
> > diff --git a/server/main_channel.c b/server/main_channel.c
> > index 0ecc9df..176584a 100644
> > --- a/server/main_channel.c
> > +++ b/server/main_channel.c
> > @@ -47,6 +47,7 @@
> >  #include "reds.h"
> >  #include "migration_protocol.h"
> >  #include "main_dispatcher.h"
> > +#include "utils.h"
> >  
> >  #define ZERO_BUF_SIZE 4096
> >  
> > @@ -590,20 +591,13 @@ void
> > main_channel_client_push_notify(MainChannelClient
> > *mcc, const char *msg)
> >      red_channel_client_pipe_add_push(&mcc->base, item);
> >  }
> >  
> > -static uint64_t get_time_stamp(void)
> > -{
> > -    struct timespec time_space;
> > -    clock_gettime(CLOCK_MONOTONIC, &time_space);
> > -    return time_space.tv_sec * 1000 * 1000 * 1000 + time_space.tv_nsec;
> > -}
> > -
> >  static void main_channel_marshall_notify(RedChannelClient *rcc,
> >                                           SpiceMarshaller *m,
> >                                           NotifyPipeItem
> > *item)
> >  {
> >      SpiceMsgNotify notify;
> >  
> >      red_channel_client_init_send_data(rcc, SPICE_MSG_NOTIFY, &item->base);
> > -    notify.time_stamp = get_time_stamp(); // TODO - move to
> > main_new_notify_item
> > +    notify.time_stamp = red_get_monotonic_time(); // TODO - move to
> > main_new_notify_item
> >      notify.severity = SPICE_NOTIFY_SEVERITY_WARN;
> >      notify.visibilty = SPICE_NOTIFY_VISIBILITY_HIGH;
> >      notify.what = SPICE_WARN_GENERAL;
> > diff --git a/server/mjpeg_encoder.c b/server/mjpeg_encoder.c
> > index c8253a7..e5f3cbd 100644
> > --- a/server/mjpeg_encoder.c
> > +++ b/server/mjpeg_encoder.c
> > @@ -21,6 +21,7 @@
> >  
> >  #include "red_common.h"
> >  #include "mjpeg_encoder.h"
> > +#include "utils.h"
> >  #include <jerror.h>
> >  #include <jpeglib.h>
> >  #include <inttypes.h>
> > @@ -714,12 +715,10 @@ static int mjpeg_encoder_start_frame(MJpegEncoder
> > *encoder,
> >  
> >      if (rate_control_is_active(encoder)) {
> >          MJpegEncoderRateControl *rate_control = &encoder->rate_control;
> > -        struct timespec time;
> >          uint64_t now;
> >          uint64_t interval;
> >  
> > -        clock_gettime(CLOCK_MONOTONIC, &time);
> > -        now = ((uint64_t) time.tv_sec) * 1000000000 + time.tv_nsec;
> > +        now = red_get_monotonic_time();
> >  
> >          if (!rate_control->adjusted_fps_start_time) {
> >              rate_control->adjusted_fps_start_time = now;
> > @@ -996,11 +995,9 @@ static void
> > mjpeg_encoder_decrease_bit_rate(MJpegEncoder
> > *encoder)
> >      rate_control->client_state.max_video_latency = 0;
> >      rate_control->client_state.max_audio_latency = 0;
> >      if (rate_control->warmup_start_time) {
> > -        struct timespec time;
> >          uint64_t now;
> >  
> > -        clock_gettime(CLOCK_MONOTONIC, &time);
> > -        now = ((uint64_t) time.tv_sec) * 1000000000 + time.tv_nsec;
> > +        now = red_get_monotonic_time();
> >          if (now - rate_control->warmup_start_time <
> > MJPEG_WARMUP_TIME*1000*1000) {
> >              spice_debug("during warmup. ignoring");
> >              return;
> > diff --git a/server/red_channel.c b/server/red_channel.c
> > index 7330ae2..2b1ce08 100644
> > --- a/server/red_channel.c
> > +++ b/server/red_channel.c
> > @@ -529,7 +529,6 @@ static void
> > red_channel_client_send_empty_msg(RedChannelClient *rcc, PipeItem *b
> >  static void red_channel_client_send_ping(RedChannelClient *rcc)
> >  {
> >      SpiceMsgPing ping;
> > -    struct timespec ts;
> >  
> >      if (!rcc->latency_monitor.warmup_was_sent) { // latency test start
> >          int delay_val;
> > @@ -561,8 +560,7 @@ static void
> > red_channel_client_send_ping(RedChannelClient
> > *rcc)
> >  
> >      red_channel_client_init_send_data(rcc, SPICE_MSG_PING, NULL);
> >      ping.id = rcc->latency_monitor.id;
> > -    clock_gettime(CLOCK_MONOTONIC, &ts);
> > -    ping.timestamp = ts.tv_sec * 1000000000LL + ts.tv_nsec;
> > +    ping.timestamp = red_get_monotonic_time();
> >      spice_marshall_msg_ping(rcc->send_data.marshaller, &ping);
> >      red_channel_client_begin_send_message(rcc);
> >  }
> > @@ -1439,12 +1437,9 @@ static void
> > red_channel_handle_migrate_data(RedChannelClient *rcc, uint32_t size
> >  
> >  static void red_channel_client_restart_ping_timer(RedChannelClient *rcc)
> >  {
> > -    struct timespec ts;
> >      uint64_t passed, timeout;
> >  
> > -    clock_gettime(CLOCK_MONOTONIC, &ts);
> > -
> > -    passed = ts.tv_sec * 1000000000LL + ts.tv_nsec;
> > +    passed = red_get_monotonic_time();
> >      passed = passed - rcc->latency_monitor.last_pong_time;
> >      passed /= 1000*1000;
> >      timeout = PING_TEST_IDLE_NET_TIMEOUT_MS;
> > @@ -1483,7 +1478,6 @@ static void
> > red_channel_client_cancel_ping_timer(RedChannelClient *rcc)
> >  static void red_channel_client_handle_pong(RedChannelClient *rcc,
> > SpiceMsgPing *ping)
> >  {
> >      uint64_t now;
> > -    struct timespec ts;
> >  
> >      /* ignoring unexpected pongs, or post-migration pongs for pings that
> >       * started just before migration */
> > @@ -1493,8 +1487,7 @@ static void
> > red_channel_client_handle_pong(RedChannelClient *rcc, SpiceMsgPing *
> >          return;
> >      }
> >  
> > -    clock_gettime(CLOCK_MONOTONIC, &ts);
> > -    now =  ts.tv_sec * 1000000000LL + ts.tv_nsec;
> > +    now = red_get_monotonic_time();
> >  
> >      if (rcc->latency_monitor.state == PING_STATE_WARMUP) {
> >          rcc->latency_monitor.state = PING_STATE_LATENCY;
> 
> 
> 
>