[Spice-devel,v6,21/26] spice-gtk: Enable adding alternative video decoders

Submitted by Francois Gouget on Oct. 16, 2015, 3:13 p.m.

Details

Message ID alpine.DEB.2.20.1510161656450.23359@amboise
State New
Headers show

Not browsing as part of any series.

Patch hide | download patch | download mbox

commit 72d4800c10801370bb95bf652d6d3d9943c02bcb
Author: Francois Gouget <fgouget@codeweavers.com>
Date:   Wed Jul 29 12:43:42 2015 +0200

    spice-gtk: Enable adding alternative video decoders
    
    The video decoder no longer stores its internal state in the
    display_stream struct, or depend on it to know which frame to decode or
    store the decoded frame. This way adding alternative implementations
    will not pile all their implementation details in display_stream and
    they will have more flexibility for their implementation.
    
    Signed-off-by: Francois Gouget <fgouget@codeweavers.com>

diff --git a/src/channel-display-mjpeg.c b/src/channel-display-mjpeg.c
index 95d5b33..78e3d5a 100644
--- a/src/channel-display-mjpeg.c
+++ b/src/channel-display-mjpeg.c
@@ -23,12 +23,33 @@ 
 
 #include "channel-display-priv.h"
 
+
+/* MJpeg decoder implementation */
+
+typedef struct MJpegDecoder {
+    VideoDecoder base;
+
+    /* ---------- The builtin mjpeg decoder ---------- */
+
+    SpiceMsgIn *frame_msg;
+    struct jpeg_source_mgr         mjpeg_src;
+    struct jpeg_decompress_struct  mjpeg_cinfo;
+    struct jpeg_error_mgr          mjpeg_jerr;
+
+    /* ---------- Output frame data ---------- */
+
+    uint8_t *out_frame;
+} MJpegDecoder;
+
+
+/* ---------- The JPEG library callbacks ---------- */
+
 static void mjpeg_src_init(struct jpeg_decompress_struct *cinfo)
 {
-    display_stream *st = SPICE_CONTAINEROF(cinfo->src, display_stream, mjpeg_src);
-    uint8_t *data;
+    MJpegDecoder *decoder = SPICE_CONTAINEROF(cinfo->src, MJpegDecoder, mjpeg_src);
 
-    cinfo->src->bytes_in_buffer = stream_get_current_frame(st, &data);
+    uint8_t *data;
+    cinfo->src->bytes_in_buffer = spice_msg_in_frame_data(decoder->frame_msg, &data);
     cinfo->src->next_input_byte = data;
 }
 
@@ -49,68 +70,57 @@  static void mjpeg_src_term(struct jpeg_decompress_struct *cinfo)
     /* nothing */
 }
 
-G_GNUC_INTERNAL
-void stream_mjpeg_init(display_stream *st)
-{
-    st->mjpeg_cinfo.err = jpeg_std_error(&st->mjpeg_jerr);
-    jpeg_create_decompress(&st->mjpeg_cinfo);
-
-    st->mjpeg_src.init_source         = mjpeg_src_init;
-    st->mjpeg_src.fill_input_buffer   = mjpeg_src_fill;
-    st->mjpeg_src.skip_input_data     = mjpeg_src_skip;
-    st->mjpeg_src.resync_to_restart   = jpeg_resync_to_restart;
-    st->mjpeg_src.term_source         = mjpeg_src_term;
-    st->mjpeg_cinfo.src               = &st->mjpeg_src;
-}
 
