[PATCH 0/10] MR8505: mfsrcsnk, winegstreamer: 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. -- https://gitlab.winehq.org/wine/wine/-/merge_requests/8505
From: Charlotte Pabst <cpabst(a)codeweavers.com> --- dlls/mfsrcsnk/tests/mfsrcsnk.c | 412 +++++++++++++++++++++++++++++++++ 1 file changed, 412 insertions(+) diff --git a/dlls/mfsrcsnk/tests/mfsrcsnk.c b/dlls/mfsrcsnk/tests/mfsrcsnk.c index 1aba4094c62..b7645d113b2 100644 --- a/dlls/mfsrcsnk/tests/mfsrcsnk.c +++ b/dlls/mfsrcsnk/tests/mfsrcsnk.c @@ -204,6 +204,418 @@ 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 event_callback +{ + IMFAsyncCallback iface; + LONG refcount; + + /* to allow synchronously waiting for events with timeout */ + IMFMediaEvent *event; + CRITICAL_SECTION cs; + CONDITION_VARIABLE produced; + CONDITION_VARIABLE consumed; +}; + +static struct event_callback *event_callback_from_iface(IMFAsyncCallback *iface) +{ + return CONTAINING_RECORD(iface, struct event_callback, iface); +} + +static HRESULT WINAPI event_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 event_callback_AddRef(IMFAsyncCallback *iface) +{ + struct event_callback *callback = event_callback_from_iface(iface); + return InterlockedIncrement(&callback->refcount); +} + +static ULONG WINAPI event_callback_Release(IMFAsyncCallback *iface) +{ + struct event_callback *callback = event_callback_from_iface(iface); + ULONG refcount = InterlockedDecrement(&callback->refcount); + if (refcount == 0) + { + DeleteCriticalSection(&callback->cs); + if (callback->event) + IMFMediaEvent_Release(callback->event); + free(callback); + } + return refcount; +} + +static HRESULT WINAPI event_callback_GetParameters(IMFAsyncCallback *iface, DWORD *flags, DWORD *queue) +{ + return E_NOTIMPL; +} + +BOOL event_callback_send_event(struct event_callback *callback, IMFMediaEvent *event) +{ + BOOL status = FALSE; + EnterCriticalSection(&callback->cs); + if (callback->event) + SleepConditionVariableCS(&callback->consumed, &callback->cs, INFINITE); + if (!callback->event) + { + status = TRUE; + callback->event = event; + WakeConditionVariable(&callback->produced); + } + LeaveCriticalSection(&callback->cs); + return status; +} + +IMFMediaEvent *event_callback_recv_event(struct event_callback *callback, DWORD timeout) +{ + IMFMediaEvent *event = NULL; + EnterCriticalSection(&callback->cs); + if (!callback->event) + SleepConditionVariableCS(&callback->produced, &callback->cs, timeout); + if (callback->event) + { + event = callback->event; + callback->event = NULL; + WakeConditionVariable(&callback->consumed); + } + LeaveCriticalSection(&callback->cs); + return event; +} + +void event_callback_shutdown(struct event_callback *callback) +{ + WakeAllConditionVariable(&callback->consumed); +} + +static HRESULT WINAPI event_callback_Invoke(IMFAsyncCallback *iface, IMFAsyncResult *result) +{ + struct event_callback *callback = event_callback_from_iface(iface); + IMFMediaEventGenerator *source; + IMFMediaEvent *event; + MediaEventType type; + PROPVARIANT value; + HRESULT hr; + + ok(result != NULL, "Unexpected result object.\n"); + + hr = IMFAsyncResult_GetState(result, (IUnknown **) &source); + ok(hr == S_OK, "Unexpected hr %#lx.\n", hr); + + hr = IMFMediaEventGenerator_EndGetEvent(source, result, &event); + if (hr == MF_E_SHUTDOWN) + { + IMFMediaEventGenerator_Release(source); + return hr; + } + ok(hr == S_OK, "Unexpected hr %#lx.\n", hr); + + if (!event_callback_send_event(callback, event)) + { + IMFMediaEvent_Release(event); + IMFMediaEventGenerator_Release(source); + return MF_E_SHUTDOWN; + } + + hr = IMFMediaEvent_GetType(event, &type); + ok(hr == S_OK, "Unexpected hr %#lx.\n", hr); + + if (type == MENewStream) + { + hr = IMFMediaEvent_GetValue(event, &value); + ok(hr == S_OK, "Unexpected hr %#lx.\n", hr); + ok(value.vt == VT_UNKNOWN, "Unexpected vt %d.\n", value.vt); + ok(!!value.punkVal, "Missing stream.\n"); + + hr = IMFMediaStream_BeginGetEvent((IMFMediaStream *) value.punkVal, iface, value.punkVal); + ok(hr == S_OK || hr == MF_S_MULTIPLE_BEGIN, "Unexpected hr %#lx.\n", hr); + + PropVariantClear(&value); + } + + hr = IMFMediaEventGenerator_BeginGetEvent(source, iface, (IUnknown *) source); + if (hr != MF_E_SHUTDOWN) + ok(hr == S_OK, "Unexpected hr %#lx.\n", hr); + + IMFMediaEventGenerator_Release(source); + + return hr; +} + +static const IMFAsyncCallbackVtbl event_callback_vtbl = +{ + event_callback_QueryInterface, + event_callback_AddRef, + event_callback_Release, + event_callback_GetParameters, + event_callback_Invoke, +}; + +static struct event_callback *create_event_callback(void) +{ + struct event_callback *callback; + + if (!(callback = calloc(1, sizeof(*callback)))) + return NULL; + + callback->refcount = 1; + callback->iface.lpVtbl = &event_callback_vtbl; + InitializeCriticalSection(&callback->cs); + InitializeConditionVariable(&callback->produced); + InitializeConditionVariable(&callback->consumed); + + return callback; +} + +/* assumes a single stream for now, this could be expanded upon later if needed */ +#define wait_for_source_start(a, b) wait_for_source_start_(__LINE__, a, b) +static void wait_for_source_start_(int line, struct event_callback *callback, + IMFMediaStream **stream) +{ + BOOL got_stream = FALSE; + BOOL source_started = FALSE; + BOOL stream_started = FALSE; + IMFMediaEvent *event; + MediaEventType type; + PROPVARIANT value; + HRESULT hr; + + while (!(source_started && got_stream && stream_started) + && (event = event_callback_recv_event(callback, 1000))) + { + hr = IMFMediaEvent_GetType(event, &type); + ok_(__FILE__, line)(hr == S_OK, "Unexpected hr %#lx.\n", hr); + + switch (type) + { + case MESourceStarted: + case MESourceSeeked: + ok_(__FILE__, line)(!source_started, "Unexpected source start.\n"); + source_started = TRUE; + break; + + case MEStreamStarted: + case MEStreamSeeked: + ok_(__FILE__, line)(got_stream && !stream_started, "Unexpected stream start.\n"); + stream_started = TRUE; + break; + + case MENewStream: + case MEUpdatedStream: + ok_(__FILE__, line)(!got_stream, "Unexpected stream.\n"); + got_stream = TRUE; + + if (stream) + { + hr = IMFMediaEvent_GetValue(event, &value); + ok_(__FILE__, line)(hr == S_OK, "Unexpected hr %#lx.\n", hr); + ok_(__FILE__, line)(value.vt == VT_UNKNOWN, "Unexpected vt %d.\n", value.vt); + ok_(__FILE__, line)(!!value.punkVal, "Missing stream.\n"); + + *stream = (IMFMediaStream *) value.punkVal; + IMFMediaStream_AddRef(*stream); + PropVariantClear(&value); + } + + break; + } + + IMFMediaEvent_Release(event); + } + + ok_(__FILE__, line)(source_started, "Expected source start.\n"); + ok_(__FILE__, line)(got_stream, "Expected stream.\n"); + ok_(__FILE__, line)(stream_started, "Expected stream start.\n"); +} + +#define wait_for_source_stop(a) wait_for_source_stop_(__LINE__, a) +static void wait_for_source_stop_(int line, struct event_callback *callback) +{ + BOOL source_stopped = FALSE; + BOOL stream_stopped = FALSE; + IMFMediaEvent *event; + MediaEventType type; + HRESULT hr; + + while (!(source_stopped && stream_stopped) + && (event = event_callback_recv_event(callback, 1000))) + { + hr = IMFMediaEvent_GetType(event, &type); + ok_(__FILE__, line)(hr == S_OK, "Unexpected hr %#lx.\n", hr); + + switch (type) + { + case MESourceStopped: + ok_(__FILE__, line)(!source_stopped, "Unexpected MESourceStopped.\n"); + source_stopped = TRUE; + break; + + case MEStreamStopped: + ok_(__FILE__, line)(!stream_stopped, "Unexpected MEStreamStopped.\n"); + stream_stopped = TRUE; + break; + } + + IMFMediaEvent_Release(event); + } + + ok_(__FILE__, line)(source_stopped, "Expected MESourceStopped.\n"); + ok_(__FILE__, line)(stream_stopped, "Expected MEStreamStopped.\n"); +} + START_TEST(mfsrcsnk) { HRESULT hr; -- GitLab 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 | 176 ++++++++++++++++++++++++++ dlls/mfsrcsnk/tests/resource.rc | 30 +++++ dlls/mfsrcsnk/tests/test_thinning.mp4 | Bin 0 -> 7199 bytes 4 files changed, 208 insertions(+), 1 deletion(-) create mode 100644 dlls/mfsrcsnk/tests/resource.rc create mode 100644 dlls/mfsrcsnk/tests/test_thinning.mp4 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 b7645d113b2..3309659df1e 100644 --- a/dlls/mfsrcsnk/tests/mfsrcsnk.c +++ b/dlls/mfsrcsnk/tests/mfsrcsnk.c @@ -616,6 +616,181 @@ static void wait_for_source_stop_(int line, struct event_callback *callback) ok_(__FILE__, line)(stream_stopped, "Expected MEStreamStopped.\n"); } +static int compare_LONGLONG(const void *va, const void *vb) +{ + LONGLONG a = *(const LONGLONG *) va, b = *(const LONGLONG *) vb; + return a < b ? -1 : a == b ? 0 : 1; +} + +static void test_sample_times_at_rate(struct event_callback *callback, IMFMediaSource *source, + IMFRateControl *rate_control, IMFPresentationDescriptor *pres_desc, FLOAT rate, BOOL thin) +{ + const UINT keyframe_interval = 5; + const UINT total_samples = 60; + const float sample_duration = 333666.666; + + IMFMediaStream *stream = NULL; + IMFMediaEvent *event; + MediaEventType type; + LONGLONG *sample_times = calloc(total_samples, sizeof(LONGLONG)); + BOOL got_rate_change = FALSE, got_thin_mode = FALSE, got_stream_end = FALSE; + UINT got_samples = 0, first_valid_sample = 0; + PROPVARIANT value; + HRESULT hr, status; + + value.vt = VT_EMPTY; + hr = IMFMediaSource_Start(source, pres_desc, &GUID_NULL, &value); + ok(hr == S_OK, "Unexpected hr %#lx.\n", hr); + wait_for_source_start(callback, &stream); + + hr = IMFMediaStream_RequestSample(stream, NULL); + 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); + + while (!got_stream_end && (event = event_callback_recv_event(callback, 1000))) + { + hr = IMFMediaEvent_GetType(event, &type); + ok(hr == S_OK, "Unexpected hr %#lx.\n", hr); + + hr = IMFMediaEvent_GetValue(event, &value); + ok(hr == S_OK, "Unexpected hr %#lx.\n", hr); + + hr = IMFMediaEvent_GetStatus(event, &status); + ok(hr == S_OK, "Unexpected hr %#lx.\n", hr); + + switch (type) + { + case MESourceRateChanged: + ok(status == S_OK, "Unexpected status %#lx.\n", status); + ok(!got_rate_change, "Unexpected MESourceRateChanged.\n"); + todo_wine + ok(value.vt == VT_R4, "Unexpected vt %d\n", value.vt); + todo_wine + ok(value.fltVal == rate, "Unexpected rate %f\n", value.fltVal); + + got_rate_change = TRUE; + break; + + case MEStreamThinMode: + /* According to MS docs, this should be VT_BOOL, but windows gives VT_INT. */ + if (value.vt == VT_INT) + { + value.vt = VT_BOOL; + value.boolVal = value.iVal ? VARIANT_TRUE : VARIANT_FALSE; + } + + ok(status == S_OK, "Unexpected status %#lx.\n", status); + ok(!got_thin_mode, "Unexpected MEStreamThinMode.\n"); + ok(value.vt == VT_BOOL, "Unexpected vt %d\n", value.vt); + ok(!!value.boolVal == !!thin, "Unexpected thin %d\n", !!value.boolVal); + + got_thin_mode = TRUE; + first_valid_sample = got_samples; + break; + + case MEMediaSample: + ok(status == S_OK, "Unexpected status %#lx.\n", status); + ok(got_samples < total_samples, "Unexpected MEMediaSample.\n"); + ok(value.vt == VT_UNKNOWN, "Unexpected vt %d\n", value.vt); + ok(!!value.punkVal, "Missing sample.\n"); + + hr = IMFSample_GetSampleTime((IMFSample *) value.punkVal, + &sample_times[got_samples++]); + ok(hr == S_OK, "Unexpected hr %#lx.\n", hr); + + hr = IMFMediaStream_RequestSample(stream, NULL); + ok(hr == S_OK, "Unexpected hr %#lx.\n", hr); + break; + + case MEEndOfStream: + ok(status == S_OK, "Unexpected status %#lx.\n", status); + got_stream_end = TRUE; + break; + } + + PropVariantClear(&value); + IMFMediaEvent_Release(event); + } + + hr = IMFMediaSource_Stop(source); + ok(hr == S_OK, "Unexpected hr %#lx.\n", hr); + wait_for_source_stop(callback); + + IMFMediaStream_Release(stream); + + ok(got_stream_end, "Expected MEEndOfStream.\n"); + todo_wine_if(thin) + ok(got_rate_change, "Expected MESourceRateChanged.\n"); + todo_wine + ok(got_thin_mode, "Expected MEStreamThinMode.\n"); + ok(got_samples >= total_samples/keyframe_interval, + "Expected at least %d samples, got %d.\n", total_samples/keyframe_interval, got_samples); + + qsort(sample_times+first_valid_sample, got_samples-first_valid_sample, + sizeof(LONGLONG), &compare_LONGLONG); + for (UINT i = first_valid_sample+1; i < got_samples; ++i) + { + LONGLONG expect_interval = sample_duration * (thin ? keyframe_interval : 1); + LONGLONG interval = sample_times[i] - sample_times[i-1]; + + LONGLONG tolerance = 10; + LONGLONG diff = expect_interval - interval; + + todo_wine_if(thin) + ok((diff < 0 ? -diff : diff) <= tolerance, + "Expected interval %lld, got %lld.\n", expect_interval, interval); + } + + free(sample_times); +} + +static void test_thinning(void) +{ + static const GUID CLSID_MPEG4ByteStreamHandler = + {0x271c3902,0x6095,0x4c45,{0xa2,0x2f,0x20,0x09,0x18,0x16,0xee,0x9e}}; + IMFMediaSource *source = NULL; + IMFRateControl *rate_control; + IMFPresentationDescriptor *pres_desc; + struct event_callback *callback; + HRESULT hr; + + hr = create_source(&CLSID_MPEG4ByteStreamHandler, + create_resource_byte_stream(L"test_thinning.mp4"), &source); + if (FAILED(hr)) + { + win_skip("Failed to create MPEG4 source: %#lx.\n", hr); + return; + } + + callback = create_event_callback(); + + hr = IMFMediaSource_BeginGetEvent(source, &callback->iface, (IUnknown *) source); + ok(hr == S_OK, "Unexpected hr %#lx.\n", hr); + + hr = MFGetService((IUnknown *)source, &MF_RATE_CONTROL_SERVICE, + &IID_IMFRateControl, (void **)&rate_control); + ok(hr == S_OK, "Unexpected hr %#lx.\n", hr); + + hr = IMFMediaSource_CreatePresentationDescriptor(source, &pres_desc); + ok(hr == S_OK, "Unexpected hr %#lx.\n", hr); + + test_sample_times_at_rate(callback, source, rate_control, pres_desc, 2.0, TRUE); + test_sample_times_at_rate(callback, source, rate_control, pres_desc, 3.0, FALSE); + + hr = IMFMediaSource_Shutdown(source); + ok(hr == S_OK, "Unexpected hr %#lx.\n", hr); + + event_callback_shutdown(callback); + + IMFRateControl_Release(rate_control); + IMFPresentationDescriptor_Release(pres_desc); + IMFMediaSource_Release(source); + IMFAsyncCallback_Release(&callback->iface); +} + START_TEST(mfsrcsnk) { HRESULT hr; @@ -624,6 +799,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..fd391b19f88 --- /dev/null +++ b/dlls/mfsrcsnk/tests/resource.rc @@ -0,0 +1,30 @@ +/* + * 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 ! mp4mux ! filesink location=dlls/mfsrcsnk/tests/test_thinning.mp4 + */ +/* @makedep: test_thinning.mp4 */ +test_thinning.mp4 RCDATA test_thinning.mp4 diff --git a/dlls/mfsrcsnk/tests/test_thinning.mp4 b/dlls/mfsrcsnk/tests/test_thinning.mp4 new file mode 100644 index 0000000000000000000000000000000000000000..7e9f7206b5686a2aa79bbe2808dee42c76016d3e GIT binary patch literal 7199 zcmZQzU{FXasVvAXFfn3aU|;~zxdkSMnZ^0JnZ@}aF^;sN)Kmrr(c0XU#1aMu1}07c z1_lPH^b`h>H8)+<4n#FGGY2&=Fn(!ZC^c~J5;@Jlz`&aE`pqUC1_q|5Y^?wPKfL4m za__U79^0qs+)%iw{O)~)k(r5tu0nEtQL2I=nCYQsWM-o88{q1$Yog#7=B$vMpOTua zpsV1VUr?EonpUD<WME*dYh+*q(wI?FQedU8UtV6WSC*NQnxB)Hr<Y%pt`9O-FQX(k zM?qI1zn~;DKd;zIAvrNAG1=Bop(r)Y)>t7WH7O(a)QIosCI%D~D%Au%s8r?NQJ)}X(a)J z%Amr~&{!ci)ixuwLZP(a)cDL2*DT%n-2($)|}#uug78tNGsSQ+RU7%1dsR-~rHgG(a)A3 z$W4teO3X`7wKX(TNX{tA&rOWaO|>;tC(a)D(K$;m9XHB_*uut-TRu{BgkF3hzxP)JEl zOsUGxOSLsJ)G;(vNJ}g(i7zP5&MW|_fEZ9%5TBoxR-9U5t81iCl2MeJm{M$Oq>z)J zpPiVInwSy~6EjpO&dE$pMF<!u<Q0MKNY2bnECIPAGp{7IC?_#FHO1CIAt|S{D6uj= zIX|}`vBcIuAvr&<xTGjCGcPqIJ~OYRC=sM4ttc@!wb<5JAt}C~vM4b(GsV_OAt^pF zC9weHi=_CZ%*0}lC7DI3V1JdTW~OJ9B-t7&<QJso#i!>N*cvE61Pg496tYt*GxJJp zO`(kV+{`>%Lxtkx)V$Q>(h^$}0|l_vpzth8EzYntP$)`{hx^sm&|IM?*)}<;NFg_= zq$m~Sr=-l1B9QNl3`{H(K&+Dd98mNq6oPzaYoKRlpio!<vIk5jR(a)j<ZDijtJm!uZh znkbYdmBr^KR)F-`f=vJkB$cKWXI7=!8X6f{DC8yP#Ag(x*yiQur7EOl=H#S;0yVQB zz9_LIGvC%w&%{6>vC!7g3X~)m8W<QD{-<`dFi8D>^WV>Y+mi~ZEyWk>zh9rdQBnGO z*yXRkcB_9$IrJgpz`LZ0Iy>)d^?Ms_@$l;9?t{l>hl_FPXq4GI-*cbxdu#gS?&&T4 zEotTdW?3lxEZw&KuhnVRuHsZj5%%1Uwwc8xXSW}Zn-r2f`_0?l0O4H)PgOPta0%@% zE1MRyb^F9f>(V81;aSe!)yKBq-}B!*pv!GzZDqU7!h%-8b1oAP&iZ$0MV46Ai#2CD zBpR{<kKH)<Tlt-$<00YLr{_Ls-%(9mpYtK(a)l}1_hA-mNcyoRmojknnv70-X<D0bq( z-!i4QizdYhub;`GGC}F|Hs(bQ;^BwBF14FC`RtOnSAuQ91%m+t0|S(a)iER`I`{C``u z--E>h7#J8h9p^b!G|MxTIKqWE80RIHIOa1NB8wzvIodNF2a6D2#xU}wb~G`t)xW9t zvtRa?ago8}tJgoaH9ucqsjI>NosYTd2FLHhsCUs(`yT$dUHjmO!P`5WSDIwjur=Rm z)G>O0R5?xD;P+Z~e@%-QCz|fRIPtNL(a)o$!$e);pC&M#+P@^otI?CjxU-jr7T{e+s` zwG-9(%cgf2J6<`Us#u~J_f+<e-MTAQITP#{r~dVk&DxmcymH3*hhghtp2hWjmK5yc zSsGatH(a)WlvuiqvqFOx4{@m)Bv^stD-?k{&)11_^J-l5!oe8t?H;6~G^_bV)?A9Fe2 zapR0`PKoXfSx1GiB}$1ulqFZmTCL#;*tYp;R<cy|j+o`Q7Cz2?c4TY%Y6i)yrdO{N z-)L`2iFUJhk8F7<a>eH%IQT#@!3~ay`hVAY-5SAy(a)Th3|&u|_|2pkpl4Cg`Vn28gt zig8|YmLpt<w8&^-ko-Ry8Mq?@DVRa|0u&iYLa(a)j{i62O0K!iw(jM1EdKW89?JZX^u z(MVckjOGk(a)WIO<uNQ8?iNH9aoq{I?XnFPrOP@&PBL4`VF;3H!+XW-8nNZ9~ZOrc~0 zNX|e~1*^azsz(a)uQM(YgxkpT%)Z~=rTGLTeJ5*ed81Ak;7rFlG&fusr+84#h-oH5$Z z7^a;*NCHPGrjUg2^fMr;NXQuigSQ9HZ>+t|ZTbq9qkL8UF{!-2u1W1&RDAqY48tBF zS+~ZLLlX~gd-~6z$97|M&+~ilSG8DpeKk<`G=Fxaql&$^S5DT)%cG8Wu9>TH&p{@Z zoP-_A4m%$cGP=GiO4Iz-sXdb=A9>zW-?iU0Mu|7i?FLWulNbI+3uadm(YS54XNY$I zs{_Xn{^($<%FWL&V_;y&$t}xB0S(kX@$(h|VFrd$0|tgya~K#IKnNrS!Hf_JPNMNa zR>S2m8B7cetS3u~60;c?7-CAY!FIBP>;qv2xK*#_U}#0N2h~vw3}Aaf1PC)QsHCQp z6f-a|$fV{Jg9evD#xa8Z!@zzCqL<}GZc1h%XrMYb1?&z6kQ+c4=J(JeAZ-k~87VnM zU={-d187_r#s<-0pmAc4#JrT8RFE2$$+?+%X$%YuB4xQ?Ly?`Wk^+&FNhwMNsR8K+ z$@7#J<tRWr&k|f*l9a>1z;M2}q!=6_5OIe6iDk*4F)f%L5RJeN3=Rw)3=Cil5(SwG zqH$x1|Nj{n7>p9jlARe-7)1U*#5eNJ2p(a)Q75J@U2f&?hY#SEeQ7#J9Ylk;<m@{)5Z z7}&s&fq_AwAhEar>U5A}B#KMG(a)d0uWh;PHd!2FVdfx)4;q!<)WAaNe3dNu|I25yiU z3=9mSpeX?c1_pTs1_l)d1_n(A1_nI_1_omW1_ldI{1%rKCxbLY^+L(a)C#V{jFaY=C% zR1QShFfg!4f}#fGWCjKXAqZx)huR|y6&Ge;7?NV3K~PY5A^YP)G9uh<AYsIQ2|^3^ zLTDHt6hxqK1IG&^11O(>^W&&GNH`6Na1kjkNzMnyBslIEKm#F>r70z#axO486-<Mq z6=7vsZbnW~Zb~Mo>;j3yu}UVS;FMWek`FRaBqb578I-{oK=WY?3=FnVgH1^>m;+=m E0NSC6RsaA1 literal 0 HcmV?d00001 -- GitLab https://gitlab.winehq.org/wine/wine/-/merge_requests/8505
From: Charlotte Pabst <cpabst(a)codeweavers.com> --- dlls/winegstreamer/gst_private.h | 2 ++ dlls/winegstreamer/main.c | 12 +++++++ dlls/winegstreamer/unixlib.h | 8 +++++ dlls/winegstreamer/wg_parser.c | 62 ++++++++++++++++++++++++++++++++ 4 files changed, 84 insertions(+) diff --git a/dlls/winegstreamer/gst_private.h b/dlls/winegstreamer/gst_private.h index 60e0c3af8bc..dac5a88bc5c 100644 --- a/dlls/winegstreamer/gst_private.h +++ b/dlls/winegstreamer/gst_private.h @@ -81,6 +81,8 @@ void wg_parser_disconnect(wg_parser_t parser); bool wg_parser_get_next_read_offset(wg_parser_t parser, uint64_t *offset, uint32_t *size); void wg_parser_push_data(wg_parser_t parser, const void *data, uint32_t size); +void wg_parser_set_thin(wg_parser_t parser, BOOL thin); + uint32_t wg_parser_get_stream_count(wg_parser_t parser); wg_parser_stream_t wg_parser_get_stream(wg_parser_t parser, uint32_t index); diff --git a/dlls/winegstreamer/main.c b/dlls/winegstreamer/main.c index d18db8b21e8..94a779bf4fb 100644 --- a/dlls/winegstreamer/main.c +++ b/dlls/winegstreamer/main.c @@ -240,6 +240,18 @@ void wg_parser_push_data(wg_parser_t parser, const void *data, uint32_t size) WINE_UNIX_CALL(unix_wg_parser_push_data, ¶ms); } +void wg_parser_set_thin(wg_parser_t parser, BOOL thin) +{ + struct wg_parser_set_thin_params params = + { + .parser = parser, + .thin = thin, + }; + TRACE("parser %#I64x, thin %d.\n", parser, thin); + + WINE_UNIX_CALL(unix_wg_parser_set_thin, ¶ms); +} + uint32_t wg_parser_get_stream_count(wg_parser_t parser) { struct wg_parser_get_stream_count_params params = diff --git a/dlls/winegstreamer/unixlib.h b/dlls/winegstreamer/unixlib.h index 179f15f78f7..bde18afd77e 100644 --- a/dlls/winegstreamer/unixlib.h +++ b/dlls/winegstreamer/unixlib.h @@ -246,6 +246,12 @@ struct wg_parser_push_data_params UINT32 size; }; +struct wg_parser_set_thin_params +{ + wg_parser_t parser; + BOOL thin; +}; + struct wg_parser_get_stream_count_params { wg_parser_t parser; @@ -430,6 +436,8 @@ enum unix_funcs unix_wg_parser_get_next_read_offset, unix_wg_parser_push_data, + unix_wg_parser_set_thin, + unix_wg_parser_get_stream_count, unix_wg_parser_get_stream, diff --git a/dlls/winegstreamer/wg_parser.c b/dlls/winegstreamer/wg_parser.c index e806298fb57..a7988513107 100644 --- a/dlls/winegstreamer/wg_parser.c +++ b/dlls/winegstreamer/wg_parser.c @@ -80,6 +80,7 @@ struct wg_parser bool output_compressed; bool no_more_pads, has_duration, error; bool err_on, warn_on; + bool thin; pthread_cond_t read_cond, read_done_cond; struct @@ -121,6 +122,11 @@ struct wg_parser_stream gchar *tags[WG_PARSER_TAG_COUNT]; }; +struct decoder_pad_probe_param +{ + struct wg_parser *parser; +}; + static struct wg_parser *get_parser(wg_parser_t parser) { return (struct wg_parser *)(ULONG_PTR)parser; @@ -226,6 +232,18 @@ static NTSTATUS wg_parser_push_data(void *args) return S_OK; } +static NTSTATUS wg_parser_set_thin(void *args) +{ + const struct wg_parser_set_thin_params *params = args; + struct wg_parser *parser = get_parser(params->parser); + + pthread_mutex_lock(&parser->mutex); + parser->thin = params->thin; + pthread_mutex_unlock(&parser->mutex); + + return S_OK; +} + static NTSTATUS wg_parser_stream_get_current_format(void *args) { const struct wg_parser_stream_get_current_format_params *params = args; @@ -600,10 +618,50 @@ static void no_more_pads_cb(GstElement *element, gpointer user) pthread_cond_signal(&parser->init_cond); } +static GstPadProbeReturn decoder_pad_probe(GstPad *pad, GstPadProbeInfo *info, gpointer user) +{ + struct decoder_pad_probe_param *param = user; + GstBuffer *buffer; + bool thin; + + pthread_mutex_lock(¶m->parser->mutex); + thin = param->parser->thin; + pthread_mutex_unlock(¶m->parser->mutex); + + if (thin && (buffer = gst_pad_probe_info_get_buffer(info))) + { + if (GST_BUFFER_FLAG_IS_SET(buffer, GST_BUFFER_FLAG_DELTA_UNIT)) + { + GST_DEBUG("Stream is thinned; discarding delta frame."); + return GST_PAD_PROBE_DROP; + } + else + { + GST_DEBUG("Stream is thinned; found key frame."); + } + } + + return GST_PAD_PROBE_OK; +} + static void deep_element_added_cb(GstBin *self, GstBin *sub_bin, GstElement *element, gpointer user) { + struct wg_parser *parser = user; + if (element) + { + GstElementFactory *fact = GST_ELEMENT_GET_CLASS(element)->elementfactory; + const char *klass = gst_element_factory_get_klass(fact); + GstPad *sink; + if (strstr(klass, GST_ELEMENT_FACTORY_KLASS_DECODER) && (sink = gst_element_get_static_pad(element, "sink"))) + { + struct decoder_pad_probe_param *param = malloc(sizeof *param); + param->parser = parser; + gst_pad_add_probe(sink, GST_PAD_PROBE_TYPE_BUFFER, decoder_pad_probe, param, free); + } + set_max_threads(element); + } } static gboolean sink_event_cb(GstPad *pad, GstObject *parent, GstEvent *event) @@ -1911,6 +1969,8 @@ const unixlib_entry_t __wine_unix_call_funcs[] = X(wg_parser_get_next_read_offset), X(wg_parser_push_data), + X(wg_parser_set_thin), + X(wg_parser_get_stream_count), X(wg_parser_get_stream), @@ -2308,6 +2368,8 @@ const unixlib_entry_t __wine_unix_call_wow64_funcs[] = X(wg_parser_get_next_read_offset), X64(wg_parser_push_data), + X(wg_parser_set_thin), + X(wg_parser_get_stream_count), X(wg_parser_get_stream), -- GitLab https://gitlab.winehq.org/wine/wine/-/merge_requests/8505
From: Charlotte Pabst <cpabst(a)codeweavers.com> --- dlls/winegstreamer/media_source.c | 10 +++++++--- 1 file changed, 7 insertions(+), 3 deletions(-) diff --git a/dlls/winegstreamer/media_source.c b/dlls/winegstreamer/media_source.c index ab842b3e438..4e9372e5d6c 100644 --- a/dlls/winegstreamer/media_source.c +++ b/dlls/winegstreamer/media_source.c @@ -202,6 +202,7 @@ struct media_source SOURCE_SHUTDOWN, } state; float rate; + BOOL thin; HANDLE read_thread; bool read_thread_shutdown; @@ -1291,8 +1292,11 @@ static HRESULT WINAPI media_source_rate_control_SetRate(IMFRateControl *iface, B if (FAILED(hr = IMFRateSupport_IsRateSupported(&source->IMFRateSupport_iface, thin, rate, NULL))) return hr; + wg_parser_set_thin(source->wg_parser, thin); + EnterCriticalSection(&source->cs); source->rate = rate; + source->thin = thin; LeaveCriticalSection(&source->cs); return IMFMediaEventQueue_QueueEventParamVar(source->event_queue, MESourceRateChanged, &GUID_NULL, S_OK, NULL); @@ -1304,11 +1308,10 @@ static HRESULT WINAPI media_source_rate_control_GetRate(IMFRateControl *iface, B TRACE("%p, %p, %p.\n", iface, thin, rate); - if (thin) - *thin = FALSE; - EnterCriticalSection(&source->cs); *rate = source->rate; + if (thin) + *thin = source->thin; LeaveCriticalSection(&source->cs); return S_OK; @@ -1734,6 +1737,7 @@ static HRESULT media_source_create(struct object_context *context, IMFMediaSourc IMFByteStream_AddRef(context->stream); object->file_size = context->file_size; object->rate = 1.0f; + object->thin = FALSE; InitializeCriticalSectionEx(&object->cs, 0, RTL_CRITICAL_SECTION_FLAG_FORCE_DEBUG_INFO); object->cs.DebugInfo->Spare[0] = (DWORD_PTR)(__FILE__ ": cs"); -- GitLab https://gitlab.winehq.org/wine/wine/-/merge_requests/8505
From: Charlotte Pabst <cpabst(a)codeweavers.com> --- dlls/winegstreamer/gst_private.h | 1 + dlls/winegstreamer/main.c | 15 ++++++++ dlls/winegstreamer/media_source.c | 13 ++++++- dlls/winegstreamer/unixlib.h | 7 ++++ dlls/winegstreamer/wg_parser.c | 61 +++++++++++++++++++++++++++++++ 5 files changed, 96 insertions(+), 1 deletion(-) diff --git a/dlls/winegstreamer/gst_private.h b/dlls/winegstreamer/gst_private.h index dac5a88bc5c..01f28956bd8 100644 --- a/dlls/winegstreamer/gst_private.h +++ b/dlls/winegstreamer/gst_private.h @@ -82,6 +82,7 @@ bool wg_parser_get_next_read_offset(wg_parser_t parser, uint64_t *offset, uint32 void wg_parser_push_data(wg_parser_t parser, const void *data, uint32_t size); void wg_parser_set_thin(wg_parser_t parser, BOOL thin); +bool wg_parser_stream_get_thin_event(wg_parser_stream_t stream, BOOL *thin); uint32_t wg_parser_get_stream_count(wg_parser_t parser); wg_parser_stream_t wg_parser_get_stream(wg_parser_t parser, uint32_t index); diff --git a/dlls/winegstreamer/main.c b/dlls/winegstreamer/main.c index 94a779bf4fb..815861d47f9 100644 --- a/dlls/winegstreamer/main.c +++ b/dlls/winegstreamer/main.c @@ -252,6 +252,21 @@ void wg_parser_set_thin(wg_parser_t parser, BOOL thin) WINE_UNIX_CALL(unix_wg_parser_set_thin, ¶ms); } +bool wg_parser_stream_get_thin_event(wg_parser_stream_t stream, BOOL *thin) +{ + struct wg_parser_stream_get_thin_event_params params = + { + .stream = stream, + }; + NTSTATUS status; + TRACE("stream %#I64x, thin %p.\n", stream, thin); + + status = WINE_UNIX_CALL(unix_wg_parser_stream_get_thin_event, ¶ms); + if (status == S_OK) + *thin = params.thin; + return !status; +} + uint32_t wg_parser_get_stream_count(wg_parser_t parser) { struct wg_parser_get_stream_count_params params = diff --git a/dlls/winegstreamer/media_source.c b/dlls/winegstreamer/media_source.c index 4e9372e5d6c..9112b5c040d 100644 --- a/dlls/winegstreamer/media_source.c +++ b/dlls/winegstreamer/media_source.c @@ -796,12 +796,23 @@ static HRESULT wait_on_sample(struct media_stream *stream, IUnknown *token) { struct media_source *source = impl_from_IMFMediaSource(stream->media_source); struct wg_parser_buffer buffer; + PROPVARIANT param; + HRESULT hr; + BOOL thin; TRACE("%p, %p\n", stream, token); while (wg_parser_stream_get_buffer(source->wg_parser, stream->wg_stream, &buffer)) { - HRESULT hr = media_stream_send_sample(stream, &buffer, token); + if (wg_parser_stream_get_thin_event(stream->wg_stream, &thin)) + { + param.vt = VT_BOOL; + param.boolVal = thin ? VARIANT_TRUE : VARIANT_FALSE; + if (FAILED(hr = IMFMediaEventQueue_QueueEventParamVar(stream->event_queue, MEStreamThinMode, &GUID_NULL, S_OK, ¶m))) + WARN("Failed to queue MEStreamThinMode event, hr %#lx\n", hr); + } + + hr = media_stream_send_sample(stream, &buffer, token); if (hr != S_FALSE) return hr; } diff --git a/dlls/winegstreamer/unixlib.h b/dlls/winegstreamer/unixlib.h index bde18afd77e..2754e1f1b03 100644 --- a/dlls/winegstreamer/unixlib.h +++ b/dlls/winegstreamer/unixlib.h @@ -252,6 +252,12 @@ struct wg_parser_set_thin_params BOOL thin; }; +struct wg_parser_stream_get_thin_event_params +{ + wg_parser_stream_t stream; + BOOL thin; +}; + struct wg_parser_get_stream_count_params { wg_parser_t parser; @@ -437,6 +443,7 @@ enum unix_funcs unix_wg_parser_push_data, unix_wg_parser_set_thin, + unix_wg_parser_stream_get_thin_event, unix_wg_parser_get_stream_count, unix_wg_parser_get_stream, diff --git a/dlls/winegstreamer/wg_parser.c b/dlls/winegstreamer/wg_parser.c index a7988513107..b290a35f29e 100644 --- a/dlls/winegstreamer/wg_parser.c +++ b/dlls/winegstreamer/wg_parser.c @@ -111,6 +111,7 @@ struct wg_parser_stream GstCaps *codec_caps; GstCaps *current_caps; GstCaps *desired_caps; + GstEvent *thin_event[2]; pthread_cond_t event_cond, event_empty_cond; GstBuffer *buffer; @@ -125,6 +126,7 @@ struct wg_parser_stream struct decoder_pad_probe_param { struct wg_parser *parser; + bool thin; }; static struct wg_parser *get_parser(wg_parser_t parser) @@ -244,6 +246,28 @@ static NTSTATUS wg_parser_set_thin(void *args) return S_OK; } +static NTSTATUS wg_parser_stream_get_thin_event(void *args) +{ + struct wg_parser_stream_get_thin_event_params *params = args; + struct wg_parser_stream *stream = get_stream(params->stream); + struct wg_parser *parser = stream->parser; + GstEvent *event = NULL; + gboolean thin; + + pthread_mutex_lock(&parser->mutex); + event = stream->thin_event[1]; + stream->thin_event[1] = NULL; + pthread_mutex_unlock(&parser->mutex); + + if (!event) + return S_FALSE; + + gst_structure_get_boolean(gst_event_get_structure(event), "thin", &thin); + gst_event_unref(event); + params->thin = thin; + return S_OK; +} + static NTSTATUS wg_parser_stream_get_current_format(void *args) { const struct wg_parser_stream_get_current_format_params *params = args; @@ -628,6 +652,14 @@ static GstPadProbeReturn decoder_pad_probe(GstPad *pad, GstPadProbeInfo *info, g thin = param->parser->thin; pthread_mutex_unlock(¶m->parser->mutex); + if (thin != param->thin) + { + GstStructure *structure = gst_structure_new("thin", "thin", G_TYPE_BOOLEAN, thin, NULL); + GstEvent *event = gst_event_new_custom(GST_EVENT_CUSTOM_DOWNSTREAM, structure); + gst_pad_send_event(pad, event); + param->thin = thin; + } + if (thin && (buffer = gst_pad_probe_info_get_buffer(info))) { if (GST_BUFFER_FLAG_IS_SET(buffer, GST_BUFFER_FLAG_DELTA_UNIT)) @@ -657,6 +689,9 @@ static void deep_element_added_cb(GstBin *self, GstBin *sub_bin, GstElement *ele { struct decoder_pad_probe_param *param = malloc(sizeof *param); param->parser = parser; + pthread_mutex_lock(&parser->mutex); + param->thin = parser->thin; + pthread_mutex_unlock(&parser->mutex); gst_pad_add_probe(sink, GST_PAD_PROBE_TYPE_BUFFER, decoder_pad_probe, param, free); } @@ -759,6 +794,17 @@ static gboolean sink_event_cb(GstPad *pad, GstObject *parent, GstEvent *event) pthread_mutex_unlock(&parser->mutex); break; + case GST_EVENT_CUSTOM_DOWNSTREAM: + if (gst_structure_has_name(gst_event_get_structure(event), "thin")) + { + pthread_mutex_lock(&parser->mutex); + if (stream->thin_event[0]) + gst_event_unref(stream->thin_event[0]); + stream->thin_event[0] = gst_event_ref(event); + pthread_mutex_unlock(&parser->mutex); + break; + } + default: GST_WARNING("Ignoring \"%s\" event.", GST_EVENT_TYPE_NAME(event)); } @@ -811,6 +857,14 @@ static GstFlowReturn sink_chain_cb(GstPad *pad, GstObject *parent, GstBuffer *bu return GST_FLOW_ERROR; } + if (stream->thin_event[0]) + { + if (stream->thin_event[1]) + gst_event_unref(stream->thin_event[1]); + stream->thin_event[1] = stream->thin_event[0]; + stream->thin_event[0] = NULL; + } + stream->buffer = buffer; pthread_mutex_unlock(&parser->mutex); @@ -944,6 +998,11 @@ static void free_stream(struct wg_parser_stream *stream) stream->buffer = NULL; } + if (stream->thin_event[0]) + gst_event_unref(stream->thin_event[0]); + if (stream->thin_event[1]) + gst_event_unref(stream->thin_event[1]); + pthread_cond_destroy(&stream->event_cond); pthread_cond_destroy(&stream->event_empty_cond); @@ -1970,6 +2029,7 @@ const unixlib_entry_t __wine_unix_call_funcs[] = X(wg_parser_push_data), X(wg_parser_set_thin), + X(wg_parser_stream_get_thin_event), X(wg_parser_get_stream_count), X(wg_parser_get_stream), @@ -2369,6 +2429,7 @@ const unixlib_entry_t __wine_unix_call_wow64_funcs[] = X64(wg_parser_push_data), X(wg_parser_set_thin), + X(wg_parser_stream_get_thin_event), X(wg_parser_get_stream_count), X(wg_parser_get_stream), -- GitLab https://gitlab.winehq.org/wine/wine/-/merge_requests/8505
From: Charlotte Pabst <cpabst(a)codeweavers.com> --- dlls/mfsrcsnk/media_source.c | 2 +- dlls/winedmo/main.c | 6 +++--- dlls/winedmo/unix_demuxer.c | 18 +++++++++++++++--- dlls/winedmo/unixlib.h | 1 + include/wine/winedmo.h | 2 +- 5 files changed, 21 insertions(+), 8 deletions(-) diff --git a/dlls/mfsrcsnk/media_source.c b/dlls/mfsrcsnk/media_source.c index d7f20e511c8..9203fd33374 100644 --- a/dlls/mfsrcsnk/media_source.c +++ b/dlls/mfsrcsnk/media_source.c @@ -588,7 +588,7 @@ static HRESULT demuxer_read_sample(struct winedmo_demuxer demuxer, UINT *index, if (FAILED(hr = create_media_buffer_sample(buffer_size, &sample, &output.pBuffer))) return hr; - if ((status = winedmo_demuxer_read(demuxer, index, &output, &buffer_size))) + if ((status = winedmo_demuxer_read(demuxer, index, &output, &buffer_size, FALSE))) { if (status == STATUS_BUFFER_TOO_SMALL) hr = E_PENDING; else if (status == STATUS_END_OF_FILE) hr = MF_E_END_OF_STREAM; diff --git a/dlls/winedmo/main.c b/dlls/winedmo/main.c index 1e91d8e2e51..6a462555f71 100644 --- a/dlls/winedmo/main.c +++ b/dlls/winedmo/main.c @@ -222,12 +222,12 @@ NTSTATUS CDECL winedmo_demuxer_destroy( struct winedmo_demuxer *demuxer ) return status; } -NTSTATUS CDECL winedmo_demuxer_read( struct winedmo_demuxer demuxer, UINT *stream, DMO_OUTPUT_DATA_BUFFER *buffer, UINT *buffer_size ) +NTSTATUS CDECL winedmo_demuxer_read( struct winedmo_demuxer demuxer, UINT *stream, DMO_OUTPUT_DATA_BUFFER *buffer, UINT *buffer_size, BOOL thin ) { - struct demuxer_read_params params = {.demuxer = demuxer}; + struct demuxer_read_params params = {.demuxer = demuxer, .thin = thin}; NTSTATUS status; - TRACE( "demuxer %#I64x, stream %p, buffer %p, buffer_size %p\n", demuxer.handle, stream, buffer, buffer_size ); + TRACE( "demuxer %#I64x, stream %p, buffer %p, buffer_size %p, thin %d\n", demuxer.handle, stream, buffer, buffer_size, thin ); buffer_lock( buffer, ¶ms.sample ); status = UNIX_CALL( demuxer_read, ¶ms ); diff --git a/dlls/winedmo/unix_demuxer.c b/dlls/winedmo/unix_demuxer.c index 7a1a262e6c9..ac41b2b8227 100644 --- a/dlls/winedmo/unix_demuxer.c +++ b/dlls/winedmo/unix_demuxer.c @@ -220,7 +220,7 @@ NTSTATUS demuxer_destroy( void *arg ) return STATUS_SUCCESS; } -static NTSTATUS demuxer_filter_packet( struct demuxer *demuxer, AVPacket **packet ) +static NTSTATUS demuxer_filter_packet( struct demuxer *demuxer, AVPacket **packet, BOOL thin ) { struct stream *stream; int i, ret; @@ -233,7 +233,19 @@ static NTSTATUS demuxer_filter_packet( struct demuxer *demuxer, AVPacket **packe if (!(stream = demuxer->last_stream)) ret = 0; else { - if (!(ret = av_bsf_receive_packet( stream->filter, *packet ))) return STATUS_SUCCESS; + if (!(ret = av_bsf_receive_packet( stream->filter, *packet ))) { + if (thin) + { + if ((*packet)->flags & AV_PKT_FLAG_KEY) TRACE("Thinning: Found key frame.\n"); + else + { + TRACE("Thinning: Skipping delta frame.\n"); + av_packet_free( packet ); + continue; + } + } + return STATUS_SUCCESS; + } if (ret == AVERROR_EOF) stream->eos = TRUE; if (!ret || ret == AVERROR_EOF || ret == AVERROR(EAGAIN)) ret = 0; else WARN( "Failed to read packet from filter, error %s.\n", debugstr_averr( ret ) ); @@ -280,7 +292,7 @@ NTSTATUS demuxer_read( void *arg ) TRACE( "demuxer %p, capacity %#x\n", demuxer, capacity ); - if ((status = demuxer_filter_packet( demuxer, &packet ))) return status; + if ((status = demuxer_filter_packet( demuxer, &packet, params->thin ))) return status; params->sample.size = packet->size; if ((capacity < packet->size)) diff --git a/dlls/winedmo/unixlib.h b/dlls/winedmo/unixlib.h index cf37bd5342a..eda808e2cd0 100644 --- a/dlls/winedmo/unixlib.h +++ b/dlls/winedmo/unixlib.h @@ -121,6 +121,7 @@ struct demuxer_read_params struct winedmo_demuxer demuxer; UINT32 stream; struct sample sample; + BOOL thin; }; struct demuxer_seek_params diff --git a/include/wine/winedmo.h b/include/wine/winedmo.h index c068dfa1de8..ab592777eab 100644 --- a/include/wine/winedmo.h +++ b/include/wine/winedmo.h @@ -46,7 +46,7 @@ NTSTATUS CDECL winedmo_demuxer_check( const char *mime_type ); NTSTATUS CDECL winedmo_demuxer_create( const WCHAR *url, struct winedmo_stream *stream, UINT64 stream_size, INT64 *duration, UINT *stream_count, WCHAR *mime_type, struct winedmo_demuxer *demuxer ); NTSTATUS CDECL winedmo_demuxer_destroy( struct winedmo_demuxer *demuxer ); -NTSTATUS CDECL winedmo_demuxer_read( struct winedmo_demuxer demuxer, UINT *stream, DMO_OUTPUT_DATA_BUFFER *buffer, UINT *buffer_size ); +NTSTATUS CDECL winedmo_demuxer_read( struct winedmo_demuxer demuxer, UINT *stream, DMO_OUTPUT_DATA_BUFFER *buffer, UINT *buffer_size, BOOL thin ); NTSTATUS CDECL winedmo_demuxer_seek( struct winedmo_demuxer demuxer, INT64 timestamp ); NTSTATUS CDECL winedmo_demuxer_stream_lang( struct winedmo_demuxer demuxer, UINT stream, WCHAR *buffer, UINT len ); NTSTATUS CDECL winedmo_demuxer_stream_name( struct winedmo_demuxer demuxer, UINT stream, WCHAR *buffer, UINT len ); -- GitLab https://gitlab.winehq.org/wine/wine/-/merge_requests/8505
From: Charlotte Pabst <cpabst(a)codeweavers.com> --- dlls/mfsrcsnk/media_source.c | 13 +++++++------ 1 file changed, 7 insertions(+), 6 deletions(-) diff --git a/dlls/mfsrcsnk/media_source.c b/dlls/mfsrcsnk/media_source.c index 9203fd33374..ccdb984567a 100644 --- a/dlls/mfsrcsnk/media_source.c +++ b/dlls/mfsrcsnk/media_source.c @@ -240,6 +240,7 @@ struct media_source IMFByteStream *stream; WCHAR *url; float rate; + BOOL thin; struct winedmo_demuxer winedmo_demuxer; struct winedmo_stream winedmo_stream; @@ -575,7 +576,7 @@ static HRESULT create_media_buffer_sample(UINT buffer_size, IMFSample **sample, return hr; } -static HRESULT demuxer_read_sample(struct winedmo_demuxer demuxer, UINT *index, IMFSample **out) +static HRESULT demuxer_read_sample(struct winedmo_demuxer demuxer, UINT *index, IMFSample **out, BOOL thin) { UINT buffer_size = 0x1000; IMFSample *sample; @@ -588,7 +589,7 @@ static HRESULT demuxer_read_sample(struct winedmo_demuxer demuxer, UINT *index, if (FAILED(hr = create_media_buffer_sample(buffer_size, &sample, &output.pBuffer))) return hr; - if ((status = winedmo_demuxer_read(demuxer, index, &output, &buffer_size, FALSE))) + if ((status = winedmo_demuxer_read(demuxer, index, &output, &buffer_size, thin))) { if (status == STATUS_BUFFER_TOO_SMALL) hr = E_PENDING; else if (status == STATUS_END_OF_FILE) hr = MF_E_END_OF_STREAM; @@ -635,7 +636,7 @@ static HRESULT media_source_read(struct media_source *source) 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) + if (FAILED(hr = demuxer_read_sample(source->winedmo_demuxer, &index, &sample, source->thin)) && hr != MF_E_END_OF_STREAM) { WARN("Failed to read stream %u data, hr %#lx\n", index, hr); return hr; @@ -1055,6 +1056,7 @@ static HRESULT WINAPI media_source_IMFRateControl_SetRate(IMFRateControl *iface, 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 +1068,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 ccdb984567a..b09a6bed665 100644 --- a/dlls/mfsrcsnk/media_source.c +++ b/dlls/mfsrcsnk/media_source.c @@ -290,28 +290,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; @@ -335,6 +313,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 b09a6bed665..adc5f407786 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 @@ -313,17 +314,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_BOOL; + param.boolVal = source->thin ? VARIANT_TRUE : VARIANT_FALSE; + 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; @@ -365,7 +375,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); } @@ -649,7 +659,7 @@ static HRESULT media_source_read(struct media_source *source) return S_OK; } - 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
From: Charlotte Pabst <cpabst(a)codeweavers.com> --- dlls/mfsrcsnk/media_source.c | 2 -- dlls/mfsrcsnk/tests/mfsrcsnk.c | 4 ---- dlls/winegstreamer/media_source.c | 3 --- 3 files changed, 9 deletions(-) diff --git a/dlls/mfsrcsnk/media_source.c b/dlls/mfsrcsnk/media_source.c index adc5f407786..e2b344584f7 100644 --- a/dlls/mfsrcsnk/media_source.c +++ b/dlls/mfsrcsnk/media_source.c @@ -1058,8 +1058,6 @@ 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; diff --git a/dlls/mfsrcsnk/tests/mfsrcsnk.c b/dlls/mfsrcsnk/tests/mfsrcsnk.c index 3309659df1e..940d54504f5 100644 --- a/dlls/mfsrcsnk/tests/mfsrcsnk.c +++ b/dlls/mfsrcsnk/tests/mfsrcsnk.c @@ -647,7 +647,6 @@ static void test_sample_times_at_rate(struct event_callback *callback, IMFMediaS 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); while (!got_stream_end && (event = event_callback_recv_event(callback, 1000))) @@ -722,9 +721,7 @@ static void test_sample_times_at_rate(struct event_callback *callback, IMFMediaS IMFMediaStream_Release(stream); ok(got_stream_end, "Expected MEEndOfStream.\n"); - todo_wine_if(thin) ok(got_rate_change, "Expected MESourceRateChanged.\n"); - todo_wine ok(got_thin_mode, "Expected MEStreamThinMode.\n"); ok(got_samples >= total_samples/keyframe_interval, "Expected at least %d samples, got %d.\n", total_samples/keyframe_interval, got_samples); @@ -739,7 +736,6 @@ static void test_sample_times_at_rate(struct event_callback *callback, IMFMediaS LONGLONG tolerance = 10; LONGLONG diff = expect_interval - interval; - todo_wine_if(thin) ok((diff < 0 ? -diff : diff) <= tolerance, "Expected interval %lld, got %lld.\n", expect_interval, interval); } diff --git a/dlls/winegstreamer/media_source.c b/dlls/winegstreamer/media_source.c index 9112b5c040d..ca9ce0ee4a0 100644 --- a/dlls/winegstreamer/media_source.c +++ b/dlls/winegstreamer/media_source.c @@ -1297,9 +1297,6 @@ static HRESULT WINAPI media_source_rate_control_SetRate(IMFRateControl *iface, B 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; -- GitLab https://gitlab.winehq.org/wine/wine/-/merge_requests/8505
Rémi Bernon (@rbernon) commented about dlls/mfsrcsnk/tests/mfsrcsnk.c:
+ 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); + + while (!got_stream_end && (event = event_callback_recv_event(callback, 1000))) + { + hr = IMFMediaEvent_GetType(event, &type); + ok(hr == S_OK, "Unexpected hr %#lx.\n", hr); + + hr = IMFMediaEvent_GetValue(event, &value); + ok(hr == S_OK, "Unexpected hr %#lx.\n", hr); + + hr = IMFMediaEvent_GetStatus(event, &status); + ok(hr == S_OK, "Unexpected hr %#lx.\n", hr); + + switch (type) I think it is better to sequentially call then check for events in a lock step, making things more deterministic and better exhibiting the order things are supposed to happen. Of course in this case some events are received on the source, and some on the stream, so it's not 100% deterministic and you may wait for events in one order or the other but still.
Similarly, this would let you check for sample timestamps directly in sequence, without sorting them. This would make the test more strict and it would catch any possible mistake with the order samples are supposed to be sent. Wrt timestamps, I would suggest to use an AVI sample format instead, which would allow (some) test to pass with exact timestamp values. You've maybe noticed that our MP4 media source doesn't have the right timestamps yet, and we're (GStreamer/FFmpeg is) probably missing some offset somewhere in the file. This isn't the purpose of this test though and using AVI instead would help avoiding this problem. In short I think something like https://gitlab.winehq.org/rbernon/wine/-/commit/3dabfcd989a08f78822d8989f46b... would be better, what do you think? -- https://gitlab.winehq.org/wine/wine/-/merge_requests/8505#note_108803
Rémi Bernon (@rbernon) commented about dlls/mfsrcsnk/tests/mfsrcsnk.c:
IMFByteStream_Release(bytestream); }
+struct source_create_callback
You'll need to squash these helpers with the next change. Also I think it'd be better to introduce another different flavor of mock test callback. I'm sure the test_callback from mf/tests/mf.c can be copied here with little adjustments to fit the needs for these tests (more in other comments). -- https://gitlab.winehq.org/wine/wine/-/merge_requests/8505#note_108802
Rémi Bernon (@rbernon) commented about dlls/mfsrcsnk/media_source.c:
if (rate < 0.0f) return MF_E_REVERSE_UNSUPPORTED; - if (thin) - return MF_E_THINNING_UNSUPPORTED;
All the previous changes are effectively dead code until this change, this should generally be avoided so that bisecting works. -- https://gitlab.winehq.org/wine/wine/-/merge_requests/8505#note_108806
Rémi Bernon (@rbernon) commented about dlls/mfsrcsnk/tests/mfsrcsnk.c:
+ + free(sample_times); +} + +static void test_thinning(void) +{ + static const GUID CLSID_MPEG4ByteStreamHandler = + {0x271c3902,0x6095,0x4c45,{0xa2,0x2f,0x20,0x09,0x18,0x16,0xee,0x9e}}; + IMFMediaSource *source = NULL; + IMFRateControl *rate_control; + IMFPresentationDescriptor *pres_desc; + struct event_callback *callback; + HRESULT hr; + + hr = create_source(&CLSID_MPEG4ByteStreamHandler, + create_resource_byte_stream(L"test_thinning.mp4"), &source); You're leaking the stream here.
-- https://gitlab.winehq.org/wine/wine/-/merge_requests/8505#note_108805
I think it is better to sequentially call then check for events in a lock step, making things more deterministic and better exhibiting the order things are supposed to happen. Of course in this case some events are received on the source, and some on the stream, so it's not 100% deterministic and you may wait for events in one order or the other but still.
To be fair, MS docs say things like "Events from the media source are not synchronized with events from the media streams." [1], so I figured it'd be more reliable to wait for source and stream events at the same time in any scenario where they could theoretically be delivered in arbitrary order - independently of if I'm able to observe a certain order on windows reliably or not (Hence also the different design from mf's test_callback - I adapted it from test_callback originally but changed it to this to support doing BeginEvent on multiple event sources in parallel). For many of these cases (like the start events), our MF media session code also accounts for arbitrary order. The issues I see with testing unspecified windows behavior more precisely especially when it comes to event order is that: 1. Some things might be racy on windows and simply be likely but not guaranteed to be delivered in a certain order, especially when the docs explicitly say that the order is not defined. 2. The windows implementation might change, and since most of this stuff go through the more robust media session, it wouldn't break applications. 3. A test that relies on less implementation details could potentially be adapted to run with a bigger matrix of different files or configurations. Which is why I've tried to mostly just rely on behavior that is either documented or something that we know games actively need. Essentially, treating the unit tests as a sort of specification. Especially some of the things like waiting for the first sample and then expecting the `MESourceRateChanged` and `MEStreamThinMode` event and updated timestamps afterwards seems incorrect - implementations may buffer a bunch of samples (I think winegstreamer does, actually? And I vaguely remember having related issues with the windows mp4 source, too), so it's not exactly guaranteed when the transition will happen - marking that is what `MEStreamThinMode` exists for. (Sidenote: if we don't rely on when the transition happens, we also can't use exact timestamp tests, which is why in this case it wouldn't matter whether we're dealing with avi or mp4. And the game I'm implementing this for also happens to just rely on the interval between timestamps being correct). And (hypothetically) what if that first sample we request takes a while to load and the `MESourceRateChanged` event goes through first? I'm also a bit wary of testing for the next event to match something. What if new, unrelated events are added and somehow get in the way? Another concern: Why make the check for `MEStreamThinMode` depend on `MESourceRateChanged` delivering `VT_R4` values? That's just an unrelated bug/oversight in wine, this MR doesn't even fix it, and at that point I'd rather check for `winetest_platform_is_wine` again. I'm not that big of a fan of wine checks anyway tho, I'd much rather just make sure to not "eat" random events in case a certain expected event doesn't get delivered. My initial approach for these tests was much more sequential and I've changed it to what it is now after considering many of these problems. Well, these are my concerns, I'm less knowledgeable/experienced in this matter so maybe I'm just overthinking things. I'm not against using your approach instead. [1] https://learn.microsoft.com/en-us/windows/win32/api/mfidl/nf-mfidl-imfrateco... -- https://gitlab.winehq.org/wine/wine/-/merge_requests/8505#note_108845
On Fri Jul 4 08:51:23 2025 +0000, Rémi Bernon wrote:
All the previous changes are effectively dead code until this change, this should generally be avoided so that bisecting works. What commit should remove the todo_wine in the tests then? Presumably the commit that makes the tests work on winegstreamer, since that's the default?
-- https://gitlab.winehq.org/wine/wine/-/merge_requests/8505#note_108846
On Fri Jul 4 18:32:26 2025 +0000, Charlotte Pabst wrote: > > I think it is better to sequentially call then check for events in a > lock step, making things more deterministic and better exhibiting the > order things are supposed to happen. Of course in this case some events > are received on the source, and some on the stream, so it's not 100% > deterministic and you may wait for events in one order or the other but still. > To be fair, MS docs say things like "Events from the media source are > not synchronized with events from the media streams." [1], so I figured > it'd be more reliable to wait for source and stream events at the same > time in any scenario where they could theoretically be delivered in > arbitrary order - independently of if I'm able to observe a certain > order on windows reliably or not (Hence also the different design from > mf's test_callback - I adapted it from test_callback originally but > changed it to this to support doing BeginEvent on multiple event sources > in parallel). > For many of these cases (like the start events), our MF media session > code also accounts for arbitrary order. > The issues I see with testing unspecified windows behavior more > precisely especially when it comes to event order is that: > 1. Some things might be racy on windows and simply be likely but not > guaranteed to be delivered in a certain order, especially when the docs > explicitly say that the order is not defined. > 2. The windows implementation might change, and since most of this stuff > go through the more robust media session, it wouldn't break applications. > 3. A test that relies on less implementation details could potentially > be adapted to run with a bigger matrix of different files or configurations. > Which is why I've tried to mostly just rely on behavior that is either > documented or something that we know games actively need. Essentially, > treating the unit tests as a sort of specification. > Especially some of the things like waiting for the first sample and then > expecting the `MESourceRateChanged` and `MEStreamThinMode` event and > updated timestamps afterwards seems incorrect - implementations may > buffer a bunch of samples (I think winegstreamer does, actually? And I > vaguely remember having related issues with the windows mp4 source, > too), so it's not exactly guaranteed when the transition will happen - > marking that is what `MEStreamThinMode` exists for. (Sidenote: if we > don't rely on when the transition happens, we also can't use exact > timestamp tests, which is why in this case it wouldn't matter whether > we're dealing with avi or mp4. And the game I'm implementing this for > also happens to just rely on the interval between timestamps being correct). > And (hypothetically) what if that first sample we request takes a while > to load and the `MESourceRateChanged` event goes through first? > I'm also a bit wary of testing for the next event to match something. > What if new, unrelated events are added and somehow get in the way? > Another concern: Why make the check for `MEStreamThinMode` depend on > `MESourceRateChanged` delivering `VT_R4` values? That's just an > unrelated bug/oversight in wine, this MR doesn't even fix it, and at > that point I'd rather check for `winetest_platform_is_wine` again. > I'm not that big of a fan of wine checks anyway tho, I'd much rather > just make sure to not "eat" random events in case a certain expected > event doesn't get delivered. > My initial approach for these tests was much more sequential and I've > changed it to what it is now after considering many of these problems. > Well, these are my concerns, I'm less knowledgeable/experienced in this > matter so maybe I'm just overthinking things. I'm not against using your > approach instead. > [1] https://learn.microsoft.com/en-us/windows/win32/api/mfidl/nf-mfidl-imfratecontrol-setrate#remarks MSDN may say something, the implementation is what matters in the end because that's what applications depend on. And more often than we would like, applications also depend on some specific implementation behavior, such as event delivery order, specific timestamps etc... Applications may even have race condition themselves that somehow only manage to succeed because Windows implementation is leaning in some specific way, that makes it less likely to trigger, and we should be trying to do the same. > 1. Some things might be racy on windows and simply be likely but not guaranteed to be delivered in a certain order, especially when the docs explicitly say that the order is not defined. > And (hypothetically) what if that first sample we request takes a while to load and the `MESourceRateChanged` event goes through first? You're right about that and although some local testing suggested that it was always sent in some specific order, it seems that running the test on the testbot sometimes shows events in a different order. As that's the case I'll suggest waiting for the first sample before changing thin mode, and make sure it's free of race conditions. For other cases where a single request causes multiple events to be sent on both source and stream, it shouldn't matter in which order we wait and check them. > 2. The windows implementation might change, and since most of this stuff go through the more robust media session, it wouldn't break applications. > > I'm also a bit wary of testing for the next event to match something. What if new, unrelated events are added and somehow get in the way? I think tests are also a way of catching Windows implementation changes in advance, it is valuable to know about changes of the implementation. Also it doesn't seem the implementation has changed in all the Windows versions we test, so I'd say it's unlikely to change much in the future. > 3. A test that relies on less implementation details could potentially be adapted to run with a bigger matrix of different files or configurations. In my experience sequential tests tend to stay simpler and are easier to adapt / copy to cover other cases. Generic tests which multiplex streams of events and check multiple things together quickly become complicated, hard to follow, debug or adjust, and in the end less useful. > Another concern: Why make the check for `MEStreamThinMode` depend on `MESourceRateChanged` delivering `VT_R4` values? That's just an unrelated bug/oversight in wine, this MR doesn't even fix it, and at that point I'd rather check for `winetest_platform_is_wine` again. > > I'm not that big of a fan of wine checks anyway tho, I'd much rather just make sure to not "eat" random events in case a certain expected event doesn't get delivered. Yes, it should probably be using `winetest_platform_is_wine` instead, it's supposed to be a temporary fixup anyway until the event is generated. -- https://gitlab.winehq.org/wine/wine/-/merge_requests/8505#note_108847
On Fri Jul 4 18:32:57 2025 +0000, Charlotte Pabst wrote:
What commit should remove the todo_wine in the tests then? Presumably the commit that makes the tests work on winegstreamer, since that's the default? Yes.
-- https://gitlab.winehq.org/wine/wine/-/merge_requests/8505#note_108848
participants (3)
-
Charlotte Pabst -
Charlotte Pabst (＠CharlottePabst)
-
Rémi Bernon