Re: [spice v10 09/27] server: Let the video encoder manage the compressed buffer

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



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

[Index of Archives]     [Linux ARM Kernel]     [Linux ARM]     [Linux Omap]     [Fedora ARM]     [IETF Annouce]     [Security]     [Bugtraq]     [Linux]     [Linux OMAP]     [Linux MIPS]     [ECOS]     [Asterisk Internet PBX]     [Linux API]     [Monitors]