[PATCH v4 0/1] MR5157: evr/dshow: List NV12 as supported, improve warnings

[David Gow (＠sulix)]

Add support for NV12 to ``evr_render()``, as otherwise no video is rendered
at all in games like Age of Empires II DE (see: https://github.com/ValveSoftware/Proton/issues/3189#issuecomment-1962984985)

Interestingly, this doesn't result in NV12 data being passed to ``evr_copy_sample_buffer()`` as I suspected, so the RGB32 copying code seems to work fine.

Nevertheless, add a warning if we get an unknown format in ``evr_copy_sample_buffer()``, as that could potentially lead to nasty memory
issues (e.g., if we get the width/lines/stride wrong).


I confess, I don't really understand what's going on here: the format of the video is definitely ``MFVideoFormat_NV12`` in ``evr_render()``, but I never see any attempt to use NV12 in ``evr_copy_sample_buffer()`` — it's always ``MFVideoFormat_RGB32``. For that reason, I've also not tried to write any tests here — I don't know the MF API well enough to plumb this all the way through. On the bright side, though, the fact that everything mysteriously ends up as RGB32 means I haven't had to write an NV12 codepath for ``evr_copy_sample_buffer()`` — I'm not sure exactly how to handle the multi-plane formats there.

--
  v4: evr/dshow: Support NV12 in evr_render
 
https://gitlab.winehq.org/wine/wine/-/merge_requests/5157

[PATCH v4 1/1] evr/dshow: Support NV12 in evr_render

[David Gow]

From: David Gow <david(a)ingeniumdigital.com>

Add support for NV12 to evr_render(), as otherwise no video is rendered
at all in games like Age of Empires II DE.

In the process, the evr_copy_sample_buffer() function has been
refactored to move the MFCopyImage() call into format-specific code (to
better handle multi-planar formats cleanly).

Signed-off-by: David Gow <david(a)ingeniumdigital.com>
---
 dlls/evr/evr.c | 36 +++++++++++++++++++++++++++---------
 1 file changed, 27 insertions(+), 9 deletions(-)

diff --git a/dlls/evr/evr.c b/dlls/evr/evr.c
index 516427c5fff..52c5de5e2c4 100644
--- a/dlls/evr/evr.c
+++ b/dlls/evr/evr.c
@@ -354,6 +354,7 @@ static HRESULT evr_copy_sample_buffer(struct evr *filter, const GUID *subtype, I
     LONG src_stride;
     HRESULT hr;
     BYTE *src;
+    BYTE *dst_uv;
 
     if (FAILED(hr = IMFMediaType_GetUINT32(filter->media_type, &MF_MT_DEFAULT_STRIDE, (UINT32 *)&src_stride)))
     {
@@ -364,14 +365,6 @@ static HRESULT evr_copy_sample_buffer(struct evr *filter, const GUID *subtype, I
     width = frame_size >> 32;
     lines = frame_size;
 
-    if (IsEqualGUID(subtype, &MFVideoFormat_YUY2))
-    {
-        width = (3 * width + 3) & ~3;
-    }
-    else
-    {
-        width *= 4;
-    }
 
     if (FAILED(hr = IMediaSample_GetPointer(input_sample, &src)))
     {
@@ -392,6 +385,30 @@ static HRESULT evr_copy_sample_buffer(struct evr *filter, const GUID *subtype, I
             if (SUCCEEDED(hr = IDirect3DSurface9_LockRect(surface, &locked_rect, NULL, D3DLOCK_DISCARD)))
             {
                 if (src_stride < 0) src += (-src_stride) * (lines - 1);
+
+                if (IsEqualGUID(subtype, &MFVideoFormat_YUY2))
+                {
+                    width = (3 * width + 3) & ~3;
+                    MFCopyImage(locked_rect.pBits, locked_rect.Pitch, src, src_stride, width, lines);
+                }
+                else if (IsEqualGUID(subtype, &MFVideoFormat_NV12))
+                {
+                    /* Width and height must be rounded up to even. */
+                    width = (width + 1) & ~1;
+                    lines = (lines + 1) & ~1;
+
+                    /* Y plane */
+                    MFCopyImage(locked_rect.pBits, locked_rect.Pitch, src, src_stride, width, lines);
+
+                    /* UV plane */
+                    dst_uv = (BYTE *)locked_rect.pBits + (lines * locked_rect.Pitch);
+                    MFCopyImage(dst_uv, locked_rect.Pitch, src + (lines * src_stride), src_stride, width, lines / 2);
+                }
+                else
+                {
+                    /* All other formats are 32-bit, single plane. */
+                    MFCopyImage(locked_rect.pBits, locked_rect.Pitch, src, src_stride, width * 4, lines);
+                }
                 MFCopyImage(locked_rect.pBits, locked_rect.Pitch, src, src_stride, width, lines);
                 IDirect3DSurface9_UnlockRect(surface);
             }
@@ -427,7 +444,8 @@ static HRESULT evr_render(struct strmbase_renderer *iface, IMediaSample *input_s
 
     if (IsEqualGUID(&subtype, &MFVideoFormat_ARGB32)
             || IsEqualGUID(&subtype, &MFVideoFormat_RGB32)
-            || IsEqualGUID(&subtype, &MFVideoFormat_YUY2))
+            || IsEqualGUID(&subtype, &MFVideoFormat_YUY2)
+            || IsEqualGUID(&subtype, &MFVideoFormat_NV12))
     {
         if (SUCCEEDED(hr = evr_copy_sample_buffer(filter, &subtype, input_sample, &sample)))
         {
-- 
GitLab

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

[David Gow (＠sulix)]

Sent out a new version, which moves the MFCopyImage() call into the format-specific if() statements. 32-bit, single plane formats are again the default 'else' case.

NV12 uses two MFCopyImage() calls.

I think this should be handling the stride correctly, for two reasons:
- We're just getting the stride from MF_MT_DEFAULT_STRIDE, which I'd hope is correct. (Similarly, the destination stride comes from LockRect.)
- The stride should be the same for the Y and UV planes (the fact that each of U and V are only half-width is cancelled out by the fact that there are two of them).

Every description of NV12 I can find says that the UV plane directly follows the Y plane, and is the same width, and half-height, so that seems to match this. There does appear to be a separate MFGetPlaneSize() function we maybe could use instead, but I've not looked into this.

My only real concern is around the width/height rounding: everything I've read agrees that the UV plane's size is taken by dividing the video size by 2 and rounding up, but I've not read anything on whether the Y plane's size is similarly rounded up to even. This code does so, but it's trivial to change it.

(Another option might be to just call something like MFCalculateImageSize(), then memcpy() the whole thing, instead of using MFCopyImage(). I've not tried that, but maybe it'd be a way of simplifying everything.)

-- 
https://gitlab.winehq.org/wine/wine/-/merge_requests/5157#note_63165