> > On Fri, Mar 03, 2017 at 11:18:57AM +0000, Frediano Ziglio wrote: > > This reverts commit c3d237075b994fe67edddd58f2b3164cb579e6f4. > > > > When you call gst_buffer_add_video_meta_full GStreamer assumes > > that buffer is contiguous. This results usually in some pixels > > shift in the video. The pixels shift you can see are artifacts > > "pixel shifts" I think (twice) > > > due to how the guest send the frames but basically are bytes > > "sends". I'd end the sentence after "send the frames", and then make a > slightly more detailed sentence for "basically are bytes inside 2 chunks > of data" (I don't understand it). > Maybe this is important to explain as is not clear how a corruption cause pixels shift. Assume you allocate the 2 chunks with 2 malloc: p1 = malloc(size); ... p2 = malloc(size); Usually the 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 are the pixel shifts. Do you have suggestion for a shorter explanation? Frediano > > Acked-by: Christophe Fergeau <cfergeau@xxxxxxxxxx> > > Christophe > > > inside 2 chunks of data. > > > > 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> > > --- > > 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); _______________________________________________ Spice-devel mailing list Spice-devel@xxxxxxxxxxxxxxxxxxxxx https://lists.freedesktop.org/mailman/listinfo/spice-devel