[Spice-devel] [spice v2] streaming: Always delegate bit rate control to the video encoder
Francois Gouget
fgouget at codeweavers.com
Thu Oct 27 17:02:01 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>
---
No one commented so it is unchanged since august:
https://lists.freedesktop.org/archives/spice-devel/2016-August/031461.html
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 de6ea92..64b32a7 100644
--- a/server/dcc-private.h
+++ b/server/dcc-private.h
@@ -55,7 +55,6 @@ struct DisplayChannelClientPrivate
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 ef67f97..5904808 100644
--- a/server/dcc-send.c
+++ b/server/dcc-send.c
@@ -1689,18 +1689,6 @@ static int red_marshall_stream_data(RedChannelClient *rcc,
}
StreamAgent *agent = &dcc->priv->stream_agents[display_channel_get_stream_id(display, stream)];
- uint64_t time_now = spice_get_monotonic_time_ns();
-
- if (!dcc->priv->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 ?
@@ -1715,7 +1703,6 @@ static int red_marshall_stream_data(RedChannelClient *rcc,
&outbuf);
switch (ret) {
case VIDEO_ENCODER_FRAME_DROP:
- spice_assert(dcc->priv->use_video_encoder_rate_control);
#ifdef STREAM_STATS
agent->stats.num_drops_fps++;
#endif
@@ -1757,7 +1744,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;
@@ -2142,7 +2128,6 @@ static void marshall_stream_start(RedChannelClient *rcc,
DisplayChannelClient *dcc = DISPLAY_CHANNEL_CLIENT(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 97b6280..d09ebb0 100644
--- a/server/dcc.c
+++ b/server/dcc.c
@@ -477,8 +477,6 @@ static void dcc_init_stream_agents(DisplayChannelClient *dcc)
region_init(&agent->vis_region);
region_init(&agent->clip);
}
- dcc->priv->use_video_encoder_rate_control =
- red_channel_client_test_remote_cap(RED_CHANNEL_CLIENT(dcc), SPICE_DISPLAY_CAP_STREAM_REPORT);
}
DisplayChannelClient *dcc_new(DisplayChannel *display,
@@ -1291,11 +1289,6 @@ spice_wan_compression_t dcc_get_zlib_glz_state(DisplayChannelClient *dcc)
return dcc->priv->zlib_glz_state;
}
-gboolean dcc_use_video_encoder_rate_control(DisplayChannelClient *dcc)
-{
- return dcc->priv->use_video_encoder_rate_control;
-}
-
uint32_t dcc_get_max_stream_latency(DisplayChannelClient *dcc)
{
return dcc->priv->streams_max_latency;
diff --git a/server/dcc.h b/server/dcc.h
index e4fe788..6fb468d 100644
--- a/server/dcc.h
+++ b/server/dcc.h
@@ -198,7 +198,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 d95c645..f3e97ef 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) {
@@ -1388,17 +1371,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 4e70222..78d3980 100644
--- a/server/stream.c
+++ b/server/stream.c
@@ -109,7 +109,7 @@ void stream_stop(DisplayChannel *display, Stream *stream)
stream_agent = dcc_get_stream_agent(dcc, display_channel_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;
GList *dpi_link, *dpi_next;
- GListIter iter;
spice_return_if_fail(stream->current);
@@ -356,54 +355,14 @@ 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_channel_client_pipe_item_is_linked(RED_CHANNEL_CLIENT(dcc),
&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_DCC(display, iter, 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)
@@ -767,30 +726,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 a415e43..2dfd9e0 100644
--- a/server/stream.h
+++ b/server/stream.h
@@ -73,12 +73,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.9.3
More information about the Spice-devel
mailing list