[Spice-devel] [spice v13 03/29] server: Add a GStreamer 1.0 MJPEG video encoder and use it by default

[Francois Gouget]

On Tue, 3 May 2016, Christophe Fergeau wrote:
[...]
> > > > +         /* Copy the line */
> > > > +         uint8_t *src = chunks->chunk[chunk_index].data + chunk_offset;
> > > > +         memcpy(dst, src, stream_stride);
> > > 
> > > Are we guaranteed that we'll have at least 'stream_stride' bytes in the
> > > chunk?
> > 
> > Yes, the is_chunk_padded() check guarantees it. I could rename it to 
> > is_chunk_stride_aligned() to make it clearer.
> 
> Hmm, this guarantees we have at least bitmap->stride bytes, which
> is (assumed to be?) bigger than stream_stride. Is there an explicit
> check/reason that bitmap->stride is bigger than stream_stride?

The red-parse-qxl patch addresses this concern which applied 
equally to all video encoders.
https://lists.freedesktop.org/archives/spice-devel/2016-May/029563.html


> Also, is there anything preventing chunks->chunk[index].len to be 0 in
> is_chunk_padded()?

It is ok for chunks->chunk[index].len to be 0. All it means is that we 
will skip that chunk (since necessarily chunk_offset >= len) and go to 
the next one. See:

         while (chunk_offset >= chunks->chunk[chunk_index].len) {
             if (is_chunk_stride_aligned(bitmap, chunk_index)) {
                 return FALSE;
             }
             chunk_offset -= chunks->chunk[chunk_index].len;
             chunk_index++;
         }

In zero_copy() it means we create a 0-byte GstMemory object which seems 
to be fine, and in chunk_copy() it means we copy 0 bytes which is fine.

-- 
Francois Gouget <fgouget at codeweavers.com>