[PATCH v5 0/5] MR8505: mfsrcsnk: Implement thinning.
Follow-up to !8184. This also adds a bunch of helpers for writing media source tests (without involving the mf session) in mfsrcsnk, these could be helpful for more tests of this kind in the future. I'm somewhat unsure as to why we haven't done tests for source behavior so far: while those are plugins, applications rely on a bunch of things in the builtin sources that microsoft provides. Since this affects both mfsrcsnk and winegstreamer sources, tests should be run both with `HKCU\\Software\\Wine\\MediaFoundation\\DisableGstByteStreamHandler` enabled and disabled. Due to this I figured it'd make sense to implement everything first and then remove the test todo_wine's and the statements rejecting the thin parameter in a single commit ("Allow thinning"). Perhaps this could be solved more nicely by setting the registry key in the tests themselves? I'm unsure if that is something we do in tests, generally. `MEStreamThinMode` events need to be emitted between the last sample using the outdated thinning parameter and the first sample using the updated thinning parameter. The winegstreamer implementation for this turned out a bit complex, if there is a simpler way to do this please let me know. Regarding winegstreamer, note that buffers need to be intercepted before the decoder because decoders often discard `GST_BUFFER_FLAG_DELTA_UNIT` flags (which is already annoying in itself - it causes all samples to be marked as `MFSampleExtension_CleanPoint`, this should potentially be worked around in the future). But even besides that, intercepting before the decoder is the "proper" implementation, since the point of thinning is increasing decoding speed by skipping delta frames, tho there are some games that rely on the semantics as well. -- v5: mfsrcsnk: Emit MEStreamThinMode event. mfsrcsnk: Move media_source_send_sample. mfsrcsnk: Implement thinning. winedmo: Fall back to dts for sample time if pts is not present. https://gitlab.winehq.org/wine/wine/-/merge_requests/8505
From: Charlotte Pabst <cpabst(a)codeweavers.com> --- dlls/mfsrcsnk/tests/Makefile.in | 3 +- dlls/mfsrcsnk/tests/mfsrcsnk.c | 528 ++++++++++++++++++++++++++ dlls/mfsrcsnk/tests/resource.rc | 31 ++ dlls/mfsrcsnk/tests/test_thinning.avi | Bin 0 -> 7716 bytes 4 files changed, 561 insertions(+), 1 deletion(-) create mode 100644 dlls/mfsrcsnk/tests/resource.rc create mode 100644 dlls/mfsrcsnk/tests/test_thinning.avi diff --git a/dlls/mfsrcsnk/tests/Makefile.in b/dlls/mfsrcsnk/tests/Makefile.in index 55a84256266..89889d6f723 100644 --- a/dlls/mfsrcsnk/tests/Makefile.in +++ b/dlls/mfsrcsnk/tests/Makefile.in @@ -2,4 +2,5 @@ TESTDLL = mfsrcsnk.dll IMPORTS = ole32 mfsrcsnk mfplat mf uuid mfuuid SOURCES = \ - mfsrcsnk.c + mfsrcsnk.c \ + resource.rc diff --git a/dlls/mfsrcsnk/tests/mfsrcsnk.c b/dlls/mfsrcsnk/tests/mfsrcsnk.c index 1aba4094c62..e78fe8adbb8 100644 --- a/dlls/mfsrcsnk/tests/mfsrcsnk.c +++ b/dlls/mfsrcsnk/tests/mfsrcsnk.c @@ -26,9 +26,31 @@ #include "mfapi.h" #include "mfidl.h" #include "mferror.h" +#include "wine/mfinternal.h" #include "wine/test.h" +static const char *debugstr_time(LONGLONG time) +{ + ULONGLONG abstime = time >= 0 ? time : -time; + unsigned int i = 0, j = 0; + char buffer[23], rev[23]; + + while (abstime || i <= 8) + { + buffer[i++] = '0' + (abstime % 10); + abstime /= 10; + if (i == 7) buffer[i++] = '.'; + } + if (time < 0) buffer[i++] = '-'; + + while (i--) rev[j++] = buffer[i]; + while (rev[j-1] == '0' && rev[j-2] != '.') --j; + rev[j] = 0; + + return wine_dbg_sprintf("%s", rev); +} + #define check_interface(a, b, c) check_interface_(__LINE__, a, b, c) static void check_interface_(unsigned int line, void *iface_ptr, REFIID iid, BOOL supported) { @@ -204,6 +226,511 @@ static void test_wave_sink(void) IMFByteStream_Release(bytestream); } +struct source_create_callback +{ + IMFAsyncCallback iface; + LONG refcount; + + IMFByteStreamHandler *handler; + HRESULT hr; + MF_OBJECT_TYPE type; + IUnknown *object; + HANDLE event; +}; + +struct source_create_callback *source_create_callback_from_iface(IMFAsyncCallback *iface) +{ + return CONTAINING_RECORD(iface, struct source_create_callback, iface); +} + +static HRESULT WINAPI source_create_callback_QueryInterface(IMFAsyncCallback *iface, REFIID riid, void **obj) +{ + if (IsEqualIID(riid, &IID_IMFAsyncCallback) || IsEqualIID(riid, &IID_IUnknown)) + { + *obj = iface; + IMFAsyncCallback_AddRef(iface); + return S_OK; + } + + *obj = NULL; + return E_NOINTERFACE; +} + +static ULONG WINAPI source_create_callback_AddRef(IMFAsyncCallback *iface) +{ + struct source_create_callback *callback = source_create_callback_from_iface(iface); + return InterlockedIncrement(&callback->refcount); +} + +static ULONG WINAPI source_create_callback_Release(IMFAsyncCallback *iface) +{ + struct source_create_callback *callback = source_create_callback_from_iface(iface); + ULONG refcount = InterlockedDecrement(&callback->refcount); + if (refcount == 0) + { + if (callback->object) + IUnknown_Release(callback->object); + IMFByteStreamHandler_Release(callback->handler); + CloseHandle(callback->event); + free(callback); + } + return refcount; +} + +static HRESULT WINAPI source_create_callback_GetParameters(IMFAsyncCallback *iface, DWORD *flags, DWORD *queue) +{ + return E_NOTIMPL; +} + +static HRESULT WINAPI source_create_callback_stream_Invoke(IMFAsyncCallback *iface, IMFAsyncResult *result) +{ + struct source_create_callback *callback = source_create_callback_from_iface(iface); + callback->hr = IMFByteStreamHandler_EndCreateObject(callback->handler, result, &callback->type, + &callback->object); + SetEvent(callback->event); + return callback->hr; +} + +static const IMFAsyncCallbackVtbl source_create_callback_vtbl = +{ + &source_create_callback_QueryInterface, + &source_create_callback_AddRef, + &source_create_callback_Release, + &source_create_callback_GetParameters, + &source_create_callback_stream_Invoke, +}; + +static HRESULT create_source(const GUID *guid_handler, IMFByteStream *stream, IMFMediaSource **source) +{ + HRESULT hr; + IMFByteStreamHandler *handler; + struct source_create_callback *callback; + + if (!(callback = calloc(1, sizeof *callback))) + return E_OUTOFMEMORY; + hr = CoCreateInstance(guid_handler, NULL, CLSCTX_INPROC_SERVER, &IID_IMFByteStreamHandler, (void **)&handler); + if (FAILED(hr)) + { + free(callback); + return hr; + } + callback->iface.lpVtbl = &source_create_callback_vtbl; + callback->refcount = 1; + callback->handler = handler; + callback->object = NULL; + callback->type = MF_OBJECT_INVALID; + callback->hr = E_PENDING; + callback->event = CreateEventW(NULL, FALSE, FALSE, NULL); + + hr = IMFByteStreamHandler_BeginCreateObject(callback->handler, stream, NULL, + MF_RESOLUTION_MEDIASOURCE, NULL, NULL, &callback->iface, NULL); + if (FAILED(hr)) + goto done; + + WaitForSingleObject(callback->event, INFINITE); + if (FAILED(hr = callback->hr)) + goto done; + if (callback->type != MF_OBJECT_MEDIASOURCE) + { + hr = E_UNEXPECTED; + goto done; + } + + hr = S_OK; + *source = (IMFMediaSource *)callback->object; + callback->object = NULL; + +done: + IMFAsyncCallback_Release(&callback->iface); + return hr; +} + +static IMFByteStream *create_byte_stream(const BYTE *data, ULONG data_len) +{ + IMFByteStream *stream; + HRESULT hr; + + hr = MFCreateTempFile(MF_ACCESSMODE_READWRITE, MF_OPENMODE_DELETE_IF_EXIST, MF_FILEFLAGS_NONE, &stream); + ok(hr == S_OK, "Unexpected hr %#lx.\n", hr); + hr = IMFByteStream_Write(stream, data, data_len, &data_len); + ok(hr == S_OK, "Unexpected hr %#lx.\n", hr); + hr = IMFByteStream_SetCurrentPosition(stream, 0); + ok(hr == S_OK, "Unexpected hr %#lx.\n", hr); + + return stream; +} + +static IMFByteStream *create_resource_byte_stream(const WCHAR *name) +{ + const BYTE *resource_data; + ULONG resource_len; + HRSRC resource; + + resource = FindResourceW(NULL, name, (const WCHAR *)RT_RCDATA); + ok(resource != 0, "FindResourceW %s failed, error %lu\n", debugstr_w(name), GetLastError()); + resource_data = LockResource(LoadResource(GetModuleHandleW(NULL), resource)); + resource_len = SizeofResource(GetModuleHandleW(NULL), resource); + + return create_byte_stream(resource_data, resource_len); +} + +struct test_callback +{ + IMFAsyncCallback IMFAsyncCallback_iface; + LONG refcount; + + HANDLE event; + IMFMediaEvent *media_event; + BOOL check_media_event; +}; + +static struct test_callback *impl_from_IMFAsyncCallback(IMFAsyncCallback *iface) +{ + return CONTAINING_RECORD(iface, struct test_callback, IMFAsyncCallback_iface); +} + +static HRESULT WINAPI testcallback_QueryInterface(IMFAsyncCallback *iface, REFIID riid, void **obj) +{ + if (IsEqualIID(riid, &IID_IMFAsyncCallback) || + IsEqualIID(riid, &IID_IUnknown)) + { + *obj = iface; + IMFAsyncCallback_AddRef(iface); + return S_OK; + } + + *obj = NULL; + return E_NOINTERFACE; +} + +static ULONG WINAPI testcallback_AddRef(IMFAsyncCallback *iface) +{ + struct test_callback *callback = impl_from_IMFAsyncCallback(iface); + return InterlockedIncrement(&callback->refcount); +} + +static ULONG WINAPI testcallback_Release(IMFAsyncCallback *iface) +{ + struct test_callback *callback = impl_from_IMFAsyncCallback(iface); + ULONG refcount = InterlockedDecrement(&callback->refcount); + + if (!refcount) + { + if (callback->media_event) + IMFMediaEvent_Release(callback->media_event); + CloseHandle(callback->event); + free(callback); + } + + return refcount; +} + +static HRESULT WINAPI testcallback_GetParameters(IMFAsyncCallback *iface, DWORD *flags, DWORD *queue) +{ + ok(flags != NULL && queue != NULL, "Unexpected arguments.\n"); + return E_NOTIMPL; +} + +static HRESULT WINAPI testcallback_Invoke(IMFAsyncCallback *iface, IMFAsyncResult *result) +{ + struct test_callback *callback = CONTAINING_RECORD(iface, struct test_callback, IMFAsyncCallback_iface); + IUnknown *object; + HRESULT hr; + + ok(result != NULL, "Unexpected result object.\n"); + + if (callback->media_event) + IMFMediaEvent_Release(callback->media_event); + + if (callback->check_media_event) + { + hr = IMFAsyncResult_GetObject(result, &object); + ok(hr == E_POINTER, "Unexpected hr %#lx.\n", hr); + + hr = IMFAsyncResult_GetState(result, &object); + ok(hr == S_OK, "Unexpected hr %#lx.\n", hr); + callback->media_event = (void *)0xdeadbeef; + hr = IMFMediaEventGenerator_EndGetEvent((IMFMediaEventGenerator *)object, + result, &callback->media_event); + ok(hr == S_OK, "Unexpected hr %#lx.\n", hr); + IUnknown_Release(object); + } + + SetEvent(callback->event); + + return S_OK; +} + +static const IMFAsyncCallbackVtbl testcallbackvtbl = +{ + testcallback_QueryInterface, + testcallback_AddRef, + testcallback_Release, + testcallback_GetParameters, + testcallback_Invoke, +}; + +static IMFAsyncCallback *create_test_callback(BOOL check_media_event) +{ + struct test_callback *callback; + + if (!(callback = calloc(1, sizeof(*callback)))) + return NULL; + + callback->refcount = 1; + callback->check_media_event = check_media_event; + callback->IMFAsyncCallback_iface.lpVtbl = &testcallbackvtbl; + callback->event = CreateEventW(NULL, FALSE, FALSE, NULL); + ok(!!callback->event, "CreateEventW failed, error %lu\n", GetLastError()); + + return &callback->IMFAsyncCallback_iface; +} + +#define next_media_event(a, b, c, d) next_media_event_(__LINE__, (IMFMediaEventGenerator *)a, b, c, d) +static HRESULT next_media_event_(int line, IMFMediaEventGenerator *source, IMFAsyncCallback *callback, DWORD timeout, + IMFMediaEvent **event) +{ + struct test_callback *impl = impl_from_IMFAsyncCallback(callback); + HRESULT hr; + DWORD ret; + + hr = IMFMediaEventGenerator_BeginGetEvent(source, &impl->IMFAsyncCallback_iface, (IUnknown *)source); + ok_(__FILE__, line)(hr == S_OK || hr == MF_S_MULTIPLE_BEGIN, "Unexpected hr %#lx.\n", hr); + ret = WaitForSingleObject(impl->event, timeout); + *event = impl->media_event; + impl->media_event = NULL; + + return ret; +} + +#define wait_media_event(a, b, c, d, e) wait_media_event_(__LINE__, (IMFMediaEventGenerator *)a, b, c, d, e) +static HRESULT wait_media_event_(int line, IMFMediaEventGenerator *source, IMFAsyncCallback *callback, + MediaEventType expect_type, DWORD timeout, PROPVARIANT *value) +{ + IMFMediaEvent *event = NULL; + MediaEventType type; + HRESULT hr, status; + DWORD ret; + GUID guid; + + do + { + if (event) IMFMediaEvent_Release(event); + ret = next_media_event(source, callback, timeout, &event); + if (ret) return MF_E_NO_EVENTS_AVAILABLE; + hr = IMFMediaEvent_GetType(event, &type); + ok_(__FILE__, line)(hr == S_OK, "Unexpected hr %#lx.\n", hr); + ok_(__FILE__, line)(type == expect_type, "got %#lx.\n", type); + } while (type != expect_type); + + hr = IMFMediaEvent_GetExtendedType(event, &guid); + ok_(__FILE__, line)(hr == S_OK, "Unexpected hr %#lx.\n", hr); + ok_(__FILE__, line)(IsEqualGUID(&guid, &GUID_NULL), "got extended type %s\n", debugstr_guid(&guid)); + + hr = IMFMediaEvent_GetValue(event, value); + ok_(__FILE__, line)(hr == S_OK, "Unexpected hr %#lx.\n", hr); + + hr = IMFMediaEvent_GetStatus(event, &status); + ok_(__FILE__, line)(hr == S_OK, "Unexpected hr %#lx.\n", hr); + + IMFMediaEvent_Release(event); + return status; +} + +static void test_sample_times_at_rate(IMFMediaSource *source, FLOAT rate, BOOL thin) +{ + static LONGLONG expect_times[] = + { + 0, 333666, + 1668333, 333666, + 2002000, 333666, + 2335666, 333666, + 2669333, 333666, + }; + static LONGLONG expect_times_thin[ARRAY_SIZE(expect_times)] = + { + 0, 333666, + 1668333, 333666, + 3336666, 333666, + 5005000, 333666, + 6673333, 333666, + }; + IMFAsyncCallback *callback; + IMFRateControl *rate_control; + IMFPresentationDescriptor *pd; + IMFMediaStream *stream; + IMFMediaEvent *event; + PROPVARIANT value; + LONGLONG time; + HRESULT hr; + DWORD ret; + + winetest_push_context("%f/%u", rate, thin); + + hr = IMFMediaSource_CreatePresentationDescriptor(source, &pd); + ok(hr == S_OK, "Unexpected hr %#lx.\n", hr); + value.vt = VT_EMPTY; + hr = IMFMediaSource_Start(source, pd, &GUID_NULL, &value); + ok(hr == S_OK, "Unexpected hr %#lx.\n", hr); + IMFPresentationDescriptor_Release(pd); + + callback = create_test_callback(TRUE); + if (!winetest_platform_is_wine) + { + hr = wait_media_event(source, callback, thin ? MENewStream : MEUpdatedStream, 100, &value); + ok(hr == S_OK, "Unexpected hr %#lx.\n", hr); + } + else + { + ret = next_media_event(source, callback, 100, &event); + ok(ret == 0, "Unexpected ret %#lx.\n", ret); + hr = IMFMediaEvent_GetType(event, &ret); + ok(hr == S_OK, "Unexpected hr %#lx.\n", hr); + todo_wine_if(!thin) + ok(ret == (thin ? MENewStream : MEUpdatedStream), "Unexpected type %#lx.\n", ret); + hr = IMFMediaEvent_GetValue(event, &value); + ok(hr == S_OK, "Unexpected hr %#lx.\n", hr); + IMFMediaEvent_Release(event); + } + ok(value.vt == VT_UNKNOWN, "got vt %u\n", value.vt); + stream = (IMFMediaStream *)value.punkVal; + IMFMediaStream_AddRef(stream); + PropVariantClear(&value); + + hr = wait_media_event(stream, callback, MEStreamStarted, 100, &value); + ok(hr == S_OK, "Unexpected hr %#lx.\n", hr); + ok(value.vt == VT_I8, "got vt %u\n", value.vt); + hr = wait_media_event(source, callback, MESourceStarted, 100, &value); + ok(hr == S_OK, "Unexpected hr %#lx.\n", hr); + ok(value.vt == VT_I8, "got vt %u\n", value.vt); + + winetest_push_context("sample 0"); + + hr = IMFMediaStream_RequestSample(stream, NULL); + ok(hr == S_OK, "Unexpected hr %#lx.\n", hr); + hr = wait_media_event(stream, callback, MEMediaSample, 100, &value); + ok(hr == S_OK, "Unexpected hr %#lx.\n", hr); + ok(value.vt == VT_UNKNOWN, "got vt %u\n", value.vt); + hr = IMFSample_GetSampleTime((IMFSample *)value.punkVal, &time); + ok(hr == S_OK, "Unexpected hr %#lx.\n", hr); + ok(time == expect_times[0], "Unexpected time %s.\n", debugstr_time(time)); + hr = IMFSample_GetSampleDuration((IMFSample *)value.punkVal, &time); + ok(hr == S_OK, "Unexpected hr %#lx.\n", hr); + ok(time == expect_times[1], "Unexpected time %s.\n", debugstr_time(time)); + hr = IMFSample_GetSampleFlags((IMFSample *)value.punkVal, &ret); + ok(hr == S_OK, "Unexpected hr %#lx.\n", hr); + ok(ret == 0, "Unexpected flags %#lx.\n", ret); + PropVariantClear(&value); + + winetest_pop_context(); + + hr = MFGetService((IUnknown *)source, &MF_RATE_CONTROL_SERVICE, &IID_IMFRateControl, (void **)&rate_control); + ok(hr == S_OK, "Unexpected hr %#lx.\n", hr); + hr = IMFRateControl_SetRate(rate_control, thin, rate); + todo_wine_if(thin && hr == MF_E_THINNING_UNSUPPORTED) + ok(hr == S_OK, "Unexpected hr %#lx.\n", hr); + IMFRateControl_Release(rate_control); + + hr = wait_media_event(source, callback, MESourceRateChanged, 100, &value); + todo_wine_if(thin) + ok(hr == S_OK, "Unexpected hr %#lx.\n", hr); + todo_wine + ok(value.vt == VT_R4, "got vt %u\n", value.vt); + ret = next_media_event(source, callback, 100, &event); + ok(ret == WAIT_TIMEOUT, "Unexpected ret %#lx.\n", ret); + + hr = IMFMediaStream_RequestSample(stream, NULL); + ok(hr == S_OK, "Unexpected hr %#lx.\n", hr); + if (!winetest_platform_is_wine) + { + hr = wait_media_event(stream, callback, MEStreamThinMode, 100, &value); + ok(hr == S_OK, "Unexpected hr %#lx.\n", hr); + ok(value.vt == VT_INT, "got vt %u\n", value.vt); + ok(value.iVal == thin, "Unexpected thin %d\n", value.iVal); + } + + winetest_push_context("sample 1"); + + hr = wait_media_event(stream, callback, MEMediaSample, 100, &value); + ok(hr == S_OK, "Unexpected hr %#lx.\n", hr); + ok(value.vt == VT_UNKNOWN, "got vt %u\n", value.vt); + hr = IMFSample_GetSampleTime((IMFSample *)value.punkVal, &time); + ok(hr == S_OK, "Unexpected hr %#lx.\n", hr); + todo_wine + ok(time == expect_times[2], "Unexpected time %s.\n", debugstr_time(time)); + hr = IMFSample_GetSampleDuration((IMFSample *)value.punkVal, &time); + ok(hr == S_OK, "Unexpected hr %#lx.\n", hr); + ok(time == expect_times[3], "Unexpected time %s.\n", debugstr_time(time)); + hr = IMFSample_GetSampleFlags((IMFSample *)value.punkVal, &ret); + ok(hr == S_OK, "Unexpected hr %#lx.\n", hr); + ok(ret == 0, "Unexpected flags %#lx.\n", ret); + PropVariantClear(&value); + + winetest_pop_context(); + + for (int i = 2; i < ARRAY_SIZE(expect_times) / 2; i++) + { + winetest_push_context("sample %u", i); + + hr = IMFMediaStream_RequestSample(stream, NULL); + ok(hr == S_OK, "Unexpected hr %#lx.\n", hr); + hr = wait_media_event(stream, callback, MEMediaSample, 100, &value); + ok(hr == S_OK, "Unexpected hr %#lx.\n", hr); + ok(value.vt == VT_UNKNOWN, "got vt %u\n", value.vt); + hr = IMFSample_GetSampleTime((IMFSample *)value.punkVal, &time); + ok(hr == S_OK, "Unexpected hr %#lx.\n", hr); + todo_wine + ok(time == (thin ? expect_times_thin[2 * i] : expect_times[2 * i]), "Unexpected time %s.\n", debugstr_time(time)); + hr = IMFSample_GetSampleDuration((IMFSample *)value.punkVal, &time); + ok(hr == S_OK, "Unexpected hr %#lx.\n", hr); + ok(time == (thin ? expect_times_thin[2 * i + 1] : expect_times[2 * i + 1]), "Unexpected time %s.\n", debugstr_time(time)); + hr = IMFSample_GetSampleFlags((IMFSample *)value.punkVal, &ret); + ok(hr == S_OK, "Unexpected hr %#lx.\n", hr); + ok(ret == 0, "Unexpected flags %#lx.\n", ret); + PropVariantClear(&value); + + winetest_pop_context(); + } + + hr = IMFMediaSource_Stop(source); + ok(hr == S_OK, "Unexpected hr %#lx.\n", hr); + hr = wait_media_event(source, callback, MESourceStopped, 100, &value); + ok(hr == S_OK, "Unexpected hr %#lx.\n", hr); + ok(value.vt == VT_EMPTY, "got vt %u\n", value.vt); + hr = wait_media_event(stream, callback, MEStreamStopped, 100, &value); + ok(hr == S_OK, "Unexpected hr %#lx.\n", hr); + ok(value.vt == VT_EMPTY, "got vt %u\n", value.vt); + + IMFMediaStream_Release(stream); + IMFAsyncCallback_Release(callback); + + winetest_pop_context(); +} + +static void test_thinning(void) +{ + IMFMediaSource *source; + IMFByteStream *stream; + HRESULT hr; + + stream = create_resource_byte_stream(L"test_thinning.avi"); + hr = create_source(&CLSID_AVIByteStreamPlugin, stream, &source); + IMFByteStream_Release(stream); + + if (FAILED(hr)) + { + win_skip("Failed to create MPEG4 source: %#lx.\n", hr); + return; + } + + test_sample_times_at_rate(source, 2.0, TRUE); + test_sample_times_at_rate(source, 3.0, FALSE); + + hr = IMFMediaSource_Shutdown(source); + ok(hr == S_OK, "Unexpected hr %#lx.\n", hr); + IMFMediaSource_Release(source); +} + START_TEST(mfsrcsnk) { HRESULT hr; @@ -212,6 +739,7 @@ START_TEST(mfsrcsnk) ok(hr == S_OK, "Unexpected hr %#lx.\n", hr); test_wave_sink(); + test_thinning(); hr = MFShutdown(); ok(hr == S_OK, "Unexpected hr %#lx.\n", hr); diff --git a/dlls/mfsrcsnk/tests/resource.rc b/dlls/mfsrcsnk/tests/resource.rc new file mode 100644 index 00000000000..7b3a6532f0a --- /dev/null +++ b/dlls/mfsrcsnk/tests/resource.rc @@ -0,0 +1,31 @@ +/* + * Resources for mfsrcsnk test suite. + * + * Copyright 2025 Charlotte Pabst for CodeWeavers + * + * This library is free software; you can redistribute it and/or + * modify it under the terms of the GNU Lesser General Public + * License as published by the Free Software Foundation; either + * version 2.1 of the License, or (at your option) any later version. + * + * This library is distributed in the hope that it will be useful, + * but WITHOUT ANY WARRANTY; without even the implied warranty of + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU + * Lesser General Public License for more details. + * + * You should have received a copy of the GNU Lesser General Public + * License along with this library; if not, write to the Free Software + * Foundation, Inc., 51 Franklin St, Fifth Floor, Boston, MA 02110-1301, USA + */ + +#include "windef.h" + +/* Generated with: + gst-launch-1.0 videotestsrc num-buffers=60 pattern=smpte100 ! \ + video/x-raw,format=I420,width=64,height=64,framerate=30000/1001 ! \ + videoflip method=clockwise ! videoconvert ! \ + x264enc key-int-max=5 ! qtmux ! filesink location=tmp.mp4 && \ + ffmpeg -i tmp.mp4 dlls/mfsrcsnk/tests/test_thinning.avi + */ +/* @makedep: test_thinning.avi */ +test_thinning.avi RCDATA test_thinning.avi diff --git a/dlls/mfsrcsnk/tests/test_thinning.avi b/dlls/mfsrcsnk/tests/test_thinning.avi new file mode 100644 index 0000000000000000000000000000000000000000..acc35cce915e86f5fd394f24a397bcf33be034c9 GIT binary patch literal 7716 zcmWIYbaRuDV_<L$^HlKh3=a9h#K4e|Qk0WemYHF}z`zjJ#K16p9xDR~2rw`(*nk)e z3=E7=+JS+A0feDkI0Z6m3KIiEaY<25aY+%_tg_6MVh<xT6SyJ<28Nf+3=9UP5D}2s zsw(a)yTGFe<wl%~PJzyQJ^yFi$cL4pBn7RXV^iWwLfyh8oFC72i(U`81jq$Hs!8Rd(a)P z5CEl3P_oHS$<0a0&B*|z6HwX%i3>3>FnId8`FjStg(a)96h1vtHeWc0)t7;^K=GC`>c zno1ctLB=qqr!a`Dx#^;IAgY;}IjDhw(a)k;|kseyZ!$Z1eU$$0%{lMVv|BOB}g|L^a( zzTErlrpNYaIyV$<D!+RVGG0MfAvwP&Rl(5AM4`yo(7-~$*f7=5GR;y!SHVNi$jn6F zH^9|h*F?cF%vm8hKP5F;L07>!zo0TFHLXO!$iTo@*T}%gSV31Iqokz3N?*Ucyj-s= zGbJ(a)YCoxYizbIWFWQ$%#Np6mUu0no6NoIatv6Vt{Vp3wVt)W6uYMQOFLP}~<PJVK> zt)Z2Hm4QNHUSdvVajLCBg|U(a)Eg`uIbLT;*UMrwsZacNR+s;#*~L2;$6A&87GO0hN6 zGcd3+&@(Vl$jz)sO^FAYXsD2z8ef!{m!4{CXr_>yQIwyX7(a)wPJYp76Cl$w*1S!`>l zU{PU_l3ZeIsE}NkYipp8lA4%Om7kYtYh<WnXsD2uSX>fcP(a)J7v08#-lps*l5KP{~| zwZvA}NTDR7C^a#q*w#oPCqF+sF(WlGB_1Ybs8F1fnVgCcFi^-V0^5<CnVVPwaz|!f zNor9}VsdJVt${*PPH9nMWqfjeZb4#+t${*veqM1&QDSCZYD#=&UP)0RNKIN%Vs2`& zt+7H<d_iSVVs2)Nt&u`fd}2ys0mv6g(a)kyD9#UM*Ei&DY<Do(a)Q!&nQW<HB`tiNX?5+ z&o8hwP=E*)*cvHhr&ea>mDrj>8S%N9dA5cM#mT98smY}!wk8G&V5>plS(I9wVQZjJ zlpGKDtF58ALQ%49a#E2(Zc<56D#%YsnI%Oa-x(R0SSWy4CHXm^=us#H`OMZp&&)uf zumEHam`<#)HM3MGEGRBXEwD9FC`&4f&rPfV>9Yl!01`+lO)Ji<O0_jKGO|#}OU#MS zC`z%-%g;+yNXyL0Nd*OJW<h*WVo7Gct)ZTYfkI-Tt)UetNiZ}pFfgWev(a)l5hfAinZ ze%q4(a)sV&79>%U*0y-`v6df4T!zjmvCNjdZ(<G{P5i8?#)Z1sB^ZSnBx<?e&WW`~P$ z>1dSMJKuAk(a)_TFg<nHM${Vi$b|7KYz{Vd(K{jb$&)~@1IM-leij<%V_C1<xEkDC;d zJp0Yt-T>iU1y5Br2XG1PFDsiCv~~N$NbAxia^YFd-qpvp-{14!JfO>MV{K)-&BB6K z!E-JX56=2`X+(a)S;)r&P}IwTsh1CQM}_*?m%qT?aq*r(?{Xx~vyT%YqH?UhDZ^&z{} z9=wLF>y5YB8Wqof<S2IH!QV2aw~Hpl39p~Yp)x_~^fu;24dUU4zAm+!H~H+6w^xE~ z89=3lFr=(y<TPMlV04_NlH-{FZ>#ouupFqo0hJP*0U)_~P8H4a3?+^rU4k%OAQ{Ga zi6xHtjD}!6XfnxJj`mE)!7_G`YMG>RDYc`Cfvx^cy`TNEzl(a)6v9$&rwv90;}0!v*D z{_lLuRW~?(7e>8{j(a)tL|$L-n&M-1NH;k?o$vxcqtR-=y5`=iQf;s(FhvioaVyg1Qx z|HX-qb&P+r?DWf@|8#yi^OC1iQ)g!n7xSjH>hCAi?5>@t&R;gY!`Si40ae8k#ki-k zf9%#>vC5fX$2j$`k8IY)B<Gbg&OZ!W7xOHx(a)3W*}AJ5Xrs<_FW_kaC1NqL!k`HJtt ziKT}{9Cm-X%NlT*b(a)2}6{^Kj=<^(sIM!jEQIsKT+0goGJbaP5{Z^$|-ge_4@{Glwl zO4e!(Pr$a#PqUJxqIbkBzqRmj_Ol~f(^oS{W;MNfrT9jBQ%ba(y?bQKOOY!+4?!U> z0t<O~eAWNE*6Y>?lEa9vrvD7*F=fE<RnKs~0YeYtyyPrLm<)OG)xse8e>A=b#usMT zJI-^e0L2%k3(a)pB|#Uv!Yz%t~;*J%DCDt}?d3MKIcHjTXa8qHtu_<8^;zep|*!C{Y5 zQjX>?`qW=TBEClR7g6~OGZ(?iLvUS$HGg61fweclddMpeN9!-5;tL$YD6v8~zA*Jr zA-+cQ7g6zrnO6wM7p5Lqe1T;~^VewqYqY<d;|T6AgL4Gd@(@#oaQhIf$6;8FZ}1yy zZ*!Zzg5(a)Y*Reww>@2_i8I~NrnKNZ8UM(a)ZJKvE<Oi!`q(zbLg?%7~S*y-uqQ87G7Ts z)IH6g9qFiI(a)9mY7_3`qk<DF~fs@!vsi6tjt$FjrD$ApZo?~2kizjbQQWXVUK_tbao zca2fv&2ziK6aD0c|Ivckl_a<5o9!9m9YDHJ;*y9M%S@>-JOG;CF-S=Q=`~_zU;vGr zgUn)LU|>jKXJ7!0jluXe><kPbbuj)kb_NE}SQm`H0f`S%cZ!{X0aW6`<OMhx7(jg$ z7+;4I)jT&eegYC7WL^~~s(A;vP|dr;g=*e6G`;{gLLOwE7B{MS1w5$c_3$9ngTiAO z8vg(iA7tJw9#r!b_)yKW;X^eq42(a)ra#0Qzz#fNI%1%6cX-teQE$0mTvS3u%}%(D_e zHLpPs)x0@^sQR~|@h>3pLFT;@L^aPq7}Y!<VN~<d(D)5Ve2{swgi+0VAcAV%9}!gZ z#6(f~21tC6d0wKZ=1mYoHE)d=s{UhW{0B&Uka(a)qvP|b6YKs7H$0(a)b`SH2wr6KFGXP b5)2HW5)+mmK1rgQ$0vnqo(3A<3Cafm!3?dO literal 0 HcmV?d00001 -- GitLab https://gitlab.winehq.org/wine/wine/-/merge_requests/8505
From: Charlotte Pabst <cpabst(a)codeweavers.com> --- dlls/winedmo/main.c | 1 + 1 file changed, 1 insertion(+) diff --git a/dlls/winedmo/main.c b/dlls/winedmo/main.c index 77411352445..671e632b16d 100644 --- a/dlls/winedmo/main.c +++ b/dlls/winedmo/main.c @@ -146,6 +146,7 @@ static void buffer_unlock( DMO_OUTPUT_DATA_BUFFER *buffer, struct sample *sample { if (sample->dts != INT64_MIN) IMFSample_SetUINT64( object, &MFSampleExtension_DecodeTimestamp, sample->dts ); if (sample->pts != INT64_MIN) IMFSample_SetSampleTime( object, sample->pts ); + else if (sample->dts != INT64_MIN) IMFSample_SetSampleTime( object, sample->dts ); if (sample->duration != INT64_MIN) IMFSample_SetSampleDuration( object, sample->duration ); if (sample->flags & SAMPLE_FLAG_SYNC_POINT) IMFSample_SetUINT32( object, &MFSampleExtension_CleanPoint, 1 ); IMFSample_Release( object ); -- GitLab https://gitlab.winehq.org/wine/wine/-/merge_requests/8505
From: Charlotte Pabst <cpabst(a)codeweavers.com> --- dlls/mfsrcsnk/media_source.c | 31 +++++++++++++++++++++++-------- 1 file changed, 23 insertions(+), 8 deletions(-) diff --git a/dlls/mfsrcsnk/media_source.c b/dlls/mfsrcsnk/media_source.c index 068f64b9f03..2b43890a232 100644 --- a/dlls/mfsrcsnk/media_source.c +++ b/dlls/mfsrcsnk/media_source.c @@ -240,6 +240,8 @@ struct media_source IMFByteStream *stream; WCHAR *url; float rate; + BOOL thin; + BOOL prev_thin; struct winedmo_demuxer winedmo_demuxer; struct winedmo_stream winedmo_stream; @@ -631,14 +633,24 @@ static HRESULT media_source_read(struct media_source *source) IMFSample *sample; UINT i, index; HRESULT hr; + BOOL thin = source->thin; if (source->state != SOURCE_RUNNING) return S_OK; - if (FAILED(hr = demuxer_read_sample(source->winedmo_demuxer, &index, &sample)) && hr != MF_E_END_OF_STREAM) + /* emulate latency */ + if (thin != source->prev_thin) { - WARN("Failed to read stream %u data, hr %#lx\n", index, hr); - return hr; + source->prev_thin = thin; + thin = TRUE; + } + + while (SUCCEEDED(hr = demuxer_read_sample(source->winedmo_demuxer, &index, &sample)) && thin) + { + UINT32 keyframe; + if (SUCCEEDED(IMFSample_GetUINT32( sample, &MFSampleExtension_CleanPoint, &keyframe )) && keyframe) + break; + IMFSample_Release(sample); } if (hr == MF_E_END_OF_STREAM) @@ -647,6 +659,11 @@ static HRESULT media_source_read(struct media_source *source) media_source_send_eos(source, source->streams[i]); return S_OK; } + else if (FAILED(hr)) + { + WARN("Failed to read stream %u data, hr %#lx\n", index, hr); + return hr; + } if ((hr = media_source_send_sample(source, index, sample)) == S_FALSE) queue_media_source_read(source); @@ -1047,14 +1064,13 @@ static HRESULT WINAPI media_source_IMFRateControl_SetRate(IMFRateControl *iface, if (rate < 0.0f) return MF_E_REVERSE_UNSUPPORTED; - if (thin) - return MF_E_THINNING_UNSUPPORTED; if (FAILED(hr = IMFRateSupport_IsRateSupported(&source->IMFRateSupport_iface, thin, rate, NULL))) return hr; EnterCriticalSection(&source->cs); source->rate = rate; + source->thin = thin; LeaveCriticalSection(&source->cs); return IMFMediaEventQueue_QueueEventParamVar(source->queue, MESourceRateChanged, &GUID_NULL, S_OK, NULL); @@ -1066,11 +1082,10 @@ static HRESULT WINAPI media_source_IMFRateControl_GetRate(IMFRateControl *iface, TRACE("source %p, thin %p, rate %p\n", source, thin, rate); - if (thin) - *thin = FALSE; - EnterCriticalSection(&source->cs); *rate = source->rate; + if (thin) + *thin = source->thin; LeaveCriticalSection(&source->cs); return S_OK; -- GitLab https://gitlab.winehq.org/wine/wine/-/merge_requests/8505
From: Charlotte Pabst <cpabst(a)codeweavers.com> --- dlls/mfsrcsnk/media_source.c | 44 ++++++++++++++++++------------------ 1 file changed, 22 insertions(+), 22 deletions(-) diff --git a/dlls/mfsrcsnk/media_source.c b/dlls/mfsrcsnk/media_source.c index 2b43890a232..62ab5b26f15 100644 --- a/dlls/mfsrcsnk/media_source.c +++ b/dlls/mfsrcsnk/media_source.c @@ -291,28 +291,6 @@ static struct media_stream *media_stream_from_index(struct media_source *source, return NULL; } -static HRESULT media_source_send_sample(struct media_source *source, UINT index, IMFSample *sample) -{ - struct media_stream *stream; - IUnknown *token; - HRESULT hr; - - if (!(stream = media_stream_from_index(source, index)) || !stream->active) - return S_FALSE; - - if (SUCCEEDED(hr = object_queue_pop(&stream->tokens, &token))) - { - media_stream_send_sample(stream, sample, token); - if (token) IUnknown_Release(token); - return S_OK; - } - - if (FAILED(hr = object_queue_push(&stream->samples, (IUnknown *)sample))) - return hr; - - return S_FALSE; -} - static void queue_media_event_object(IMFMediaEventQueue *queue, MediaEventType type, IUnknown *object) { HRESULT hr; @@ -336,6 +314,28 @@ static void queue_media_source_read(struct media_source *source) source->pending_reads++; } +static HRESULT media_source_send_sample(struct media_source *source, UINT index, IMFSample *sample) +{ + struct media_stream *stream; + IUnknown *token; + HRESULT hr; + + if (!(stream = media_stream_from_index(source, index)) || !stream->active) + return S_FALSE; + + if (SUCCEEDED(hr = object_queue_pop(&stream->tokens, &token))) + { + media_stream_send_sample(stream, sample, token); + if (token) IUnknown_Release(token); + return S_OK; + } + + if (FAILED(hr = object_queue_push(&stream->samples, (IUnknown *)sample))) + return hr; + + return S_FALSE; +} + static void media_stream_start(struct media_stream *stream, UINT index, const PROPVARIANT *position) { struct media_source *source = media_source_from_IMFMediaSource(stream->source); -- GitLab https://gitlab.winehq.org/wine/wine/-/merge_requests/8505
From: Charlotte Pabst <cpabst(a)codeweavers.com> --- dlls/mfsrcsnk/media_source.c | 16 +++++++++++++--- 1 file changed, 13 insertions(+), 3 deletions(-) diff --git a/dlls/mfsrcsnk/media_source.c b/dlls/mfsrcsnk/media_source.c index 62ab5b26f15..769232f5b40 100644 --- a/dlls/mfsrcsnk/media_source.c +++ b/dlls/mfsrcsnk/media_source.c @@ -220,6 +220,7 @@ struct media_stream BOOL active; BOOL eos; + BOOL thin; }; struct media_source @@ -314,17 +315,26 @@ static void queue_media_source_read(struct media_source *source) source->pending_reads++; } -static HRESULT media_source_send_sample(struct media_source *source, UINT index, IMFSample *sample) +static HRESULT media_source_send_sample(struct media_source *source, UINT index, IMFSample *sample, BOOL update_thin_mode) { struct media_stream *stream; IUnknown *token; HRESULT hr; + PROPVARIANT param; if (!(stream = media_stream_from_index(source, index)) || !stream->active) return S_FALSE; if (SUCCEEDED(hr = object_queue_pop(&stream->tokens, &token))) { + if (update_thin_mode && stream->thin != source->thin) + { + param.vt = VT_INT; + param.iVal = source->thin; + queue_media_event_value(stream->queue, MEStreamThinMode, ¶m); + stream->thin = source->thin; + } + media_stream_send_sample(stream, sample, token); if (token) IUnknown_Release(token); return S_OK; @@ -366,7 +376,7 @@ static void media_stream_start(struct media_stream *stream, UINT index, const PR list_move_head(&samples, &stream->samples); while (object_queue_pop(&samples, (IUnknown **)&sample) != E_PENDING) { - media_source_send_sample(source, index, sample); + media_source_send_sample(source, index, sample, FALSE); IMFSample_Release(sample); } @@ -665,7 +675,7 @@ static HRESULT media_source_read(struct media_source *source) return hr; } - if ((hr = media_source_send_sample(source, index, sample)) == S_FALSE) + if ((hr = media_source_send_sample(source, index, sample, TRUE)) == S_FALSE) queue_media_source_read(source); IMFSample_Release(sample); -- GitLab https://gitlab.winehq.org/wine/wine/-/merge_requests/8505
On Wed Nov 12 17:49:04 2025 +0000, Charlotte Pabst wrote:
Essentially, the windows source seems to buffer one sample ahead (at least that's what it looks like), this means that when thinned is changed to non-thinned the next sample returned is still a keyframe, and after that it starts returning delta frames again. This is tested here: https://gitlab.winehq.org/wine/wine/-/merge_requests/8505/diffs#9c89a7a7f07e... However when non-thinned is changed to thinned, the already buffered sample is discarded until a keyframe is found, hence the line `thin = TRUE` (test here https://gitlab.winehq.org/wine/wine/-/merge_requests/8505/diffs#9c89a7a7f07e...). This doesn't look right though. I think what happens is that there's more asynchronicity involved. Changing the test a bit, it passes too with something like this:
```C IMFMediaStream_RequestSample(stream, NULL); IMFMediaStream_RequestSample(stream, NULL); IMFMediaStream_RequestSample(stream, NULL); IMFRateControl_SetRate(rate_control, thin, rate); IMFMediaStream_RequestSample(stream, NULL); IMFMediaStream_RequestSample(stream, NULL); IMFMediaStream_RequestSample(stream, NULL); ok(S_OK == wait_media_event(stream, callback, MEMediaSample, 100, &value), "\n"); ok(S_OK == wait_media_event(stream, callback, MEMediaSample, 100, &value), "\n"); ok(S_OK == wait_media_event(stream, callback, MEMediaSample, 100, &value), "\n"); ok(S_OK == wait_media_event(source, callback, MESourceRateChanged, 100, &value), "\n"); ok(S_OK == wait_media_event(stream, callback, MEMediaSample, 100, &value), "\n"); ok(S_OK == wait_media_event(stream, callback, MEMediaSample, 100, &value), "\n"); ok(S_OK == wait_media_event(stream, callback, MEMediaSample, 100, &value), "\n"); ok(MF_E_NO_EVENTS_AVAILABLE == wait_media_event(stream, callback, MEStreamThinMode, 1000, &value), "\n"); IMFMediaStream_RequestSample(stream, NULL); ok(S_OK == wait_media_event(stream, callback, MEStreamThinMode, 1000, &value), "\n"); ok(S_OK == wait_media_event(stream, callback, MEMediaSample, 100, &value), "\n"); ``` So I think what happens is that `IMFRateControl_SetRate` queues an asynchronous command to update the source rate and thinning mode, which queues the MESourceRateChanged event when it is executed and updates the source thin / rate. Then, the source thin / rate gets committed to each stream when IMFMediaStream_RequestSample gets called. It could probably be written right away to the stream, but using an asynchronous command too would probably be nicer as it seems to be how everything works and it would make async commands sequence match the event sequence. -- https://gitlab.winehq.org/wine/wine/-/merge_requests/8505#note_121813
On Thu Nov 13 11:14:15 2025 +0000, Rémi Bernon wrote:
This doesn't look right though. I think what happens is that there's more asynchronicity involved. Changing the test a bit, it passes too with something like this: ```c++ IMFMediaStream_RequestSample(stream, NULL); IMFMediaStream_RequestSample(stream, NULL); IMFMediaStream_RequestSample(stream, NULL); IMFRateControl_SetRate(rate_control, thin, rate); IMFMediaStream_RequestSample(stream, NULL); IMFMediaStream_RequestSample(stream, NULL); IMFMediaStream_RequestSample(stream, NULL); ok(S_OK == wait_media_event(stream, callback, MEMediaSample, 100, &value), "\n"); ok(S_OK == wait_media_event(stream, callback, MEMediaSample, 100, &value), "\n"); ok(S_OK == wait_media_event(stream, callback, MEMediaSample, 100, &value), "\n"); ok(S_OK == wait_media_event(source, callback, MESourceRateChanged, 100, &value), "\n"); ok(S_OK == wait_media_event(stream, callback, MEMediaSample, 100, &value), "\n"); ok(S_OK == wait_media_event(stream, callback, MEMediaSample, 100, &value), "\n"); ok(S_OK == wait_media_event(stream, callback, MEMediaSample, 100, &value), "\n"); ok(MF_E_NO_EVENTS_AVAILABLE == wait_media_event(stream, callback, MEStreamThinMode, 1000, &value), "\n"); IMFMediaStream_RequestSample(stream, NULL); ok(S_OK == wait_media_event(stream, callback, MEStreamThinMode, 1000, &value), "\n"); ok(S_OK == wait_media_event(stream, callback, MEMediaSample, 100, &value), "\n"); ``` So I think what happens is that `IMFRateControl_SetRate` queues an asynchronous command to update the source rate and thinning mode, which queues the MESourceRateChanged event when it is executed and updates the source thin / rate. Then, the source thin / rate gets committed to each stream when IMFMediaStream_RequestSample gets called. It could probably be written right away to the stream, but using an asynchronous command too would probably be nicer as it seems to be how everything works and it would make async commands sequence match the event sequence. Also note that calling `IMFRateControl_SetRate` multiple times quickly sends as many `MESourceRateChanged` events but if thinning mode is toggled back and forth there's no `MEStreamThinMode` sent with the last sample request.
-- https://gitlab.winehq.org/wine/wine/-/merge_requests/8505#note_121814
On Thu Nov 13 11:17:25 2025 +0000, Rémi Bernon wrote:
Also note that calling `IMFRateControl_SetRate` multiple times quickly sends as many `MESourceRateChanged` events but if thinning mode is toggled back and forth there's no `MEStreamThinMode` sent with the last sample request. I've implemented this locally now and I understand why it makes sense on its own and solves the problem/test you describe, but it doesn't seem to solve the problem that the discussed code snippet aims to solve.
Your suggested change is concerned with at what point rate/thin are updated and when MEStreamThinMode & MESourceRateChanged are sent, but regardless of when it happens, the first sample after a MEStreamThinMode is still affected by the source having operated in thin=true mode previously, even when the source was changed to thin=false and MESourceRateChanged was successfully received before the sample was requested - and this is asymmetric, a previous thin=false does not affect the next requested sample after a thin=true change. It only really makes sense when you think of it as the source buffering the next sample and subjecting it to thin-discarding both when it is about to be returned, and when looking for the next sample to buffer after a previously buffered sample was just returned. My code snippet just emulates this behavior. Perhaps it would be better to actually implement it in the way I suspect windows does it. -- https://gitlab.winehq.org/wine/wine/-/merge_requests/8505#note_121876
On Thu Nov 13 17:22:54 2025 +0000, Charlotte Pabst wrote:
I've implemented this locally now and I understand why it makes sense on its own and solves the problem/test you describe, but it doesn't seem to solve the problem that the discussed code snippet aims to solve. Your suggested change is concerned with at what point rate/thin are updated and when MEStreamThinMode & MESourceRateChanged are sent, but regardless of when it happens, the first sample after a MEStreamThinMode is still affected by the source having operated in thin=true mode previously, even when the source was changed to thin=false and MESourceRateChanged was successfully received before the sample was requested - and this is asymmetric, a previous thin=false does not affect the next requested sample after a thin=true change. It only really makes sense when you think of it as the source buffering the next sample and subjecting it to thin-discarding both when it is about to be returned, and when looking for the next sample to buffer after a previously buffered sample was just returned. My code snippet just emulates this behavior. Perhaps it would be better to actually implement it in the way I suspect windows does it. Hmm I think the reason is rather that when disabling thinning, it probably needs to skip to the next keyframe before starting outputting samples again, to make sure the decoder can start from a clean point. Also this seems to be a per-stream requirement, and would need to be implemented as such.
Another thing I noted from varying the test snippet above is that MEStreamThinMode is sent only *if* some samples have been skipped. If the next sample to be read was a keyframe already, `IMFMediaStream_RequestSample` would not trigger the thinning mode change and only the next request would. This could also confirm your buffering analysis, where each stream has its next sample read in advance and decides whether thinning mode is to be updated based on its properties. -- https://gitlab.winehq.org/wine/wine/-/merge_requests/8505#note_121991
when disabling thinning, it probably needs to skip to the next keyframe before starting outputting samples again, to make sure the decoder can start from a clean point
But shouldn't it already be at a keyframe due to being in thinned mode before?
Changing the test a bit, it passes too with something like this
This snippet only passes if it is pasted at the beginning of the test. If it is done afterwards, the MESourceRateChanged is received before the samples from the requests that preceded it. I think if that event was sent by an async command this wouldn't really happen because the request commands would get processed first? Or perhaps this is related to async request triggering an async read (i.e. the async requests would get processed before the async rate change but the async reads afterwards)? I can consistently reproduce all your remarks about MEStreamThinMode however. -- https://gitlab.winehq.org/wine/wine/-/merge_requests/8505#note_124033
But shouldn't it already be at a keyframe due to being in thinned mode before?
Not necessarily from the demuxer point of view? I think we read samples from any stream then dispatch the sample to the right stream. If we are disabling thinning mode, we perhaps just read a keyframe for some stream but the next sample read might be for a different stream and might not be a keyframe itself, and might references samples before it that we didn't send to the decoder. -- https://gitlab.winehq.org/wine/wine/-/merge_requests/8505#note_124034
This snippet only passes if it is pasted at the beginning of the test. If it is done afterwards, the MESourceRateChanged is received before the samples from the requests that preceded it.
After some further investigation, it seems like this actually happens because after a previous waiting for timeout on the source (`ret = next_media_event(source, callback, 100, &event); ok(ret == WAIT_TIMEOUT, "Unexpected ret %#lx.\n", ret);`), the BeginGetEvent call remains pending on the source. Once the rate change event fires it then overwrites sample events. This can be fixed by using separate callbacks for stream and source events. Sorry for the confusion. -- https://gitlab.winehq.org/wine/wine/-/merge_requests/8505#note_124039
participants (3)
-
Charlotte Pabst -
Charlotte Pabst (＠CharlottePabst)
-
Rémi Bernon