-G_GNUC_INTERNAL
-void stream_mjpeg_data(display_stream *st)
+/* ---------- VideoDecoder's public API ---------- */
+
+static uint8_t* mjpeg_decoder_decode_frame(VideoDecoder *video_decoder,
+                                           SpiceMsgIn *frame_msg)
 {
-    gboolean back_compat = st->channel->priv->peer_hdr.major_version == 1;
+    MJpegDecoder *decoder = (MJpegDecoder*)video_decoder;
+    gboolean back_compat = decoder->base.stream->channel->priv->peer_hdr.major_version == 1;
     int width;
     int height;
     uint8_t *dest;
     uint8_t *lines[4];
 
-    stream_get_dimensions(st, &width, &height);
-    dest = g_malloc0(width * height * 4);
+    decoder->frame_msg = frame_msg;
+    stream_get_dimensions(decoder->base.stream, frame_msg, &width, &height);
+    g_free(decoder->out_frame);
+    dest = decoder->out_frame = g_malloc0(width * height * 4);
 
-    g_free(st->out_frame);
-    st->out_frame = dest;
-
-    jpeg_read_header(&st->mjpeg_cinfo, 1);
+    jpeg_read_header(&decoder->mjpeg_cinfo, 1);
 #ifdef JCS_EXTENSIONS
     // requires jpeg-turbo
     if (back_compat)
-        st->mjpeg_cinfo.out_color_space = JCS_EXT_RGBX;
+        decoder->mjpeg_cinfo.out_color_space = JCS_EXT_RGBX;
     else
-        st->mjpeg_cinfo.out_color_space = JCS_EXT_BGRX;
+        decoder->mjpeg_cinfo.out_color_space = JCS_EXT_BGRX;
 #else
 #warning "You should consider building with libjpeg-turbo"
-    st->mjpeg_cinfo.out_color_space = JCS_RGB;
+    decoder->mjpeg_cinfo.out_color_space = JCS_RGB;
 #endif
 
 #ifndef SPICE_QUALITY
-    st->mjpeg_cinfo.dct_method = JDCT_IFAST;
-    st->mjpeg_cinfo.do_fancy_upsampling = FALSE;
-    st->mjpeg_cinfo.do_block_smoothing = FALSE;
-    st->mjpeg_cinfo.dither_mode = JDITHER_ORDERED;
+    decoder->mjpeg_cinfo.dct_method = JDCT_IFAST;
+    decoder->mjpeg_cinfo.do_fancy_upsampling = FALSE;
+    decoder->mjpeg_cinfo.do_block_smoothing = FALSE;
+    decoder->mjpeg_cinfo.dither_mode = JDITHER_ORDERED;
 #endif
     // TODO: in theory should check cinfo.output_height match with our height
-    jpeg_start_decompress(&st->mjpeg_cinfo);
+    jpeg_start_decompress(&decoder->mjpeg_cinfo);
     /* rec_outbuf_height is the recommended size of the output buffer we
      * pass to libjpeg for optimum performance
      */
-    if (st->mjpeg_cinfo.rec_outbuf_height > G_N_ELEMENTS(lines)) {
-        jpeg_abort_decompress(&st->mjpeg_cinfo);
-        g_return_if_reached();
+    if (decoder->mjpeg_cinfo.rec_outbuf_height > G_N_ELEMENTS(lines)) {
+        jpeg_abort_decompress(&decoder->mjpeg_cinfo);
+        g_return_val_if_reached(NULL);
     }
 
-    while (st->mjpeg_cinfo.output_scanline < st->mjpeg_cinfo.output_height) {
+    while (decoder->mjpeg_cinfo.output_scanline < decoder->mjpeg_cinfo.output_height) {
         /* only used when JCS_EXTENSIONS is undefined */
         G_GNUC_UNUSED unsigned int lines_read;
 
-        for (unsigned int j = 0; j < st->mjpeg_cinfo.rec_outbuf_height; j++) {
+        for (unsigned int j = 0; j < decoder->mjpeg_cinfo.rec_outbuf_height; j++) {
             lines[j] = dest;
 #ifdef JCS_EXTENSIONS
             dest += 4 * width;
@@ -118,8 +128,8 @@  void stream_mjpeg_data(display_stream *st)
             dest += 3 * width;
 #endif
         }
-        lines_read = jpeg_read_scanlines(&st->mjpeg_cinfo, lines,
-                                st->mjpeg_cinfo.rec_outbuf_height);
+        lines_read = jpeg_read_scanlines(&decoder->mjpeg_cinfo, lines,
+                                decoder->mjpeg_cinfo.rec_outbuf_height);
 #ifndef JCS_EXTENSIONS
         {
             uint8_t *s = lines[0];
@@ -142,15 +152,44 @@  void stream_mjpeg_data(display_stream *st)
             }
         }
 #endif
-        dest = &st->out_frame[st->mjpeg_cinfo.output_scanline * width * 4];
+        dest = &(decoder->out_frame[decoder->mjpeg_cinfo.output_scanline * width * 4]);
     }
-    jpeg_finish_decompress(&st->mjpeg_cinfo);
+    jpeg_finish_decompress(&decoder->mjpeg_cinfo);
+
+    return decoder->out_frame;
+}
+
+static void mjpeg_decoder_destroy(VideoDecoder* video_decoder)
+{
+    MJpegDecoder *decoder = (MJpegDecoder*)video_decoder;
+    jpeg_destroy_decompress(&decoder->mjpeg_cinfo);
+    g_free(decoder->out_frame);
+    free(decoder);
 }
 
 G_GNUC_INTERNAL
-void stream_mjpeg_cleanup(display_stream *st)
+VideoDecoder* create_mjpeg_decoder(int codec_type, display_stream *stream)
 {
-    jpeg_destroy_decompress(&st->mjpeg_cinfo);
-    g_free(st->out_frame);
-    st->out_frame = NULL;
+    g_return_val_if_fail(codec_type == SPICE_VIDEO_CODEC_TYPE_MJPEG, NULL);
+
+    MJpegDecoder *decoder = spice_new0(MJpegDecoder, 1);
+
+    decoder->base.destroy = &mjpeg_decoder_destroy;
+    decoder->base.decode_frame = &mjpeg_decoder_decode_frame;
+    decoder->base.codec_type = codec_type;
+    decoder->base.stream = stream;
+
+    decoder->mjpeg_cinfo.err = jpeg_std_error(&decoder->mjpeg_jerr);
+    jpeg_create_decompress(&decoder->mjpeg_cinfo);
+
+    decoder->mjpeg_src.init_source         = mjpeg_src_init;
+    decoder->mjpeg_src.fill_input_buffer   = mjpeg_src_fill;
+    decoder->mjpeg_src.skip_input_data     = mjpeg_src_skip;
+    decoder->mjpeg_src.resync_to_restart   = jpeg_resync_to_restart;
+    decoder->mjpeg_src.term_source         = mjpeg_src_term;
+    decoder->mjpeg_cinfo.src               = &decoder->mjpeg_src;
+
+    /* All the other fields are initialized to zero by spice_new0(). */
+
+    return (VideoDecoder*)decoder;
 }
diff --git a/src/channel-display-priv.h b/src/channel-display-priv.h
index 71f5d17..4ca80b6 100644
--- a/src/channel-display-priv.h
+++ b/src/channel-display-priv.h
@@ -34,6 +34,39 @@ 
 
 G_BEGIN_DECLS
 
+typedef struct display_stream display_stream;
+
+typedef struct VideoDecoder VideoDecoder;
+struct VideoDecoder {
+    /* Releases the video decoder's resources */
+    void (*destroy)(VideoDecoder *decoder);
+
+    /* Decompresses the specified frame.
+     *
+     * @decoder:   The video decoder.
+     * @frame_msg: The Spice message containing the compressed frame.
+     * @return:    A pointer to the buffer holding the decoded frame. This
+     *             buffer will be invalidated by the next call to
+     *             decode_frame().
+     */
+    uint8_t* (*decode_frame)(VideoDecoder *decoder, SpiceMsgIn *frame_msg);
+
+    /* The format of the encoded video. */
+    int codec_type;
+
+    /* The associated display stream. */
+    display_stream *stream;
+};
+
+
+/* Instantiates the video decoder for the specified codec.
+ *
+ * @codec_type: The format of the video.
+ * @stream:     The associated video stream.
+ * @return:     A pointer to a structure implementing the VideoDecoder methods.
+ */
+VideoDecoder* create_mjpeg_decoder(int codec_type, display_stream *stream);
+
 
 typedef struct display_surface {
     guint32                     surface_id;
@@ -54,24 +87,18 @@  typedef struct drops_sequence_stats {
     uint32_t duration;
 } drops_sequence_stats;
 
-typedef struct display_stream {
+struct display_stream {
     SpiceMsgIn                  *msg_create;
     SpiceMsgIn                  *msg_clip;
-    SpiceMsgIn                  *msg_data;
 
     /* from messages */
     display_surface             *surface;
     SpiceClip                   *clip;
     QRegion                     region;
     int                         have_region;
-    int                         codec;
 
-    /* mjpeg decoder */
-    struct jpeg_source_mgr         mjpeg_src;
-    struct jpeg_decompress_struct  mjpeg_cinfo;
-    struct jpeg_error_mgr          mjpeg_jerr;
+    VideoDecoder                *video_decoder;
 
-    uint8_t                     *out_frame;
     GQueue                      *msgq;
     guint                       timeout;
     SpiceChannel                *channel;
@@ -98,15 +125,11 @@  typedef struct display_stream {
     uint32_t report_num_frames;
     uint32_t report_num_drops;
     uint32_t report_drops_seq_len;
-} display_stream;
+};
 
-void stream_get_dimensions(display_stream *st, int *width, int *height);
-uint32_t stream_get_current_frame(display_stream *st, uint8_t **data);
+void stream_get_dimensions(display_stream *st, SpiceMsgIn *frame_msg, int *width, int *height);
+uint32_t spice_msg_in_frame_data(SpiceMsgIn *frame_msg, uint8_t **data);
 
-/* channel-display-mjpeg.c */
-void stream_mjpeg_init(display_stream *st);
-void stream_mjpeg_data(display_stream *st);
-void stream_mjpeg_cleanup(display_stream *st);
 
 G_END_DECLS
 
diff --git a/src/channel-display.c b/src/channel-display.c
index 9e42dd9..5c916cc 100644
--- a/src/channel-display.c
+++ b/src/channel-display.c
@@ -996,7 +996,6 @@  static void display_handle_stream_create(SpiceChannel *channel, SpiceMsgIn *in)
     st->msg_create = in;
     spice_msg_in_ref(in);
     st->clip = &op->clip;
-    st->codec = op->codec_type;
     st->surface = find_surface(c, op->surface_id);
     st->msgq = g_queue_new();
     st->channel = channel;
@@ -1005,10 +1004,15 @@  static void display_handle_stream_create(SpiceChannel *channel, SpiceMsgIn *in)
     region_init(&st->region);
     display_update_stream_region(st);
 
-    switch (st->codec) {
+    switch (op->codec_type) {
     case SPICE_VIDEO_CODEC_TYPE_MJPEG:
-        stream_mjpeg_init(st);
+        st->video_decoder = create_mjpeg_decoder(op->codec_type, st);
         break;
+    default:
+        st->video_decoder = NULL;
+    }
+    if (st->video_decoder == NULL) {
+        spice_printerr("could not create a video decoder for codec %d", op->codec_type);
     }
 }
 
@@ -1051,15 +1055,15 @@  static gboolean display_stream_schedule(display_stream *st)
     return FALSE;
 }
 
-static SpiceRect *stream_get_dest(display_stream *st)
+static SpiceRect *stream_get_dest(display_stream *st, SpiceMsgIn *frame_msg)
 {
-    if (st->msg_data == NULL ||
-        spice_msg_in_type(st->msg_data) != SPICE_MSG_DISPLAY_STREAM_DATA_SIZED) {
+    if (frame_msg == NULL ||
+        spice_msg_in_type(frame_msg) != SPICE_MSG_DISPLAY_STREAM_DATA_SIZED) {
         SpiceMsgDisplayStreamCreate *info = spice_msg_in_parsed(st->msg_create);
 
         return &info->dest;
     } else {
-        SpiceMsgDisplayStreamDataSized *op = spice_msg_in_parsed(st->msg_data);
+        SpiceMsgDisplayStreamDataSized *op = spice_msg_in_parsed(frame_msg);
 
         return &op->dest;
    }
@@ -1074,21 +1078,16 @@  static uint32_t stream_get_flags(display_stream *st)
 }
 
 G_GNUC_INTERNAL
-uint32_t stream_get_current_frame(display_stream *st, uint8_t **data)
+uint32_t spice_msg_in_frame_data(SpiceMsgIn *frame_msg, uint8_t **data)
 {
-    if (st->msg_data == NULL) {
-        *data = NULL;
-        return 0;
-    }
-
-    switch (spice_msg_in_type(st->msg_data)) {
+    switch (spice_msg_in_type(frame_msg)) {
     case SPICE_MSG_DISPLAY_STREAM_DATA: {
-        SpiceMsgDisplayStreamData *op = spice_msg_in_parsed(st->msg_data);
+        SpiceMsgDisplayStreamData *op = spice_msg_in_parsed(frame_msg);
         *data = op->data;
         return op->data_size;
     }
     case SPICE_MSG_DISPLAY_STREAM_DATA_SIZED: {
-        SpiceMsgDisplayStreamDataSized *op = spice_msg_in_parsed(st->msg_data);
+        SpiceMsgDisplayStreamDataSized *op = spice_msg_in_parsed(frame_msg);
         *data = op->data;
         return op->data_size;
     }
@@ -1099,19 +1098,19 @@  uint32_t stream_get_current_frame(display_stream *st, uint8_t **data)
 }
 
 G_GNUC_INTERNAL
-void stream_get_dimensions(display_stream *st, int *width, int *height)
+void stream_get_dimensions(display_stream *st, SpiceMsgIn *frame_msg, int *width, int *height)
 {
     g_return_if_fail(width != NULL);
     g_return_if_fail(height != NULL);
 
-    if (st->msg_data == NULL ||
-        spice_msg_in_type(st->msg_data) != SPICE_MSG_DISPLAY_STREAM_DATA_SIZED) {
+    if (frame_msg == NULL ||
+        spice_msg_in_type(frame_msg) != SPICE_MSG_DISPLAY_STREAM_DATA_SIZED) {
         SpiceMsgDisplayStreamCreate *info = spice_msg_in_parsed(st->msg_create);
 
         *width = info->stream_width;
         *height = info->stream_height;
     } else {
-        SpiceMsgDisplayStreamDataSized *op = spice_msg_in_parsed(st->msg_data);
+        SpiceMsgDisplayStreamDataSized *op = spice_msg_in_parsed(frame_msg);
 
         *width = op->width;
         *height = op->height;
@@ -1129,24 +1128,21 @@  static gboolean display_stream_render(display_stream *st)
 
         g_return_val_if_fail(in != NULL, FALSE);
 
-        st->msg_data = in;
-        switch (st->codec) {
-        case SPICE_VIDEO_CODEC_TYPE_MJPEG:
-            stream_mjpeg_data(st);
-            break;
+        uint8_t *out_frame = NULL;
+        if (st->video_decoder) {
+            out_frame = st->video_decoder->decode_frame(st->video_decoder, in);
         }
-
-        if (st->out_frame) {
+        if (out_frame) {
             int width;
             int height;
             SpiceRect *dest;
             uint8_t *data;
             int stride;
 
-            stream_get_dimensions(st, &width, &height);
-            dest = stream_get_dest(st);
+            stream_get_dimensions(st, in, &width, &height);
+            dest = stream_get_dest(st, in);
 
-            data = st->out_frame;
+            data = out_frame;
             stride = width * sizeof(uint32_t);
             if (!(stream_get_flags(st) & SPICE_STREAM_FLAGS_TOP_DOWN)) {
                 data += stride * (height - 1);
@@ -1169,7 +1165,6 @@  static gboolean display_stream_render(display_stream *st)
                     dest->bottom - dest->top);
         }
 
-        st->msg_data = NULL;
         spice_msg_in_unref(in);
 
         in = g_queue_peek_head(st->msgq);
@@ -1478,10 +1473,8 @@  static void destroy_stream(SpiceChannel *channel, int id)
 
     g_array_free(st->drops_seqs_stats_arr, TRUE);
 
-    switch (st->codec) {
-    case SPICE_VIDEO_CODEC_TYPE_MJPEG:
-        stream_mjpeg_cleanup(st);
-        break;
+    if (st->video_decoder) {
+        st->video_decoder->destroy(st->video_decoder);
     }
 
     if (st->msg_clip)