[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 <bmcgrath(a)codeweavers.com>

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 =
-- 
GitLab

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

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

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

[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?

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

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

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

[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.
-- 
https://gitlab.winehq.org/wine/wine/-/merge_requests/8436