Revert "gstreamer: Avoid memory copy if strides are different"

This reverts commit c3d237075b.

When you call gst_buffer_add_video_meta_full GStreamer assumes that
buffer is contiguous. Specifically GStreamer assumes that the first
chunk is big enough to hold a whole frame. This results usually in
some pixel shifts in the video. The pixel shifts you can see are
artifacts due to how the guest sends the frames.

Assuming you allocate the 2 chunks with 2 malloc:

   p1 = malloc(size);
   ...
   p2 = malloc(size);

Usually the memory allocator tend to allocate linearly if there are
space adding a prefix to describe next block leading to a memory
arrangement as:

   +-------------------+
   | p1 prefix         |
   +-------------------+
   | p1 buffer         |
   +-------------------+
   | p2 prefix         |
   +-------------------+
   | p2 buffer         |
   +-------------------+

now if you take p1 pointer and you assume it points to a 2 * size
buffer you will get p2 prefix in the middle, this prefix is the pixel
shifts.

Problems happens specifically in gst_video_frame_map_id.
This bug is reported in https://bugzilla.gnome.org/show_bug.cgi?id=779524.

Signed-off-by: Frediano Ziglio <fziglio@redhat.com>
Acked-by: Christophe Fergeau <cfergeau@redhat.com>
This commit is contained in:
Frediano Ziglio 2017-03-03 11:16:44 +00:00
parent 4b1c95beee
commit e3e2cbcb3a

View File

@ -1236,6 +1236,8 @@ static void clear_zero_copy_queue(SpiceGstEncoder *encoder, gboolean unref_queue
/* Nothing to do */
}
#endif
/* A helper for push_raw_frame() */
static inline int line_copy(SpiceGstEncoder *encoder, const SpiceBitmap *bitmap,
uint32_t chunk_offset, uint32_t stream_stride,
@ -1266,8 +1268,6 @@ static inline int line_copy(SpiceGstEncoder *encoder, const SpiceBitmap *bitmap,
return TRUE;
}
#endif
/* A helper for push_raw_frame() */
static inline int chunk_copy(SpiceGstEncoder *encoder, const SpiceBitmap *bitmap,
uint32_t chunk_index, uint32_t chunk_offset,
@ -1350,8 +1350,8 @@ static int push_raw_frame(SpiceGstEncoder *encoder,
gpointer bitmap_opaque)
{
uint32_t height = src->bottom - src->top;
uint32_t len;
uint32_t chunk_index = 0;
uint32_t stream_stride = (src->right - src->left) * encoder->format->bpp / 8;
uint32_t len = stream_stride * height;
GstBuffer *buffer = gst_buffer_new();
/* TODO Use GST_MAP_INFO_INIT once GStreamer 1.4.5 is no longer relevant */
GstMapInfo map = { .memory = NULL };
@ -1362,10 +1362,6 @@ static int push_raw_frame(SpiceGstEncoder *encoder,
uint32_t skip_lines = top_down ? src->top : bitmap->y - (src->bottom - 0);
uint32_t chunk_offset = bitmap->stride * skip_lines;
#ifndef DO_ZERO_COPY
uint32_t stream_stride = (src->right - src->left) * encoder->format->bpp / 8;
len = stream_stride * height;
if (stream_stride != bitmap->stride) {
/* We have to do a line-by-line copy because for each we have to
* leave out pixels on the left or right.
@ -1381,19 +1377,9 @@ static int push_raw_frame(SpiceGstEncoder *encoder,
return VIDEO_ENCODER_FRAME_UNSUPPORTED;
}
} else {
#else
{
/* using GStreamer 1.0, we can avoid cropping the image by simply passing
* the appropriate offset and stride as metadata */
gsize offset[] = { src->left * encoder->format->bpp / 8 };
gint stride[] = { bitmap->stride };
gst_buffer_add_video_meta_full(buffer, GST_VIDEO_FRAME_FLAG_NONE,
encoder->format->gst_format, bitmap->x, bitmap->y,
1, offset, stride);
len = bitmap->stride * height;
/* We can copy the bitmap chunk by chunk */
uint32_t chunk_index = 0;
#ifdef DO_ZERO_COPY
if (!zero_copy(encoder, bitmap, bitmap_opaque, buffer, &chunk_index,
&chunk_offset, &len)) {
gst_buffer_unref(buffer);