[PATCH 0/1] MR2883: evr: Respect RGB format stride in GetCurrentImage.

[Rémi Bernon (＠rbernon)]

This fixes some upside-down videos in Secret of Mana. The game plays a NV12 video using MF session and EVR, then calls GetCurrentImage to get the frame RGB data and display it itself.

[PATCH 1/1] evr: Respect RGB format stride in GetCurrentImage.

[Rémi Bernon]

From: Rémi Bernon

---
 dlls/evr/presenter.c | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/dlls/evr/presenter.c b/dlls/evr/presenter.c
index d8bdbee3915..313c4fc6309 100644
--- a/dlls/evr/presenter.c
+++ b/dlls/evr/presenter.c
@@ -1531,7 +1531,8 @@ static HRESULT WINAPI video_presenter_control_GetCurrentImage(IMFVideoDisplayCon
         {
             if (SUCCEEDED(hr = IDirect3DSurface9_LockRect(readback, &mapped_rect, NULL, D3DLOCK_READONLY)))
             {
-                memcpy(*dib, mapped_rect.pBits, *dib_size);
+                hr = MFCopyImage(stride < 0 ? *dib + *dib_size - stride : *dib, stride,
+                        mapped_rect.pBits, mapped_rect.Pitch, surface_desc.Width * 4, surface_desc.Height);
                 IDirect3DSurface9_UnlockRect(readback);
             }
         }

[Nikolay Sivov (＠nsivov)]

Nikolay Sivov (@nsivov) commented about dlls/evr/presenter.c:
>      {
>          if (SUCCEEDED(hr = IDirect3DSurface9_LockRect(readback, &mapped_rect, NULL, D3DLOCK_READONLY)))
>          {
> -
>            memcpy(*dib, mapped_rect.pBits, *dib_size);
> -
>            hr = MFCopyImage(stride < 0 ? *dib + *dib_size - stride : *dib, stride,
> -
>                    mapped_rect.pBits, mapped_rect.Pitch, surface_desc.Width * 4, surface_desc.Height);
>            IDirect3DSurface9_UnlockRect(readback);

Could this use same abs(stride) instead of width * 4?

[Rémi Bernon (＠rbernon)]

On Tue May 23 18:09:04 2023 +0000, Nikolay Sivov wrote:
> Could this use same abs(stride) instead of width * 4?

Sure, but I was thinking that stride isn't necessarily the same as the size to copy on each pixel line.

[Nikolay Sivov (＠nsivov)]

On Tue May 23 18:52:28 2023 +0000, Rémi Bernon wrote:
> Sure, but I was thinking that stride isn't necessarily the same as the
> size to copy on each pixel line.

Right, maybe it's not. What I noticed was an unconditional 4 multiplier, which is ought to depend on format as well. Unless it never does in practice, for d3d9 formats we will ever get. Regarding using MFCopyImage(), it's worth noting that this change, for positive stride, will produce Height of memcpy() calls, comparing to one before it.

[eric pouech (＠epo)]

On Tue May 23 20:53:11 2023 +0000, Nikolay Sivov wrote:
> Right, maybe it's not. What I noticed was an unconditional 4 multiplier,
> which is ought to depend on format as well. Unless it never does in
> practice, for d3d9 formats we will ever get. Regarding using
> MFCopyImage(), it's worth noting that this change, for positive stride,
> will produce Height of memcpy() calls, comparing to one before it.

shouldn't we optimize CopyImage then? (when src/dst stride are equal and positive)