[Spice-devel] [PATCH spice-server] Revert "gstreamer: Avoid memory copy if strides are different"

[Christophe Fergeau]

On Thu, Mar 02, 2017 at 05:00:14PM +0000, Frediano Ziglio wrote:
> This reverts commit c3d237075b994fe67edddd58f2b3164cb579e6f4.

I would add an explanation about what is broken (ie a copy and paste of
what you said in your email about this, link to a gstreamer bug if you
filed one, ..), otherwise in 2 years from now, we won't remember what
the problem was nor how to decide whether to revert this revert.

Christophe

> ---
>  server/gstreamer-encoder.c | 26 ++++++--------------------
>  1 file changed, 6 insertions(+), 20 deletions(-)
> 
> 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 at lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/spice-devel
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 801 bytes
Desc: not available
URL: <https://lists.freedesktop.org/archives/spice-devel/attachments/20170303/011a329d/attachment.sig>