On Tue, Mar 01, 2016 at 04:53:03PM +0100, 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 | 55 +++++++++++++++++++-----------
> server/mjpeg-encoder.c | 85 +++++++++++++++++++++++++++++-----------------
> server/video-encoder.h | 26 ++++++++++----
> 6 files changed, 122 insertions(+), 77 deletions(-)
>
> diff --git a/server/dcc-send.c b/server/dcc-send.c
> index 6cf16e2..68ea085 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,18 +1705,16 @@ 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->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);
> @@ -1728,7 +1730,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;
> @@ -1737,7 +1738,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 {
> @@ -1747,7 +1748,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;
> @@ -1756,12 +1757,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 df650c9..88196f9 100644
> --- a/server/dcc.c
> +++ b/server/dcc.c
> @@ -385,10 +385,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));
> @@ -504,7 +500,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 c2ef871..35cb2a4 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 3dfbdfb..17dd0cb 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;
>
> @@ -80,6 +86,23 @@ typedef struct SpiceGstEncoder {
> } SpiceGstEncoder;
>
>
> +/* ---------- The SpiceGstVideoBuffer implementation ---------- */
> +
> +static void gst_video_buffer_free(VideoBuffer *video_buffer)
> +{
> + SpiceGstVideoBuffer *buffer = (SpiceGstVideoBuffer*)video_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 = &gst_video_buffer_free;
> + return buffer;
I forgot to mention that we usually don't use & for function pointers,
just direct assignment. I've noticed it in a few places in this patch.
Christophe
Attachment:
signature.asc
Description: PGP signature
_______________________________________________ Spice-devel mailing list Spice-devel@xxxxxxxxxxxxxxxxxxxxx https://lists.freedesktop.org/mailman/listinfo/spice-devel