[PATCH 0/1] MR5978: winegstreamer: Optimise copy to DXGI buffer.

[Brendan McGrath (＠redmcg)]

Using IMF2DBuffer2_Lock2DSize with LockFlags_Write dramatically improves
performance over IMFMediaBuffer_Lock when using a DXGI buffer.

IMFMediaBuffer_Lock does not know that this buffer will not be read and
therefore performs an unnecessary transfer of the texture from GPU to CPU
before it is overwritten.

[PATCH 1/1] winegstreamer: Optimise copy to DXGI buffer.

[Brendan McGrath]

From: Brendan McGrath

Using IMF2DBuffer2_Lock2DSize with LockFlags_Write dramatically improves
performance over IMFMediaBuffer_Lock when using a DXGI buffer.

IMFMediaBuffer_Lock does not know that this buffer will not be read and
therefore performs an unnecessary transfer of the texture from GPU to CPU
before it is overwritten.
---
 dlls/winegstreamer/wg_sample.c | 25 ++++++++++++++++++++++---
 1 file changed, 22 insertions(+), 3 deletions(-)

diff --git a/dlls/winegstreamer/wg_sample.c b/dlls/winegstreamer/wg_sample.c
index 116dbb1f3ec..08acc650b46 100644
--- a/dlls/winegstreamer/wg_sample.c
+++ b/dlls/winegstreamer/wg_sample.c
@@ -52,6 +52,7 @@ struct sample
         {
             IMFSample *sample;
             IMFMediaBuffer *buffer;
+            IMF2DBuffer2 *buffer2d2;
         } mf;
         struct
         {
@@ -79,7 +80,15 @@ static void mf_sample_destroy(struct wg_sample *wg_sample)

TRACE_(mfplat)("wg_sample %p.\n", wg_sample);

-    IMFMediaBuffer_Unlock(sample->u.mf.buffer);
+    if (sample->u.mf.buffer2d2)
+    {
+        IMF2DBuffer2_Unlock2D(sample->u.mf.buffer2d2);
+        IMF2DBuffer2_Release(sample->u.mf.buffer2d2);
+    }
+    else
+    {
+        IMFMediaBuffer_Unlock(sample->u.mf.buffer);
+    }
     IMFMediaBuffer_Release(sample->u.mf.buffer);
     IMFSample_Release(sample->u.mf.sample);
 }
@@ -92,15 +101,25 @@ static const struct wg_sample_ops mf_sample_ops =
 HRESULT wg_sample_create_mf(IMFSample *mf_sample, struct wg_sample **out)
 {
     DWORD current_length, max_length;
+    LONG pitch;
     struct sample *sample;
-    BYTE *buffer;
+    BYTE *buffer, *scanline;
     HRESULT hr;

if (!(sample = calloc(1, sizeof(*sample))))
         return E_OUTOFMEMORY;
     if (FAILED(hr = IMFSample_ConvertToContiguousBuffer(mf_sample, &sample->u.mf.buffer)))
         goto fail;
-    if (FAILED(hr = IMFMediaBuffer_Lock(sample->u.mf.buffer, &buffer, &max_length, &current_length)))
+    if (SUCCEEDED(hr = IMFMediaBuffer_QueryInterface(sample->u.mf.buffer, &IID_IMF2DBuffer2, (void**)&sample->u.mf.buffer2d2)) &&
+            FAILED(hr = IMF2DBuffer2_Lock2DSize(sample->u.mf.buffer2d2, MF2DBuffer_LockFlags_Write, &scanline, &pitch, &buffer, &max_length)))
+    {
+        IMF2DBuffer2_Release(sample->u.mf.buffer2d2);
+        sample->u.mf.buffer2d2 = NULL;
+    }
+
+    if (SUCCEEDED(hr))
+        current_length = max_length;
+    else if (FAILED(hr = IMFMediaBuffer_Lock(sample->u.mf.buffer, &buffer, &max_length, &current_length)))
         goto fail;

IMFSample_AddRef((sample->u.mf.sample = mf_sample));

[PATCH 1/1] winegstreamer: Optimise copy to DXGI buffer.

[Marvin]

Hi,

It looks like your patch introduced the new failures shown below.
Please investigate and fix them before resubmitting your patch.
If they are not new, fixing them anyway would help a lot. Otherwise
please ask for the known failures list to be updated.

The tests also ran into some preexisting test failures. If you know how
to fix them that would be helpful. See the TestBot job for the details:

The full results can be found at:
https://testbot.winehq.org/JobDetails.pl?Key=146773

Your paranoid android.

=== debian11b (64 bit WoW report) ===

Report validation errors:
shell32:shelllink crashed (c0000005)

[Rémi Bernon (＠rbernon)]

Rémi Bernon (@rbernon) commented about dlls/winegstreamer/wg_sample.c:
> HRESULT wg_sample_create_mf(IMFSample *mf_sample, struct wg_sample **out)
>  {
>      DWORD current_length, max_length;
> - LONG pitch;
> struct sample *sample;
> - BYTE *buffer;
> -
>
> BYTE *buffer, *scanline;
> HRESULT hr;
>
> if (!(sample = calloc(1, sizeof(*sample))))
>     return E_OUTOFMEMORY;
> if (FAILED(hr = IMFSample_ConvertToContiguousBuffer(mf_sample, &sample->u.mf.buffer)))
>     goto fail;
> - if (FAILED(hr = IMFMediaBuffer_Lock(sample->u.mf.buffer, &buffer, &max_length, &current_length)))
> - if (SUCCEEDED(hr = IMFMediaBuffer_QueryInterface(sample->u.mf.buffer, &IID_IMF2DBuffer2, (void**)&sample->u.mf.buffer2d2)) &&
> -
>        FAILED(hr = IMF2DBuffer2_Lock2DSize(sample->u.mf.buffer2d2, MF2DBuffer_LockFlags_Write, &scanline, &pitch, &buffer, &max_length)))

I wanted to try doing that, but right now we don't support buffers with pitch != width * bpp. Are we sure that it's not the case?

[Brendan McGrath (＠redmcg)]

On Wed Jul  3 05:28:55 2024 +0000, Rémi Bernon wrote:
> I wanted to try doing that, but right now we don't support buffers with
> pitch != width * bpp. Are we sure that it's not the case?

I'm writing some tests to check. It seems to work with RGBA and padding, but I may have hit an issue with the NV12 format and a pixel width of 1390 (which results in GStreamer adding 2 pixels of padding), but I'm still investigating.

I'll let you know when I get to the bottom of it.

[Rémi Bernon (＠rbernon)]

On Thu Jul  4 11:17:47 2024 +0000, Brendan McGrath wrote:
> I'm writing some tests to check. It seems to work with RGBA and padding,
> but I may have hit an issue with the NV12 format and a pixel width of
> 1390 (which results in GStreamer adding 2 pixels of padding), but I'm
> still investigating.
> I'll let you know when I get to the bottom of it.

Fwiw since 39ad87840a3f383421a89dd93ba69b7bdfd159db the wg_transform now have some way to tell GStreamer about buffer strides. It's currently set globally, as a property of the input/output media types, but maybe it can be tweaked so that we can override it with a per-buffer stride information.