[PATCH xf86-video-vmware] vmwgfx: Fix YV12 video corruption issue

[Matt Turner]

On Sun, Mar 28, 2021 at 10:57 AM Doug Brown <doug at schmorgal.com> wrote:
>
> This fixes an issue I discovered with YV12 video that isn't a multiple
> of 8 pixels in width (after the round up to an even width value). Due to
> extra padding because the pitch doesn't match the width, the wrong data
> gets copied into the buffer.
>
> Example GStreamer command demonstrating the issue:
> gst-launch-1.0 videotestsrc ! \
> video/x-raw,format=YV12,width=782,height=482 ! xvimagesink
>
> Working width values:
> 775, 776, 783, 784
> Broken widths prior to this patch:
> 777-782
>
> Signed-off-by: Doug Brown <doug at schmorgal.com>
> ---
> To test the change to vmwgfx_overlay.c, use a VMware VM with 3D
> acceleration turned off. To test the change to vmwgfx_tex_video.c, use a
> VMware VM with 3D acceleration turned on, or a VirtualBox VM with VMSVGA
> graphics and 3D acceleration turned on.
>
> I'm concerned that the fix in vmwgfx_overlay.c feels kind of dirty. The
> pitches array is getting passed onto the kernel driver, which I believe
> passes it onto the virtual hardware. I feel like it should be the
> responsibility of the virtual hardware to interpret the data correctly
> given that it is aware of the difference between width and pitch. But it
> doesn't seem to be paying attention to it. Is there somewhere else where
> the patch for that should go instead? Perhaps I should change the
> pitches array being sent out so it matches the data I send?
>
>  vmwgfx/vmwgfx_overlay.c   | 34 +++++++++++++++++++++++++++++++++-
>  vmwgfx/vmwgfx_tex_video.c | 19 +++++++++++++++----
>  2 files changed, 48 insertions(+), 5 deletions(-)
>
> diff --git a/vmwgfx/vmwgfx_overlay.c b/vmwgfx/vmwgfx_overlay.c
> index 94d738c..e02c6fa 100644
> --- a/vmwgfx/vmwgfx_overlay.c
> +++ b/vmwgfx/vmwgfx_overlay.c
> @@ -478,7 +478,39 @@ vmw_video_port_play(ScrnInfoPtr pScrn, struct vmwgfx_overlay_port *port,
>                            clipBoxes, pDraw);
>      }
>
> -    memcpy(port->bufs[port->currBuf].data, buf, port->size);
> +    switch (format) {
> +    case FOURCC_YV12: {
> +        unsigned char *ystart, *vstart, *ustart;
> +        unsigned char *yp, *up, *vp;
> +        int yidx, uidx, vidx;
> +        int i, j;
> +        yidx = uidx = vidx = 0;
> +        ystart = port->bufs[port->currBuf].data;
> +        vstart = ystart + w*h;
> +        ustart = vstart + w*h/4;
> +        yp = buf + port->offsets[0];
> +        vp = buf + port->offsets[1];
> +        up = buf + port->offsets[2];
> +        for (i = 0; i < h; ++i) {
> +            for (j = 0; j < w; ++j) {
> +                ystart[yidx++] = yp[j];
> +            }
> +            yp += port->pitches[0];
> +            if (i % 2 == 0) {
> +                for (j = 0; j < w>>1; ++j) {
> +                    vstart[vidx++] = vp[j];
> +                    ustart[uidx++] = up[j];
> +                }
> +                vp += port->pitches[1];
> +                up += port->pitches[2];
> +            }
> +        }
> +        break;
> +    }
> +    default:
> +        memcpy(port->bufs[port->currBuf].data, buf, port->size);
> +        break;
> +    }
>
>      memset(&arg, 0, sizeof(arg));
>
> diff --git a/vmwgfx/vmwgfx_tex_video.c b/vmwgfx/vmwgfx_tex_video.c
> index acc2b56..0c79220 100644
> --- a/vmwgfx/vmwgfx_tex_video.c
> +++ b/vmwgfx/vmwgfx_tex_video.c
> @@ -603,7 +603,7 @@ copy_packed_data(ScrnInfoPtr pScrn,
>                   int top,
>                   unsigned short w, unsigned short h)
>  {
> -    int i;
> +    int i, j;
>     struct xa_surface **yuv = port->yuv[port->current_set];
>     char *ymap, *vmap, *umap;
>     unsigned char _y1, _y2, u, v;
> @@ -634,9 +634,20 @@ copy_packed_data(ScrnInfoPtr pScrn,
>        yp = buf + offsets[0];
>        vp = buf + offsets[1];
>        up = buf + offsets[2];
> -      memcpy(ymap, yp, w*h);
> -      memcpy(vmap, vp, w*h/4);
> -      memcpy(umap, up, w*h/4);
> +      for (i = 0; i < h; ++i) {
> +          for (j = 0; j < w; ++j) {
> +              ymap[yidx++] = yp[j];
> +          }
> +          yp += pitches[0];
> +          if (i % 2 == 0) {
> +              for (j = 0; j < w>>1; ++j) {
> +                  vmap[vidx++] = vp[j];
> +                  umap[uidx++] = up[j];
> +              }
> +              vp += pitches[1];
> +              up += pitches[2];
> +          }
> +      }
>        break;
>     }
>     case FOURCC_UYVY:
> --

FWIW, we finally got them to allow merge requests on gitlab. If you
wouldn't mind and are still interested, please try making a merge
request at https://gitlab.freedesktop.org/xorg/driver/xf86-video-vmware