[Spice-devel] [spice v13 11/29] server: Avoid copying the input frame in the GStreamer encoder

[Francois Gouget]

On Tue, 10 May 2016, Christophe Fergeau wrote:
[...]
> > diff --git a/server/gstreamer-encoder.c b/server/gstreamer-encoder.c
> > index f9b3579..7929dd8 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
> > +
> 
> Why do we want to  make it possible to compile out? I believe the
> implementation is more robust now that it initially was? Do you still
> expect some people will want to disable it?

It provides an easy way to compare the behavior and performance impact 
of the zero-copy code. I could remove the ifdefs if that's preferred.


> > +    int max_mem = gst_buffer_get_max_memory();
> 
> 
> The gstreamer name isn't so great, but could we use something less
> confusing than 'max_mem' as the variable name? I keep thinking it's a
> size in bytes. 'max_block_count' maybe?

Ok.


> > +    BitmapWrapper *wrapper = NULL;
> > +    while (*len && *chunk_index < max_mem) {
> > +        if (is_chunk_padded(bitmap, *chunk_index)) {
> > +            return FALSE;
> > +        }
> > +        if (wrapper) {
> > +            wrapper->refs++;
> 
> It would probably be safer to only change this with g_atomic_int_inc()

Ok.

-- 
Francois Gouget <fgouget at codeweavers.com>