[Spice-devel] [spice-server 2/3] gstreamer: Use RedChunkIterator

Christophe Fergeau cfergeau at redhat.com
Fri Sep 9 13:37:21 UTC 2016


---
 server/gstreamer-encoder.c | 148 +++++++++++----------------------------------
 1 file changed, 36 insertions(+), 112 deletions(-)

diff --git a/server/gstreamer-encoder.c b/server/gstreamer-encoder.c
index d5fa072..c9cd3fb 100644
--- a/server/gstreamer-encoder.c
+++ b/server/gstreamer-encoder.c
@@ -27,6 +27,7 @@
 #include <gst/app/gstappsink.h>
 
 #include "red-common.h"
+#include "red-chunk-iterator.h"
 #include "video-encoder.h"
 #include "utils.h"
 
@@ -37,7 +38,6 @@
 # define DO_ZERO_COPY
 #endif
 
-
 typedef struct {
     SpiceBitmapFmt spice_format;
     char format[8];
@@ -1132,44 +1132,21 @@ static gboolean configure_pipeline(SpiceGstEncoder *encoder)
     return TRUE;
 }
 
-/* A helper for the *_copy() functions */
-static int is_chunk_stride_aligned(const SpiceBitmap *bitmap, uint32_t index)
-{
-    SpiceChunks *chunks = bitmap->data;
-    if (chunks->chunk[index].len % bitmap->stride != 0) {
-        /* A line straddles two chunks. This is not supported */
-        spice_warning("chunk %d/%d contains an incomplete line, cannot copy",
-                      index, chunks->num_chunks);
-        return FALSE;
-    }
-    return TRUE;
-}
-
 /* A helper for push_raw_frame() */
 static inline int line_copy(SpiceGstEncoder *encoder, const SpiceBitmap *bitmap,
                             uint32_t chunk_offset, uint32_t stream_stride,
                             uint32_t height, uint8_t *buffer)
 {
      uint8_t *dst = buffer;
-     SpiceChunks *chunks = bitmap->data;
-     uint32_t chunk_index = 0;
+     RedChunkIterator it;
+
+     red_chunk_iterator_init(&it, bitmap->data);
+     red_chunk_iterator_skip_bytes(&it, chunk_offset);
+     g_assert(bitmap->stride >= stream_stride);
      for (int l = 0; l < height; l++) {
-         /* We may have to move forward by more than one chunk the first
-          * time around. This also protects us against 0-byte chunks.
-          */
-         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++;
-         }
-
-         /* Copy the line */
-         uint8_t *src = chunks->chunk[chunk_index].data + chunk_offset;
-         memcpy(dst, src, stream_stride);
+         red_chunk_iterator_peek_data(&it, dst, stream_stride);
          dst += stream_stride;
-         chunk_offset += bitmap->stride;
+         red_chunk_iterator_skip_bytes(&it, bitmap->stride);
      }
      spice_return_val_if_fail(dst - buffer == stream_stride * height, FALSE);
      return TRUE;
@@ -1214,56 +1191,34 @@ static void bitmap_wrapper_unref(gpointer data)
 
 
 /* A helper for push_raw_frame() */
-static inline int zero_copy(SpiceGstEncoder *encoder,
-                            const SpiceBitmap *bitmap, gpointer bitmap_opaque,
-                            GstBuffer *buffer, uint32_t *chunk_index,
-                            uint32_t *chunk_offset, uint32_t *len)
+static inline int zero_copy(SpiceGstEncoder *encoder, gpointer bitmap_opaque,
+                            GstBuffer *buffer, RedChunkIterator *it,
+                            uint32_t len)
 {
-    const SpiceChunks *chunks = bitmap->data;
-    while (*chunk_index < chunks->num_chunks &&
-           *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)++;
-    }
-
     int max_block_count = gst_buffer_get_max_memory();
-    if (chunks->num_chunks - *chunk_index > max_block_count) {
-        /* There are more chunks than we can fit memory objects in a
-         * buffer. This will cause the buffer to merge memory objects to
-         * fit the extra chunks, which means doing wasteful memory copies.
-         * So use the zero-copy approach for the first max_mem-1 chunks, and
-         * let push_raw_frame() deal with the rest.
-         */
-        max_block_count = *chunk_index + max_block_count - 1;
-    } else {
-        max_block_count = chunks->num_chunks;
-    }
-
     BitmapWrapper *wrapper = NULL;
