[Spice-devel] [PATCH v2 spice 09/11] server/red_worker: streams: moving mjpeg_encoder from Stream to StreamAgent

[Yonit Halperin]

The mjpeg_encoder should be client specific, and not shared between
different clients**, for the following reasons:
(1) Since we use abbreviated jpeg datastream for mjpeg, employing the same
    mjpeg_encoder for different clients might cause errors when the
    clients decode the jpeg data.
(2) The next patch introduces bit rate control to the mjpeg_encoder.
    This feature depends on the bandwidth available, which is client
    specific.

** at least till we change multi-clients not to re-encode the same
   streams.
---
 server/red_worker.c |   38 ++++++++++++++++++++++----------------
 1 files changed, 22 insertions(+), 16 deletions(-)

diff --git a/server/red_worker.c b/server/red_worker.c
index 1e74662..ccc2bfc 100644
--- a/server/red_worker.c
+++ b/server/red_worker.c
@@ -389,7 +389,6 @@ struct Stream {
     int width;
     int height;
     SpiceRect dest_area;
-    MJpegEncoder *mjpeg_encoder;
     int top_down;
     Stream *next;
     RingItem link;
@@ -402,6 +401,7 @@ typedef struct StreamAgent {
     PipeItem destroy_item;
     Stream *stream;
     uint64_t last_send_time;
+    MJpegEncoder *mjpeg_encoder;
 
     int frames;
     int drops;
@@ -2413,9 +2413,6 @@ static void red_release_stream(RedWorker *worker, Stream *stream)
 {
     if (!--stream->refs) {
         spice_assert(!ring_item_is_linked(&stream->link));
-        if (stream->mjpeg_encoder) {
-            mjpeg_encoder_destroy(stream->mjpeg_encoder);
-        }
         red_free_stream(worker, stream);
         worker->stream_count--;
     }
@@ -2524,8 +2521,13 @@ static void red_stop_stream(RedWorker *worker, Stream *stream)
     spice_assert(!stream->current);
     WORKER_FOREACH_DCC(worker, item, dcc) {
         StreamAgent *stream_agent;
+
         stream_agent = &dcc->stream_agents[stream - worker->streams_buf];
         region_clear(&stream_agent->vis_region);
+        if (stream_agent->mjpeg_encoder) {
+            mjpeg_encoder_destroy(stream_agent->mjpeg_encoder);
+            stream_agent->mjpeg_encoder = NULL;
+        }
         spice_assert(!pipe_item_is_linked(&stream_agent->destroy_item));
         stream->refs++;
         red_channel_client_pipe_add(&dcc->common.base, &stream_agent->destroy_item);
@@ -2756,6 +2758,7 @@ static void red_display_create_stream(DisplayChannelClient *dcc, Stream *stream)
     agent->drops = 0;
     agent->fps = MAX_FPS;
     reset_rate(dcc, agent);
+    agent->mjpeg_encoder = mjpeg_encoder_new();
     red_channel_client_pipe_add(&dcc->common.base, &agent->create_item);
 }
 
@@ -2781,8 +2784,6 @@ static void red_create_stream(RedWorker *worker, Drawable *drawable)
     stream_width = src_rect->right - src_rect->left;
     stream_height = src_rect->bottom - src_rect->top;
 
-    stream->mjpeg_encoder = mjpeg_encoder_new();
-
     ring_add(&worker->streams, &stream->link);
     stream->current = drawable;
     stream->last_time = drawable->creation_time;
@@ -2836,6 +2837,10 @@ static void red_display_destroy_streams(DisplayChannelClient *dcc)
     for (i = 0; i < NUM_STREAMS; i++) {
         StreamAgent *agent = &dcc->stream_agents[i];
         region_destroy(&agent->vis_region);
+        if (agent->mjpeg_encoder) {
+            mjpeg_encoder_destroy(agent->mjpeg_encoder);
+            agent->mjpeg_encoder = NULL;
+        }
     }
 }
 
@@ -8015,7 +8020,7 @@ static inline void display_begin_send_message(RedChannelClient *rcc)
     red_channel_client_begin_send_message(rcc);
 }
 
-static inline uint8_t *red_get_image_line(RedWorker *worker, SpiceChunks *chunks, size_t *offset,
+static inline uint8_t *red_get_image_line(SpiceChunks *chunks, size_t *offset,
                                           int *chunk_nr, int stride)
 {
     uint8_t *ret;
@@ -8041,13 +8046,14 @@ static inline uint8_t *red_get_image_line(RedWorker *worker, SpiceChunks *chunks
     return ret;
 }
 
-static int encode_frame (RedWorker *worker, const SpiceRect *src,
-                         const SpiceBitmap *image, Stream *stream)
+static int encode_frame(DisplayChannelClient *dcc, const SpiceRect *src,
+                        const SpiceBitmap *image, Stream *stream)
 {
     SpiceChunks *chunks;
     uint32_t image_stride;
     size_t offset;
     int i, chunk;
+    StreamAgent *agent = &dcc->stream_agents[stream - dcc->common.worker->streams_buf];
 
     chunks = image->data;
     offset = 0;
@@ -8056,7 +8062,7 @@ static int encode_frame (RedWorker *worker, const SpiceRect *src,
 
     const int skip_lines = stream->top_down ? src->top : image->y - (src->bottom - 0);
     for (i = 0; i < skip_lines; i++) {
-        red_get_image_line(worker, chunks, &offset, &chunk, image_stride);
+        red_get_image_line(chunks, &offset, &chunk, image_stride);
     }
 
     const unsigned int stream_height = src->bottom - src->top;
@@ -8064,14 +8070,14 @@ static int encode_frame (RedWorker *worker, const SpiceRect *src,
 
     for (i = 0; i < stream_height; i++) {
         uint8_t *src_line =
-            (uint8_t *)red_get_image_line(worker, chunks, &offset, &chunk, image_stride);
+            (uint8_t *)red_get_image_line(chunks, &offset, &chunk, image_stride);
 
         if (!src_line) {
             return FALSE;
         }
 
-        src_line += src->left * mjpeg_encoder_get_bytes_per_pixel(stream->mjpeg_encoder);
-        if (mjpeg_encoder_encode_scanline(stream->mjpeg_encoder, src_line, stream_width) == 0)
+        src_line += src->left * mjpeg_encoder_get_bytes_per_pixel(agent->mjpeg_encoder);
+        if (mjpeg_encoder_encode_scanline(agent->mjpeg_encoder, src_line, stream_width) == 0)
             return FALSE;
     }
 
@@ -8125,17 +8131,17 @@ static inline int red_marshall_stream_data(RedChannelClient *rcc,
     }
 
     outbuf_size = dcc->send_data.stream_outbuf_size;
-    if (!mjpeg_encoder_start_frame(stream->mjpeg_encoder, image->u.bitmap.format,
+    if (!mjpeg_encoder_start_frame(agent->mjpeg_encoder, image->u.bitmap.format,
                                    width, height,
                                    &dcc->send_data.stream_outbuf,
                                    &outbuf_size)) {
         return FALSE;
     }
-    if (!encode_frame(worker, &drawable->red_drawable->u.copy.src_area,
+    if (!encode_frame(dcc, &drawable->red_drawable->u.copy.src_area,
                       &image->u.bitmap, stream)) {
         return FALSE;
     }
-    n = mjpeg_encoder_end_frame(stream->mjpeg_encoder);
+    n = mjpeg_encoder_end_frame(agent->mjpeg_encoder);
     dcc->send_data.stream_outbuf_size = outbuf_size;
 
     if (!drawable->sized_stream) {
-- 
1.7.7.6