diff --git a/server/dcc-send.c b/server/dcc-send.c index 472575d5..9ed3a209 100644 --- a/server/dcc-send.c +++ b/server/dcc-send.c @@ -1678,7 +1678,8 @@ static void red_release_video_encoder_buffer(uint8_t *data, void *opaque) } static int red_marshall_stream_data(RedChannelClient *rcc, - SpiceMarshaller *base_marshaller, Drawable *drawable) + SpiceMarshaller *base_marshaller, + Drawable *drawable) { DisplayChannelClient *dcc = RCC_TO_DCC(rcc); DisplayChannel *display = DCC_TO_DC(dcc); @@ -1727,6 +1728,7 @@ static int red_marshall_stream_data(RedChannelClient *rcc, frame_mm_time, ©->src_bitmap->u.bitmap, ©->src_area, stream->top_down, + drawable->red_drawable, &outbuf); switch (ret) { case VIDEO_ENCODER_FRAME_DROP: diff --git a/server/gstreamer-encoder.c b/server/gstreamer-encoder.c index 08a1afa3..92428148 100644 --- a/server/gstreamer-encoder.c +++ b/server/gstreamer-encoder.c @@ -30,6 +30,8 @@ #define SPICE_GST_DEFAULT_FPS 30 +#define DO_ZERO_COPY + typedef struct { SpiceBitmapFmt spice_format; @@ -46,6 +48,14 @@ typedef struct SpiceGstVideoBuffer { typedef struct SpiceGstEncoder { VideoEncoder base; + /* Callbacks to adjust the refcount of the bitmap being encoded. */ + bitmap_ref_t bitmap_ref; + bitmap_unref_t bitmap_unref; + +#ifdef DO_ZERO_COPY + GAsyncQueue *unused_bitmap_opaques; +#endif + /* Rate control callbacks */ VideoEncoderRateControlCbs cbs; @@ -460,12 +470,109 @@ static inline int line_copy(SpiceGstEncoder *encoder, const SpiceBitmap *bitmap, return TRUE; } +#ifdef DO_ZERO_COPY +typedef struct { + gint refs; + SpiceGstEncoder *encoder; + gpointer opaque; +} BitmapWrapper; + +static void clear_zero_copy_queue(SpiceGstEncoder *encoder, gboolean unref_queue) +{ + gpointer bitmap_opaque; + while ((bitmap_opaque = g_async_queue_try_pop(encoder->unused_bitmap_opaques))) { + encoder->bitmap_unref(bitmap_opaque); + } + if (unref_queue) { + g_async_queue_unref(encoder->unused_bitmap_opaques); + } +} + +static BitmapWrapper *bitmap_wrapper_new(SpiceGstEncoder *encoder, gpointer bitmap_opaque) +{ + BitmapWrapper *wrapper = spice_new(BitmapWrapper, 1); + wrapper->refs = 1; + wrapper->encoder = encoder; + wrapper->opaque = bitmap_opaque; + encoder->bitmap_ref(bitmap_opaque); + return wrapper; +} + +static void bitmap_wrapper_unref(gpointer data) +{ + BitmapWrapper *wrapper = data; + if (g_atomic_int_dec_and_test(&wrapper->refs)) { + g_async_queue_push(wrapper->encoder->unused_bitmap_opaques, wrapper->opaque); + free(wrapper); + } +} + + +/* A helper for push_raw_frame() */ +static inline int zero_copy(SpiceGstEncoder *encoder, + const SpiceBitmap *bitmap, gpointer bitmap_opaque, + GstBuffer *buffer, uint32_t *chunk_index, + uint32_t *chunk_offset, uint32_t *len) +{ + const SpiceChunks *chunks = bitmap->data; + while (*chunk_index < chunks->num_chunks && + *chunk_offset >= chunks->chunk[*chunk_index].len) { + if (!is_chunk_stride_aligned(bitmap, *chunk_index)) { + return FALSE; + } + *chunk_offset -= chunks->chunk[*chunk_index].len; + (*chunk_index)++; + } + + int max_block_count = gst_buffer_get_max_memory(); + if (chunks->num_chunks - *chunk_index > max_block_count) { + /* There are more chunks than we can fit memory objects in a + * buffer. This will cause the buffer to merge memory objects to + * fit the extra chunks, which means doing wasteful memory copies. + * So use the zero-copy approach for the first max_mem-1 chunks, and + * let push_raw_frame() deal with the rest. + */ + max_block_count = *chunk_index + max_block_count - 1; + } else { + max_block_count = chunks->num_chunks; + } + + BitmapWrapper *wrapper = NULL; + while (*len && *chunk_index < max_block_count) { + if (!is_chunk_stride_aligned(bitmap, *chunk_index)) { + return FALSE; + } + if (wrapper) { + g_atomic_int_inc(&wrapper->refs); + } else { + wrapper = bitmap_wrapper_new(encoder, bitmap_opaque); + } + uint32_t thislen = MIN(chunks->chunk[*chunk_index].len - *chunk_offset, *len); + GstMemory *mem = gst_memory_new_wrapped(GST_MEMORY_FLAG_READONLY, + chunks->chunk[*chunk_index].data, + chunks->chunk[*chunk_index].len, + *chunk_offset, thislen, + wrapper, bitmap_wrapper_unref); + gst_buffer_append_memory(buffer, mem); + *len -= thislen; + *chunk_offset = 0; + (*chunk_index)++; + } + return TRUE; +} +#else +static void clear_zero_copy_queue(SpiceGstEncoder *encoder, gboolean unref_queue) +{ + /* Nothing to do */ +} +#endif + /* A helper for push_raw_frame() */ static inline int chunk_copy(SpiceGstEncoder *encoder, const SpiceBitmap *bitmap, - uint32_t chunk_offset, uint32_t len, uint8_t *dst) + uint32_t chunk_index, uint32_t chunk_offset, + uint32_t len, uint8_t *dst) { SpiceChunks *chunks = bitmap->data; - uint32_t chunk_index = 0; /* Skip chunks until we find the start of the frame */ while (chunk_index < chunks->num_chunks && chunk_offset >= chunks->chunk[chunk_index].len) { @@ -493,17 +600,34 @@ static inline int chunk_copy(SpiceGstEncoder *encoder, const SpiceBitmap *bitmap return TRUE; } +/* A helper for push_raw_frame() */ +static uint8_t *allocate_and_map_memory(gsize size, GstMapInfo *map, GstBuffer *buffer) +{ + GstMemory *mem = gst_allocator_alloc(NULL, size, NULL); + if (!mem) { + gst_buffer_unref(buffer); + return NULL; + } + if (!gst_memory_map(mem, map, GST_MAP_WRITE)) { + gst_memory_unref(mem); + gst_buffer_unref(buffer); + return NULL; + } + return map->data; +} + /* A helper for spice_gst_encoder_encode_frame() */ -static int push_raw_frame(SpiceGstEncoder *encoder, const SpiceBitmap *bitmap, - const SpiceRect *src, int top_down) +static int push_raw_frame(SpiceGstEncoder *encoder, + const SpiceBitmap *bitmap, + const SpiceRect *src, int top_down, + gpointer bitmap_opaque) { uint32_t height = src->bottom - src->top; uint32_t stream_stride = (src->right - src->left) * encoder->format->bpp / 8; uint32_t len = stream_stride * height; - GstBuffer *buffer = gst_buffer_new_and_alloc(len); - GstMapInfo map; - gst_buffer_map(buffer, &map, GST_MAP_WRITE); - uint8_t *dst = map.data; + GstBuffer *buffer = gst_buffer_new(); + /* TODO Use GST_MAP_INFO_INIT once GStreamer 1.4.5 is no longer relevant */ + GstMapInfo map = { .memory = NULL }; /* Note that we should not reorder the lines, even if top_down is false. * It just changes the number of lines to skip at the start of the bitmap. @@ -515,21 +639,51 @@ static int push_raw_frame(SpiceGstEncoder *encoder, const SpiceBitmap *bitmap, /* We have to do a line-by-line copy because for each we have to * leave out pixels on the left or right. */ + uint8_t *dst = allocate_and_map_memory(len, &map, buffer); + if (!dst) { + return VIDEO_ENCODER_FRAME_UNSUPPORTED; + } + chunk_offset += src->left * encoder->format->bpp / 8; if (!line_copy(encoder, bitmap, chunk_offset, stream_stride, height, dst)) { - gst_buffer_unmap(buffer, &map); + gst_memory_unmap(map.memory, &map); + gst_memory_unref(map.memory); gst_buffer_unref(buffer); return VIDEO_ENCODER_FRAME_UNSUPPORTED; } } else { /* We can copy the bitmap chunk by chunk */ - if (!chunk_copy(encoder, bitmap, chunk_offset, len, dst)) { - gst_buffer_unmap(buffer, &map); + uint32_t chunk_index = 0; +#ifdef DO_ZERO_COPY + if (!zero_copy(encoder, bitmap, bitmap_opaque, buffer, &chunk_index, + &chunk_offset, &len)) { gst_buffer_unref(buffer); return VIDEO_ENCODER_FRAME_UNSUPPORTED; } + /* len now contains the remaining number of bytes to copy. + * However we must avoid any write to the GstBuffer object as it + * would cause a copy of the read-only memory objects we just added. + * Fortunately we can append extra writable memory objects instead. + */ +#endif + + if (len) { + uint8_t *dst = allocate_and_map_memory(len, &map, buffer); + if (!dst) { + return VIDEO_ENCODER_FRAME_UNSUPPORTED; + } + if (!chunk_copy(encoder, bitmap, chunk_index, chunk_offset, len, dst)) { + gst_memory_unmap(map.memory, &map); + gst_memory_unref(map.memory); + gst_buffer_unref(buffer); + return VIDEO_ENCODER_FRAME_UNSUPPORTED; + } + } + } + if (map.memory) { + gst_memory_unmap(map.memory, &map); + gst_buffer_append_memory(buffer, map.memory); } - gst_buffer_unmap(buffer, &map); GstFlowReturn ret = gst_app_src_push_buffer(encoder->appsrc, buffer); if (ret != GST_FLOW_OK) { @@ -573,6 +727,9 @@ static void spice_gst_encoder_destroy(VideoEncoder *video_encoder) g_mutex_clear(&encoder->outbuf_mutex); g_cond_clear(&encoder->outbuf_cond); + /* Unref any lingering bitmap opaque structures from past frames */ + clear_zero_copy_queue(encoder, TRUE); + free(encoder); } @@ -580,12 +737,16 @@ static int spice_gst_encoder_encode_frame(VideoEncoder *video_encoder, uint32_t frame_mm_time, const SpiceBitmap *bitmap, const SpiceRect *src, int top_down, + gpointer bitmap_opaque, VideoBuffer **outbuf) { SpiceGstEncoder *encoder = (SpiceGstEncoder*)video_encoder; g_return_val_if_fail(outbuf != NULL, VIDEO_ENCODER_FRAME_UNSUPPORTED); *outbuf = NULL; + /* Unref the last frame's bitmap_opaque structures if any */ + clear_zero_copy_queue(encoder, FALSE); + uint32_t width = src->right - src->left; uint32_t height = src->bottom - src->top; if (width != encoder->width || height != encoder->height || @@ -610,7 +771,7 @@ static int spice_gst_encoder_encode_frame(VideoEncoder *video_encoder, return VIDEO_ENCODER_FRAME_UNSUPPORTED; } - int rc = push_raw_frame(encoder, bitmap, src, top_down); + int rc = push_raw_frame(encoder, bitmap, src, top_down, bitmap_opaque); if (rc == VIDEO_ENCODER_FRAME_ENCODE_DONE) { rc = pull_compressed_buffer(encoder, outbuf); if (rc != VIDEO_ENCODER_FRAME_ENCODE_DONE) { @@ -622,6 +783,9 @@ static int spice_gst_encoder_encode_frame(VideoEncoder *video_encoder, free_pipeline(encoder); } } + + /* Unref the last frame's bitmap_opaque structures if any */ + clear_zero_copy_queue(encoder, FALSE); return rc; } @@ -669,7 +833,9 @@ static void spice_gst_encoder_get_stats(VideoEncoder *video_encoder, VideoEncoder *gstreamer_encoder_new(SpiceVideoCodecType codec_type, uint64_t starting_bit_rate, - VideoEncoderRateControlCbs *cbs) + VideoEncoderRateControlCbs *cbs, + bitmap_ref_t bitmap_ref, + bitmap_unref_t bitmap_unref) { spice_return_val_if_fail(codec_type == SPICE_VIDEO_CODEC_TYPE_MJPEG || codec_type == SPICE_VIDEO_CODEC_TYPE_VP8, NULL); @@ -689,11 +855,16 @@ VideoEncoder *gstreamer_encoder_new(SpiceVideoCodecType codec_type, encoder->base.get_bit_rate = spice_gst_encoder_get_bit_rate; encoder->base.get_stats = spice_gst_encoder_get_stats; encoder->base.codec_type = codec_type; +#ifdef DO_ZERO_COPY + encoder->unused_bitmap_opaques = g_async_queue_new(); +#endif if (cbs) { encoder->cbs = *cbs; } encoder->starting_bit_rate = starting_bit_rate; + encoder->bitmap_ref = bitmap_ref; + encoder->bitmap_unref = bitmap_unref; g_mutex_init(&encoder->outbuf_mutex); g_cond_init(&encoder->outbuf_cond); diff --git a/server/mjpeg-encoder.c b/server/mjpeg-encoder.c index 2db6ca93..1649516a 100644 --- a/server/mjpeg-encoder.c +++ b/server/mjpeg-encoder.c @@ -948,6 +948,7 @@ static int mjpeg_encoder_encode_frame(VideoEncoder *video_encoder, uint32_t frame_mm_time, const SpiceBitmap *bitmap, const SpiceRect *src, int top_down, + gpointer bitmap_opaque, VideoBuffer **outbuf) { MJpegEncoder *encoder = (MJpegEncoder*)video_encoder; @@ -1367,7 +1368,9 @@ static void mjpeg_encoder_get_stats(VideoEncoder *video_encoder, VideoEncoder *mjpeg_encoder_new(SpiceVideoCodecType codec_type, uint64_t starting_bit_rate, - VideoEncoderRateControlCbs *cbs) + VideoEncoderRateControlCbs *cbs, + bitmap_ref_t bitmap_ref, + bitmap_unref_t bitmap_unref) { MJpegEncoder *encoder = spice_new0(MJpegEncoder, 1); diff --git a/server/stream.c b/server/stream.c index 3d3e6276..8a137a1d 100644 --- a/server/stream.c +++ b/server/stream.c @@ -706,6 +706,18 @@ static void update_client_playback_delay(void *opaque, uint32_t delay_ms) agent->dcc->streams_max_latency); } +void bitmap_ref(gpointer data) +{ + RedDrawable *red_drawable = (RedDrawable*)data; + red_drawable_ref(red_drawable); +} + +void bitmap_unref(gpointer data) +{ + RedDrawable *red_drawable = (RedDrawable*)data; + red_drawable_unref(red_drawable); +} + /* A helper for dcc_create_stream(). */ static VideoEncoder* dcc_create_video_encoder(DisplayChannelClient *dcc, uint64_t starting_bit_rate, @@ -730,7 +742,7 @@ static VideoEncoder* dcc_create_video_encoder(DisplayChannelClient *dcc, continue; } - VideoEncoder* video_encoder = video_codec->create(video_codec->type, starting_bit_rate, cbs); + VideoEncoder* video_encoder = video_codec->create(video_codec->type, starting_bit_rate, cbs, bitmap_ref, bitmap_unref); if (video_encoder) { return video_encoder; } @@ -738,7 +750,7 @@ static VideoEncoder* dcc_create_video_encoder(DisplayChannelClient *dcc, /* Try to use the builtin MJPEG video encoder as a fallback */ if (!client_has_multi_codec || red_channel_client_test_remote_cap(rcc, SPICE_DISPLAY_CAP_CODEC_MJPEG)) { - return mjpeg_encoder_new(SPICE_VIDEO_CODEC_TYPE_MJPEG, starting_bit_rate, cbs); + return mjpeg_encoder_new(SPICE_VIDEO_CODEC_TYPE_MJPEG, starting_bit_rate, cbs, bitmap_ref, bitmap_unref); } return NULL; diff --git a/server/video-encoder.h b/server/video-encoder.h index f4be396a..34231184 100644 --- a/server/video-encoder.h +++ b/server/video-encoder.h @@ -65,6 +65,8 @@ struct VideoEncoder { * @bitmap: A bitmap containing the source video frame. * @src: A rectangle specifying the area occupied by the video. * @top_down: If true the first video line is specified by src.top. + * @bitmap_opaque: The parameter for the bitmap_ref() and bitmap_unref() + * callbacks. * @outbuf: A pointer to a VideoBuffer structure containing the * compressed frame if successful. Call the buffer's * free() method as soon as it is no longer needed. @@ -77,7 +79,7 @@ struct VideoEncoder { int (*encode_frame)(VideoEncoder *encoder, uint32_t frame_mm_time, const SpiceBitmap *bitmap, const SpiceRect *src, int top_down, - VideoBuffer** outbuf); + gpointer bitmap_opaque, VideoBuffer** outbuf); /* * Bit rate control methods. @@ -166,6 +168,8 @@ typedef struct VideoEncoderRateControlCbs { void (*update_client_playback_delay)(void *opaque, uint32_t delay_ms); } VideoEncoderRateControlCbs; +typedef void (*bitmap_ref_t)(gpointer data); +typedef void (*bitmap_unref_t)(gpointer data); /* Instantiates the video encoder. * @@ -173,22 +177,35 @@ typedef struct VideoEncoderRateControlCbs { * @starting_bit_rate: An initial estimate of the available stream bit rate * or zero if the client does not support rate control. * @cbs: A set of callback methods to be used for rate control. + * @bitmap_ref: A callback that the encoder can use to increase the + * bitmap refcount. + * This must be called from the main context. + * @bitmap_unref: A callback that the encoder can use to decrease the + * bitmap refcount. + * This must be called from the main context. * @return: A pointer to a structure implementing the VideoEncoder * methods. */ typedef VideoEncoder* (*new_video_encoder_t)(SpiceVideoCodecType codec_type, uint64_t starting_bit_rate, - VideoEncoderRateControlCbs *cbs); + VideoEncoderRateControlCbs *cbs, + bitmap_ref_t bitmap_ref, + bitmap_unref_t bitmap_unref); VideoEncoder* mjpeg_encoder_new(SpiceVideoCodecType codec_type, uint64_t starting_bit_rate, - VideoEncoderRateControlCbs *cbs); + VideoEncoderRateControlCbs *cbs, + bitmap_ref_t bitmap_ref, + bitmap_unref_t bitmap_unref); #ifdef HAVE_GSTREAMER_1_0 VideoEncoder* gstreamer_encoder_new(SpiceVideoCodecType codec_type, uint64_t starting_bit_rate, - VideoEncoderRateControlCbs *cbs); + VideoEncoderRateControlCbs *cbs, + bitmap_ref_t bitmap_ref, + bitmap_unref_t bitmap_unref); #endif + typedef struct RedVideoCodec { new_video_encoder_t create; SpiceVideoCodecType type;