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@xxxxxxxxxxxxxxx> _______________________________________________ Spice-devel mailing list Spice-devel@xxxxxxxxxxxxxxxxxxxxx https://lists.freedesktop.org/mailman/listinfo/spice-devel