[Spice-commits] server/gstreamer-encoder.c

[Frediano Ziglio]

 server/gstreamer-encoder.c |   26 ++++++--------------------
 1 file changed, 6 insertions(+), 20 deletions(-)

New commits:
commit e3e2cbcb3aa42c4af84b2bb74531bb73a280475f
Author: Frediano Ziglio <fziglio at redhat.com>
Date:   Fri Mar 3 11:16:44 2017 +0000

    Revert "gstreamer: Avoid memory copy if strides are different"
    
    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 at redhat.com>
    Acked-by: Christophe Fergeau <cfergeau at redhat.com>

diff --git a/server/gstreamer-encoder.c b/server/gstreamer-encoder.c
index 991eb510..df54cad5 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);