-    while (*len && *chunk_index < max_block_count) {
-        if (!is_chunk_stride_aligned(bitmap, *chunk_index)) {
-            return FALSE;
-        }
+    gsize copied_len = 0;
+
+    for (unsigned int i = 0; i < max_block_count; i++) {
         if (wrapper) {
             g_atomic_int_inc(&wrapper->refs);
         } else {
             wrapper = bitmap_wrapper_new(encoder, bitmap_opaque);
         }
-        uint32_t thislen = MIN(chunks->chunk[*chunk_index].len - *chunk_offset, *len);
-        GstMemory *mem = gst_memory_new_wrapped(GST_MEMORY_FLAG_READONLY,
-                                                chunks->chunk[*chunk_index].data,
-                                                chunks->chunk[*chunk_index].len,
-                                                *chunk_offset, thislen,
-                                                wrapper, bitmap_wrapper_unref);
+        GstMemory *mem = red_chunk_iterator_get_current_chunk_as_gst(it, len, wrapper, bitmap_wrapper_unref);
+        if (mem == NULL) {
+            /* We had less than "max_block_count" chunks, this is fine */
+            break;
+        }
         gst_buffer_append_memory(buffer, mem);
-        *len -= thislen;
-        *chunk_offset = 0;
-        (*chunk_index)++;
+
+        copied_len += gst_memory_get_sizes(mem, NULL, NULL);
+        len -= gst_memory_get_sizes(mem, NULL, NULL);
+        if (len == 0) {
+            break;
+        }
     }
-    return TRUE;
+    return copied_len;
 }
 #else
 static void clear_zero_copy_queue(SpiceGstEncoder *encoder, gboolean unref_queue)
@@ -1272,39 +1227,6 @@ static void clear_zero_copy_queue(SpiceGstEncoder *encoder, gboolean unref_queue
 }
 #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,
-                             uint32_t len, uint8_t *dst)
-{
-    SpiceChunks *chunks = bitmap->data;
-    /* Skip chunks until we find the start of the frame */
-    while (chunk_index < chunks->num_chunks &&
-           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++;
-    }
-
-    /* We can copy the frame chunk by chunk */
-    while (len && chunk_index < chunks->num_chunks) {
-        if (!is_chunk_stride_aligned(bitmap, chunk_index)) {
-            return FALSE;
-        }
-        uint8_t *src = chunks->chunk[chunk_index].data + chunk_offset;
-        uint32_t thislen = MIN(chunks->chunk[chunk_index].len - chunk_offset, len);
-        memcpy(dst, src, thislen);
-        dst += thislen;
-        len -= thislen;
-        chunk_offset = 0;
-        chunk_index++;
-    }
-    spice_return_val_if_fail(len == 0, FALSE);
-    return TRUE;
-}
-
 #ifdef HAVE_GSTREAMER_0_10
 /* Dummy structure to avoid too many #ifdef in the main codepaths */
 typedef struct {
@@ -1382,13 +1304,15 @@ static int push_raw_frame(SpiceGstEncoder *encoder,
         }
     } else {
         /* 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);
-            return VIDEO_ENCODER_FRAME_UNSUPPORTED;
+        RedChunkIterator it;
+        red_chunk_iterator_init(&it, bitmap->data);
+        if (!red_chunk_iterator_skip_bytes(&it, chunk_offset)) {
+            return FALSE;
         }
+
+#ifdef DO_ZERO_COPY
+        len -= zero_copy(encoder, bitmap_opaque, buffer, &it, len);
+
         /* len now contains the remaining number of bytes to copy.
          * However we must avoid any write to the GstBuffer object as it
          * would cause a copy of the read-only memory objects we just added.
@@ -1401,8 +1325,8 @@ static int push_raw_frame(SpiceGstEncoder *encoder,
             if (!dst) {
                 return VIDEO_ENCODER_FRAME_UNSUPPORTED;
             }
-            if (!chunk_copy(encoder, bitmap, chunk_index, chunk_offset,
-                            len, dst)) {
+            len -= red_chunk_iterator_get_data(&it, dst, len);
+            if (len != 0) {
                 unmap_and_release_memory(&map, buffer);
                 return VIDEO_ENCODER_FRAME_UNSUPPORTED;
             }
-- 
2.7.4



More information about the Spice-devel mailing list