[PATCH v2 0/2] MR2345: Close the bytestream in IMFMediaSource::Shutdown.

[Derek Lesho (＠dlesho)]

--
  v2: winegstreamer/media_source: Close bytestream in ::Shutdown.
      mfplat/tests: Test bytestream closing behavior in IMFMediaSource::Shutdown.

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

[PATCH v2 1/2] mfplat/tests: Test bytestream closing behavior in IMFMediaSource::Shutdown.

[Derek Lesho]

From: Derek Lesho

Signed-off-by: Derek Lesho 
---
 dlls/mfplat/tests/mfplat.c | 21 +++++++++++++++++++++
 1 file changed, 21 insertions(+)

diff --git a/dlls/mfplat/tests/mfplat.c b/dlls/mfplat/tests/mfplat.c
index 33b17b8df41..73cdadef2da 100644
--- a/dlls/mfplat/tests/mfplat.c
+++ b/dlls/mfplat/tests/mfplat.c
@@ -720,9 +720,19 @@ static BOOL get_event(IMFMediaEventGenerator *generator, MediaEventType expected
     return ret;
 }

+static const IMFByteStreamVtbl *bytestream_vtbl_orig;
+
+static int bytestream_closed = 0;
+static HRESULT WINAPI bytestream_wrapper_Close(IMFByteStream *iface)
+{
+    bytestream_closed = 1;
+    return bytestream_vtbl_orig->Close(iface);
+}
+
 static void test_source_resolver(void)
 {
     struct test_callback *callback, *callback2;
+    IMFByteStreamVtbl bytestream_vtbl_wrapper;
     IMFSourceResolver *resolver, *resolver2;
     IMFPresentationDescriptor *descriptor;
     IMFSchemeHandler *scheme_handler;
@@ -826,6 +836,12 @@ static void test_source_resolver(void)
     hr = MFCreateFile(MF_ACCESSMODE_READ, MF_OPENMODE_FAIL_IF_NOT_EXIST, MF_FILEFLAGS_NONE, filename, &stream);
     ok(hr == S_OK, "Unexpected hr %#lx.\n", hr);

+    /* Wrap ::Close to test when the media source calls it */
+    bytestream_vtbl_orig = stream->lpVtbl;
+    bytestream_vtbl_wrapper = *bytestream_vtbl_orig;
+    bytestream_vtbl_wrapper.Close = bytestream_wrapper_Close;
+    stream->lpVtbl = &bytestream_vtbl_wrapper;
+
     hr = IMFByteStream_QueryInterface(stream, &IID_IMFAttributes, (void **)&attributes);
     ok(hr == S_OK, "Unexpected hr %#lx.\n", hr);
     hr = IMFAttributes_SetString(attributes, &MF_BYTESTREAM_CONTENT_TYPE, L"video/mp4");
@@ -1059,9 +1075,14 @@ static void test_source_resolver(void)
     IMFMediaTypeHandler_Release(handler);
     IMFPresentationDescriptor_Release(descriptor);

+    ok(!bytestream_closed, "IMFByteStream::Close called unexpectedly\n");
+
     hr = IMFMediaSource_Shutdown(mediasource);
     ok(hr == S_OK, "Unexpected hr %#lx.\n", hr);

+    todo_wine
+    ok(bytestream_closed, "Missing IMFByteStream::Close call\n");
+
     hr = IMFMediaSource_CreatePresentationDescriptor(mediasource, NULL);
     ok(hr == MF_E_SHUTDOWN, "Unexpected hr %#lx.\n", hr);

[PATCH v2 2/2] winegstreamer/media_source: Close bytestream in ::Shutdown.

[Derek Lesho]

From: Derek Lesho

Signed-off-by: Derek Lesho 
---
 dlls/mfplat/tests/mfplat.c        | 1 -
 dlls/winegstreamer/media_source.c | 1 +
 2 files changed, 1 insertion(+), 1 deletion(-)

diff --git a/dlls/mfplat/tests/mfplat.c b/dlls/mfplat/tests/mfplat.c
index 73cdadef2da..540001676bf 100644
--- a/dlls/mfplat/tests/mfplat.c
+++ b/dlls/mfplat/tests/mfplat.c
@@ -1080,7 +1080,6 @@ static void test_source_resolver(void)
     hr = IMFMediaSource_Shutdown(mediasource);
     ok(hr == S_OK, "Unexpected hr %#lx.\n", hr);

-    todo_wine
     ok(bytestream_closed, "Missing IMFByteStream::Close call\n");

hr = IMFMediaSource_CreatePresentationDescriptor(mediasource, NULL);
diff --git a/dlls/winegstreamer/media_source.c b/dlls/winegstreamer/media_source.c
index 542189b28f5..1b57d8bfba8 100644
--- a/dlls/winegstreamer/media_source.c
+++ b/dlls/winegstreamer/media_source.c
@@ -1344,6 +1344,7 @@ static HRESULT WINAPI media_source_Shutdown(IMFMediaSource *iface)

IMFPresentationDescriptor_Release(source->pres_desc);
     IMFMediaEventQueue_Shutdown(source->event_queue);
+    IMFByteStream_Close(source->byte_stream);
     IMFByteStream_Release(source->byte_stream);

for (i = 0; i < source->stream_count; i++)

[Nikolay Sivov (＠nsivov)]

Looks good. I'm not sure we need to have such test though, a full wrapper would be better. I didn't check what's going on with reported crashes, it didn't crash on my machine.

[Derek Lesho (＠dlesho)]

On Wed Mar  8 09:52:53 2023 +0000, Nikolay Sivov wrote:
> Looks good. I'm not sure we need to have such test though, a full
> wrapper would be better. I didn't check what's going on with reported
> crashes, it didn't crash on my machine.

I think the crashes were a result of the wrong calling convention on 32 bit.  Adding WINAPI for v2 seemed to fix it.  I can extend the wrapper or drop the test, whichever you prefer.

[Nikolay Sivov (＠nsivov)]

On Wed Mar  8 09:52:53 2023 +0000, Derek Lesho wrote:
> I think the crashes were a result of the wrong calling convention on 32
> bit.  Adding WINAPI for v2 seemed to fix it.  I can extend the wrapper
> or drop the test, whichever you prefer.

Ok, let's keep it the way it is for now.

[PATCH v2 0/2] MR2345: Close the bytestream in IMFMediaSource::Shutdown. - approved

[Nikolay Sivov (＠nsivov)]

This merge request was approved by Nikolay Sivov.