[Spice-devel] [spice] streaming: Always delegate bit rate control to the video encoder

Francois Gouget fgouget at codeweavers.com
Thu Aug 18 16:02:30 UTC 2016


The video encoders already have sophisticated bit rate control code that
can react to both stream reports sent by the client, and server frame
drop notifications. Furthermore they can adjust both the frame rate and
the image quality to best match the network conditions.

But if the client cannot send stream reports all this is bypassed and
instead the streaming code performs its own primitive bit rate control
that can only adjust the frame rate.

So this patch removes the code duplication and lets the video encoders
do their job.

Signed-off-by: Francois Gouget <fgouget at codeweavers.com>
---
 server/dcc-private.h       |  1 -
 server/dcc-send.c          | 15 ---------
 server/dcc.c               |  7 -----
 server/dcc.h               |  1 -
 server/gstreamer-encoder.c |  9 ++----
 server/mjpeg-encoder.c     | 76 ++++++++++++++++------------------------------
 server/stream.c            | 69 ++++++-----------------------------------
 server/stream.h            |  3 --
 8 files changed, 39 insertions(+), 142 deletions(-)

diff --git a/server/dcc-private.h b/server/dcc-private.h
index 02b51dd..28f63a2 100644
--- a/server/dcc-private.h
+++ b/server/dcc-private.h
@@ -54,7 +54,6 @@ struct DisplayChannelClient {
     QRegion surface_client_lossy_region[NUM_SURFACES];
 
     StreamAgent stream_agents[NUM_STREAMS];
-    int use_video_encoder_rate_control;
     uint32_t streams_max_latency;
     uint64_t streams_max_bit_rate;
     bool gl_draw_ongoing;
diff --git a/server/dcc-send.c b/server/dcc-send.c
index 406907b..7fefcdb 100644
--- a/server/dcc-send.c
+++ b/server/dcc-send.c
@@ -1706,18 +1706,6 @@ static int red_marshall_stream_data(RedChannelClient *rcc,
     }
 
     StreamAgent *agent = &dcc->stream_agents[get_stream_id(display, stream)];
-    uint64_t time_now = spice_get_monotonic_time_ns();
-
-    if (!dcc->use_video_encoder_rate_control) {
-        if (time_now - agent->last_send_time < (1000 * 1000 * 1000) / agent->fps) {
-            agent->frames--;
-#ifdef STREAM_STATS
-            agent->stats.num_drops_fps++;
-#endif
-            return TRUE;
-        }
-    }
-
     VideoBuffer *outbuf;
     /* workaround for vga streams */
     frame_mm_time =  drawable->red_drawable->mm_time ?
@@ -1732,7 +1720,6 @@ static int red_marshall_stream_data(RedChannelClient *rcc,
                                              &outbuf);
     switch (ret) {
     case VIDEO_ENCODER_FRAME_DROP:
-        spice_assert(dcc->use_video_encoder_rate_control);
 #ifdef STREAM_STATS
         agent->stats.num_drops_fps++;
 #endif
@@ -1774,7 +1761,6 @@ static int red_marshall_stream_data(RedChannelClient *rcc,
     }
     spice_marshaller_add_ref_full(base_marshaller, outbuf->data, outbuf->size,
                                   &red_release_video_encoder_buffer, outbuf);
-    agent->last_send_time = time_now;
 #ifdef STREAM_STATS
     agent->stats.num_frames_sent++;
     agent->stats.size_sent += outbuf->size;
@@ -2157,7 +2143,6 @@ static void marshall_stream_start(RedChannelClient *rcc,
     DisplayChannelClient *dcc = RCC_TO_DCC(rcc);
     Stream *stream = agent->stream;
 
-    agent->last_send_time = 0;
     spice_assert(stream);
     if (!agent->video_encoder) {
         /* Without a video encoder nothing will be streamed */
diff --git a/server/dcc.c b/server/dcc.c
index ddc23e2..1501629 100644
--- a/server/dcc.c
+++ b/server/dcc.c
@@ -346,8 +346,6 @@ static void dcc_init_stream_agents(DisplayChannelClient *dcc)
         region_init(&agent->vis_region);
         region_init(&agent->clip);
     }
-    dcc->use_video_encoder_rate_control =
-        red_channel_client_test_remote_cap(RED_CHANNEL_CLIENT(dcc), SPICE_DISPLAY_CAP_STREAM_REPORT);
 }
 
 #define DISPLAY_FREE_LIST_DEFAULT_SIZE 128
@@ -1161,11 +1159,6 @@ spice_wan_compression_t dcc_get_zlib_glz_state(DisplayChannelClient *dcc)
     return dcc->zlib_glz_state;
 }
 
-gboolean dcc_use_video_encoder_rate_control(DisplayChannelClient *dcc)
-{
-    return dcc->use_video_encoder_rate_control;
-}
-
 uint32_t dcc_get_max_stream_latency(DisplayChannelClient *dcc)
 {
     return dcc->streams_max_latency;
diff --git a/server/dcc.h b/server/dcc.h
index 672fb2f..2deff8b 100644
--- a/server/dcc.h
+++ b/server/dcc.h
@@ -165,7 +165,6 @@ StreamAgent *              dcc_get_stream_agent                      (DisplayCha
 ImageEncoders *dcc_get_encoders(DisplayChannelClient *dcc);
 spice_wan_compression_t    dcc_get_jpeg_state                        (DisplayChannelClient *dcc);
 spice_wan_compression_t    dcc_get_zlib_glz_state                    (DisplayChannelClient *dcc);
-gboolean                   dcc_use_video_encoder_rate_control        (DisplayChannelClient *dcc);
 uint32_t dcc_get_max_stream_latency(DisplayChannelClient *dcc);
 void dcc_set_max_stream_latency(DisplayChannelClient *dcc, uint32_t latency);
 uint64_t dcc_get_max_stream_bit_rate(DisplayChannelClient *dcc);
diff --git a/server/gstreamer-encoder.c b/server/gstreamer-encoder.c
index d575c67..cb04569 100644
--- a/server/gstreamer-encoder.c
+++ b/server/gstreamer-encoder.c
@@ -1495,9 +1495,8 @@ static int spice_gst_encoder_encode_frame(VideoEncoder *video_encoder,
         return VIDEO_ENCODER_FRAME_UNSUPPORTED;
     }
 
-    if (rate_control_is_active(encoder) &&
-        (handle_server_drops(encoder, frame_mm_time) ||
-         frame_mm_time < encoder->next_frame_mm_time)) {
+    if (handle_server_drops(encoder, frame_mm_time) ||
+        frame_mm_time < encoder->next_frame_mm_time) {
         /* Drop the frame to limit the outgoing bit rate. */
         return VIDEO_ENCODER_FRAME_DROP;
     }
@@ -1711,10 +1710,8 @@ VideoEncoder *gstreamer_encoder_new(SpiceVideoCodecType codec_type,
     encoder->unused_bitmap_opaques = g_async_queue_new();
 #endif
 
-    if (cbs) {
-        encoder->cbs = *cbs;
-    }
     encoder->starting_bit_rate = starting_bit_rate;
+    encoder->cbs = *cbs;
     encoder->bitmap_ref = bitmap_ref;
     encoder->bitmap_unref = bitmap_unref;
     encoder->format = GSTREAMER_FORMAT_INVALID;
diff --git a/server/mjpeg-encoder.c b/server/mjpeg-encoder.c
index 1649516..027ffe0 100644
--- a/server/mjpeg-encoder.c
+++ b/server/mjpeg-encoder.c
@@ -32,8 +32,6 @@
 #define MJPEG_QUALITY_SAMPLE_NUM 7
 static const int mjpeg_quality_samples[MJPEG_QUALITY_SAMPLE_NUM] = {20, 30, 40, 50, 60, 70, 80};
 
-#define MJPEG_LEGACY_STATIC_QUALITY_ID 5 // jpeg quality 70
-
 #define MJPEG_IMPROVE_QUALITY_FPS_STRICT_TH 10
 #define MJPEG_IMPROVE_QUALITY_FPS_PERMISSIVE_TH 5
 
@@ -208,11 +206,6 @@ static MJpegVideoBuffer* create_mjpeg_video_buffer(void)
     return buffer;
 }
 
-static inline int rate_control_is_active(MJpegEncoder* encoder)
-{
-    return encoder->cbs.get_roundtrip_ms != NULL;
-}
-
 static void mjpeg_encoder_destroy(VideoEncoder *video_encoder)
 {
     MJpegEncoder *encoder = (MJpegEncoder*)video_encoder;
@@ -592,8 +585,6 @@ static void mjpeg_encoder_adjust_params_to_bit_rate(MJpegEncoder *encoder)
     uint32_t latency = 0;
     uint32_t src_fps;
 
-    spice_assert(rate_control_is_active(encoder));
-
     rate_control = &encoder->rate_control;
     quality_eval = &rate_control->quality_eval_data;
 
@@ -677,8 +668,6 @@ static void mjpeg_encoder_adjust_fps(MJpegEncoder *encoder, uint64_t now)
     MJpegEncoderRateControl *rate_control = &encoder->rate_control;
     uint64_t adjusted_fps_time_passed;
 
-    spice_assert(rate_control_is_active(encoder));
-
     adjusted_fps_time_passed = (now - rate_control->adjusted_fps_start_time) / NSEC_PER_MILLISEC;
 
     if (!rate_control->during_quality_eval &&
@@ -731,37 +720,35 @@ static int mjpeg_encoder_start_frame(MJpegEncoder *encoder,
 {
     uint32_t quality;
 
-    if (rate_control_is_active(encoder)) {
-        MJpegEncoderRateControl *rate_control = &encoder->rate_control;
-        uint64_t now;
-        uint64_t interval;
+    MJpegEncoderRateControl *rate_control = &encoder->rate_control;
+    uint64_t now;
+    uint64_t interval;
 
-        now = spice_get_monotonic_time_ns();
+    now = spice_get_monotonic_time_ns();
 
-        if (!rate_control->adjusted_fps_start_time) {
-            rate_control->adjusted_fps_start_time = now;
-        }
-        mjpeg_encoder_adjust_fps(encoder, now);
-        interval = (now - rate_control->bit_rate_info.last_frame_time);
+    if (!rate_control->adjusted_fps_start_time) {
+        rate_control->adjusted_fps_start_time = now;
+    }
+    mjpeg_encoder_adjust_fps(encoder, now);
+    interval = (now - rate_control->bit_rate_info.last_frame_time);
 
-        if (interval < NSEC_PER_SEC / rate_control->adjusted_fps) {
-            return VIDEO_ENCODER_FRAME_DROP;
-        }
+    if (interval < NSEC_PER_SEC / rate_control->adjusted_fps) {
+        return VIDEO_ENCODER_FRAME_DROP;
+    }
 
-        mjpeg_encoder_adjust_params_to_bit_rate(encoder);
+    mjpeg_encoder_adjust_params_to_bit_rate(encoder);
 
-        if (!rate_control->during_quality_eval ||
-            rate_control->quality_eval_data.reason == MJPEG_QUALITY_EVAL_REASON_SIZE_CHANGE) {
-            MJpegEncoderBitRateInfo *bit_rate_info;
+    if (!rate_control->during_quality_eval ||
+        rate_control->quality_eval_data.reason == MJPEG_QUALITY_EVAL_REASON_SIZE_CHANGE) {
+        MJpegEncoderBitRateInfo *bit_rate_info;
 
-            bit_rate_info = &encoder->rate_control.bit_rate_info;
+        bit_rate_info = &encoder->rate_control.bit_rate_info;
 
-            if (!bit_rate_info->change_start_time) {
-                bit_rate_info->change_start_time = now;
-                bit_rate_info->change_start_mm_time = frame_mm_time;
-            }
-            bit_rate_info->last_frame_time = now;
+        if (!bit_rate_info->change_start_time) {
+            bit_rate_info->change_start_time = now;
+            bit_rate_info->change_start_mm_time = frame_mm_time;
         }
+        bit_rate_info->last_frame_time = now;
     }
 
     encoder->cinfo.in_color_space   = JCS_RGB;
@@ -1222,10 +1209,6 @@ static void mjpeg_encoder_client_stream_report(VideoEncoder *video_encoder,
                 end_frame_mm_time - start_frame_mm_time,
                 end_frame_delay, audio_delay);
 
-    if (!rate_control_is_active(encoder)) {
-        spice_debug("rate control was not activated: ignoring");
-        return;
-    }
     if (rate_control->during_quality_eval) {
         if (rate_control->quality_eval_data.type == MJPEG_QUALITY_EVAL_TYPE_DOWNGRADE &&
             rate_control->quality_eval_data.reason == MJPEG_QUALITY_EVAL_REASON_RATE_CHANGE) {
@@ -1387,17 +1370,12 @@ VideoEncoder *mjpeg_encoder_new(SpiceVideoCodecType codec_type,
     encoder->rate_control.byte_rate = starting_bit_rate / 8;
     encoder->starting_bit_rate = starting_bit_rate;
 
-    if (cbs) {
-        encoder->cbs = *cbs;
-        mjpeg_encoder_reset_quality(encoder, MJPEG_QUALITY_SAMPLE_NUM / 2, 5, 0);
-        encoder->rate_control.during_quality_eval = TRUE;
-        encoder->rate_control.quality_eval_data.type = MJPEG_QUALITY_EVAL_TYPE_SET;
-        encoder->rate_control.quality_eval_data.reason = MJPEG_QUALITY_EVAL_REASON_RATE_CHANGE;
-        encoder->rate_control.warmup_start_time = spice_get_monotonic_time_ns();
-    } else {
-        encoder->cbs.get_roundtrip_ms = NULL;
-        mjpeg_encoder_reset_quality(encoder, MJPEG_LEGACY_STATIC_QUALITY_ID, MJPEG_MAX_FPS, 0);
-    }
+    encoder->cbs = *cbs;
+    mjpeg_encoder_reset_quality(encoder, MJPEG_QUALITY_SAMPLE_NUM / 2, 5, 0);
+    encoder->rate_control.during_quality_eval = TRUE;
+    encoder->rate_control.quality_eval_data.type = MJPEG_QUALITY_EVAL_TYPE_SET;
+    encoder->rate_control.quality_eval_data.reason = MJPEG_QUALITY_EVAL_REASON_RATE_CHANGE;
+    encoder->rate_control.warmup_start_time = spice_get_monotonic_time_ns();
 
     encoder->cinfo.err = jpeg_std_error(&encoder->jerr);
     jpeg_create_compress(&encoder->cinfo);
diff --git a/server/stream.c b/server/stream.c
index e070202..5239468 100644
--- a/server/stream.c
+++ b/server/stream.c
@@ -108,7 +108,7 @@ void stream_stop(DisplayChannel *display, Stream *stream)
         stream_agent = dcc_get_stream_agent(dcc, get_stream_id(display, stream));
         region_clear(&stream_agent->vis_region);
         region_clear(&stream_agent->clip);
-        if (stream_agent->video_encoder && dcc_use_video_encoder_rate_control(dcc)) {
+        if (stream_agent->video_encoder) {
             uint64_t stream_bit_rate = stream_agent->video_encoder->get_bit_rate(stream_agent->video_encoder);
 
             if (stream_bit_rate > dcc_get_max_stream_bit_rate(dcc)) {
@@ -336,7 +336,6 @@ static void before_reattach_stream(DisplayChannel *display,
     int index;
     StreamAgent *agent;
     RingItem *ring_item, *next;
-    GList *link, *link_next;
 
     spice_return_if_fail(stream->current);
 
@@ -354,53 +353,13 @@ static void before_reattach_stream(DisplayChannel *display,
         dcc = dpi->dcc;
         agent = dcc_get_stream_agent(dcc, index);
 
-        if (!dcc_use_video_encoder_rate_control(dcc) &&
-            !dcc_is_low_bandwidth(dcc)) {
-            continue;
-        }
-
         if (red_pipe_item_is_linked(&dpi->dpi_pipe_item)) {
 #ifdef STREAM_STATS
             agent->stats.num_drops_pipe++;
 #endif
-            if (dcc_use_video_encoder_rate_control(dcc)) {
-                agent->video_encoder->notify_server_frame_drop(agent->video_encoder);
-            } else {
-                ++agent->drops;
-            }
+            agent->video_encoder->notify_server_frame_drop(agent->video_encoder);
         }
     }
-
-
-    FOREACH_CLIENT(display, link, link_next, dcc) {
-        double drop_factor;
-
-        agent = dcc_get_stream_agent(dcc, index);
-
-        if (dcc_use_video_encoder_rate_control(dcc)) {
-            continue;
-        }
-        if (agent->frames / agent->fps < FPS_TEST_INTERVAL) {
-            agent->frames++;
-            continue;
-        }
-        drop_factor = ((double)agent->frames - (double)agent->drops) /
-            (double)agent->frames;
-        spice_debug("stream %d: #frames %u #drops %u", index, agent->frames, agent->drops);
-        if (drop_factor == 1) {
-            if (agent->fps < MAX_FPS) {
-                agent->fps++;
-                spice_debug("stream %d: fps++ %u", index, agent->fps);
-            }
-        } else if (drop_factor < 0.9) {
-            if (agent->fps > 1) {
-                agent->fps--;
-                spice_debug("stream %d: fps--%u", index, agent->fps);
-            }
-        }
-        agent->frames = 1;
-        agent->drops = 0;
-    }
 }
 
 static Stream *display_channel_stream_try_new(DisplayChannel *display)
@@ -761,30 +720,20 @@ void dcc_create_stream(DisplayChannelClient *dcc, Stream *stream)
     spice_return_if_fail(region_is_empty(&agent->vis_region));
 
     if (stream->current) {
-        agent->frames = 1;
         region_clone(&agent->vis_region, &stream->current->tree_item.base.rgn);
         region_clone(&agent->clip, &agent->vis_region);
-    } else {
-        agent->frames = 0;
     }
-    agent->drops = 0;
     agent->fps = MAX_FPS;
     agent->dcc = dcc;
 
-    if (dcc_use_video_encoder_rate_control(dcc)) {
-        VideoEncoderRateControlCbs video_cbs;
-        uint64_t initial_bit_rate;
-
-        video_cbs.opaque = agent;
-        video_cbs.get_roundtrip_ms = get_roundtrip_ms;
-        video_cbs.get_source_fps = get_source_fps;
-        video_cbs.update_client_playback_delay = update_client_playback_delay;
+    VideoEncoderRateControlCbs video_cbs;
+    video_cbs.opaque = agent;
+    video_cbs.get_roundtrip_ms = get_roundtrip_ms;
+    video_cbs.get_source_fps = get_source_fps;
+    video_cbs.update_client_playback_delay = update_client_playback_delay;
 
-        initial_bit_rate = get_initial_bit_rate(dcc, stream);
-        agent->video_encoder = dcc_create_video_encoder(dcc, initial_bit_rate, &video_cbs);
-    } else {
-        agent->video_encoder = dcc_create_video_encoder(dcc, 0, NULL);
-    }
+    uint64_t initial_bit_rate = get_initial_bit_rate(dcc, stream);
+    agent->video_encoder = dcc_create_video_encoder(dcc, initial_bit_rate, &video_cbs);
     red_channel_client_pipe_add(RED_CHANNEL_CLIENT(dcc), stream_create_item_new(agent));
 
     if (red_channel_client_test_remote_cap(RED_CHANNEL_CLIENT(dcc), SPICE_DISPLAY_CAP_STREAM_REPORT)) {
diff --git a/server/stream.h b/server/stream.h
index 053b23d..9e9dc42 100644
--- a/server/stream.h
+++ b/server/stream.h
@@ -76,12 +76,9 @@ typedef struct StreamAgent {
                            displays fragments of the video */
 
     Stream *stream;
-    uint64_t last_send_time;
     VideoEncoder *video_encoder;
     DisplayChannelClient *dcc;
 
-    int frames;
-    int drops;
     int fps;
 
     uint32_t report_id;
-- 
2.8.1


More information about the Spice-devel mailing list