This reverts commit c3d237075b994fe67edddd58f2b3164cb579e6f4. 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@xxxxxxxxxx> Acked-by: Christophe Fergeau <cfergeau@xxxxxxxxxx> --- server/gstreamer-encoder.c | 26 ++++++-------------------- 1 file changed, 6 insertions(+), 20 deletions(-) Changes since v2: - improve comment. Changes since v1: - add more comment on pixel shifts reason. diff --git a/server/gstreamer-encoder.c b/server/gstreamer-encoder.c index 991eb51..df54cad 100644 --- a/server/gstreamer-encoder.c +++ b/server/gstreamer-encoder.c @@ -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); -- 2.9.3 _______________________________________________ Spice-devel mailing list Spice-devel@xxxxxxxxxxxxxxxxxxxxx https://lists.freedesktop.org/mailman/listinfo/spice-devel