[Spice-devel,1/3] server: Add time constants to go with spice_get_monotonic_time_ns()

Submitted by Francois Gouget on Dec. 11, 2015, 6:07 p.m.

Details

Message ID 68da9e487d1e2b9d860ec94c34b2e6811a1f53ad.1449856957.git.fgouget@free.fr
State New
Headers show
Series "Series without cover letter" ( rev: 1 ) in Spice

Not browsing as part of any series.

Commit Message

Francois Gouget Dec. 11, 2015, 6:07 p.m.
They clarify the time unit being used, reduce the need for casts and
simplify calculations.

Signed-off-by: Francois Gouget <fgouget@codeweavers.com>
---
 server/dcc.h           |  4 ++--
 server/mjpeg-encoder.c | 10 +++++-----
 server/red-channel.c   |  8 +++-----
 server/red-worker.c    |  2 +-
 server/red-worker.h    |  2 +-
 server/stream.c        |  4 ++--
 server/stream.h        |  8 ++++----
 server/utils.h         |  5 ++++-
 8 files changed, 22 insertions(+), 21 deletions(-)

Patch hide | download patch | download mbox

diff --git a/server/dcc.h b/server/dcc.h
index f9703be..4401384 100644
--- a/server/dcc.h
+++ b/server/dcc.h
@@ -31,8 +31,8 @@ 
 #define PALETTE_CACHE_HASH_KEY(id) ((id) & PALETTE_CACHE_HASH_MASK)
 #define CLIENT_PALETTE_CACHE_SIZE 128
 
-#define DISPLAY_CLIENT_TIMEOUT 30000000000ULL //nano
-#define DISPLAY_CLIENT_MIGRATE_DATA_TIMEOUT 10000000000ULL //nano, 10 sec
+#define DISPLAY_CLIENT_TIMEOUT (NSEC_PER_SEC * 30)
+#define DISPLAY_CLIENT_MIGRATE_DATA_TIMEOUT (NSEC_PER_SEC * 10)
 #define DISPLAY_CLIENT_RETRY_INTERVAL 10000 //micro
 
 /* Each drawable can refer to at most 3 images: src, brush and mask */
