[PATCH v2 0/1] MR8436: Draft: mfplat: Optimise 1d Lock of 2d buffer when pitch equals width.

[Brendan McGrath (＠redmcg)]

When the pitch and width are the same, the original 2D buffer can be
used. This saves a potentially expensive allocation and memory copy.

--
  v2: mfplat: Optimise 1d Lock of 2d buffer when pitch equals width.

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

[PATCH v2 1/1] mfplat: Optimise 1d Lock of 2d buffer when pitch equals width.

[Brendan McGrath]

From: Brendan McGrath

When the pitch and width are the same, the original 2D buffer can be
used. This saves a potentially expensive allocation and memory copy.
---
 dlls/mfplat/buffer.c | 33 +++++++++++++++++++++++++++++----
 1 file changed, 29 insertions(+), 4 deletions(-)

diff --git a/dlls/mfplat/buffer.c b/dlls/mfplat/buffer.c
index 6d59245b40c..65814f67508 100644
--- a/dlls/mfplat/buffer.c
+++ b/dlls/mfplat/buffer.c
@@ -290,6 +290,9 @@ static HRESULT WINAPI memory_1d_2d_buffer_QueryInterface(IMFMediaBuffer *iface,
     return S_OK;
 }

+static HRESULT memory_2d_buffer_lock(struct buffer *buffer, BYTE **scanline0, LONG *pitch,
+        BYTE **buffer_start, DWORD *buffer_length);
+
 static HRESULT WINAPI memory_1d_2d_buffer_Lock(IMFMediaBuffer *iface, BYTE **data, DWORD *max_length, DWORD *current_length)
 {
     struct buffer *buffer = impl_from_IMFMediaBuffer(iface);
@@ -305,8 +308,18 @@ static HRESULT WINAPI memory_1d_2d_buffer_Lock(IMFMediaBuffer *iface, BYTE **dat

EnterCriticalSection(&buffer->cs);

-    if (!buffer->_2d.linear_buffer && buffer->_2d.locks)
+    if (!buffer->_2d.linear_buffer && buffer->_2d.width == buffer->_2d.pitch)
+    {
+        BYTE *scanline;
+        LONG pitch;
+
+        /* width and pitch are the same, so this avoids a potentially expensive copy */
+        hr = memory_2d_buffer_lock(buffer, &scanline, &pitch, data, NULL);
+    }
+    else if (!buffer->_2d.linear_buffer && buffer->_2d.locks)
+    {
         hr = MF_E_INVALIDREQUEST;
+    }
     else if (!buffer->_2d.linear_buffer)
     {
         if (!(buffer->_2d.linear_buffer = malloc(buffer->_2d.plane_size)))
@@ -323,10 +336,14 @@ static HRESULT WINAPI memory_1d_2d_buffer_Lock(IMFMediaBuffer *iface, BYTE **dat
         }
     }

-    if (SUCCEEDED(hr))
+    if (SUCCEEDED(hr) && buffer->_2d.linear_buffer)
     {
         ++buffer->_2d.locks;
         *data = buffer->_2d.linear_buffer;
+    }
+
+    if (SUCCEEDED(hr))
+    {
         if (max_length)
             *max_length = buffer->_2d.plane_size;
         if (current_length)
@@ -341,12 +358,20 @@ static HRESULT WINAPI memory_1d_2d_buffer_Lock(IMFMediaBuffer *iface, BYTE **dat
 static HRESULT WINAPI memory_1d_2d_buffer_Unlock(IMFMediaBuffer *iface)
 {
     struct buffer *buffer = impl_from_IMFMediaBuffer(iface);
+    HRESULT hr = S_OK;

TRACE("%p.\n", iface);

EnterCriticalSection(&buffer->cs);

-    if (buffer->_2d.linear_buffer && !--buffer->_2d.locks)
+    if (!buffer->_2d.linear_buffer && buffer->_2d.width == buffer->_2d.pitch)
+    {
+        if (buffer->_2d.locks)
+            --buffer->_2d.locks;
+        else
+            hr = HRESULT_FROM_WIN32(ERROR_WAS_UNLOCKED);
+    }
+    else if (buffer->_2d.linear_buffer && !--buffer->_2d.locks)
     {
         int pitch = buffer->_2d.pitch;

@@ -361,7 +386,7 @@ static HRESULT WINAPI memory_1d_2d_buffer_Unlock(IMFMediaBuffer *iface)

LeaveCriticalSection(&buffer->cs);

-    return S_OK;
+    return hr;
 }

static const IMFMediaBufferVtbl memory_1d_2d_buffer_vtbl =

[PATCH v2 0/1] MR8436: mfplat: Optimise 1d Lock of 2d buffer when pitch equals width.

[Brendan McGrath (＠redmcg)]

Tests fixed, marking as ready again.

[PATCH v2 0/1] MR8436: mfplat: Optimise 1d Lock of 2d buffer when pitch equals width.

[Nikolay Sivov (＠nsivov)]

Nikolay Sivov (@nsivov) commented about dlls/mfplat/buffer.c:
>  EnterCriticalSection(&buffer->cs);
>
> - if (!buffer->_2d.linear_buffer && buffer->_2d.locks)
> - if (!buffer->_2d.linear_buffer && buffer->_2d.width == buffer->_2d.pitch)
> - {
> -
>    BYTE *scanline;
> -
>    LONG pitch;
> -
> -
>    /* width and pitch are the same, so this avoids a potentially expensive copy */
> -
>    hr = memory_2d_buffer_lock(buffer, &scanline, &pitch, data, NULL);
> - }
> - else if (!buffer->_2d.linear_buffer && buffer->_2d.locks)
> - {
>     hr = MF_E_INVALIDREQUEST;
> - }

Is it correct to swap those and ignore "!linear_buffer && locks" condition that was a failing path before?

[PATCH v2 0/1] MR8436: mfplat: Optimise 1d Lock of 2d buffer when pitch equals width.

[Brendan McGrath (＠redmcg)]

On Thu Jun 26 09:41:16 2025 +0000, Nikolay Sivov wrote:
> Is it correct to swap those and ignore "!linear_buffer && locks"
> condition that was a failing path before?

I just tested the behavior on Windows, and what we currently have is correct. Apologies, I got tunnel vision on the optimisation and didn't test the Windows behavior before raising the MR. I'll close this, as I don't think it belongs upstream.

[PATCH v2 0/1] MR8436: mfplat: Optimise 1d Lock of 2d buffer when pitch equals width. - closed

[Brendan McGrath (＠redmcg)]

This merge request was closed by Brendan McGrath.