[PATCH v2 0/2] MR6069: quartz: Properly copy data to render surfaces of planar formats in VMR9.

[Francisco Casas (＠fcasas)]

Fixes Bug 23029 (Devil May Cry® 3 Special Edition) (6550) Intro Video is covered by green, transparent square for a majority of it.

Before these patches, surface data for planar formats is not copied
correctly when the application uses a custom allocator-presenter and
allocates a surface of different size than the VMR9 source.

Patch 2/2 adds support for performing this copy correctly when the
source dimensions are less or equal than the rendering surface
dimensions.

---

I have some questions:
- Should I also make it work for when the dst is smaller than the src?
- If not, should the FIXME() in 1/2 be promoted to an error? Otherwise we might have segfaults, writing out of scope.
- Should I use copy_plane(), introduced in 2/2 also in the implementation of the other formats? Changing what needs to be changed to preserve behavior of course.

--
  v2: quartz: Properly copy data to render surfaces of planar formats in VMR9.
      quartz: Emit FIXME when the rendering surface is smaller than the source in VMR9.

https://gitlab.winehq.org/wine/wine/-/merge_requests/6069

[PATCH v2 1/2] quartz: Emit FIXME when the rendering surface is smaller than the source in VMR9.

[Francisco Casas]

From: Francisco Casas

---
 dlls/quartz/vmr9.c | 11 +++++++++++
 1 file changed, 11 insertions(+)

diff --git a/dlls/quartz/vmr9.c b/dlls/quartz/vmr9.c
index 9d95cda9def..270277f0196 100644
--- a/dlls/quartz/vmr9.c
+++ b/dlls/quartz/vmr9.c
@@ -189,6 +189,7 @@ static HRESULT vmr_render(struct strmbase_renderer *iface, IMediaSample *sample)
     REFERENCE_TIME start_time, end_time;
     VMR9PresentationInfo info = {};
     D3DLOCKED_RECT locked_rect;
+    D3DSURFACE_DESC dst_desc;
     BYTE *data = NULL;
     HRESULT hr;
     int height;
@@ -240,6 +241,16 @@ static HRESULT vmr_render(struct strmbase_renderer *iface, IMediaSample *sample)
     info.szAspectRatio.cy = height;
     info.lpSurf = filter->surfaces[(++filter->cur_surface) % filter->num_surfaces];

+    if (FAILED(hr = IDirect3DSurface9_GetDesc(info.lpSurf, &dst_desc)))
+    {
+        ERR("Failed to get rendering surface description.\n");
+        return hr;
+    }
+
+    if (width > dst_desc.Width || abs(height) > dst_desc.Height)
+        FIXME("src surface (%ux%u) larger than rendering surface (%ux%u).\n", width, height,
+                dst_desc.Width, dst_desc.Height);
+
     if (FAILED(hr = IDirect3DSurface9_LockRect(info.lpSurf, &locked_rect, NULL, D3DLOCK_DISCARD)))
     {
         ERR("Failed to lock surface, hr %#lx.\n", hr);

[PATCH v2 2/2] quartz: Properly copy data to render surfaces of planar formats in VMR9.

[Francisco Casas]

From: Francisco Casas

Before this patch, surface data for planar formats is not copied
correctly when the application uses a custom allocator-presenter and
allocates a surface of different size than the VMR9 source.

This patch adds support for performing this copy correctly when the
source dimensions are less or equal than the rendering surface
dimensions.
---
 dlls/quartz/vmr9.c | 38 +++++++++++++++++++++++++++++++++++++-
 1 file changed, 37 insertions(+), 1 deletion(-)

diff --git a/dlls/quartz/vmr9.c b/dlls/quartz/vmr9.c
index 270277f0196..a195a684579 100644
--- a/dlls/quartz/vmr9.c
+++ b/dlls/quartz/vmr9.c
@@ -181,6 +181,25 @@ static inline struct quartz_vmr *impl_from_IBaseFilter(IBaseFilter *iface)
     return CONTAINING_RECORD(iface, struct quartz_vmr, renderer.filter.IBaseFilter_iface);
 }

+static void copy_plane(BYTE **dstp, unsigned int dst_pitch, unsigned int dst_height,
+                       const BYTE **srcp, unsigned int src_pitch, unsigned int src_height)
+{
+    const BYTE *src = *srcp;
+    BYTE *dst = *dstp;
+    unsigned int i;
+
+    for (i = 0; i < src_height; ++i)
+    {
+        memcpy(dst, src, src_pitch);
+        dst += dst_pitch;
+        src += src_pitch;
+    }
+    dst += (dst_height - src_height) * dst_height;
+
+    *srcp = src;
+    *dstp = dst;
+}
+
 static HRESULT vmr_render(struct strmbase_renderer *iface, IMediaSample *sample)
 {
     struct quartz_vmr *filter = impl_from_IBaseFilter(&iface->filter.IBaseFilter_iface);
@@ -257,7 +276,24 @@ static HRESULT vmr_render(struct strmbase_renderer *iface, IMediaSample *sample)
         return hr;
     }

-    if (height > 0 && bitmap_header->biCompression == BI_RGB)
+    if (bitmap_header->biCompression == mmioFOURCC('N','V','1','2'))
+    {
+        BYTE *dst = locked_rect.pBits;
+        const BYTE *src = data;
+
+        copy_plane(&dst, locked_rect.Pitch, dst_desc.Height, &src, src_pitch, height);
+        copy_plane(&dst, locked_rect.Pitch, dst_desc.Height/2, &src, src_pitch, height/2);
+    }
+    else if (bitmap_header->biCompression == mmioFOURCC('Y','V','1','2'))
+    {
+        BYTE *dst = locked_rect.pBits;
+        const BYTE *src = data;
+
+        copy_plane(&dst, locked_rect.Pitch, dst_desc.Height, &src, src_pitch, height);
+        copy_plane(&dst, locked_rect.Pitch/2, dst_desc.Height/2, &src, src_pitch/2, height/2);
+        copy_plane(&dst, locked_rect.Pitch/2, dst_desc.Height/2, &src, src_pitch/2, height/2);
+    }
+    else if (height > 0 && bitmap_header->biCompression == BI_RGB)
     {
         BYTE *dst = (BYTE *)locked_rect.pBits + (height * locked_rect.Pitch);
         const BYTE *src = data;

[Elizabeth Figura (＠zfigura)]

> - Should I also make it work for when the dst is smaller than the src?
> - If not, should the FIXME() in 1/2 be promoted to an error? Otherwise we might have segfaults, writing out of scope.

I think leaving it as a FIXME is fine, in the sense of "we don't know what to do with this, and it's a bridge we can cross when we get to" (and I find it rather unlikely that we will, personally).
> - Should I use copy_plane(), introduced in 2/2 also in the implementation of the other formats? Changing what needs to be changed to preserve behavior of course.

I would at least use it for the manual cases, but leaving the memcpy() fast path seems like a good idea.