diff --git a/server/mjpeg-encoder.c b/server/mjpeg-encoder.c
index 9c51f7b..2a040f5 100644
--- a/server/mjpeg-encoder.c
+++ b/server/mjpeg-encoder.c
@@ -68,7 +68,7 @@  static const int mjpeg_quality_samples[MJPEG_QUALITY_SAMPLE_NUM] = {20, 30, 40,
  * are not necessarily related to mis-estimation of the bit rate, and we would
  * like to wait till the stream stabilizes.
  */
-#define MJPEG_WARMUP_TIME 3000000000LL // 3 sec
+#define MJPEG_WARMUP_TIME (NSEC_PER_SEC * 3)
 
 enum {
     MJPEG_QUALITY_EVAL_TYPE_SET,
@@ -659,7 +659,7 @@  static void mjpeg_encoder_adjust_fps(MJpegEncoder *encoder, uint64_t now)
 
     spice_assert(rate_control_is_active(encoder));
 
-    adjusted_fps_time_passed = (now - rate_control->adjusted_fps_start_time) / 1000 / 1000;
+    adjusted_fps_time_passed = (now - rate_control->adjusted_fps_start_time) / NSEC_PER_MILLISEC;
 
     if (!rate_control->during_quality_eval &&
         adjusted_fps_time_passed > MJPEG_ADJUST_FPS_TIMEOUT &&
@@ -724,7 +724,7 @@  static int mjpeg_encoder_start_frame(MJpegEncoder *encoder,
         mjpeg_encoder_adjust_fps(encoder, now);
         interval = (now - rate_control->bit_rate_info.last_frame_time);
 
-        if (interval < (1000*1000*1000) / rate_control->adjusted_fps) {
+        if (interval < NSEC_PER_SEC / rate_control->adjusted_fps) {
             return MJPEG_ENCODER_FRAME_DROP;
         }
 
@@ -1009,7 +1009,7 @@  static void mjpeg_encoder_decrease_bit_rate(MJpegEncoder *encoder)
         double duration_sec;
 
         duration_sec = (bit_rate_info->last_frame_time - bit_rate_info->change_start_time);
-        duration_sec /= (1000.0 * 1000.0 * 1000.0);
+        duration_sec /= NSEC_PER_SEC;
         measured_byte_rate = bit_rate_info->sum_enc_size / duration_sec;
         measured_fps = bit_rate_info->num_enc_frames / duration_sec;
         decrease_size = bit_rate_info->sum_enc_size / bit_rate_info->num_enc_frames;
@@ -1078,7 +1078,7 @@  static void mjpeg_encoder_increase_bit_rate(MJpegEncoder *encoder)
         double duration_sec;
 
         duration_sec = (bit_rate_info->last_frame_time - bit_rate_info->change_start_time);
-        duration_sec /= (1000.0 * 1000.0 * 1000.0);
+        duration_sec /= NSEC_PER_SEC;
         measured_byte_rate = bit_rate_info->sum_enc_size / duration_sec;
         measured_fps = bit_rate_info->num_enc_frames / duration_sec;
         avg_frame_size = bit_rate_info->sum_enc_size / bit_rate_info->num_enc_frames;
diff --git a/server/red-channel.c b/server/red-channel.c
index 361f1a5..f10810e 100644
--- a/server/red-channel.c
+++ b/server/red-channel.c
@@ -1370,7 +1370,7 @@  int red_channel_client_get_roundtrip_ms(RedChannelClient *rcc)
     if (rcc->latency_monitor.roundtrip < 0) {
         return rcc->latency_monitor.roundtrip;
     }
-    return rcc->latency_monitor.roundtrip / 1000 / 1000;
+    return rcc->latency_monitor.roundtrip / NSEC_PER_MILLISEC;
 }
 
 static void red_channel_client_init_outgoing_messages_window(RedChannelClient *rcc)
@@ -1432,9 +1432,7 @@  static void red_channel_client_restart_ping_timer(RedChannelClient *rcc)
 {
     uint64_t passed, timeout;
 
-    passed = spice_get_monotonic_time_ns();
-    passed = passed - rcc->latency_monitor.last_pong_time;
-    passed /= 1000*1000;
+    passed = (spice_get_monotonic_time_ns() - rcc->latency_monitor.last_pong_time) / NSEC_PER_MILLISEC;
     timeout = PING_TEST_IDLE_NET_TIMEOUT_MS;
     if (passed  < PING_TEST_TIMEOUT_MS) {
         timeout += PING_TEST_TIMEOUT_MS - passed;
@@ -1511,7 +1509,7 @@  static void red_channel_client_handle_pong(RedChannelClient *rcc, SpiceMsgPing *
     if (rcc->latency_monitor.roundtrip < 0 ||
         now - ping->timestamp < rcc->latency_monitor.roundtrip) {
         rcc->latency_monitor.roundtrip = now - ping->timestamp;
-        spice_debug("update roundtrip %.2f(ms)", rcc->latency_monitor.roundtrip/1000.0/1000.0);
+        spice_debug("update roundtrip %.2f(ms)", ((double)rcc->latency_monitor.roundtrip)/NSEC_PER_MILLISEC);
     }
 
     rcc->latency_monitor.last_pong_time = now;
diff --git a/server/red-worker.c b/server/red-worker.c
index 37d19cd..5771442 100644
--- a/server/red-worker.c
+++ b/server/red-worker.c
@@ -330,7 +330,7 @@  static int red_process_display(RedWorker *worker, uint32_t max_pipe_size, int *r
         n++;
         if ((worker->display_channel &&
              red_channel_all_blocked(&worker->display_channel->common.base))
-            || spice_get_monotonic_time_ns() - start > 10 * 1000 * 1000) {
+            || spice_get_monotonic_time_ns() - start > NSEC_PER_SEC / 100) {
             worker->event_timeout = 0;
             return n;
         }
diff --git a/server/red-worker.h b/server/red-worker.h
index 44f35f7..e6e3404 100644
--- a/server/red-worker.h
+++ b/server/red-worker.h
@@ -33,7 +33,7 @@  typedef struct CommonChannelClient {
 } CommonChannelClient;
 
 #define COMMON_CHANNEL_CLIENT(Client) ((CommonChannelClient*)(Client))
-#define DISPLAY_CLIENT_TIMEOUT 30000000000ULL //nano
+#define DISPLAY_CLIENT_TIMEOUT (NSEC_PER_SEC * 30)
 
 #define CHANNEL_RECEIVE_BUF_SIZE 1024
 typedef struct CommonChannel {
diff --git a/server/stream.c b/server/stream.c
index ae70296..811f7d3 100644
--- a/server/stream.c
+++ b/server/stream.c
@@ -430,8 +430,8 @@  static void display_channel_create_stream(DisplayChannel *display, Drawable *dra
      * the nearest integer, for instance 24 for 23.976.
      */
     uint64_t duration = drawable->creation_time - drawable->first_frame_time;
-    if (duration > (uint64_t)drawable->frames_count * 1000 * 1000 * 1000 / MAX_FPS) {
-        stream->input_fps = ((uint64_t)drawable->frames_count * 1000 * 1000 * 1000 + duration / 2) / duration;
+    if (duration > NSEC_PER_SEC * drawable->frames_count / MAX_FPS) {
+        stream->input_fps = (NSEC_PER_SEC * drawable->frames_count + duration / 2) / duration;
     } else {
         stream->input_fps = MAX_FPS;
     }
diff --git a/server/stream.h b/server/stream.h
index e20c8de..7b63402 100644
--- a/server/stream.h
+++ b/server/stream.h
@@ -25,14 +25,14 @@ 
 #include "red-channel.h"
 #include "image-cache.h"
 
-#define RED_STREAM_DETACTION_MAX_DELTA ((1000 * 1000 * 1000) / 5) // 1/5 sec
-#define RED_STREAM_CONTINUS_MAX_DELTA (1000 * 1000 * 1000)
-#define RED_STREAM_TIMEOUT (1000 * 1000 * 1000)
+#define RED_STREAM_DETACTION_MAX_DELTA (NSEC_PER_SEC / 5)
+#define RED_STREAM_CONTINUS_MAX_DELTA NSEC_PER_SEC
+#define RED_STREAM_TIMEOUT NSEC_PER_SEC
 #define RED_STREAM_FRAMES_START_CONDITION 20
 #define RED_STREAM_GRADUAL_FRAMES_START_CONDITION 0.2
 #define RED_STREAM_FRAMES_RESET_CONDITION 100
 #define RED_STREAM_MIN_SIZE (96 * 96)
-#define RED_STREAM_INPUT_FPS_TIMEOUT ((uint64_t)5 * 1000 * 1000 * 1000) // 5 sec
+#define RED_STREAM_INPUT_FPS_TIMEOUT (NSEC_PER_SEC * 5)
 #define RED_STREAM_CHANNEL_CAPACITY 0.8
 /* the client's stream report frequency is the minimum of the 2 values below */
 #define RED_STREAM_CLIENT_REPORT_WINDOW 5 // #frames
diff --git a/server/utils.h b/server/utils.h
index 3aa5be0..b85b104 100644
--- a/server/utils.h
+++ b/server/utils.h
@@ -50,13 +50,16 @@  static inline int test_bit(int index, uint32_t val)
 
 typedef int64_t red_time_t;
 
+#define NSEC_PER_SEC      1000000000LL
+#define NSEC_PER_MILLISEC 1000000LL
+
 /* FIXME: consider g_get_monotonic_time (), but in microseconds */
 static inline red_time_t spice_get_monotonic_time_ns(void)
 {
     struct timespec time;
 
     clock_gettime(CLOCK_MONOTONIC, &time);
-    return (red_time_t) time.tv_sec * (1000 * 1000 * 1000) + time.tv_nsec;
+    return NSEC_PER_SEC * time.tv_sec + time.tv_nsec;
 }
 
 static inline red_time_t spice_get_monotonic_time_ms(void)

Comments

Acked-by: Christophe Fergeau <cfergeau@redhat.com>

For what it's worth, I find "3 * NSEC_PER_SEC" to be more readable than
"NSEC_PER_SEC * 3" (ie the unit comes last, not first) (but no need to
change that now imo).

Christophe

On Fri, Dec 11, 2015 at 07:07:22PM +0100, Francois Gouget wrote:
> They clarify the time unit being used, reduce the need for casts and
> simplify calculations.
> 
> Signed-off-by: Francois Gouget <fgouget@codeweavers.com>
> ---
>  server/dcc.h           |  4 ++--
>  server/mjpeg-encoder.c | 10 +++++-----
>  server/red-channel.c   |  8 +++-----
>  server/red-worker.c    |  2 +-
>  server/red-worker.h    |  2 +-
>  server/stream.c        |  4 ++--
>  server/stream.h        |  8 ++++----
>  server/utils.h         |  5 ++++-
>  8 files changed, 22 insertions(+), 21 deletions(-)
> 
> diff --git a/server/dcc.h b/server/dcc.h
> index f9703be..4401384 100644
> --- a/server/dcc.h
> +++ b/server/dcc.h
> @@ -31,8 +31,8 @@
>  #define PALETTE_CACHE_HASH_KEY(id) ((id) & PALETTE_CACHE_HASH_MASK)
>  #define CLIENT_PALETTE_CACHE_SIZE 128
>  
> -#define DISPLAY_CLIENT_TIMEOUT 30000000000ULL //nano
> -#define DISPLAY_CLIENT_MIGRATE_DATA_TIMEOUT 10000000000ULL //nano, 10 sec
> +#define DISPLAY_CLIENT_TIMEOUT (NSEC_PER_SEC * 30)
> +#define DISPLAY_CLIENT_MIGRATE_DATA_TIMEOUT (NSEC_PER_SEC * 10)
>  #define DISPLAY_CLIENT_RETRY_INTERVAL 10000 //micro
>  
>  /* Each drawable can refer to at most 3 images: src, brush and mask */
> diff --git a/server/mjpeg-encoder.c b/server/mjpeg-encoder.c
> index 9c51f7b..2a040f5 100644
> --- a/server/mjpeg-encoder.c
> +++ b/server/mjpeg-encoder.c
> @@ -68,7 +68,7 @@ static const int mjpeg_quality_samples[MJPEG_QUALITY_SAMPLE_NUM] = {20, 30, 40,
>   * are not necessarily related to mis-estimation of the bit rate, and we would
>   * like to wait till the stream stabilizes.
>   */
> -#define MJPEG_WARMUP_TIME 3000000000LL // 3 sec
> +#define MJPEG_WARMUP_TIME (NSEC_PER_SEC * 3)
>  
>  enum {
>      MJPEG_QUALITY_EVAL_TYPE_SET,
> @@ -659,7 +659,7 @@ static void mjpeg_encoder_adjust_fps(MJpegEncoder *encoder, uint64_t now)
>  
>      spice_assert(rate_control_is_active(encoder));
>  
> -    adjusted_fps_time_passed = (now - rate_control->adjusted_fps_start_time) / 1000 / 1000;
> +    adjusted_fps_time_passed = (now - rate_control->adjusted_fps_start_time) / NSEC_PER_MILLISEC;
>  
>      if (!rate_control->during_quality_eval &&
>          adjusted_fps_time_passed > MJPEG_ADJUST_FPS_TIMEOUT &&
> @@ -724,7 +724,7 @@ static int mjpeg_encoder_start_frame(MJpegEncoder *encoder,
>          mjpeg_encoder_adjust_fps(encoder, now);
>          interval = (now - rate_control->bit_rate_info.last_frame_time);
>  
> -        if (interval < (1000*1000*1000) / rate_control->adjusted_fps) {
> +        if (interval < NSEC_PER_SEC / rate_control->adjusted_fps) {
>              return MJPEG_ENCODER_FRAME_DROP;
>          }
>  
> @@ -1009,7 +1009,7 @@ static void mjpeg_encoder_decrease_bit_rate(MJpegEncoder *encoder)
>          double duration_sec;
>  
>          duration_sec = (bit_rate_info->last_frame_time - bit_rate_info->change_start_time);
> -        duration_sec /= (1000.0 * 1000.0 * 1000.0);
> +        duration_sec /= NSEC_PER_SEC;
>          measured_byte_rate = bit_rate_info->sum_enc_size / duration_sec;
>          measured_fps = bit_rate_info->num_enc_frames / duration_sec;
>          decrease_size = bit_rate_info->sum_enc_size / bit_rate_info->num_enc_frames;
> @@ -1078,7 +1078,7 @@ static void mjpeg_encoder_increase_bit_rate(MJpegEncoder *encoder)
>          double duration_sec;
>  
>          duration_sec = (bit_rate_info->last_frame_time - bit_rate_info->change_start_time);
> -        duration_sec /= (1000.0 * 1000.0 * 1000.0);
> +        duration_sec /= NSEC_PER_SEC;
>          measured_byte_rate = bit_rate_info->sum_enc_size / duration_sec;
>          measured_fps = bit_rate_info->num_enc_frames / duration_sec;
>          avg_frame_size = bit_rate_info->sum_enc_size / bit_rate_info->num_enc_frames;
> diff --git a/server/red-channel.c b/server/red-channel.c
> index 361f1a5..f10810e 100644
> --- a/server/red-channel.c
> +++ b/server/red-channel.c
> @@ -1370,7 +1370,7 @@ int red_channel_client_get_roundtrip_ms(RedChannelClient *rcc)
>      if (rcc->latency_monitor.roundtrip < 0) {
>          return rcc->latency_monitor.roundtrip;
>      }
> -    return rcc->latency_monitor.roundtrip / 1000 / 1000;
> +    return rcc->latency_monitor.roundtrip / NSEC_PER_MILLISEC;
>  }
>  
>  static void red_channel_client_init_outgoing_messages_window(RedChannelClient *rcc)
> @@ -1432,9 +1432,7 @@ static void red_channel_client_restart_ping_timer(RedChannelClient *rcc)
>  {
>      uint64_t passed, timeout;
>  
> -    passed = spice_get_monotonic_time_ns();
> -    passed = passed - rcc->latency_monitor.last_pong_time;
> -    passed /= 1000*1000;
> +    passed = (spice_get_monotonic_time_ns() - rcc->latency_monitor.last_pong_time) / NSEC_PER_MILLISEC;
>      timeout = PING_TEST_IDLE_NET_TIMEOUT_MS;
>      if (passed  < PING_TEST_TIMEOUT_MS) {
>          timeout += PING_TEST_TIMEOUT_MS - passed;
> @@ -1511,7 +1509,7 @@ static void red_channel_client_handle_pong(RedChannelClient *rcc, SpiceMsgPing *
>      if (rcc->latency_monitor.roundtrip < 0 ||
>          now - ping->timestamp < rcc->latency_monitor.roundtrip) {
>          rcc->latency_monitor.roundtrip = now - ping->timestamp;
> -        spice_debug("update roundtrip %.2f(ms)", rcc->latency_monitor.roundtrip/1000.0/1000.0);
> +        spice_debug("update roundtrip %.2f(ms)", ((double)rcc->latency_monitor.roundtrip)/NSEC_PER_MILLISEC);
>      }
>  
>      rcc->latency_monitor.last_pong_time = now;
> diff --git a/server/red-worker.c b/server/red-worker.c
> index 37d19cd..5771442 100644
> --- a/server/red-worker.c
> +++ b/server/red-worker.c
> @@ -330,7 +330,7 @@ static int red_process_display(RedWorker *worker, uint32_t max_pipe_size, int *r
>          n++;
>          if ((worker->display_channel &&
>               red_channel_all_blocked(&worker->display_channel->common.base))
> -            || spice_get_monotonic_time_ns() - start > 10 * 1000 * 1000) {
> +            || spice_get_monotonic_time_ns() - start > NSEC_PER_SEC / 100) {
>              worker->event_timeout = 0;
>              return n;
>          }
> diff --git a/server/red-worker.h b/server/red-worker.h
> index 44f35f7..e6e3404 100644
> --- a/server/red-worker.h
> +++ b/server/red-worker.h
> @@ -33,7 +33,7 @@ typedef struct CommonChannelClient {
>  } CommonChannelClient;
>  
>  #define COMMON_CHANNEL_CLIENT(Client) ((CommonChannelClient*)(Client))
> -#define DISPLAY_CLIENT_TIMEOUT 30000000000ULL //nano
> +#define DISPLAY_CLIENT_TIMEOUT (NSEC_PER_SEC * 30)
>  
>  #define CHANNEL_RECEIVE_BUF_SIZE 1024
>  typedef struct CommonChannel {
> diff --git a/server/stream.c b/server/stream.c
> index ae70296..811f7d3 100644
> --- a/server/stream.c
> +++ b/server/stream.c
> @@ -430,8 +430,8 @@ static void display_channel_create_stream(DisplayChannel *display, Drawable *dra
>       * the nearest integer, for instance 24 for 23.976.
>       */
>      uint64_t duration = drawable->creation_time - drawable->first_frame_time;
> -    if (duration > (uint64_t)drawable->frames_count * 1000 * 1000 * 1000 / MAX_FPS) {
> -        stream->input_fps = ((uint64_t)drawable->frames_count * 1000 * 1000 * 1000 + duration / 2) / duration;
> +    if (duration > NSEC_PER_SEC * drawable->frames_count / MAX_FPS) {
> +        stream->input_fps = (NSEC_PER_SEC * drawable->frames_count + duration / 2) / duration;
>      } else {
>          stream->input_fps = MAX_FPS;
>      }
> diff --git a/server/stream.h b/server/stream.h
> index e20c8de..7b63402 100644
> --- a/server/stream.h
> +++ b/server/stream.h
> @@ -25,14 +25,14 @@
>  #include "red-channel.h"
>  #include "image-cache.h"
>  
> -#define RED_STREAM_DETACTION_MAX_DELTA ((1000 * 1000 * 1000) / 5) // 1/5 sec
> -#define RED_STREAM_CONTINUS_MAX_DELTA (1000 * 1000 * 1000)
> -#define RED_STREAM_TIMEOUT (1000 * 1000 * 1000)
> +#define RED_STREAM_DETACTION_MAX_DELTA (NSEC_PER_SEC / 5)
> +#define RED_STREAM_CONTINUS_MAX_DELTA NSEC_PER_SEC
> +#define RED_STREAM_TIMEOUT NSEC_PER_SEC
>  #define RED_STREAM_FRAMES_START_CONDITION 20
>  #define RED_STREAM_GRADUAL_FRAMES_START_CONDITION 0.2
>  #define RED_STREAM_FRAMES_RESET_CONDITION 100
>  #define RED_STREAM_MIN_SIZE (96 * 96)
> -#define RED_STREAM_INPUT_FPS_TIMEOUT ((uint64_t)5 * 1000 * 1000 * 1000) // 5 sec
> +#define RED_STREAM_INPUT_FPS_TIMEOUT (NSEC_PER_SEC * 5)
>  #define RED_STREAM_CHANNEL_CAPACITY 0.8
>  /* the client's stream report frequency is the minimum of the 2 values below */
>  #define RED_STREAM_CLIENT_REPORT_WINDOW 5 // #frames
> diff --git a/server/utils.h b/server/utils.h
> index 3aa5be0..b85b104 100644
> --- a/server/utils.h
> +++ b/server/utils.h
> @@ -50,13 +50,16 @@ static inline int test_bit(int index, uint32_t val)
>  
>  typedef int64_t red_time_t;
>  
> +#define NSEC_PER_SEC      1000000000LL
> +#define NSEC_PER_MILLISEC 1000000LL
> +
>  /* FIXME: consider g_get_monotonic_time (), but in microseconds */
>  static inline red_time_t spice_get_monotonic_time_ns(void)
>  {
>      struct timespec time;
>  
>      clock_gettime(CLOCK_MONOTONIC, &time);
> -    return (red_time_t) time.tv_sec * (1000 * 1000 * 1000) + time.tv_nsec;
> +    return NSEC_PER_SEC * time.tv_sec + time.tv_nsec;
>  }
>  
>  static inline red_time_t spice_get_monotonic_time_ms(void)
> -- 
> 2.6.2
> 
> _______________________________________________
> Spice-devel mailing list
> Spice-devel@lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/spice-devel