[PATCH v3 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.

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

[PATCH v3 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.

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

diff --git a/dlls/evr/evr.c b/dlls/evr/evr.c
index 516427c5fff..1a652cebb22 100644
--- a/dlls/evr/evr.c
+++ b/dlls/evr/evr.c
@@ -368,10 +368,24 @@ static HRESULT evr_copy_sample_buffer(struct evr *filter, const GUID *subtype, I
     {
         width = (3 * width + 3) & ~3;
     }
-    else
+    else if (IsEqualGUID(subtype, &MFVideoFormat_NV12))
+    {
+        /* Width and height must be rounded up to even. */
+        width = (width + 1) & ~1;
+        lines = (lines + 1) & ~1;
+        /* UV plane has same width as Y (half the pixels, twice the components), but half height. */
+        lines = (lines * 3) / 2;
+    }
+    else if (IsEqualGUID(subtype, &MFVideoFormat_ARGB32)
+                || IsEqualGUID(subtype, &MFVideoFormat_RGB32))
     {
         width *= 4;
     }
+    else
+    {
+        ERR("unsupported video format %s\n", debugstr_guid(subtype));
+        return -E_UNEXPECTED;
+    }
 
     if (FAILED(hr = IMediaSample_GetPointer(input_sample, &src)))
     {
@@ -427,7 +441,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

[Zebediah Figura (＠zfigura)]

I don't think this handles stride correctly. I also don't think it should be *that* hard to implement correctly, especially if we just move the format switch down to where we're calling MFCopyImage().

The "else" block is unreachable, so I don't think it should be added.

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