[PATCH 0/2] MR10583: amstream: Don't return MediaType if it hasn't changed.

[Brendan McGrath (＠redmcg)]

Windows will only include a media type on a sample if there was a change in
media type compared to the last time the buffer was retrieved.

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

[PATCH 1/2] amstream/tests: Test that we only include media type on change.

[Brendan McGrath]

From: Brendan McGrath <[e-mail]>

Windows will only include a media type on a sample if there was a change in
media type compared to the last time the buffer was retrieved.
---
 dlls/amstream/tests/amstream.c | 7 +++++++
 1 file changed, 7 insertions(+)

diff --git a/dlls/amstream/tests/amstream.c b/dlls/amstream/tests/amstream.c
index 8da987a6367..a402a32021d 100644
--- a/dlls/amstream/tests/amstream.c
+++ b/dlls/amstream/tests/amstream.c
@@ -9077,6 +9077,13 @@ static void test_ddrawstream_mem_allocator(void)
     hr = IMemAllocator_GetBuffer(mem_allocator, &media_sample1, NULL, NULL, 0);
     ok(hr == S_OK, "Got hr %#lx.\n", hr);
 
+    sample_mt = (AM_MEDIA_TYPE*)0xc0ffee;
+    hr = IMediaSample_GetMediaType(media_sample1, &sample_mt);
+    todo_wine
+    ok(hr == S_FALSE, "Got hr %#lx.\n", hr);
+    todo_wine
+    ok(sample_mt == NULL, "Got sample_mt %p.\n", sample_mt);
+
     start = end = 0xdeadbeef;
     hr = IMediaSample_GetTime(media_sample1, &start, &end);
     ok(hr == S_OK, "Got hr %#lx.\n", hr);
-- 
GitLab


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

[PATCH 2/2] amstream: Don't return MediaType if it hasn't changed.

[Brendan McGrath]

From: Brendan McGrath <[e-mail]>

---
 dlls/amstream/ddrawstream.c    | 9 +++++++++
 dlls/amstream/tests/amstream.c | 2 --
 2 files changed, 9 insertions(+), 2 deletions(-)

diff --git a/dlls/amstream/ddrawstream.c b/dlls/amstream/ddrawstream.c
index 29e17f54542..9f5b1fe2c4f 100644
--- a/dlls/amstream/ddrawstream.c
+++ b/dlls/amstream/ddrawstream.c
@@ -92,6 +92,7 @@ struct ddraw_sample
     struct list entry;
     HRESULT update_hr;
     bool pending;
+    bool needs_mt;
 };
 
 static HRESULT ddrawstreamsample_create(struct ddraw_stream *parent, IDirectDrawSurface *surface,
@@ -2136,6 +2137,7 @@ static ULONG WINAPI media_sample_Release(IMediaSample *iface)
     if (!refcount)
     {
         IDirectDrawSurface_Unlock(sample->surface, NULL);
+        sample->needs_mt = false;
 
         WakeConditionVariable(&sample->update_cv);
 
@@ -2288,6 +2290,12 @@ static HRESULT WINAPI media_sample_GetMediaType(IMediaSample *iface, AM_MEDIA_TY
 
     TRACE("sample %p, ret_mt %p.\n", sample, ret_mt);
 
+    if (!sample->needs_mt)
+    {
+        *ret_mt = NULL;
+        return S_FALSE;
+    }
+
     /* Note that this usually matches the media type we pass to QueryAccept(),
      * but not if there's a sub-rect.
      * That's amstream just breaking the DirectShow rules.
@@ -2415,6 +2423,7 @@ static HRESULT ddrawstreamsample_create(struct ddraw_stream *parent, IDirectDraw
     object->ref = 1;
     object->parent = parent;
     object->mmstream = parent->parent;
+    object->needs_mt = true;
     InitializeConditionVariable(&object->update_cv);
     IAMMediaStream_AddRef(&parent->IAMMediaStream_iface);
     if (object->mmstream)
diff --git a/dlls/amstream/tests/amstream.c b/dlls/amstream/tests/amstream.c
index a402a32021d..5e1afbe8cdf 100644
--- a/dlls/amstream/tests/amstream.c
+++ b/dlls/amstream/tests/amstream.c
@@ -9079,9 +9079,7 @@ static void test_ddrawstream_mem_allocator(void)
 
     sample_mt = (AM_MEDIA_TYPE*)0xc0ffee;
     hr = IMediaSample_GetMediaType(media_sample1, &sample_mt);
-    todo_wine
     ok(hr == S_FALSE, "Got hr %#lx.\n", hr);
-    todo_wine
     ok(sample_mt == NULL, "Got sample_mt %p.\n", sample_mt);
 
     start = end = 0xdeadbeef;
-- 
GitLab

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

[Elizabeth Figura (＠zfigura)]

The phrasing is a bit odd, since if I'm not mistaken, the media type for a given IMediaSample can't change. But that raises a question: is it actually per-sample, or is it about the surface desc on two consecutive samples for the same stream? That is, what if you create two samples with the same surface desc and then call GetMediaType() on both of them?

```
@@ -2288,6 +2290,12 @@ static HRESULT WINAPI media_sample_GetMediaType(IMediaSample *iface, AM_MEDIA_TY
 
     TRACE("sample %p, ret_mt %p.\n", sample, ret_mt);
 
+    if (!sample->needs_mt)
+    {
+        *ret_mt = NULL;
+        return S_FALSE;
+    }
+
     /* Note that this usually matches the media type we pass to QueryAccept(),
      * but not if there's a sub-rect.
      * That's amstream just breaking the DirectShow rules.
```

Are we sure this is correct? What if you call GetMediaType() twice without releasing the sample? What if you don't call GetMediaType() the first time you retrieve the sample but you do call it the second time?

-- 
https://gitlab.winehq.org/wine/wine/-/merge_requests/10583#note_135600