[Spice-devel] [PATCH spice 03/11] server: video streaming: add support for frames of different sizes

Yonit Halperin yhalperi at redhat.com
Tue Apr 10 07:28:23 PDT 2012


Hi,
please wait with the review for this one. I may have a better way to 
identify such frames, using the current tree. If it works, I will send 
it in v2.

On 04/08/2012 06:43 PM, Yonit Halperin wrote:
> When playing a youtube video on Windows guest, the driver sometimes sends
> images which contain the video frames, but also other parts of the
> screen (e.g., the youtube process bar). In order to prevent glitches, we send these
> images as part of the stream, using SPICE_MSG_DISPLAY_STREAM_DATA_SIZED.
>
> This patch includes and update to the spice-common submodule.
>
> Signed-off-by: Yonit Halperin<yhalperi at redhat.com>
> ---
>   server/mjpeg_encoder.c |   15 ++---
>   server/mjpeg_encoder.h |    3 +-
>   server/red_worker.c    |  183 +++++++++++++++++++++++++++++++++++++-----------
>   spice-common           |    2 +-
>   4 files changed, 151 insertions(+), 52 deletions(-)
>
> diff --git a/server/mjpeg_encoder.c b/server/mjpeg_encoder.c
> index 4692315..b3685f8 100644
> --- a/server/mjpeg_encoder.c
> +++ b/server/mjpeg_encoder.c
> @@ -25,8 +25,6 @@
>   #include<jpeglib.h>
>
>   struct MJpegEncoder {
> -    int width;
> -    int height;
>       uint8_t *row;
>       int first_frame;
>       int quality;
> @@ -38,15 +36,13 @@ struct MJpegEncoder {
>       void (*pixel_converter)(uint8_t *src, uint8_t *dest);
>   };
>
> -MJpegEncoder *mjpeg_encoder_new(int width, int height)
> +MJpegEncoder *mjpeg_encoder_new()
>   {
>       MJpegEncoder *enc;
>
>       enc = spice_new0(MJpegEncoder, 1);
>
>       enc->first_frame = TRUE;
> -    enc->width = width;
> -    enc->height = height;
>       enc->quality = 70;
>       enc->cinfo.err = jpeg_std_error(&enc->jerr);
>       jpeg_create_compress(&enc->cinfo);
> @@ -200,6 +196,7 @@ spice_jpeg_mem_dest(j_compress_ptr cinfo,
>   /* end of code from libjpeg */
>
>   int mjpeg_encoder_start_frame(MJpegEncoder *encoder, SpiceBitmapFmt format,
> +                              int width, int height,
>                                 uint8_t **dest, size_t *dest_len)
>   {
>       encoder->cinfo.in_color_space   = JCS_RGB;
> @@ -233,9 +230,9 @@ int mjpeg_encoder_start_frame(MJpegEncoder *encoder, SpiceBitmapFmt format,
>       }
>
>       if ((encoder->pixel_converter != NULL)&&  (encoder->row == NULL)) {
> -        unsigned int stride = encoder->width * 3;
> +        unsigned int stride = width * 3;
>           /* check for integer overflow */
> -        if (stride<  encoder->width) {
> +        if (stride<  width) {
>               return FALSE;
>           }
>           encoder->row = spice_malloc(stride);
> @@ -243,8 +240,8 @@ int mjpeg_encoder_start_frame(MJpegEncoder *encoder, SpiceBitmapFmt format,
>
>       spice_jpeg_mem_dest(&encoder->cinfo, dest, dest_len);
>
> -    encoder->cinfo.image_width      = encoder->width;
> -    encoder->cinfo.image_height     = encoder->height;
> +    encoder->cinfo.image_width      = width;
> +    encoder->cinfo.image_height     = height;
>       jpeg_set_defaults(&encoder->cinfo);
>       encoder->cinfo.dct_method       = JDCT_IFAST;
>       jpeg_set_quality(&encoder->cinfo, encoder->quality, TRUE);
> diff --git a/server/mjpeg_encoder.h b/server/mjpeg_encoder.h
> index c43827f..3a005b7 100644
> --- a/server/mjpeg_encoder.h
> +++ b/server/mjpeg_encoder.h
> @@ -23,11 +23,12 @@
>
>   typedef struct MJpegEncoder MJpegEncoder;
>
> -MJpegEncoder *mjpeg_encoder_new(int width, int height);
> +MJpegEncoder *mjpeg_encoder_new();
>   void mjpeg_encoder_destroy(MJpegEncoder *encoder);
>
>   uint8_t mjpeg_encoder_get_bytes_per_pixel(MJpegEncoder *encoder);
>   int mjpeg_encoder_start_frame(MJpegEncoder *encoder, SpiceBitmapFmt format,
> +                              int width, int height,
>                                 uint8_t **dest, size_t *dest_len);
>   int mjpeg_encoder_encode_scanline(MJpegEncoder *encoder, uint8_t *src_pixels,
>                                     size_t image_width);
> diff --git a/server/red_worker.c b/server/red_worker.c
> index b2fa348..fd0e32e 100644
> --- a/server/red_worker.c
> +++ b/server/red_worker.c
> @@ -374,6 +374,12 @@ typedef struct ImageItem {
>
>   typedef struct Drawable Drawable;
>
> +enum {
> +    STREAM_FRAME_NONE,
> +    STREAM_FRAME_NATIVE,
> +    STREAM_FRAME_CONTAINER,
> +};
> +
>   typedef struct Stream Stream;
>   struct Stream {
>       uint8_t refs;
> @@ -784,6 +790,7 @@ struct Drawable {
>       int gradual_frames_count;
>       int last_gradual_frame;
>       Stream *stream;
> +    Stream *sized_stream;
>       int streamable;
>       BitmapGradualType copy_bitmap_graduality;
>       uint32_t group_id;
> @@ -2773,7 +2780,7 @@ 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(stream_width, stream_height);
> +    stream->mjpeg_encoder = mjpeg_encoder_new();
>
>       ring_add(&worker->streams,&stream->link);
>       stream->current = drawable;
> @@ -2850,34 +2857,63 @@ static inline int __red_is_next_stream_frame(RedWorker *worker,
>                                                const int other_src_height,
>                                                const SpiceRect *other_dest,
>                                                const red_time_t other_time,
> -                                             const Stream *stream)
> +                                             const Stream *stream,
> +                                             int container_candidate_allowed)
>   {
>       RedDrawable *red_drawable;
> +    int is_frame_container = FALSE;
>
>       if (candidate->creation_time - other_time>
>               (stream ? RED_STREAM_CONTINUS_MAX_DELTA : RED_STREAM_DETACTION_MAX_DELTA)) {
> -        return FALSE;
> +        return STREAM_FRAME_NONE;
>       }
>
>       red_drawable = candidate->red_drawable;
> +    if (!container_candidate_allowed) {
> +        SpiceRect* candidate_src;
>
> -    if (!rect_is_equal(&red_drawable->bbox, other_dest)) {
> -        return FALSE;
> -    }
> +        if (!rect_is_equal(&red_drawable->bbox, other_dest)) {
> +            return STREAM_FRAME_NONE;
> +        }
>
> -    SpiceRect* candidate_src =&red_drawable->u.copy.src_area;
> -    if (candidate_src->right - candidate_src->left != other_src_width ||
> -        candidate_src->bottom - candidate_src->top != other_src_height) {
> -        return FALSE;
> +        candidate_src =&red_drawable->u.copy.src_area;
> +        if (candidate_src->right - candidate_src->left != other_src_width ||
> +            candidate_src->bottom - candidate_src->top != other_src_height) {
> +            return STREAM_FRAME_NONE;
> +        }
> +    } else {
> +        if (rect_contains(&red_drawable->bbox, other_dest)) {
> +            int candidate_area = rect_get_area(&red_drawable->bbox);
> +            int other_area = rect_get_area(other_dest);
> +
> +            if (candidate_area>  1.3 * other_area) {
> +                spice_debug("Too big candidate: (%d, %d) (%d, %d), new: (%d, %d) (%d, %d)",
> +                            other_dest->left, other_dest->top,
> +                            other_dest->right, other_dest->bottom,
> +                            red_drawable->bbox.left, red_drawable->bbox.top,
> +                            red_drawable->bbox.right, red_drawable->bbox.bottom);
> +                return STREAM_FRAME_NONE;
> +            }
> +
> +            if (candidate_area>  other_area) {
> +                is_frame_container = TRUE;
> +            }
> +        } else {
> +            return STREAM_FRAME_NONE;
> +        }
>       }
>
>       if (stream) {
>           SpiceBitmap *bitmap =&red_drawable->u.copy.src_bitmap->u.bitmap;
>           if (stream->top_down != !!(bitmap->flags&  SPICE_BITMAP_FLAGS_TOP_DOWN)) {
> -            return FALSE;
> +            return STREAM_FRAME_NONE;
>           }
>       }
> -    return TRUE;
> +    if (is_frame_container) {
> +        return STREAM_FRAME_CONTAINER;
> +    } else {
> +        return STREAM_FRAME_NATIVE;
> +    }
>   }
>
>   static inline int red_is_next_stream_frame(RedWorker *worker, const Drawable *candidate,
> @@ -2891,7 +2927,8 @@ static inline int red_is_next_stream_frame(RedWorker *worker, const Drawable *ca
>       return __red_is_next_stream_frame(worker, candidate, prev_src->right - prev_src->left,
>                                         prev_src->bottom - prev_src->top,
>                                         &prev->red_drawable->bbox, prev->creation_time,
> -                                      prev->stream);
> +                                      prev->stream,
> +                                      FALSE);
>   }
>
>   static void reset_rate(DisplayChannelClient *dcc, StreamAgent *stream_agent)
> @@ -3033,20 +3070,31 @@ static inline void red_stream_maintenance(RedWorker *worker, Drawable *candidate
>           return;
>       }
>
> -    if (!red_is_next_stream_frame(worker, candidate, prev)) {
> -        return;
> -    }
> -
>       if ((stream = prev->stream)) {
> -        pre_stream_item_swap(worker, stream);
> -        red_detach_stream(worker, stream);
> -        prev->streamable = FALSE; //prevent item trace
> -        red_attach_stream(worker, candidate, stream);
> +        int is_next_frame = __red_is_next_stream_frame(worker,
> +                                                       candidate,
> +                                                       stream->width,
> +                                                       stream->height,
> +&stream->dest_area,
> +                                                       stream->last_time,
> +                                                       stream,
> +                                                       TRUE);
> +        if (is_next_frame != STREAM_FRAME_NONE) {
> +            pre_stream_item_swap(worker, stream);
> +            red_detach_stream(worker, stream);
> +            prev->streamable = FALSE; //prevent item trace
> +            red_attach_stream(worker, candidate, stream);
> +            if (is_next_frame == STREAM_FRAME_CONTAINER) {
> +                candidate->sized_stream = stream;
> +            }
> +        }
>       } else {
> -        red_stream_add_frame(worker, candidate,
> -                             prev->frames_count,
> -                             prev->gradual_frames_count,
> -                             prev->last_gradual_frame);
> +        if (red_is_next_stream_frame(worker, candidate, prev) != STREAM_FRAME_NONE) {
> +            red_stream_add_frame(worker, candidate,
> +                                 prev->frames_count,
> +                                 prev->gradual_frames_count,
> +                                 prev->last_gradual_frame);
> +        }
>       }
>   }
>
> @@ -3174,14 +3222,24 @@ static inline void red_use_stream_trace(RedWorker *worker, Drawable *drawable)
>
>       while (item) {
>           Stream *stream = SPICE_CONTAINEROF(item, Stream, link);
> -        if (!stream->current&&  __red_is_next_stream_frame(worker,
> -                                                           drawable,
> -                                                           stream->width,
> -                                                           stream->height,
> -&stream->dest_area,
> -                                                           stream->last_time,
> -                                                           stream)) {
> +        int is_next_frame = __red_is_next_stream_frame(worker,
> +                                                       drawable,
> +                                                       stream->width,
> +                                                       stream->height,
> +&stream->dest_area,
> +                                                       stream->last_time,
> +                                                       stream,
> +                                                       TRUE);
> +        if (is_next_frame != STREAM_FRAME_NONE) {
> +            if (stream->current) {
> +                stream->current->streamable = FALSE; //prevent item trace
> +                pre_stream_item_swap(worker, stream);
> +                red_detach_stream(worker, stream);
> +            }
>               red_attach_stream(worker, drawable, stream);
> +            if (is_next_frame == STREAM_FRAME_CONTAINER) {
> +                drawable->sized_stream = stream;
> +            }
>               return;
>           }
>           item = ring_next(ring, item);
> @@ -3191,7 +3249,8 @@ static inline void red_use_stream_trace(RedWorker *worker, Drawable *drawable)
>       trace_end = trace + NUM_TRACE_ITEMS;
>       for (; trace<  trace_end; trace++) {
>           if (__red_is_next_stream_frame(worker, drawable, trace->width, trace->height,
> -&trace->dest_area, trace->time, NULL)) {
> +&trace->dest_area, trace->time, NULL, FALSE) !=
> +                                       STREAM_FRAME_NONE) {
>               if (red_stream_add_frame(worker, drawable,
>                                        trace->frames_count,
>                                        trace->gradual_frames_count,
> @@ -8027,8 +8086,12 @@ static inline int red_marshall_stream_data(RedChannelClient *rcc,
>       SpiceImage *image;
>       RedWorker *worker = dcc->common.worker;
>       int n;
> +    int width, height;
>
> -    spice_assert(stream);
> +    if (!stream) {
> +        spice_assert(drawable->sized_stream);
> +        stream = drawable->sized_stream;
> +    }
>       spice_assert(drawable->red_drawable->type == QXL_DRAW_COPY);
>
>       worker = display_channel->common.worker;
> @@ -8038,6 +8101,20 @@ static inline int red_marshall_stream_data(RedChannelClient *rcc,
>           return FALSE;
>       }
>
> +    if (drawable->sized_stream) {
> +        if (red_channel_client_test_remote_cap(rcc, SPICE_DISPLAY_CAP_SIZED_STREAM)) {
> +            SpiceRect *src_rect =&drawable->red_drawable->u.copy.src_area;
> +
> +            width = src_rect->right - src_rect->left;
> +            height = src_rect->bottom - src_rect->top;
> +        } else {
> +            return FALSE;
> +        }
> +    } else {
> +        width = stream->width;
> +        height = stream->height;
> +    }
> +
>       StreamAgent *agent =&dcc->stream_agents[stream - worker->streams_buf];
>       uint64_t time_now = red_now();
>       size_t outbuf_size;
> @@ -8048,6 +8125,7 @@ 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,
> +                                   width, height,
>                                      &dcc->send_data.stream_outbuf,
>                                      &outbuf_size)) {
>           return FALSE;
> @@ -8059,14 +8137,35 @@ static inline int red_marshall_stream_data(RedChannelClient *rcc,
>       n = mjpeg_encoder_end_frame(stream->mjpeg_encoder);
>       dcc->send_data.stream_outbuf_size = outbuf_size;
>
> -    red_channel_client_init_send_data(rcc, SPICE_MSG_DISPLAY_STREAM_DATA, NULL);
> +    if (!drawable->sized_stream) {
> +        SpiceMsgDisplayStreamData stream_data;
> +
> +        red_channel_client_init_send_data(rcc, SPICE_MSG_DISPLAY_STREAM_DATA, NULL);
>
> -    SpiceMsgDisplayStreamData stream_data;
> +        stream_data.base.id = stream - worker->streams_buf;
> +        stream_data.base.multi_media_time = drawable->red_drawable->mm_time;
> +        stream_data.data_size = n;
>
> -    stream_data.id = stream - worker->streams_buf;
> -    stream_data.multi_media_time = drawable->red_drawable->mm_time;
> -    stream_data.data_size = n;
> -    spice_marshall_msg_display_stream_data(base_marshaller,&stream_data);
> +        spice_marshall_msg_display_stream_data(base_marshaller,&stream_data);
> +    } else {
> +        SpiceMsgDisplayStreamDataSized stream_data;
> +
> +        red_channel_client_init_send_data(rcc, SPICE_MSG_DISPLAY_STREAM_DATA_SIZED, NULL);
> +
> +        stream_data.base.id = stream - worker->streams_buf;
> +        stream_data.base.multi_media_time = drawable->red_drawable->mm_time;
> +        stream_data.data_size = n;
> +        stream_data.width = width;
> +        stream_data.height = height;
> +        stream_data.dest = drawable->red_drawable->bbox;
> +
> +        spice_debug("stream %d sized frame: dest %dx%d (%d,%d) (%d,%d))", stream_data.base.id,
> +                    stream_data.dest.right - stream_data.dest.left,
> +                    stream_data.dest.bottom - stream_data.dest.top,
> +                    stream_data.dest.left, stream_data.dest.top,
> +                    stream_data.dest.right, stream_data.dest.bottom);
> +        spice_marshall_msg_display_stream_data_sized(base_marshaller,&stream_data);
> +    }
>       spice_marshaller_add_ref(base_marshaller,
>                                dcc->send_data.stream_outbuf, n);
>       agent->last_send_time = time_now;
> @@ -8080,7 +8179,9 @@ static inline void marshall_qxl_drawable(RedChannelClient *rcc,
>       DisplayChannel *display_channel = SPICE_CONTAINEROF(rcc->channel, DisplayChannel, common.base);
>
>       spice_assert(display_channel&&  rcc);
> -    if (item->stream&&  red_marshall_stream_data(rcc, m, item)) {
> +    /* allow sized frames to be streamed, even if they where detached from the stream, since
> +     * unlike other frames, they are not dropped */
> +    if ((item->stream || item->sized_stream)&&  red_marshall_stream_data(rcc, m, item)) {
>           return;
>       }
>       if (!display_channel->enable_jpeg)
> diff --git a/spice-common b/spice-common
> index 1b41d15..cc613c0 160000
> --- a/spice-common
> +++ b/spice-common
> @@ -1 +1 @@
> -Subproject commit 1b41d15a99dfcddb99975d17cfbcd61d8870a887
> +Subproject commit cc613c0c3c1ba3fe7017528d0da998a210a35d75



More information about the Spice-devel mailing list