On Tue, Apr 19, 2016 at 09:49:33AM +0200, Francois Gouget wrote:
> This way the video encoder is not forced to use malloc()/free().
> This also allows more flexibility in how the video encoder manages the
> buffer which allows for a zero-copy implementation in both video
> encoders.
>
> Signed-off-by: Francois Gouget <fgouget@xxxxxxxxxxxxxxx>
> ---
> server/dcc-send.c | 25 +++++++-------
> server/dcc.c | 5 ---
> server/dcc.h | 3 --
> server/gstreamer-encoder.c | 59 +++++++++++++++++++++-----------
> server/mjpeg-encoder.c | 85 +++++++++++++++++++++++++++++-----------------
> server/video-encoder.h | 26 ++++++++++----
> 6 files changed, 126 insertions(+), 77 deletions(-)
>
> diff --git a/server/dcc-send.c b/server/dcc-send.c
> index 54907f3..a2aea2a 100644
> --- a/server/dcc-send.c
> +++ b/server/dcc-send.c
> @@ -1649,6 +1649,12 @@ static void red_lossy_marshall_qxl_draw_text(RedChannelClient *rcc,
> }
> }
>
> +static void red_release_video_encoder_buffer(uint8_t *data, void *opaque)
> +{
> + VideoBuffer *buffer = (VideoBuffer*)opaque;
> + buffer->free(buffer);
> +}
> +
> static int red_marshall_stream_data(RedChannelClient *rcc,
> SpiceMarshaller *base_marshaller, Drawable *drawable)
> {
> @@ -1657,7 +1663,6 @@ static int red_marshall_stream_data(RedChannelClient *rcc,
> Stream *stream = drawable->stream;
> SpiceImage *image;
> uint32_t frame_mm_time;
> - int n;
> int width, height;
> int ret;
>
> @@ -1689,7 +1694,6 @@ static int red_marshall_stream_data(RedChannelClient *rcc,
>
> StreamAgent *agent = &dcc->stream_agents[get_stream_id(display, stream)];
> uint64_t time_now = spice_get_monotonic_time_ns();
> - size_t outbuf_size;
>
> if (!dcc->use_video_encoder_rate_control) {
> if (time_now - agent->last_send_time < (1000 * 1000 * 1000) / agent->fps) {
> @@ -1701,19 +1705,17 @@ static int red_marshall_stream_data(RedChannelClient *rcc,
> }
> }
>
> + VideoBuffer *outbuf;
> /* workaround for vga streams */
> frame_mm_time = drawable->red_drawable->mm_time ?
> drawable->red_drawable->mm_time :
> reds_get_mm_time();
> - outbuf_size = dcc->send_data.stream_outbuf_size;
> ret = !agent->video_encoder ? VIDEO_ENCODER_FRAME_UNSUPPORTED :
> agent->video_encoder->encode_frame(agent->video_encoder,
> frame_mm_time,
> &image->u.bitmap, width, height,
> &drawable->red_drawable->u.copy.src_area,
> - stream->top_down,
> - &dcc->send_data.stream_outbuf,
> - &outbuf_size, &n);
> + stream->top_down, &outbuf);
> switch (ret) {
> case VIDEO_ENCODER_FRAME_DROP:
> spice_assert(dcc->use_video_encoder_rate_control);
> @@ -1729,7 +1731,6 @@ static int red_marshall_stream_data(RedChannelClient *rcc,
> spice_error("bad return value (%d) from VideoEncoder::encode_frame", ret);
> return FALSE;
> }
> - dcc->send_data.stream_outbuf_size = outbuf_size;
>
> if (!drawable->sized_stream) {
> SpiceMsgDisplayStreamData stream_data;
> @@ -1738,7 +1739,7 @@ static int red_marshall_stream_data(RedChannelClient *rcc,
>
> stream_data.base.id = get_stream_id(display, stream);
> stream_data.base.multi_media_time = frame_mm_time;
> - stream_data.data_size = n;
> + stream_data.data_size = outbuf->size;
>
> spice_marshall_msg_display_stream_data(base_marshaller, &stream_data);
> } else {
> @@ -1748,7 +1749,7 @@ static int red_marshall_stream_data(RedChannelClient *rcc,
>
> stream_data.base.id = get_stream_id(display, stream);
> stream_data.base.multi_media_time = frame_mm_time;
> - stream_data.data_size = n;
> + stream_data.data_size = outbuf->size;
> stream_data.width = width;
> stream_data.height = height;
> stream_data.dest = drawable->red_drawable->bbox;
> @@ -1757,12 +1758,12 @@ static int red_marshall_stream_data(RedChannelClient *rcc,
> rect_debug(&stream_data.dest);
> spice_marshall_msg_display_stream_data_sized(base_marshaller, &stream_data);
> }
> - spice_marshaller_add_ref(base_marshaller,
> - dcc->send_data.stream_outbuf, n);
> + spice_marshaller_add_ref_full(base_marshaller, outbuf->data, outbuf->size,
> + &red_release_video_encoder_buffer, outbuf);
> agent->last_send_time = time_now;
> #ifdef STREAM_STATS
> agent->stats.num_frames_sent++;
> - agent->stats.size_sent += n;
> + agent->stats.size_sent += outbuf->size;
> agent->stats.end = frame_mm_time;
> #endif
>
> diff --git a/server/dcc.c b/server/dcc.c
> index 48d5b3d..14cd992 100644
> --- a/server/dcc.c
> +++ b/server/dcc.c
> @@ -374,10 +374,6 @@ DisplayChannelClient *dcc_new(DisplayChannel *display,
> // TODO: tune quality according to bandwidth
> dcc->jpeg_quality = 85;
>
> - size_t stream_buf_size;
> - stream_buf_size = 32*1024;
> - dcc->send_data.stream_outbuf = spice_malloc(stream_buf_size);
> - dcc->send_data.stream_outbuf_size = stream_buf_size;
> dcc->send_data.free_list.res =
> spice_malloc(sizeof(SpiceResourceList) +
> DISPLAY_FREE_LIST_DEFAULT_SIZE * sizeof(SpiceResourceID));
> @@ -485,7 +481,6 @@ void dcc_stop(DisplayChannelClient *dcc)
> dcc->pixmap_cache = NULL;
> dcc_release_glz(dcc);
> dcc_palette_cache_reset(dcc);
> - free(dcc->send_data.stream_outbuf);
> free(dcc->send_data.free_list.res);
> dcc_destroy_stream_agents(dcc);
> dcc_encoders_free(dcc);
> diff --git a/server/dcc.h b/server/dcc.h
> index 24471ba..4e95422 100644
> --- a/server/dcc.h
> +++ b/server/dcc.h
> @@ -88,9 +88,6 @@ struct DisplayChannelClient {
> uint32_t palette_cache_items;
>
> struct {
> - uint32_t stream_outbuf_size;
> - uint8_t *stream_outbuf; // caution stream buffer is also used as compress bufs!!!
> -
> FreeList free_list;
> uint64_t pixmap_cache_items[MAX_DRAWABLE_PIXMAP_CACHE_ITEMS];
> int num_pixmap_cache_items;
> diff --git a/server/gstreamer-encoder.c b/server/gstreamer-encoder.c
> index 0834863..04a74cb 100644
> --- a/server/gstreamer-encoder.c
> +++ b/server/gstreamer-encoder.c
> @@ -37,6 +37,12 @@ typedef struct {
> uint32_t bpp;
> } SpiceFormatForGStreamer;
>
> +typedef struct SpiceGstVideoBuffer {
> + VideoBuffer base;
> + GstBuffer *gst_buffer;
> + GstMapInfo map;
> +} SpiceGstVideoBuffer;
> +
> typedef struct SpiceGstEncoder {
> VideoEncoder base;
>
> @@ -81,6 +87,25 @@ typedef struct SpiceGstEncoder {
> } SpiceGstEncoder;
>
>
> +/* ---------- The SpiceGstVideoBuffer implementation ---------- */
> +
> +static void spice_gst_video_buffer_free(VideoBuffer *video_buffer)
> +{
> + SpiceGstVideoBuffer *buffer = (SpiceGstVideoBuffer*)video_buffer;
> + if (buffer->gst_buffer) {
> + gst_buffer_unref(buffer->gst_buffer);
> + }
> + free(buffer);
> +}
> +
> +static SpiceGstVideoBuffer* create_gst_video_buffer(void)
> +{
> + SpiceGstVideoBuffer *buffer = spice_new0(SpiceGstVideoBuffer, 1);
> + buffer->base.free = spice_gst_video_buffer_free;
> + return buffer;
> +}
> +
> +
> /* ---------- Miscellaneous SpiceGstEncoder helpers ---------- */
>
> static inline double get_mbps(uint64_t bit_rate)
> @@ -469,29 +494,22 @@ static int push_raw_frame(SpiceGstEncoder *encoder, const SpiceBitmap *bitmap,
>
> /* A helper for spice_gst_encoder_encode_frame() */
> static int pull_compressed_buffer(SpiceGstEncoder *encoder,
> - uint8_t **outbuf, size_t *outbuf_size,
> - int *data_size)
> + VideoBuffer **outbuf)
> {
> - spice_return_val_if_fail(outbuf && outbuf_size, VIDEO_ENCODER_FRAME_UNSUPPORTED);
> -
> GstSample *sample = gst_app_sink_pull_sample(encoder->appsink);
> if (sample) {
> - GstMapInfo map;
> - GstBuffer *buffer = gst_sample_get_buffer(sample);
> - if (buffer && gst_buffer_map(buffer, &map, GST_MAP_READ)) {
> - int size = gst_buffer_get_size(buffer);
> - if (!*outbuf || *outbuf_size < size) {
> - free(*outbuf);
> - *outbuf = spice_malloc(size);
> - *outbuf_size = size;
> - }
> - /* TODO Try to avoid this copy by changing the GstBuffer handling */
> - memcpy(*outbuf, map.data, size);
> - *data_size = size;
> - gst_buffer_unmap(buffer, &map);
> + SpiceGstVideoBuffer *buffer = create_gst_video_buffer();
> + buffer->gst_buffer = gst_sample_get_buffer(sample);
> + if (buffer->gst_buffer &&
> + gst_buffer_map(buffer->gst_buffer, &buffer->map, GST_MAP_READ)) {
The corresponding gst_buffer_unmap() is missing from
spice_gst_video_buffer_free (it's only re-added in the 0.10 support patch).
Acked-by: Christophe Fergeau <cfergeau@xxxxxxxxxx>
Christophe
Attachment:
signature.asc
Description: PGP signature
_______________________________________________ Spice-devel mailing list Spice-devel@xxxxxxxxxxxxxxxxxxxxx https://lists.freedesktop.org/mailman/listinfo/spice-devel