[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?

As far as I can tell, if stream_stride was larger than bitmap->stride it 
would mean that the video we try to encode is wider than the provided 
bitmap. I don't think that makes sense or that it can happen.


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

It would mean the chunk contains no data which is already wrong. And it 
would only be a problem if we still had data to copy which could only 
happen if the video is higher than the provided bitmap. Again I don't 
think that makes sense or that it can happen.


-- 
Francois Gouget <fgouget at codeweavers.com>