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

[Francois Gouget]

On Thu, 3 Mar 2016, Christophe Fergeau wrote:
[...]
> > +static void gst_video_buffer_free(VideoBuffer *video_buffer)
> > +{
> > +    SpiceGstVideoBuffer *buffer = (SpiceGstVideoBuffer*)video_buffer;
> > +    gst_buffer_unref(buffer->gst_buffer);
> 
> You need a if (buffer->gst_buffer != NULL) test.
> pull_compressed_buffer() can call this method with a NULL gst_buffer in
> error cases, and gst_buffer_unref() warns on NULL.

Done.

[...]
> >  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);
> 
> Maybe g_return_val_if_fail(video_buffer != NULL, VIDEO_ENCODER_FRAME_UNSUPPORTED); ?

[...]
> > +        buffer->base.free((VideoBuffer*)buffer);
> >          gst_sample_unref(sample);
> 
> A *video_buffer = NULL; would not hurt in error cases in case the
> caller is careless and tries to use the result without checking for
> errors.
[...]
> Same comment here about setting *video_buffer to NULL in error cases.

Done although I centralized all these in encode_frame().


-- 
Francois Gouget <fgouget at codeweavers.com>