[Spice-devel] [PATCH v5 16/20] spice-gtk: Refactor the video decoding to use a more object oriented design.
Francois Gouget
fgouget at codeweavers.com
Thu Aug 27 12:03:01 PDT 2015
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 have more flexibility for their implementation.
Signed-off-by: Francois Gouget <fgouget at codeweavers.com>
---
Changes since take 2:
- None.
src/channel-display-mjpeg.c | 140 +++++++++++++++++++++++++++++---------------
src/channel-display-priv.h | 56 +++++++++++++-----
src/channel-display.c | 65 +++++++++-----------
3 files changed, 165 insertions(+), 96 deletions(-)
diff --git a/src/channel-display-mjpeg.c b/src/channel-display-mjpeg.c
index 95d5b33..37b28f7 100644
--- a/src/channel-display-mjpeg.c
+++ b/src/channel-display-mjpeg.c
@@ -21,14 +21,38 @@
#include "spice-common.h"
#include "spice-channel-priv.h"
+typedef struct MJpegDecoder MJpegDecoder;
+#define VIDEO_DECODER_T MJpegDecoder
#include "channel-display-priv.h"
+
+/* MJpeg decoder implementation */
+
+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;
+ uint32_t out_size;
+};
+
+
+/* ---------- 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 +73,62 @@ 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(MJpegDecoder *decoder,
+ SpiceMsgIn *frame_msg)
{
- gboolean back_compat = st->channel->priv->peer_hdr.major_version == 1;
+ 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);
-
- g_free(st->out_frame);
- st->out_frame = dest;
+ decoder->frame_msg = frame_msg;
+ stream_get_dimensions(decoder->base.stream, frame_msg, &width, &height);
+ if (decoder->out_size < width * height * 4) {
+ if (decoder->out_frame) {
+ free(decoder->out_frame);
+ }
+ decoder->out_size = width * height * 4;
+ decoder->out_frame = spice_malloc(decoder->out_size);
+ }
+ dest = decoder->out_frame;
- 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 +136,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 +160,45 @@ 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(MJpegDecoder* decoder)
+{
+ jpeg_destroy_decompress(&decoder->mjpeg_cinfo);
+ if (decoder->out_frame) {
+ free(decoder->out_frame);
+ }
+ free(decoder);
}
G_GNUC_INTERNAL
-void stream_mjpeg_cleanup(display_stream *st)
+MJpegDecoder* create_mjpeg_decoder(int codec_type, display_stream *stream)
{
- jpeg_destroy_decompress(&st->mjpeg_cinfo);
- g_free(st->out_frame);
- st->out_frame = NULL;
+ spice_assert(codec_type == SPICE_VIDEO_CODEC_TYPE_MJPEG);
+
+ 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 decoder;
}
diff --git a/src/channel-display-priv.h b/src/channel-display-priv.h
index 71f5d17..a01183b 100644
--- a/src/channel-display-priv.h
+++ b/src/channel-display-priv.h
@@ -34,6 +34,43 @@
G_BEGIN_DECLS
+typedef struct display_stream display_stream;
+
+typedef struct VideoDecoder VideoDecoder;
+#ifndef VIDEO_DECODER_T
+# define VIDEO_DECODER_T VideoDecoder
+#endif
+
+struct VideoDecoder {
+ /* Releases the video decoder's resources */
+ void (*destroy)(VIDEO_DECODER_T *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)(VIDEO_DECODER_T *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.
+ */
+VIDEO_DECODER_T* create_mjpeg_decoder(int codec_type, display_stream *stream);
+
typedef struct display_surface {
guint32 surface_id;
@@ -54,7 +91,7 @@ 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;
@@ -64,14 +101,9 @@ typedef struct display_stream {
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 +130,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 a65f926..4033b2f 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,22 +1078,17 @@ 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;
- }
-
- if (spice_msg_in_type(st->msg_data) == SPICE_MSG_DISPLAY_STREAM_DATA) {
- SpiceMsgDisplayStreamData *op = spice_msg_in_parsed(st->msg_data);
+ if (spice_msg_in_type(frame_msg) == SPICE_MSG_DISPLAY_STREAM_DATA) {
+ SpiceMsgDisplayStreamData *op = spice_msg_in_parsed(frame_msg);
*data = op->data;
return op->data_size;
} else {
- SpiceMsgDisplayStreamDataSized *op = spice_msg_in_parsed(st->msg_data);
+ SpiceMsgDisplayStreamDataSized *op = spice_msg_in_parsed(frame_msg);
- g_return_val_if_fail(spice_msg_in_type(st->msg_data) ==
+ g_return_val_if_fail(spice_msg_in_type(frame_msg) ==
SPICE_MSG_DISPLAY_STREAM_DATA_SIZED, 0);
*data = op->data;
return op->data_size;
@@ -1098,19 +1097,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;
@@ -1128,24 +1127,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);
@@ -1168,7 +1164,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);
@@ -1477,10 +1472,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)
--
2.5.0
More information about the Spice-devel
mailing list