[PATCH v2 0/2] MR10583: amstream: Don't return MediaType if it hasn't changed.
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. -- v2: amstream: Only provide MediaType on first retrieval. amstream/tests: Test media type is only on the first of each sample. https://gitlab.winehq.org/wine/wine/-/merge_requests/10583
From: Brendan McGrath <bmcgrath@codeweavers.com> The MS documentation for IMediaSample::GetMediaType states that this method: "retrieves the media type, if the media type differs from the previous sample." For the amstream direct draw samples, which don't change, this just means to include it in each sample the first time it is retrieved. --- dlls/amstream/tests/amstream.c | 29 +++++++++++++++++++++++++++++ 1 file changed, 29 insertions(+) diff --git a/dlls/amstream/tests/amstream.c b/dlls/amstream/tests/amstream.c index 8da987a6367..064aa777185 100644 --- a/dlls/amstream/tests/amstream.c +++ b/dlls/amstream/tests/amstream.c @@ -8985,6 +8985,28 @@ static void test_ddrawstream_mem_allocator(void) /* GetBuffer() again blocks, even with AM_GBF_NOWAIT. */ ok(media_sample1 != media_sample2, "Expected different samples.\n"); + ok(media_sample2 != media_sample3, "Expected different samples.\n"); + ok(media_sample1 != media_sample3, "Expected different samples.\n"); + + /* Release sample3 without ever calling GetMediaType */ + ref = IMediaSample_Release(media_sample3); + ok(!ref, "Got refcount %ld.\n", ref); + + hr = IMemAllocator_GetBuffer(mem_allocator, &media_sample3, NULL, NULL, AM_GBF_NOWAIT); + ok(hr == S_OK, "Got hr %#lx.\n", hr); + + /* Confirm that because sample3 has been released, we no longer get the media type */ + sample_mt = (AM_MEDIA_TYPE*)0xc0ffee; + hr = IMediaSample_GetMediaType(media_sample3, &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); + + /* Check that we still get MediaType on a second call to sample1 */ + hr = IMediaSample_GetMediaType(media_sample1, &sample_mt); + ok(hr == S_OK, "Got hr %#lx.\n", hr); + DeleteMediaType(sample_mt); check_interface(media_sample1, &IID_IDirectDrawStreamSample, FALSE); check_interface(media_sample1, &IID_IMediaSample2, FALSE); @@ -9077,6 +9099,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
From: Brendan McGrath <bmcgrath@codeweavers.com> --- dlls/amstream/ddrawstream.c | 9 +++++++++ dlls/amstream/tests/amstream.c | 4 ---- 2 files changed, 9 insertions(+), 4 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 064aa777185..eb9c8f3e745 100644 --- a/dlls/amstream/tests/amstream.c +++ b/dlls/amstream/tests/amstream.c @@ -8998,9 +8998,7 @@ static void test_ddrawstream_mem_allocator(void) /* Confirm that because sample3 has been released, we no longer get the media type */ sample_mt = (AM_MEDIA_TYPE*)0xc0ffee; hr = IMediaSample_GetMediaType(media_sample3, &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); /* Check that we still get MediaType on a second call to sample1 */ @@ -9101,9 +9099,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
The phrasing is a bit odd, since if I'm not mistaken, the media type for a given IMediaSample can't change.
You are right. My initial description was based on the MS documentation, which doesn't quite match up with the amstream direct draw sample implementation. So I've modified the commit messages (and MR title/description) to better articulate that.
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?
This was actually already being tested. Both sample1 and sample2 were fetching the media data successfully on their first retrieval.
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?
I've just added tests for both of these scenarios. It shows you can call `GetMediaType()` as many times as you like on the first retrieval. But whether you call `GetMediaType()` or not, once you release the sample, `GetMediaType()` will no longer return a media type. -- https://gitlab.winehq.org/wine/wine/-/merge_requests/10583#note_135621
v2: - Change commit messages to reflect the specifics of amstream - Add additional tests to show that `GetMediaType()`: - will provide a media type every time on the first retrieval; and - will never provide a media type when called on subsequent retrievals -- https://gitlab.winehq.org/wine/wine/-/merge_requests/10583#note_135622
This merge request was approved by Elizabeth Figura. -- https://gitlab.winehq.org/wine/wine/-/merge_requests/10583
participants (3)
-
Brendan McGrath -
Brendan McGrath (＠redmcg)
-
Elizabeth Figura (＠zfigura)