[Spice-devel] GStreamer's zero-copy code is broken

[Francois Gouget]

On Fri, 24 Feb 2017, Christophe de Dinechin wrote:
[...]
> Looking at the patch, I wonder why we pass an offset and a stride from 
> different sources (offset is from the source, but stride is from the 
> bitmap)? Shouldn’t we use the stream stride?

I think that's correct:
 * bitmap->{x,y} are the width and height of the source image.

 * bitmap->stride is the size in bytes of each line. In other words 
   that's how much you must add to the pointer to get the pixel of the 
   next line.

 * src->{left,right,top,bottom} identifies which subset of the source 
   image should be encoded and sent to the client. So the offset 
   corresponds to the src->left.

However I don't see any way to tell gst_buffer_add_video_meta_full() 
that you only want to take 400 pixels out of every 640 pixel line, or 
only 200 lines out of the 480 lines.

For instance if we have
  bitmap->x = 640
  bitmap->y = 480
  bitmap->stride = 2560
  src->left = src->top = 0
  src->right = 200
  src->bottom = 100

Then we will call:

gst_buffer_add_video_meta_full(buffer, GST_VIDEO_FRAME_FLAG_NONE,
    encoder->format->gst_format, 
    640 /* bitmap->x */, 480 /* bitmap->y */,
    1,
    { 0 } /* offset */,
    { 2560 } /* stride */);

How will GStreamer know that it should encode a 200x100 video since we 
did not tell it that:
    src->right - src->left = 200
    src>bottom - src->top = 100


(not that any of this explains why it does not work for me since in my 
case src->right-src->left==bitmap->x and src->bottom-src->top==bitmap->y)

-- 
Francois Gouget <fgouget at codeweavers.com>