[Spice-devel] [PATCH v5 07/20] server: Let the video encoder manage the compressed buffer and avoid copying it.
Christophe Fergeau
cfergeau at redhat.com
Tue Sep 22 05:52:09 PDT 2015
Hey,
On Thu, Aug 27, 2015 at 09:01:02PM +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.
> The current implementations also ensure that there is no reallocation of the VideoBuffer structure.
>
> Signed-off-by: Francois Gouget <fgouget at codeweavers.com>
> ---
>
> Changes since take 4:
> - Some formatting fixes.
commit log formatting needs work too (comment which applies to most of
your commits)
>
> server/gstreamer_encoder.c | 81 ++++++++++++++++++++++++++--------
> server/mjpeg_encoder.c | 105 +++++++++++++++++++++++++++++++--------------
> server/red_worker.c | 31 ++++++-------
> server/video_encoder.h | 45 ++++++++++++++++---
> 4 files changed, 186 insertions(+), 76 deletions(-)
>
> diff --git a/server/red_worker.c b/server/red_worker.c
> index 6587047..fa24412 100644
> --- a/server/red_worker.c
> +++ b/server/red_worker.c
> @@ -701,9 +701,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!!!
> -
> RedCompressBuf *used_compress_bufs;
>
> FreeList free_list;
> @@ -8529,6 +8526,12 @@ static inline void display_begin_send_message(RedChannelClient *rcc)
> red_channel_client_begin_send_message(rcc);
> }
>
> +static void red_release_video_encoder_buffer(uint8_t *data, void *opaque)
> +{
> + VideoBuffer *buffer = (VideoBuffer*)opaque;
> + buffer->unref(buffer);
> +}
> +
> static inline int red_marshall_stream_data(RedChannelClient *rcc,
> SpiceMarshaller *base_marshaller, Drawable *drawable)
> {
> @@ -8538,7 +8541,7 @@ static inline int red_marshall_stream_data(RedChannelClient *rcc,
> SpiceImage *image;
> RedWorker *worker = dcc->common.worker;
> uint32_t frame_mm_time;
> - int n;
> + VideoBuffer *outbuf;
> int width, height;
> int ret;
>
> @@ -8571,7 +8574,6 @@ static inline int red_marshall_stream_data(RedChannelClient *rcc,
>
> StreamAgent *agent = &dcc->stream_agents[get_stream_id(worker, stream)];
> uint64_t time_now = red_now();
> - size_t outbuf_size;
>
> if (!dcc->use_video_encoder_rate_control) {
> if (time_now - agent->last_send_time < (1000 * 1000 * 1000) / agent->fps) {
> @@ -8588,12 +8590,11 @@ static inline int red_marshall_stream_data(RedChannelClient *rcc,
> 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,
> &image->u.bitmap, width, height,
> &drawable->red_drawable->u.copy.src_area,
> stream->top_down, frame_mm_time,
> - &dcc->send_data.stream_outbuf, &outbuf_size, &n);
> + &outbuf);
You get a VideoBuffer from the encoder...
>
> switch (ret) {
> case VIDEO_ENCODER_FRAME_DROP:
> @@ -8610,7 +8611,6 @@ static inline 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;
> @@ -8619,7 +8619,7 @@ static inline int red_marshall_stream_data(RedChannelClient *rcc,
>
> stream_data.base.id = get_stream_id(worker, stream);
> stream_data.base.multi_media_time = frame_mm_time;
> - stream_data.data_size = n;
> + stream_data.data_size = outbuf->size;
... then you get the size from the previously obtained VideoBuffer
>
> spice_marshall_msg_display_stream_data(base_marshaller, &stream_data);
> } else {
> @@ -8629,7 +8629,7 @@ static inline int red_marshall_stream_data(RedChannelClient *rcc,
>
> stream_data.base.id = get_stream_id(worker, 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;
> @@ -8638,12 +8638,12 @@ static inline 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);
.. and here you need the data, the data size, the VideoBuffer, and some
function which is going to release the VideoBuffer memory.
What I'm getting at is that VideoBuffer seems to only be used from red_marshall_stream_data(),
and that all that the non-VideoEncoder code needs is
- some data buffer
- the size of that data buffer
- some opaque pointer which manages this data
- a function to free the opaque pointer when it's no longer needed.
It seems to me that VideoBuffer could be stack-allocated (or even
removed), and that the gstreamer backend could just return a directly
GstBuffer with gst_buffer_unref() as its free function, rather than
what is done currently where you create a GstVideoBuffer wrapping a
GstBuffer, and then you have some more code to try and reuse existing
GstVideoBuffer instances rather than allocating a new one.
The GStreamer 1.0 situation is a bit more complicated as you need a
GstBuffer and a GstMapInfo in the free function, I think I would use
gst_mini_object_set_qdata() to attach the GstMapInfo data to the
GstBuffer.
The needed data, data size, opaque pointer, free func could
alternatively be returned by a new encoder vfunc, or this could also be
all hidden in an video_encoder_marshall(...) vfunc
pull_compressed_buffer() could do (something like):
buffer->opaque = gst_app_sink_pull_buffer(encoder->appsink);
buffer->free_func = gst_buffer_unref;
if (buffer->opaque) {
buffer->data = GST_BUFFER_DATA(video_buffer->opaque);
buffer->size = GST_BUFFER_SIZE(video_buffer->opaque);
buffer->free_func = gst_buffer_unref;
return VIDEO_ENCODER_FRAME_ENCODE_DONE;
}
and red_marshall_stream_data() would do:
VideoBuffer buffer;
agent->video_encoder->encode_frame(..., &buffer);
spice_marshaller_add_ref_full(base_marshaller, buffer->data, buffer->size,
buffer->free_func, buffer->opaque);
(this is very schematic). I think this would work and could make the
code simpler.
> 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
>
> @@ -9405,7 +9405,6 @@ static void display_channel_client_on_disconnect(RedChannelClient *rcc)
> red_release_pixmap_cache(dcc);
> red_release_glz(dcc);
> red_reset_palette_cache(dcc);
> - free(dcc->send_data.stream_outbuf);
> red_display_reset_compress_buf(dcc);
> free(dcc->send_data.free_list.res);
> red_display_destroy_streams_agents(dcc);
> @@ -10776,7 +10775,6 @@ static void handle_new_display_channel(RedWorker *worker, RedClient *client, Red
> {
> DisplayChannel *display_channel;
> DisplayChannelClient *dcc;
> - size_t stream_buf_size;
>
> if (!worker->display_channel) {
> spice_warning("Display channel was not created");
> @@ -10792,9 +10790,6 @@ static void handle_new_display_channel(RedWorker *worker, RedClient *client, Red
> return;
> }
> spice_info("New display (client %p) dcc %p stream %p", client, dcc, stream);
> - stream_buf_size = 32*1024;
> - dcc->send_data.stream_outbuf = spice_malloc(stream_buf_size);
> - dcc->send_data.stream_outbuf_size = stream_buf_size;
> red_display_init_glz_data(dcc);
>
> dcc->send_data.free_list.res =
> diff --git a/server/gstreamer_encoder.c b/server/gstreamer_encoder.c
> index 2318f40..425be55 100644
> --- a/server/gstreamer_encoder.c
> +++ b/server/gstreamer_encoder.c
> @@ -26,6 +26,8 @@
>
> #include "red_common.h"
>
> +typedef struct GstVideoBuffer GstVideoBuffer;
> +#define VIDEO_BUFFER_T GstVideoBuffer
> typedef struct GstEncoder GstEncoder;
> #define VIDEO_ENCODER_T GstEncoder
> #include "video_encoder.h"
> @@ -44,6 +46,12 @@ typedef struct {
> uint32_t red_mask;
> } SpiceFormatForGStreamer;
>
> +struct GstVideoBuffer {
> + VideoBuffer base;
> + GstBuffer *gst_buffer;
> + gboolean persistent;
> +};
> +
> struct GstEncoder {
> VideoEncoder base;
>
> @@ -72,6 +80,9 @@ struct GstEncoder {
> GstElement *gstenc;
> GstAppSink *appsink;
>
> + /* The default video buffer */
> + GstVideoBuffer *default_buffer;
> +
Do we really need a default video buffer rather than always allocating a
new one? How much performance do we gain from that? Imo splitting this
in a separate commit (or removing it) would make the whole patch much
simpler (no need for refcounting in particular).
> /* The frame counter for GStreamer buffers */
> uint32_t frame;
>
> @@ -86,6 +97,35 @@ struct GstEncoder {
> };
>
>
> +/* ---------- The GstVideoBuffer implementation ---------- */
> +
> +static inline GstVideoBuffer* gst_video_buffer_ref(GstVideoBuffer *buffer)
> +{
> + buffer->base.ref_count++;
> + return buffer;
> +}
> +
> +static void gst_video_buffer_unref(GstVideoBuffer *buffer)
> +{
> + if (--buffer->base.ref_count == 0) {
> + gst_buffer_unref(buffer->gst_buffer);
> + if (!buffer->persistent) {
> + free(buffer);
> + }
> + }
> +}
> +
> +static GstVideoBuffer* create_gst_video_buffer(gboolean persistent)
> +{
> + GstVideoBuffer *buffer = spice_new0(GstVideoBuffer, 1);
> + buffer->base.ref = &gst_video_buffer_ref;
> + buffer->base.unref = &gst_video_buffer_unref;
> + buffer->persistent = persistent;
> + buffer->base.ref_count = persistent ? 0 : 1;
A new object with a 0 refcount is very odd to me, especially as it seems
it's not just internal state, but can be returned to the user this way
(I think there are even codepaths where it could wrap back to UINT_MAX)
> + return buffer;
> +}
> +
> +
> /* ---------- Miscellaneous GstEncoder helpers ---------- */
>
> static inline double get_mbps(uint64_t bit_rate)
> @@ -386,24 +426,24 @@ static int push_raw_frame(GstEncoder *encoder, const SpiceBitmap *bitmap,
> }
>
> /* A helper for gst_encoder_encode_frame(). */
> -static int pull_compressed_buffer(GstEncoder *encoder,
> - uint8_t **outbuf, size_t *outbuf_size,
> - int *data_size)
> +static int pull_compressed_buffer(GstEncoder *encoder, GstVideoBuffer **buffer)
> {
> - GstBuffer *buffer = gst_app_sink_pull_buffer(encoder->appsink);
> - if (buffer) {
> - int len = GST_BUFFER_SIZE(buffer);
> - spice_assert(outbuf && outbuf_size);
> - if (!*outbuf || *outbuf_size < len) {
> - *outbuf = spice_realloc(*outbuf, len);
> - *outbuf_size = len;
> - }
> - /* TODO Try to avoid this copy by changing the GstBuffer handling */
> - memcpy(*outbuf, GST_BUFFER_DATA(buffer), len);
> - gst_buffer_unref(buffer);
> - *data_size = len;
> + GstVideoBuffer *video_buffer;
> + if (encoder->default_buffer->base.ref_count == 0) {
> + /* The default buffer is unused so we can reuse it. */
> + video_buffer = encoder->default_buffer;
> + video_buffer->base.ref(video_buffer);
> + } else {
> + video_buffer = create_gst_video_buffer(FALSE);
> + }
> + video_buffer->gst_buffer = gst_app_sink_pull_buffer(encoder->appsink);
> + if (video_buffer->gst_buffer) {
> + video_buffer->base.data = GST_BUFFER_DATA(video_buffer->gst_buffer);
> + video_buffer->base.size = GST_BUFFER_SIZE(video_buffer->gst_buffer);
> + *buffer = video_buffer;
> return VIDEO_ENCODER_FRAME_ENCODE_DONE;
> }
> + video_buffer->base.unref(video_buffer);
> return VIDEO_ENCODER_FRAME_UNSUPPORTED;
> }
>
> @@ -412,6 +452,11 @@ static int pull_compressed_buffer(GstEncoder *encoder,
>
> static void gst_encoder_destroy(GstEncoder *encoder)
> {
> + if (encoder->default_buffer->base.ref_count == 0) {
> + free(encoder->default_buffer);
> + } else {
> + encoder->default_buffer->persistent = FALSE;
> + }
> reset_pipeline(encoder);
> free(encoder);
> }
> @@ -421,8 +466,7 @@ static int gst_encoder_encode_frame(GstEncoder *encoder,
> int width, int height,
> const SpiceRect *src, int top_down,
> uint32_t frame_mm_time,
> - uint8_t **outbuf, size_t *outbuf_size,
> - int *data_size)
> + GstVideoBuffer **buffer)
> {
> if (width != encoder->width || height != encoder->height ||
> encoder->spice_format != bitmap->format) {
> @@ -447,7 +491,7 @@ static int gst_encoder_encode_frame(GstEncoder *encoder,
>
> int rc = push_raw_frame(encoder, bitmap, src, top_down);
> if (rc == VIDEO_ENCODER_FRAME_ENCODE_DONE) {
> - rc = pull_compressed_buffer(encoder, outbuf, outbuf_size, data_size);
> + rc = pull_compressed_buffer(encoder, buffer);
> }
> return rc;
> }
> @@ -515,6 +559,7 @@ GstEncoder *create_gstreamer_encoder(SpiceVideoCodecType codec_type,
> encoder->base.get_bit_rate = &gst_encoder_get_bit_rate;
> encoder->base.get_stats = &gst_encoder_get_stats;
> encoder->base.codec_type = codec_type;
> + encoder->default_buffer = create_gst_video_buffer(TRUE);
>
> if (cbs) {
> encoder->cbs = *cbs;
> diff --git a/server/mjpeg_encoder.c b/server/mjpeg_encoder.c
> index 9f1b392..0c982b7 100644
> --- a/server/mjpeg_encoder.c
> +++ b/server/mjpeg_encoder.c
> @@ -21,6 +21,8 @@
>
> #include "red_common.h"
>
> +typedef struct MJpegVideoBuffer MJpegVideoBuffer;
> +#define VIDEO_BUFFER_T MJpegVideoBuffer
> typedef struct MJpegEncoder MJpegEncoder;
> #define VIDEO_ENCODER_T MJpegEncoder
> #include "video_encoder.h"
> @@ -73,6 +75,9 @@ static const int mjpeg_quality_samples[MJPEG_QUALITY_SAMPLE_NUM] = {20, 30, 40,
> */
> #define MJPEG_WARMUP_TIME 3000LL // 3 sec
>
> +/* The compressed buffer initial size. */
> +#define MJPEG_INITIAL_BUFFER_SIZE (32 * 1024)
> +
> enum {
> MJPEG_QUALITY_EVAL_TYPE_SET,
> MJPEG_QUALITY_EVAL_TYPE_UPGRADE,
> @@ -157,12 +162,19 @@ typedef struct MJpegEncoderRateControl {
> uint64_t warmup_start_time;
> } MJpegEncoderRateControl;
>
> +struct MJpegVideoBuffer {
> + VideoBuffer base;
> + size_t maxsize;
> +};
> +
> struct MJpegEncoder {
> VideoEncoder base;
> uint8_t *row;
> uint32_t row_size;
> int first_frame;
>
> + MJpegVideoBuffer* default_buffer;
> +
Same comment about this 'default_buffer', I think this patch would be
much simpler without it, and this seems to be an additional optimization
more than something required by this change, can this be moved to a
different patch?
For what it's worth, my feeling is that the mjpeg encoder could be
changed to use GByteArray rather than directly using malloc/free, but
that's a bit more work than what's done in this commit
(mem_destination_mgr would need to be changed)
Christophe
> struct jpeg_compress_struct cinfo;
> struct jpeg_error_mgr jerr;
>
> @@ -184,6 +196,31 @@ static uint32_t get_min_required_playback_delay(uint64_t frame_enc_size,
> uint64_t byte_rate,
> uint32_t latency);
>
> +static inline MJpegVideoBuffer* mjpeg_buffer_ref(MJpegVideoBuffer *buffer)
> +{
> + buffer->base.ref_count++;
> + return buffer;
> +}
> +
> +static void mjpeg_buffer_unref(MJpegVideoBuffer *buffer)
S> +{
> + if (--buffer->base.ref_count == 0) {
> + free(buffer->base.data);
> + free(buffer);
> + }
> +}
> +
> +static MJpegVideoBuffer* create_mjpeg_video_buffer(void)
> +{
> + MJpegVideoBuffer *buffer = spice_new0(MJpegVideoBuffer, 1);
> + buffer->base.ref = &mjpeg_buffer_ref;
> + buffer->base.unref = &mjpeg_buffer_unref;
> + buffer->maxsize = MJPEG_INITIAL_BUFFER_SIZE;
> + buffer->base.data = malloc(buffer->maxsize);
> + buffer->base.ref_count = 1;
> + return buffer;
> +}
> +
> static inline int rate_control_is_active(MJpegEncoder* encoder)
> {
> return encoder->cbs.get_roundtrip_ms != NULL;
> @@ -192,6 +229,7 @@ static inline int rate_control_is_active(MJpegEncoder* encoder)
> static void mjpeg_encoder_destroy(MJpegEncoder *encoder)
> {
> jpeg_destroy_compress(&encoder->cinfo);
> + encoder->default_buffer->base.unref(encoder->default_buffer);
> free(encoder->row);
> free(encoder);
> }
> @@ -285,24 +323,22 @@ static void term_mem_destination(j_compress_ptr cinfo)
>
> /*
> * Prepare for output to a memory buffer.
> - * The caller may supply an own initial buffer with appropriate size.
> - * Otherwise, or when the actual data output exceeds the given size,
> - * the library adapts the buffer size as necessary.
> - * The standard library functions malloc/free are used for allocating
> - * larger memory, so the buffer is available to the application after
> - * finishing compression, and then the application is responsible for
> - * freeing the requested memory.
> + * The caller must supply its own initial buffer and size.
> + * When the actual data output exceeds the given size, the library
> + * will adapt the buffer size as necessary using the malloc()/free()
> + * functions. The buffer is available to the application after the
> + * compression and the application is then responsible for freeing it.
> */
> -
> static void
> spice_jpeg_mem_dest(j_compress_ptr cinfo,
> unsigned char ** outbuffer, size_t * outsize)
> {
> mem_destination_mgr *dest;
> -#define OUTPUT_BUF_SIZE 4096 /* choose an efficiently fwrite'able size */
>
> - if (outbuffer == NULL || outsize == NULL) /* sanity check */
> + if (outbuffer == NULL || *outbuffer == NULL ||
> + outsize == NULL || *outsize == 0) { /* sanity check */
> ERREXIT(cinfo, JERR_BUFFER_SIZE);
> + }
>
> /* The destination object is made permanent so that multiple JPEG images
> * can be written to the same buffer without re-executing jpeg_mem_dest.
> @@ -317,13 +353,6 @@ spice_jpeg_mem_dest(j_compress_ptr cinfo,
> dest->pub.term_destination = term_mem_destination;
> dest->outbuffer = outbuffer;
> dest->outsize = outsize;
> - if (*outbuffer == NULL || *outsize == 0) {
> - /* Allocate initial buffer */
> - *outbuffer = malloc(OUTPUT_BUF_SIZE);
> - if (*outbuffer == NULL)
> - ERREXIT1(cinfo, JERR_OUT_OF_MEMORY, 10);
> - *outsize = OUTPUT_BUF_SIZE;
> - }
>
> dest->pub.next_output_byte = dest->buffer = *outbuffer;
> dest->pub.free_in_buffer = dest->bufsize = *outsize;
> @@ -700,7 +729,7 @@ static void mjpeg_encoder_adjust_fps(MJpegEncoder *encoder, uint64_t now)
> static int mjpeg_encoder_start_frame(MJpegEncoder *encoder,
> SpiceBitmapFmt format,
> int width, int height,
> - uint8_t **dest, size_t *dest_len,
> + MJpegVideoBuffer *buffer,
> uint32_t frame_mm_time)
> {
> uint32_t quality;
> @@ -784,7 +813,7 @@ static int mjpeg_encoder_start_frame(MJpegEncoder *encoder,
> }
> }
>
> - spice_jpeg_mem_dest(&encoder->cinfo, dest, dest_len);
> + spice_jpeg_mem_dest(&encoder->cinfo, &buffer->base.data, &buffer->maxsize);
>
> encoder->cinfo.image_width = width;
> encoder->cinfo.image_height = height;
> @@ -927,25 +956,34 @@ static int mjpeg_encoder_encode_frame(MJpegEncoder *encoder,
> int width, int height,
> const SpiceRect *src,
> int top_down, uint32_t frame_mm_time,
> - uint8_t **outbuf, size_t *outbuf_size,
> - int *data_size)
> + MJpegVideoBuffer **buffer)
> {
> - int ret;
> -
> - ret = mjpeg_encoder_start_frame(encoder, bitmap->format,
> - width, height, outbuf, outbuf_size,
> - frame_mm_time);
> - if (ret != VIDEO_ENCODER_FRAME_ENCODE_DONE) {
> - return ret;
> + MJpegVideoBuffer *video_buffer;
> + if (encoder->default_buffer->base.ref_count == 1) {
> + /* Only the MJpegEncoder is referencing the default buffer
> + * so we can reuse it.
> + */
> + video_buffer = encoder->default_buffer;
> + video_buffer->base.ref(video_buffer);
> + } else {
> + video_buffer = create_mjpeg_video_buffer();
> }
>
> - if (!encode_mjpeg_frame(encoder, src, bitmap, top_down)) {
> - return VIDEO_ENCODER_FRAME_UNSUPPORTED;
> + int ret = mjpeg_encoder_start_frame(encoder, bitmap->format, width, height,
> + video_buffer, frame_mm_time);
> + if (ret == VIDEO_ENCODER_FRAME_ENCODE_DONE) {
> + if (encode_mjpeg_frame(encoder, src, bitmap, top_down)) {
> + video_buffer->base.size = mjpeg_encoder_end_frame(encoder);
> + *buffer = video_buffer;
> + } else {
> + ret = VIDEO_ENCODER_FRAME_UNSUPPORTED;
> + }
> }
>
> - *data_size = mjpeg_encoder_end_frame(encoder);
> -
> - return VIDEO_ENCODER_FRAME_ENCODE_DONE;
> + if (ret != VIDEO_ENCODER_FRAME_ENCODE_DONE) {
> + video_buffer->base.unref(video_buffer);
> + }
> + return ret;
> }
>
>
> @@ -1355,6 +1393,7 @@ MJpegEncoder *create_mjpeg_encoder(SpiceVideoCodecType codec_type,
> encoder->base.get_stats = &mjpeg_encoder_get_stats;
> encoder->base.codec_type = codec_type;
> encoder->first_frame = TRUE;
> + encoder->default_buffer = create_mjpeg_video_buffer();
> encoder->rate_control.byte_rate = starting_bit_rate / 8;
> encoder->starting_bit_rate = starting_bit_rate;
>
> diff --git a/server/video_encoder.h b/server/video_encoder.h
> index ac96589..fb378c3 100644
> --- a/server/video_encoder.h
> +++ b/server/video_encoder.h
> @@ -23,6 +23,38 @@
>
> #include "red_common.h"
>
> +
> +typedef struct VideoBuffer VideoBuffer;
> +#ifndef VIDEO_BUFFER_T
> +# define VIDEO_BUFFER_T VideoBuffer
> +#endif
> +
> +/* A structure containing the data for a compressed frame. See encode_frame(). */
> +struct VideoBuffer {
> + /* A pointer to the compressed frame data. */
> + uint8_t *data;
> +
> + /* The size of the compressed frame in bytes. */
> + uint32_t size;
> +
> + /* The buffer's reference count. */
> + uint32_t ref_count;
> +
> + /* Increments the video buffer's reference count and returns the new count.
> + *
> + * @buffer: The video buffer.
> + * @return: The new reference count.
> + */
> + VIDEO_BUFFER_T* (*ref)(VIDEO_BUFFER_T *buffer);
> +
> + /* Decrements the video buffer's reference count and deallocates it as
> + * appropriate.
> + *
> + * @buffer: The video buffer.
> + */
> + void (*unref)(VIDEO_BUFFER_T *buffer);
> +};
> +
> enum {
> VIDEO_ENCODER_FRAME_UNSUPPORTED = -1,
> VIDEO_ENCODER_FRAME_DROP,
> @@ -52,11 +84,10 @@ struct VideoEncoder {
> * @height: The height of the video area. This always matches src.
> * @src: A rectangle specifying the area occupied by the video.
> * @top_down: If true the first video line is specified by src.top.
> - * @outbuf: The buffer for the compressed frame. This must either be
> - * NULL or point to a buffer allocated by malloc since it may be
> - * reallocated, if its size is too small.
> - * @outbuf_size: The size of the outbuf buffer.
> - * @data_size: The size of the compressed frame.
> + * @buffer: A pointer to a VideoBuffer structure containing the
> + * compressed frame if successful. This buffer should be
> + * unref()-ed as soon as no longer needed, optimally before the
> + * next call to encode_frame().
> * @return:
> * VIDEO_ENCODER_FRAME_ENCODE_DONE if successful.
> * VIDEO_ENCODER_FRAME_UNSUPPORTED if the frame cannot be encoded.
> @@ -65,8 +96,8 @@ struct VideoEncoder {
> */
> int (*encode_frame)(VIDEO_ENCODER_T *encoder, const SpiceBitmap *bitmap,
> int width, int height, const SpiceRect *src,
> - int top_down, uint32_t frame_mm_time,uint8_t **outbuf,
> - size_t *outbuf_size, int *data_size);
> + int top_down, uint32_t frame_mm_time,
> + VIDEO_BUFFER_T** buffer);
>
> /*
> * Bit rate control methods.
> --
> 2.5.0
>
> _______________________________________________
> Spice-devel mailing list
> Spice-devel at lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/spice-devel
-------------- next part --------------
A non-text attachment was scrubbed...
Name: not available
Type: application/pgp-signature
Size: 819 bytes
Desc: not available
URL: <http://lists.freedesktop.org/archives/spice-devel/attachments/20150922/31bb10fd/attachment-0001.sig>
More information about the Spice-devel
mailing list