[Spice-devel] [PATCH spice-server 08/28] mjpeg_encoder: move the control over frame drops to mjpeg_encoder

[Yonit Halperin]

---
 server/mjpeg_encoder.c | 20 +++++++++-----------
 server/mjpeg_encoder.h | 21 ++++++++++++++-------
 server/red_worker.c    | 21 ++++++++++++++++-----
 3 files changed, 39 insertions(+), 23 deletions(-)

diff --git a/server/mjpeg_encoder.c b/server/mjpeg_encoder.c
index 70b6338..c124286 100644
--- a/server/mjpeg_encoder.c
+++ b/server/mjpeg_encoder.c
@@ -641,9 +641,15 @@ int mjpeg_encoder_start_frame(MJpegEncoder *encoder, SpiceBitmapFmt format,
         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;
+        interval = (now - rate_control->bit_rate_info.last_frame_time);
+
+        if (interval < (1000*1000*1000) / rate_control->fps) {
+            return MJPEG_ENCODER_FRAME_DROP;
+        }
 
         mjpeg_encoder_adjust_params_to_bit_rate(encoder);
 
@@ -690,14 +696,14 @@ int mjpeg_encoder_start_frame(MJpegEncoder *encoder, SpiceBitmapFmt format,
         break;
     default:
         spice_warning("unsupported format %d", format);
-        return FALSE;
+        return MJPEG_ENCODER_FRAME_UNSUPPORTED;
     }
 
     if (encoder->pixel_converter != NULL) {
         unsigned int stride = width * 3;
         /* check for integer overflow */
         if (stride < width) {
-            return FALSE;
+            return MJPEG_ENCODER_FRAME_UNSUPPORTED;
         }
         if (encoder->row_size < stride) {
             encoder->row = spice_realloc(encoder->row, stride);
@@ -715,7 +721,7 @@ int mjpeg_encoder_start_frame(MJpegEncoder *encoder, SpiceBitmapFmt format,
     jpeg_set_quality(&encoder->cinfo, quality, TRUE);
     jpeg_start_compress(&encoder->cinfo, encoder->first_frame);
 
-    return TRUE;
+    return MJPEG_ENCODER_FRAME_ENCODE_START;
 }
 
 int mjpeg_encoder_encode_scanline(MJpegEncoder *encoder, uint8_t *src_pixels,
@@ -773,14 +779,6 @@ size_t mjpeg_encoder_end_frame(MJpegEncoder *encoder)
     return encoder->rate_control.last_enc_size;
 }
 
-uint32_t mjpeg_encoder_get_fps(MJpegEncoder *encoder)
-{
-    if (!encoder->rate_control_is_active) {
-        spice_warning("bit rate control is not active");
-    }
-    return encoder->rate_control.fps;
-}
-
 static void mjpeg_encoder_quality_eval_stop(MJpegEncoder *encoder)
 {
     MJpegEncoderRateControl *rate_control = &encoder->rate_control;
diff --git a/server/mjpeg_encoder.h b/server/mjpeg_encoder.h
index f9ae43c..0ee2e96 100644
--- a/server/mjpeg_encoder.h
+++ b/server/mjpeg_encoder.h
@@ -21,6 +21,12 @@
 
 #include "red_common.h"
 
+enum {
+    MJPEG_ENCODER_FRAME_UNSUPPORTED = -1,
+    MJPEG_ENCODER_FRAME_DROP,
+    MJPEG_ENCODER_FRAME_ENCODE_START,
+};
+
 typedef struct MJpegEncoder MJpegEncoder;
 
 /*
@@ -44,8 +50,15 @@ void mjpeg_encoder_destroy(MJpegEncoder *encoder);
 uint8_t mjpeg_encoder_get_bytes_per_pixel(MJpegEncoder *encoder);
 
 /*
- * *dest must be either NULL or allocated by malloc, since it might be freed
+ * dest must be either NULL or allocated by malloc, since it might be freed
  * during the encoding, if its size is too small.
+ *
+ * return:
+ *  MJPEG_ENCODER_FRAME_UNSUPPORTED : frame cannot be encoded
+ *  MJPEG_ENCODER_FRAME_DROP        : frame should be dropped. This value can only be returned
+ *                                    if mjpeg rate control is active.
+ *  MJPEG_ENCODER_FRAME_ENCODE_START: frame encoding started. Continue with
+ *                                    mjpeg_encoder_encode_scanline.
  */
 int mjpeg_encoder_start_frame(MJpegEncoder *encoder, SpiceBitmapFmt format,
                               int width, int height,
@@ -60,12 +73,6 @@ size_t mjpeg_encoder_end_frame(MJpegEncoder *encoder);
  */
 
 /*
- * The recommended output frame rate (per second) for the
- * current available bit rate.
- */
-uint32_t mjpeg_encoder_get_fps(MJpegEncoder *encoder);
-
-/*
  * Data that should be periodically obtained from the client. The report contains:
  * num_frames         : the number of frames that reached the client during the time
  *                      the report is referring to.
diff --git a/server/red_worker.c b/server/red_worker.c
index 568b8e5..a390629 100644
--- a/server/red_worker.c
+++ b/server/red_worker.c
@@ -8352,6 +8352,7 @@ static inline int red_marshall_stream_data(RedChannelClient *rcc,
     RedWorker *worker = dcc->common.worker;
     int n;
     int width, height;
+    int ret;
 
     if (!stream) {
         spice_assert(drawable->sized_stream);
@@ -8389,13 +8390,23 @@ static inline int red_marshall_stream_data(RedChannelClient *rcc,
     }
 
     outbuf_size = dcc->send_data.stream_outbuf_size;
-    if (!mjpeg_encoder_start_frame(agent->mjpeg_encoder, image->u.bitmap.format,
-                                   width, height,
-                                   &dcc->send_data.stream_outbuf,
-                                   &outbuf_size,
-                                   drawable->red_drawable->mm_time)) {
+    ret = mjpeg_encoder_start_frame(agent->mjpeg_encoder, image->u.bitmap.format,
+                                    width, height,
+                                    &dcc->send_data.stream_outbuf,
+                                    &outbuf_size,
+                                    drawable->red_drawable->mm_time);
+    switch (ret) {
+    case MJPEG_ENCODER_FRAME_DROP:
+        spice_warning("mjpeg rate control is not supported yet");
+        return TRUE;
+    case MJPEG_ENCODER_FRAME_UNSUPPORTED:
         return FALSE;
+    case MJPEG_ENCODER_FRAME_ENCODE_START:
+        break;
+    default:
+        spice_error("bad return value (%d) from mjpeg_encoder_start_frame", ret);
     }
+
     if (!encode_frame(dcc, &drawable->red_drawable->u.copy.src_area,
                       &image->u.bitmap, stream)) {
         return FALSE;
-- 
1.8.1