Re: [PATCH spice 2/11] server: Remove the rate_control_is_active field from MJpegEncoder.

[Marc-André Lureau <marcandre.lureau@xxxxxxxxx>]

ack

On Wed, May 13, 2015 at 10:26 PM, Francois Gouget <fgouget@xxxxxxxxxxxxxxx> wrote:

It is redundant with the corresponding callbacks.
---

This patch only depends on patch 1/11 and is independent from the rest
of the series.

 server/mjpeg_encoder.c | 22 +++++++++++++---------
 server/mjpeg_encoder.h |  2 +-
 server/red_worker.c    |  4 ++--
 3 files changed, 16 insertions(+), 12 deletions(-)

diff --git a/server/mjpeg_encoder.c b/server/mjpeg_encoder.c
index 95d841f..9a41ef3 100644
--- a/server/mjpeg_encoder.c
+++ b/server/mjpeg_encoder.c
@@ -164,7 +164,6 @@ struct MJpegEncoder {
     unsigned int bytes_per_pixel; /* bytes per pixel of the input buffer */
     void (*pixel_converter)(uint8_t *src, uint8_t *dest);

-    int rate_control_is_active;
     MJpegEncoderRateControl rate_control;
     MJpegEncoderRateControlCbs cbs;
     void *cbs_opaque;
@@ -185,21 +184,25 @@ static uint32_t get_min_required_playback_delay(uint64_t frame_enc_size,
                                                 uint64_t byte_rate,
                                                 uint32_t latency);

-MJpegEncoder *mjpeg_encoder_new(int bit_rate_control, uint64_t starting_bit_rate,
+static inline int rate_control_is_active(MJpegEncoder* encoder)
+{
+    return encoder->cbs.get_roundtrip_ms != NULL;
+}
+
+MJpegEncoder *mjpeg_encoder_new(uint64_t starting_bit_rate,
                                 MJpegEncoderRateControlCbs *cbs, void *opaque)
 {
     MJpegEncoder *enc;

-    spice_assert(!bit_rate_control || (cbs && cbs->get_roundtrip_ms && cbs->get_source_fps));
+    spice_assert(!cbs || (cbs && cbs->get_roundtrip_ms && cbs->get_source_fps));

     enc = spice_new0(MJpegEncoder, 1);

     enc->first_frame = TRUE;
-    enc->rate_control_is_active = bit_rate_control;
     enc->rate_control.byte_rate = starting_bit_rate / 8;
     enc->starting_bit_rate = starting_bit_rate;

-    if (bit_rate_control) {
+    if (cbs) {
         struct timespec time;

         clock_gettime(CLOCK_MONOTONIC, &time);
@@ -211,6 +214,7 @@ MJpegEncoder *mjpeg_encoder_new(int bit_rate_control, uint64_t starting_bit_rate
         enc->rate_control.quality_eval_data.reason = MJPEG_QUALITY_EVAL_REASON_RATE_CHANGE;
         enc->rate_control.warmup_start_time = ((uint64_t) time.tv_sec) * 1000000000 + time.tv_nsec;
     } else {
+        enc->cbs.get_roundtrip_ms = NULL;
         mjpeg_encoder_reset_quality(enc, MJPEG_LEGACY_STATIC_QUALITY_ID, MJPEG_MAX_FPS, 0);
     }

@@ -607,7 +611,7 @@ static void mjpeg_encoder_adjust_params_to_bit_rate(MJpegEncoder *encoder)
     uint32_t latency = 0;
     uint32_t src_fps;

-    spice_assert(encoder->rate_control_is_active);
+    spice_assert(rate_control_is_active(encoder));

     rate_control = &encoder->rate_control;
     quality_eval = &rate_control->quality_eval_data;
@@ -692,7 +696,7 @@ 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(encoder->rate_control_is_active);
+    spice_assert(rate_control_is_active(encoder));

     adjusted_fps_time_passed = (now - rate_control->adjusted_fps_start_time) / 1000 / 1000;

@@ -734,7 +738,7 @@ int mjpeg_encoder_start_frame(MJpegEncoder *encoder, SpiceBitmapFmt format,
 {
     uint32_t quality;

-    if (encoder->rate_control_is_active) {
+    if (rate_control_is_active(encoder)) {
         MJpegEncoderRateControl *rate_control = &encoder->rate_control;
         struct timespec time;
         uint64_t now;
@@ -1131,7 +1135,7 @@ void mjpeg_encoder_client_stream_report(MJpegEncoder *encoder,
                 end_frame_mm_time - start_frame_mm_time,
                 end_frame_delay, audio_delay);

-    if (!encoder->rate_control_is_active) {
+    if (!rate_control_is_active(encoder)) {
         spice_debug("rate control was not activated: ignoring");
         return;
     }
diff --git a/server/mjpeg_encoder.h b/server/mjpeg_encoder.h
index 741ea1c..d584b92 100644
--- a/server/mjpeg_encoder.h
+++ b/server/mjpeg_encoder.h
@@ -49,7 +49,7 @@ typedef struct MJpegEncoderStats {
     double avg_quality;
 } MJpegEncoderStats;

-MJpegEncoder *mjpeg_encoder_new(int bit_rate_control, uint64_t starting_bit_rate,
+MJpegEncoder *mjpeg_encoder_new(uint64_t starting_bit_rate,
                                 MJpegEncoderRateControlCbs *cbs, void *opaque);
 void mjpeg_encoder_destroy(MJpegEncoder *encoder);

diff --git a/server/red_worker.c b/server/red_worker.c
index 5deb30b..e0ff8e9 100644
--- a/server/red_worker.c
+++ b/server/red_worker.c
@@ -3100,9 +3100,9 @@ static void red_display_create_stream(DisplayChannelClient *dcc, Stream *stream)
         mjpeg_cbs.update_client_playback_delay = red_stream_update_client_playback_latency;

         initial_bit_rate = red_stream_get_initial_bit_rate(dcc, stream);
-        agent->mjpeg_encoder = mjpeg_encoder_new(TRUE, initial_bit_rate, &mjpeg_cbs, agent);
+        agent->mjpeg_encoder = mjpeg_encoder_new(initial_bit_rate, &mjpeg_cbs, agent);
     } else {
-        agent->mjpeg_encoder = mjpeg_encoder_new(FALSE, 0, NULL, NULL);
+        agent->mjpeg_encoder = mjpeg_encoder_new(0, NULL, NULL);
     }
     red_channel_client_pipe_add(&dcc->common.base, &agent->create_item);

--
2.1.4

_______________________________________________
Spice-devel mailing list
Spice-devel@xxxxxxxxxxxxxxxxxxxxx
http://lists.freedesktop.org/mailman/listinfo/spice-devel

--

Marc-André Lureau

_______________________________________________
Spice-devel mailing list
Spice-devel@xxxxxxxxxxxxxxxxxxxxx
http://lists.freedesktop.org/mailman/listinfo/spice-devel