[PATCH 1/7] winegstreamer: Emit absolute timestamp in media source.
Signed-off-by: Giovanni Mascellani <gmascellani(a)codeweavers.com> --- dlls/winegstreamer/media_source.c | 5 +---- 1 file changed, 1 insertion(+), 4 deletions(-) diff --git a/dlls/winegstreamer/media_source.c b/dlls/winegstreamer/media_source.c index 01ab626254a..cd8957db7b1 100644 --- a/dlls/winegstreamer/media_source.c +++ b/dlls/winegstreamer/media_source.c @@ -100,8 +100,6 @@ struct media_source SOURCE_SHUTDOWN, } state; - LONGLONG start_time; - HANDLE read_thread; bool read_thread_shutdown; }; @@ -274,7 +272,6 @@ static void start_pipeline(struct media_source *source, struct source_async_comm position->vt = VT_I8; position->hVal.QuadPart = 0; } - source->start_time = position->hVal.QuadPart; for (i = 0; i < source->stream_count; i++) { @@ -427,7 +424,7 @@ static void send_buffer(struct media_stream *stream, const struct wg_parser_even goto out; } - if (FAILED(hr = IMFSample_SetSampleTime(sample, event->u.buffer.pts - stream->parent_source->start_time))) + if (FAILED(hr = IMFSample_SetSampleTime(sample, event->u.buffer.pts))) { ERR("Failed to set sample time, hr %#x.\n", hr); goto out; -- 2.33.0
Signed-off-by: Giovanni Mascellani <gmascellani(a)codeweavers.com> --- dlls/winegstreamer/media_source.c | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/dlls/winegstreamer/media_source.c b/dlls/winegstreamer/media_source.c index cd8957db7b1..51ef84bf88f 100644 --- a/dlls/winegstreamer/media_source.c +++ b/dlls/winegstreamer/media_source.c @@ -329,8 +329,9 @@ static void start_pipeline(struct media_source *source, struct source_async_comm source->state = SOURCE_RUNNING; - unix_funcs->wg_parser_stream_seek(source->streams[0]->wg_stream, 1.0, - position->hVal.QuadPart, 0, AM_SEEKING_AbsolutePositioning, AM_SEEKING_NoPositioning); + if (position->vt == VT_I8) + unix_funcs->wg_parser_stream_seek(source->streams[0]->wg_stream, 1.0, + position->hVal.QuadPart, 0, AM_SEEKING_AbsolutePositioning, AM_SEEKING_NoPositioning); unix_funcs->wg_parser_end_flush(source->wg_parser); } -- 2.33.0
Hi, when a Start command is issued to a media source, the caller can optionally specify a timestamp to seek to just before starting playback. This is expressed by mean of a PROPVARIANT, which can be empty if no seek is desired, or can be set to 64 bit int, which is interpreted at the timestamp to seek to. When no seek is requested, the playback will restart from the beginning if it was previously stopped, or from where it was if it was previously paused. The current version doesn't check the type of the PROPVARIANT before using it. This is a bug, both because the content of an empty PROPVARIANT is not specified and should therefore not be used, and because supplying an empty PROPVARIANT explicitly has the meaning of not requesting any seek. This patch fixes this bug by simply gating the seek with the check that the PROPVARIANT has the right type. Again, you can use patches 4/7 through 7/7 to test this one. See my explanation for 1/7. Giovanni. Il 06/09/21 17:11, Giovanni Mascellani ha scritto:
Signed-off-by: Giovanni Mascellani <gmascellani(a)codeweavers.com> --- dlls/winegstreamer/media_source.c | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-)
diff --git a/dlls/winegstreamer/media_source.c b/dlls/winegstreamer/media_source.c index cd8957db7b1..51ef84bf88f 100644 --- a/dlls/winegstreamer/media_source.c +++ b/dlls/winegstreamer/media_source.c @@ -329,8 +329,9 @@ static void start_pipeline(struct media_source *source, struct source_async_comm
source->state = SOURCE_RUNNING;
- unix_funcs->wg_parser_stream_seek(source->streams[0]->wg_stream, 1.0, - position->hVal.QuadPart, 0, AM_SEEKING_AbsolutePositioning, AM_SEEKING_NoPositioning); + if (position->vt == VT_I8) + unix_funcs->wg_parser_stream_seek(source->streams[0]->wg_stream, 1.0, + position->hVal.QuadPart, 0, AM_SEEKING_AbsolutePositioning, AM_SEEKING_NoPositioning); unix_funcs->wg_parser_end_flush(source->wg_parser); }
Signed-off-by: Giovanni Mascellani <gmascellani(a)codeweavers.com> --- This patch was already signed off by Derek Lesho in an earlier submission. dlls/winegstreamer/media_source.c | 102 ++++++++++++++++++++++++++++-- 1 file changed, 97 insertions(+), 5 deletions(-) diff --git a/dlls/winegstreamer/media_source.c b/dlls/winegstreamer/media_source.c index 51ef84bf88f..2066051765e 100644 --- a/dlls/winegstreamer/media_source.c +++ b/dlls/winegstreamer/media_source.c @@ -37,6 +37,10 @@ struct media_stream struct wg_parser_stream *wg_stream; + IUnknown **token_queue; + LONG token_queue_count; + LONG token_queue_cap; + enum { STREAM_INACTIVE, @@ -50,6 +54,7 @@ struct media_stream enum source_async_op { SOURCE_ASYNC_START, + SOURCE_ASYNC_PAUSE, SOURCE_ASYNC_STOP, SOURCE_ASYNC_REQUEST_SAMPLE, }; @@ -96,6 +101,7 @@ struct media_source { SOURCE_OPENING, SOURCE_STOPPED, + SOURCE_PAUSED, SOURCE_RUNNING, SOURCE_SHUTDOWN, } state; @@ -260,6 +266,54 @@ static IMFStreamDescriptor *stream_descriptor_from_id(IMFPresentationDescriptor return NULL; } +static BOOL enqueue_token(struct media_stream *stream, IUnknown *token) +{ + if (stream->token_queue_count == stream->token_queue_cap) + { + IUnknown **buf; + stream->token_queue_cap = stream->token_queue_cap * 2 + 1; + buf = realloc(stream->token_queue, stream->token_queue_cap * sizeof(*buf)); + if (buf) + stream->token_queue = buf; + else + { + stream->token_queue_cap = stream->token_queue_count; + return FALSE; + } + } + stream->token_queue[stream->token_queue_count++] = token; + return TRUE; +} + +static void flush_token_queue(struct media_stream *stream, BOOL send) +{ + LONG i; + + for (i = 0; i < stream->token_queue_count; i++) + { + if (send) + { + HRESULT hr; + struct source_async_command *command; + if (SUCCEEDED(hr = source_create_async_op(SOURCE_ASYNC_REQUEST_SAMPLE, &command))) + { + command->u.request_sample.stream = stream; + command->u.request_sample.token = stream->token_queue[i]; + + hr = MFPutWorkItem(stream->parent_source->async_commands_queue, &stream->parent_source->async_commands_callback, &command->IUnknown_iface); + } + if (FAILED(hr)) + WARN("Could not enqueue sample request, hr %#x\n", hr); + } + else if (stream->token_queue[i]) + IUnknown_Release(stream->token_queue[i]); + } + free(stream->token_queue); + stream->token_queue = NULL; + stream->token_queue_count = 0; + stream->token_queue_cap = 0; +} + static void start_pipeline(struct media_source *source, struct source_async_command *command) { PROPVARIANT *position = &command->u.start.position; @@ -333,6 +387,27 @@ static void start_pipeline(struct media_source *source, struct source_async_comm unix_funcs->wg_parser_stream_seek(source->streams[0]->wg_stream, 1.0, position->hVal.QuadPart, 0, AM_SEEKING_AbsolutePositioning, AM_SEEKING_NoPositioning); unix_funcs->wg_parser_end_flush(source->wg_parser); + + for (i = 0; i < source->stream_count; i++) + flush_token_queue(source->streams[i], position->vt == VT_EMPTY); +} + +static void pause_pipeline(struct media_source *source) +{ + unsigned int i; + + for (i = 0; i < source->stream_count; i++) + { + struct media_stream *stream = source->streams[i]; + if (stream->state != STREAM_INACTIVE) + { + IMFMediaEventQueue_QueueEventParamVar(stream->event_queue, MEStreamPaused, &GUID_NULL, S_OK, NULL); + } + } + + IMFMediaEventQueue_QueueEventParamVar(source->event_queue, MESourcePaused, &GUID_NULL, S_OK, NULL); + + source->state = SOURCE_PAUSED; } static void stop_pipeline(struct media_source *source) @@ -354,6 +429,9 @@ static void stop_pipeline(struct media_source *source) IMFMediaEventQueue_QueueEventParamVar(source->event_queue, MESourceStopped, &GUID_NULL, S_OK, NULL); source->state = SOURCE_STOPPED; + + for (i = 0; i < source->stream_count; i++) + flush_token_queue(source->streams[i], FALSE); } static void dispatch_end_of_presentation(struct media_source *source) @@ -502,11 +580,17 @@ static HRESULT WINAPI source_async_commands_Invoke(IMFAsyncCallback *iface, IMFA case SOURCE_ASYNC_START: start_pipeline(source, command); break; + case SOURCE_ASYNC_PAUSE: + pause_pipeline(source); + break; case SOURCE_ASYNC_STOP: stop_pipeline(source); break; case SOURCE_ASYNC_REQUEST_SAMPLE: - wait_on_sample(command->u.request_sample.stream, command->u.request_sample.token); + if (source->state == SOURCE_PAUSED) + enqueue_token(command->u.request_sample.stream, command->u.request_sample.token); + else + wait_on_sample(command->u.request_sample.stream, command->u.request_sample.token); break; } @@ -597,6 +681,7 @@ static ULONG WINAPI media_stream_Release(IMFMediaStream *iface) { if (stream->event_queue) IMFMediaEventQueue_Release(stream->event_queue); + flush_token_queue(stream, FALSE); free(stream); } @@ -699,7 +784,6 @@ static HRESULT WINAPI media_stream_RequestSample(IMFMediaStream *iface, IUnknown IUnknown_AddRef(token); command->u.request_sample.token = token; - /* Once pause support is added, this will need to put into a stream queue, and synchronization will need to be added*/ hr = MFPutWorkItem(stream->parent_source->async_commands_queue, &stream->parent_source->async_commands_callback, &command->IUnknown_iface); } @@ -1124,7 +1208,7 @@ static HRESULT WINAPI media_source_GetCharacteristics(IMFMediaSource *iface, DWO if (source->state == SOURCE_SHUTDOWN) return MF_E_SHUTDOWN; - *characteristics = MFMEDIASOURCE_CAN_SEEK; + *characteristics = MFMEDIASOURCE_CAN_SEEK | MFMEDIASOURCE_CAN_PAUSE; return S_OK; } @@ -1188,13 +1272,21 @@ static HRESULT WINAPI media_source_Stop(IMFMediaSource *iface) static HRESULT WINAPI media_source_Pause(IMFMediaSource *iface) { struct media_source *source = impl_from_IMFMediaSource(iface); + struct source_async_command *command; + HRESULT hr; - FIXME("%p: stub\n", iface); + TRACE("%p.\n", iface); if (source->state == SOURCE_SHUTDOWN) return MF_E_SHUTDOWN; - return E_NOTIMPL; + if (source->state != SOURCE_RUNNING) + return MF_E_INVALID_STATE_TRANSITION; + + if (SUCCEEDED(hr = source_create_async_op(SOURCE_ASYNC_PAUSE, &command))) + hr = MFPutWorkItem(source->async_commands_queue, &source->async_commands_callback, &command->IUnknown_iface); + + return S_OK; } static HRESULT WINAPI media_source_Shutdown(IMFMediaSource *iface) -- 2.33.0
Hi, this patch implements the possibility to pause the media source provided by winegstreamer. This feature is, for example, required by Deep Rock Galactic (the game uses it to pause and restart the preview videos in the character selection screen, which you can reach from the computer on your right in the spawn pod, just after loading the game; without this patch the videos will not restart after you switch between them). Since the media source does not work synchronously (the caller can request a sample at any time, independently of the timestamps at which they should presented), the paused state is not very different from the playing state. The most important difference is that RequestSample commands are not executed when the media source is paused, but stored in an internal queue (via the enqueue_token helper). As soon as the source is restarted, they will be immediately issued (if, instead, the source is later stopped, they are dropped), which happens in the flush_token_queue helper. This internal queue just needs to store the token associated with that RequestSample command, because that's the only state RequestSample has. Besides that, this patch ensures that the MESourcePaused and MEStreamPaused are emitted when there is a transition to paused state and that the MFMEDIASOURCE_CAN_PAUSE characteristic is exposed. Again, you can use patches 4/7 through 7/7 to test this one. See my explanation for 1/7. Thanks, Giovanni. Il 06/09/21 17:11, Giovanni Mascellani ha scritto:
Signed-off-by: Giovanni Mascellani <gmascellani(a)codeweavers.com> --- This patch was already signed off by Derek Lesho in an earlier submission.
dlls/winegstreamer/media_source.c | 102 ++++++++++++++++++++++++++++-- 1 file changed, 97 insertions(+), 5 deletions(-)
diff --git a/dlls/winegstreamer/media_source.c b/dlls/winegstreamer/media_source.c index 51ef84bf88f..2066051765e 100644 --- a/dlls/winegstreamer/media_source.c +++ b/dlls/winegstreamer/media_source.c @@ -37,6 +37,10 @@ struct media_stream
struct wg_parser_stream *wg_stream;
+ IUnknown **token_queue; + LONG token_queue_count; + LONG token_queue_cap; + enum { STREAM_INACTIVE, @@ -50,6 +54,7 @@ struct media_stream enum source_async_op { SOURCE_ASYNC_START, + SOURCE_ASYNC_PAUSE, SOURCE_ASYNC_STOP, SOURCE_ASYNC_REQUEST_SAMPLE, }; @@ -96,6 +101,7 @@ struct media_source { SOURCE_OPENING, SOURCE_STOPPED, + SOURCE_PAUSED, SOURCE_RUNNING, SOURCE_SHUTDOWN, } state; @@ -260,6 +266,54 @@ static IMFStreamDescriptor *stream_descriptor_from_id(IMFPresentationDescriptor return NULL; }
+static BOOL enqueue_token(struct media_stream *stream, IUnknown *token) +{ + if (stream->token_queue_count == stream->token_queue_cap) + { + IUnknown **buf; + stream->token_queue_cap = stream->token_queue_cap * 2 + 1; + buf = realloc(stream->token_queue, stream->token_queue_cap * sizeof(*buf)); + if (buf) + stream->token_queue = buf; + else + { + stream->token_queue_cap = stream->token_queue_count; + return FALSE; + } + } + stream->token_queue[stream->token_queue_count++] = token; + return TRUE; +} + +static void flush_token_queue(struct media_stream *stream, BOOL send) +{ + LONG i; + + for (i = 0; i < stream->token_queue_count; i++) + { + if (send) + { + HRESULT hr; + struct source_async_command *command; + if (SUCCEEDED(hr = source_create_async_op(SOURCE_ASYNC_REQUEST_SAMPLE, &command))) + { + command->u.request_sample.stream = stream; + command->u.request_sample.token = stream->token_queue[i]; + + hr = MFPutWorkItem(stream->parent_source->async_commands_queue, &stream->parent_source->async_commands_callback, &command->IUnknown_iface); + } + if (FAILED(hr)) + WARN("Could not enqueue sample request, hr %#x\n", hr); + } + else if (stream->token_queue[i]) + IUnknown_Release(stream->token_queue[i]); + } + free(stream->token_queue); + stream->token_queue = NULL; + stream->token_queue_count = 0; + stream->token_queue_cap = 0; +} + static void start_pipeline(struct media_source *source, struct source_async_command *command) { PROPVARIANT *position = &command->u.start.position; @@ -333,6 +387,27 @@ static void start_pipeline(struct media_source *source, struct source_async_comm unix_funcs->wg_parser_stream_seek(source->streams[0]->wg_stream, 1.0, position->hVal.QuadPart, 0, AM_SEEKING_AbsolutePositioning, AM_SEEKING_NoPositioning); unix_funcs->wg_parser_end_flush(source->wg_parser); + + for (i = 0; i < source->stream_count; i++) + flush_token_queue(source->streams[i], position->vt == VT_EMPTY); +} + +static void pause_pipeline(struct media_source *source) +{ + unsigned int i; + + for (i = 0; i < source->stream_count; i++) + { + struct media_stream *stream = source->streams[i]; + if (stream->state != STREAM_INACTIVE) + { + IMFMediaEventQueue_QueueEventParamVar(stream->event_queue, MEStreamPaused, &GUID_NULL, S_OK, NULL); + } + } + + IMFMediaEventQueue_QueueEventParamVar(source->event_queue, MESourcePaused, &GUID_NULL, S_OK, NULL); + + source->state = SOURCE_PAUSED; }
static void stop_pipeline(struct media_source *source) @@ -354,6 +429,9 @@ static void stop_pipeline(struct media_source *source) IMFMediaEventQueue_QueueEventParamVar(source->event_queue, MESourceStopped, &GUID_NULL, S_OK, NULL);
source->state = SOURCE_STOPPED; + + for (i = 0; i < source->stream_count; i++) + flush_token_queue(source->streams[i], FALSE); }
static void dispatch_end_of_presentation(struct media_source *source) @@ -502,11 +580,17 @@ static HRESULT WINAPI source_async_commands_Invoke(IMFAsyncCallback *iface, IMFA case SOURCE_ASYNC_START: start_pipeline(source, command); break; + case SOURCE_ASYNC_PAUSE: + pause_pipeline(source); + break; case SOURCE_ASYNC_STOP: stop_pipeline(source); break; case SOURCE_ASYNC_REQUEST_SAMPLE: - wait_on_sample(command->u.request_sample.stream, command->u.request_sample.token); + if (source->state == SOURCE_PAUSED) + enqueue_token(command->u.request_sample.stream, command->u.request_sample.token); + else + wait_on_sample(command->u.request_sample.stream, command->u.request_sample.token); break; }
@@ -597,6 +681,7 @@ static ULONG WINAPI media_stream_Release(IMFMediaStream *iface) { if (stream->event_queue) IMFMediaEventQueue_Release(stream->event_queue); + flush_token_queue(stream, FALSE); free(stream); }
@@ -699,7 +784,6 @@ static HRESULT WINAPI media_stream_RequestSample(IMFMediaStream *iface, IUnknown IUnknown_AddRef(token); command->u.request_sample.token = token;
- /* Once pause support is added, this will need to put into a stream queue, and synchronization will need to be added*/ hr = MFPutWorkItem(stream->parent_source->async_commands_queue, &stream->parent_source->async_commands_callback, &command->IUnknown_iface); }
@@ -1124,7 +1208,7 @@ static HRESULT WINAPI media_source_GetCharacteristics(IMFMediaSource *iface, DWO if (source->state == SOURCE_SHUTDOWN) return MF_E_SHUTDOWN;
- *characteristics = MFMEDIASOURCE_CAN_SEEK; + *characteristics = MFMEDIASOURCE_CAN_SEEK | MFMEDIASOURCE_CAN_PAUSE;
return S_OK; } @@ -1188,13 +1272,21 @@ static HRESULT WINAPI media_source_Stop(IMFMediaSource *iface) static HRESULT WINAPI media_source_Pause(IMFMediaSource *iface) { struct media_source *source = impl_from_IMFMediaSource(iface); + struct source_async_command *command; + HRESULT hr;
- FIXME("%p: stub\n", iface); + TRACE("%p.\n", iface);
if (source->state == SOURCE_SHUTDOWN) return MF_E_SHUTDOWN;
- return E_NOTIMPL; + if (source->state != SOURCE_RUNNING) + return MF_E_INVALID_STATE_TRANSITION; + + if (SUCCEEDED(hr = source_create_async_op(SOURCE_ASYNC_PAUSE, &command))) + hr = MFPutWorkItem(source->async_commands_queue, &source->async_commands_callback, &command->IUnknown_iface); + + return S_OK; }
static HRESULT WINAPI media_source_Shutdown(IMFMediaSource *iface)
On Mon, Sep 06, 2021 at 05:11:05PM +0200, Giovanni Mascellani wrote:
+static void flush_token_queue(struct media_stream *stream, BOOL send) +{ + LONG i; + + for (i = 0; i < stream->token_queue_count; i++) + { + if (send) + { + HRESULT hr; + struct source_async_command *command; + if (SUCCEEDED(hr = source_create_async_op(SOURCE_ASYNC_REQUEST_SAMPLE, &command))) + { + command->u.request_sample.stream = stream; + command->u.request_sample.token = stream->token_queue[i]; + + hr = MFPutWorkItem(stream->parent_source->async_commands_queue, &stream->parent_source->async_commands_callback, &command->IUnknown_iface);
This could do with a linewrap---155 is a bit too long.
@@ -699,7 +784,6 @@ static HRESULT WINAPI media_stream_RequestSample(IMFMediaStream *iface, IUnknown IUnknown_AddRef(token); command->u.request_sample.token = token;
- /* Once pause support is added, this will need to put into a stream queue, and synchronization will need to be added*/ hr = MFPutWorkItem(stream->parent_source->async_commands_queue, &stream->parent_source->async_commands_callback, &command->IUnknown_iface);
And while you're touching code near it, this is rather long too. Huw.
Signed-off-by: Giovanni Mascellani <gmascellani(a)codeweavers.com> --- This patch and the following are tests to justify the first three in the series. Since they fail on Windows 7 (I investigated this issue and came to the conclusion that it's a Windows 7 bug) and I was told that it not a good idea to commit patches that depend too much on the ins and outs of how events work on Windows, they are not meant to be included in Wine. They're just there to help reviewers check that the first three patches work sensibly. dlls/mfplat/tests/mfplat.c | 3 +++ 1 file changed, 3 insertions(+) diff --git a/dlls/mfplat/tests/mfplat.c b/dlls/mfplat/tests/mfplat.c index 54af1ee151d..90440a10c62 100644 --- a/dlls/mfplat/tests/mfplat.c +++ b/dlls/mfplat/tests/mfplat.c @@ -872,6 +872,9 @@ todo_wine video_stream = (IMFMediaStream *)var.punkVal; } + get_event((IMFMediaEventGenerator *)mediasource, MESourceStarted, NULL); + get_event((IMFMediaEventGenerator *)video_stream, MEStreamStarted, NULL); + sample_count = 10; for (i = 0; i < sample_count; ++i) -- 2.33.0
Hi, While running your changed tests, I think I found new failures. Being a bot and all I'm not very good at pattern recognition, so I might be wrong, but could you please double-check? Full results can be found at: https://testbot.winehq.org/JobDetails.pl?Key=97475 Your paranoid android. === debiant2 (32 bit German report) === mfplat: mfplat.c:3756: Test failed: Unexpected refcount 1. Unhandled exception: page fault on execute access to 0x00000000 in 32-bit code (0x00000000).
An asynchronous media event request cannot be cancelled. This means that we cannot test for no event firing within a certain timeout, because if we did we would leave a pending thread that will spoil future tests. Using synchronous APIs and repeated requests, we can wait for an even with a certain timeout, without doing damages if the timeout is reached. Signed-off-by: Giovanni Mascellani <gmascellani(a)codeweavers.com> --- dlls/mfplat/tests/mfplat.c | 106 ++++++++++++++----------------------- 1 file changed, 41 insertions(+), 65 deletions(-) diff --git a/dlls/mfplat/tests/mfplat.c b/dlls/mfplat/tests/mfplat.c index 90440a10c62..a18fb9256d6 100644 --- a/dlls/mfplat/tests/mfplat.c +++ b/dlls/mfplat/tests/mfplat.c @@ -565,77 +565,53 @@ static const IMFAsyncCallbackVtbl test_create_from_file_handler_callback_vtbl = test_create_from_file_handler_callback_Invoke, }; -static HRESULT WINAPI source_events_callback_Invoke(IMFAsyncCallback *iface, IMFAsyncResult *result) +#define get_event(a, b, c) get_event_(a, b, c, __LINE__) +static BOOL get_event_(IMFMediaEventGenerator *generator, MediaEventType *met, PROPVARIANT *value, int line) { - struct test_callback *callback = impl_from_IMFAsyncCallback(iface); - IMFMediaEventGenerator *generator; - HRESULT hr; - - ok(!!result, "Unexpected result object.\n"); - - generator = (IMFMediaEventGenerator *)IMFAsyncResult_GetStateNoAddRef(result); - - hr = IMFMediaEventGenerator_EndGetEvent(generator, result, &callback->media_event); - ok(hr == S_OK, "Failed to create an object, hr %#x.\n", hr); - - SetEvent(callback->event); - - return S_OK; -} - -static const IMFAsyncCallbackVtbl events_callback_vtbl = -{ - testcallback_QueryInterface, - testcallback_AddRef, - testcallback_Release, - testcallback_GetParameters, - source_events_callback_Invoke, -}; - -static BOOL get_event(IMFMediaEventGenerator *generator, MediaEventType expected_event_type, PROPVARIANT *value) -{ - struct test_callback callback = {{ 0 }}; - MediaEventType event_type; - BOOL ret = FALSE; + IMFMediaEvent *event; HRESULT hr; + int i; - callback.IMFAsyncCallback_iface.lpVtbl = &events_callback_vtbl; - callback.event = CreateEventA(NULL, FALSE, FALSE, NULL); - - for (;;) + for (i = 0; i < 10; i++) { - hr = IMFMediaEventGenerator_BeginGetEvent(generator, &callback.IMFAsyncCallback_iface, - (IUnknown *)generator); - ok(hr == S_OK, "Unexpected hr %#x.\n", hr); - - if (WaitForSingleObject(callback.event, 1000) == WAIT_TIMEOUT) - { - ok(0, "timeout\n"); + hr = IMFMediaEventGenerator_GetEvent(generator, MF_EVENT_FLAG_NO_WAIT, &event); + if (hr == S_OK) break; + if (hr != MF_E_NO_EVENTS_AVAILABLE) + { + ok_(__FILE__, line)(FALSE, "Unexpected hr %#x.\n", hr); + return FALSE; } - Sleep(10); + } - hr = IMFMediaEvent_GetType(callback.media_event, &event_type); - ok(hr == S_OK, "Failed to event type, hr %#x.\n", hr); - - if ((ret = (event_type == expected_event_type))) - { - if (value) - { - hr = IMFMediaEvent_GetValue(callback.media_event, value); - ok(hr == S_OK, "Failed to get value of event, hr %#x.\n", hr); - } + if (i == 10) + return FALSE; - break; - } + if (met) + { + hr = IMFMediaEvent_GetType(event, met); + ok_(__FILE__, line)(hr == S_OK, "Failed to get event type, hr %#x.\n", hr); + } + if (value) + { + hr = IMFMediaEvent_GetValue(event, value); + ok_(__FILE__, line)(hr == S_OK, "Failed to get event value, hr %#x.\n", hr); } + IMFMediaEvent_Release(event); - CloseHandle(callback.event); - if (callback.media_event) - IMFMediaEvent_Release(callback.media_event); + return TRUE; +} + +#define expect_event(a, b, c) expect_event_(a, b, c, __LINE__) +static BOOL expect_event_(IMFMediaEventGenerator *generator, MediaEventType expected_met, PROPVARIANT *value, int line) +{ + MediaEventType met; - return ret; + if (!get_event_(generator, &met, value, line)) + return FALSE; + ok_(__FILE__, line)(met == expected_met, "Got event %d instead of %d.\n", met, expected_met); + return met == expected_met; } static void test_source_resolver(void) @@ -866,14 +842,14 @@ todo_wine ok(hr == S_OK, "Failed to start media source, hr %#x.\n", hr); video_stream = NULL; - if (get_event((IMFMediaEventGenerator *)mediasource, MENewStream, &var)) + if (expect_event((IMFMediaEventGenerator *)mediasource, MENewStream, &var)) { ok(var.vt == VT_UNKNOWN, "Unexpected value type.\n"); video_stream = (IMFMediaStream *)var.punkVal; } - get_event((IMFMediaEventGenerator *)mediasource, MESourceStarted, NULL); - get_event((IMFMediaEventGenerator *)video_stream, MEStreamStarted, NULL); + expect_event((IMFMediaEventGenerator *)mediasource, MESourceStarted, NULL); + expect_event((IMFMediaEventGenerator *)video_stream, MEStreamStarted, NULL); sample_count = 10; @@ -895,7 +871,7 @@ todo_wine IMFSample *sample; BOOL ret; - ret = get_event((IMFMediaEventGenerator *)video_stream, MEMediaSample, &var); + ret = expect_event((IMFMediaEventGenerator *)video_stream, MEMediaSample, &var); ok(ret, "Sample %u not received.\n", i + 1); if (!ret) break; @@ -929,14 +905,14 @@ todo_wine hr = IMFMediaStream_RequestSample(video_stream, NULL); ok (hr == S_OK || hr == MF_E_END_OF_STREAM, "Unexpected hr %#x.\n", hr); - get_event((IMFMediaEventGenerator *)video_stream, MEEndOfStream, NULL); + expect_event((IMFMediaEventGenerator *)video_stream, MEEndOfStream, NULL); } hr = IMFMediaStream_RequestSample(video_stream, NULL); ok(hr == MF_E_END_OF_STREAM, "Unexpected hr %#x.\n", hr); - get_event((IMFMediaEventGenerator *)mediasource, MEEndOfPresentation, NULL); + expect_event((IMFMediaEventGenerator *)mediasource, MEEndOfPresentation, NULL); IMFMediaStream_Release(video_stream); IMFMediaTypeHandler_Release(handler); -- 2.33.0
Hi, While running your changed tests, I think I found new failures. Being a bot and all I'm not very good at pattern recognition, so I might be wrong, but could you please double-check? Full results can be found at: https://testbot.winehq.org/JobDetails.pl?Key=97476 Your paranoid android. === w7u_2qxl (32 bit report) === mfplat: 0838:mfplat: unhandled exception c0000005 at 6F0C7D24 === w7u_adm (32 bit report) === mfplat: 0864:mfplat: unhandled exception c0000005 at 6EAB7D24 === w7u_el (32 bit report) === mfplat: 0850:mfplat: unhandled exception c0000005 at 6EA97D24
On 9/6/21 6:11 PM, Giovanni Mascellani wrote:
An asynchronous media event request cannot be cancelled. This means that we cannot test for no event firing within a certain timeout, because if we did we would leave a pending thread that will spoil future tests. Using synchronous APIs and repeated requests, we can wait for an even with a certain timeout, without doing damages if the timeout is reached.
I believe we talked about this at some point. Synchronous GetEvent() is a problem because it will block forever if something goes wrong. I don't see how doing NO_WAIT a couple of times solves anything, if didn't get your event doing this batch of calls, it doesn't mean you won't get it during following tests, unless you use new source instance every time. Then again, nothing stops you from filtering unrelated events in async callback, so they don't interfere with tests to follow. Apparently there are some crashes on Windows 7 still, after 5/7, so hard to tell if this change helped anything. For pausing, I think some manual testing with higher level API, like mfplay, and audio-only pipeline for simplicity, should be enough.
Hi, Il 08/09/21 14:13, Nikolay Sivov ha scritto:
Apparently there are some crashes on Windows 7 still, after 5/7, so hard to tell if this change helped anything. For pausing, I think some manual testing with higher level API, like mfplay, and audio-only pipeline for simplicity, should be enough.
As I said in the comment of 4/7, patches from 4/7 on are not meant to be committed (precisely because we had already discussed this). They are just there to help the reviewer of 1-3/7 to check that those are correct. Giovanni.
Signed-off-by: Giovanni Mascellani <gmascellani(a)codeweavers.com> --- dlls/mfplat/tests/mfplat.c | 20 +++++++++++++++----- 1 file changed, 15 insertions(+), 5 deletions(-) diff --git a/dlls/mfplat/tests/mfplat.c b/dlls/mfplat/tests/mfplat.c index a18fb9256d6..a2aa082fbbb 100644 --- a/dlls/mfplat/tests/mfplat.c +++ b/dlls/mfplat/tests/mfplat.c @@ -614,6 +614,16 @@ static BOOL expect_event_(IMFMediaEventGenerator *generator, MediaEventType expe return met == expected_met; } +#define expect_no_event(a) expect_no_event_(a, __LINE__) +static void expect_no_event_(IMFMediaEventGenerator *generator, int line) +{ + MediaEventType met; + BOOL ret; + + ret = get_event_(generator, &met, NULL, line); + ok_(__FILE__, line)(!ret, "Got event %d when none was expected.\n", met); +} + static void test_source_resolver(void) { struct test_callback callback = { { &test_create_from_url_callback_vtbl } }; @@ -849,7 +859,9 @@ todo_wine } expect_event((IMFMediaEventGenerator *)mediasource, MESourceStarted, NULL); + expect_no_event((IMFMediaEventGenerator *)mediasource); expect_event((IMFMediaEventGenerator *)video_stream, MEStreamStarted, NULL); + expect_no_event((IMFMediaEventGenerator *)video_stream); sample_count = 10; @@ -896,16 +908,13 @@ todo_wine if (i == sample_count) { - IMFMediaEvent *event; - /* MEEndOfStream isn't queued until after a one request beyond the last frame is submitted */ - Sleep(100); - hr = IMFMediaEventGenerator_GetEvent((IMFMediaEventGenerator *)video_stream, MF_EVENT_FLAG_NO_WAIT, &event); - ok (hr == MF_E_NO_EVENTS_AVAILABLE, "Unexpected hr %#x.\n", hr); + expect_no_event((IMFMediaEventGenerator *)video_stream); hr = IMFMediaStream_RequestSample(video_stream, NULL); ok (hr == S_OK || hr == MF_E_END_OF_STREAM, "Unexpected hr %#x.\n", hr); expect_event((IMFMediaEventGenerator *)video_stream, MEEndOfStream, NULL); + expect_no_event((IMFMediaEventGenerator *)video_stream); } @@ -913,6 +922,7 @@ todo_wine ok(hr == MF_E_END_OF_STREAM, "Unexpected hr %#x.\n", hr); expect_event((IMFMediaEventGenerator *)mediasource, MEEndOfPresentation, NULL); + expect_no_event((IMFMediaEventGenerator *)mediasource); IMFMediaStream_Release(video_stream); IMFMediaTypeHandler_Release(handler); -- 2.33.0
Hi, While running your changed tests, I think I found new failures. Being a bot and all I'm not very good at pattern recognition, so I might be wrong, but could you please double-check? Full results can be found at: https://testbot.winehq.org/JobDetails.pl?Key=97477 Your paranoid android. === w7u_2qxl (32 bit report) === mfplat: 0834:mfplat: unhandled exception c0000005 at 6EEF7D24 === w7u_adm (32 bit report) === mfplat: 083c:mfplat: unhandled exception c0000005 at 6ED97D24 === w7u_el (32 bit report) === mfplat: 0860:mfplat: unhandled exception c0000005 at 6EDD7D24
Signed-off-by: Giovanni Mascellani <gmascellani(a)codeweavers.com> --- dlls/mfplat/tests/mfplat.c | 161 ++++++++++++++++++++++++++++++++++++- 1 file changed, 157 insertions(+), 4 deletions(-) diff --git a/dlls/mfplat/tests/mfplat.c b/dlls/mfplat/tests/mfplat.c index a2aa082fbbb..ffc939f54af 100644 --- a/dlls/mfplat/tests/mfplat.c +++ b/dlls/mfplat/tests/mfplat.c @@ -644,13 +644,14 @@ static void test_source_resolver(void) IMFGetService *get_service; IMFRateSupport *rate_support; WCHAR pathW[MAX_PATH]; - int i, sample_count; + int i, sample_count, pause_count, while_paused_count, restart_count, seek_count, last_seen; WCHAR *filename; PROPVARIANT var; HRESULT hr; GUID guid; float rate; UINT32 rotation; + BOOL ret, pause_seen; if (!pMFCreateSourceResolver) { @@ -864,9 +865,19 @@ todo_wine expect_no_event((IMFMediaEventGenerator *)video_stream); sample_count = 10; + pause_count = 2; + while_paused_count = 6; + restart_count = 7; + seek_count = 3; - for (i = 0; i < sample_count; ++i) + for (i = 0; i < while_paused_count; ++i) { + if (i == pause_count) + { + hr = IMFMediaSource_Pause(mediasource); + ok(hr == S_OK, "Could not pause media source, hr %#x.\n", hr); + } + hr = IMFMediaStream_RequestSample(video_stream, NULL); if (i == sample_count) break; @@ -875,13 +886,84 @@ todo_wine break; } - for (i = 0; i < sample_count; ++i) + pause_seen = FALSE; + + for (i = 0; ; ++i) + { + static const LONGLONG MILLI_TO_100_NANO = 10000; + LONGLONG duration, time; + DWORD buffer_count; + IMFSample *sample; + MediaEventType met; + + ret = get_event((IMFMediaEventGenerator *)video_stream, &met, &var); + if (!ret) + break; + + if (met == MEStreamPaused) + { + ok(!pause_seen, "Pause seen twice\n"); + pause_seen = TRUE; + i--; + continue; + } + + ok(i < while_paused_count, "Too many samples: %d\n", i); + ok(met == MEMediaSample, "wrong type %d\n", met); + + ok(var.vt == VT_UNKNOWN, "Unexpected value type %u from MEMediaSample event.\n", var.vt); + sample = (IMFSample *)var.punkVal; + + hr = IMFSample_GetBufferCount(sample, &buffer_count); + ok(hr == S_OK, "Failed to get buffer count, hr %#x.\n", hr); + ok(buffer_count == 1, "Unexpected buffer count %u.\n", buffer_count); + + hr = IMFSample_GetSampleDuration(sample, &duration); + ok(hr == S_OK, "Failed to get sample duration, hr %#x.\n", hr); + ok(duration == 40 * MILLI_TO_100_NANO, "Unexpected duration %s.\n", wine_dbgstr_longlong(duration)); + + hr = IMFSample_GetSampleTime(sample, &time); + ok(hr == S_OK, "Failed to get sample time, hr %#x.\n", hr); + ok(time == i * 40 * MILLI_TO_100_NANO, "Unexpected time %s.\n", wine_dbgstr_longlong(time)); + + IMFSample_Release(sample); + } + + last_seen = i; + + ok(pause_seen, "Pause not seen\n"); + + expect_event((IMFMediaEventGenerator *)mediasource, MESourcePaused, &var); + expect_no_event((IMFMediaEventGenerator *)mediasource); + expect_no_event((IMFMediaEventGenerator *)video_stream); + + for (i = while_paused_count; i < restart_count; ++i) + { + hr = IMFMediaStream_RequestSample(video_stream, NULL); + if (i == sample_count) + break; + ok(hr == S_OK, "Failed to request sample %u, hr %#x.\n", i + 1, hr); + if (hr != S_OK) + break; + } + + expect_no_event((IMFMediaEventGenerator *)mediasource); + expect_no_event((IMFMediaEventGenerator *)video_stream); + + var.vt = VT_EMPTY; + hr = IMFMediaSource_Start(mediasource, descriptor, &GUID_NULL, &var); + ok(hr == S_OK, "Failed to restart the media source, hr %#x.\n", hr); + + expect_event((IMFMediaEventGenerator *)mediasource, MEUpdatedStream, &var); + expect_event((IMFMediaEventGenerator *)mediasource, MESourceStarted, &var); + expect_event((IMFMediaEventGenerator *)video_stream, MEStreamStarted, &var); + + for (i = last_seen; i < restart_count; ++i) { static const LONGLONG MILLI_TO_100_NANO = 10000; LONGLONG duration, time; DWORD buffer_count; IMFSample *sample; - BOOL ret; ret = expect_event((IMFMediaEventGenerator *)video_stream, MEMediaSample, &var); ok(ret, "Sample %u not received.\n", i + 1); @@ -906,6 +988,77 @@ todo_wine IMFSample_Release(sample); } + expect_no_event((IMFMediaEventGenerator *)mediasource); + expect_no_event((IMFMediaEventGenerator *)video_stream); + + hr = IMFMediaSource_Pause(mediasource); + ok(hr == S_OK, "cannot pause %#x\n", hr); + + expect_event((IMFMediaEventGenerator *)mediasource, MESourcePaused, &var); + expect_event((IMFMediaEventGenerator *)video_stream, MEStreamPaused, &var); + expect_no_event((IMFMediaEventGenerator *)mediasource); + expect_no_event((IMFMediaEventGenerator *)video_stream); + + for (i = 0; i < 200; i++) + { + hr = IMFMediaStream_RequestSample(video_stream, NULL); + ok(hr == S_OK, "Failed to request sample %u, hr %#x.\n", i + 1, hr); + } + + expect_no_event((IMFMediaEventGenerator *)mediasource); + expect_no_event((IMFMediaEventGenerator *)video_stream); + + var.vt = VT_I8; + var.hVal.QuadPart = seek_count * 40 * 10000; + hr = IMFMediaSource_Start(mediasource, descriptor, &GUID_NULL, &var); + ok(hr == S_OK, "Failed to restart the media source, hr %#x.\n", hr); + + expect_event((IMFMediaEventGenerator *)mediasource, MEUpdatedStream, &var); + expect_event((IMFMediaEventGenerator *)mediasource, MESourceSeeked, &var); + expect_event((IMFMediaEventGenerator *)video_stream, MEStreamSeeked, &var); + expect_no_event((IMFMediaEventGenerator *)mediasource); + expect_no_event((IMFMediaEventGenerator *)video_stream); + + for (i = seek_count; i < sample_count; ++i) + { + hr = IMFMediaStream_RequestSample(video_stream, NULL); + if (i == sample_count) + break; + ok(hr == S_OK, "Failed to request sample %u, hr %#x.\n", i + 1, hr); + if (hr != S_OK) + break; + } + + for (i = seek_count; i < sample_count; ++i) + { + static const LONGLONG MILLI_TO_100_NANO = 10000; + LONGLONG duration, time; + DWORD buffer_count; + IMFSample *sample; + + ret = expect_event((IMFMediaEventGenerator *)video_stream, MEMediaSample, &var); + ok(ret, "Sample %u not received.\n", i + 1); + if (!ret) + break; + + ok(var.vt == VT_UNKNOWN, "Unexpected value type %u from MEMediaSample event.\n", var.vt); + sample = (IMFSample *)var.punkVal; + + hr = IMFSample_GetBufferCount(sample, &buffer_count); + ok(hr == S_OK, "Failed to get buffer count, hr %#x.\n", hr); + ok(buffer_count == 1, "Unexpected buffer count %u.\n", buffer_count); + + hr = IMFSample_GetSampleDuration(sample, &duration); + ok(hr == S_OK, "Failed to get sample duration, hr %#x.\n", hr); + ok(duration == 40 * MILLI_TO_100_NANO, "Unexpected duration %s.\n", wine_dbgstr_longlong(duration)); + + hr = IMFSample_GetSampleTime(sample, &time); + ok(hr == S_OK, "Failed to get sample time, hr %#x.\n", hr); + ok(time == i * 40 * MILLI_TO_100_NANO, "Unexpected time %s %s %d %d.\n", wine_dbgstr_longlong(time), wine_dbgstr_longlong(i * 40 * MILLI_TO_100_NANO), i, (int)(time / (40 * MILLI_TO_100_NANO))); + + IMFSample_Release(sample); + } + if (i == sample_count) { /* MEEndOfStream isn't queued until after a one request beyond the last frame is submitted */ -- 2.33.0
Hi, While running your changed tests, I think I found new failures. Being a bot and all I'm not very good at pattern recognition, so I might be wrong, but could you please double-check? Full results can be found at: https://testbot.winehq.org/JobDetails.pl?Key=97478 Your paranoid android. === w7u_2qxl (32 bit report) === mfplat: 05c8:mfplat: unhandled exception c0000005 at 6EF07D24 === w7u_adm (32 bit report) === mfplat: 08d8:mfplat: unhandled exception c0000005 at 6ECF7D24 === w7u_el (32 bit report) === mfplat: 0878:mfplat: unhandled exception c0000005 at 6EDD7D24
Hi, this patch is unfortunately getting little attention, so I'll try to add a little bit of context in the hope to convince somebody to have a look at it. Each sample emitted by a media source has attached a timestamp (often called "presentation timestamp", or PTS), which is used by the sink that will receive the sample to ensure that it is rendered at the right time. The current Wine behavior is to emit a presentation timestamp relative to the moment where playback was last started or restarted. I.e., the frame that sits two seconds in the media file would get PTS = 2 when playback started from beginning, but PTS = 1 if, for instance, playback was stopped, seeked to 1 second in the file and then restarted. This behavior is, according to my tests, wrong. Windows always emits PTS = 2 for the frame two seconds in the file, independently of the point at which playback started. I have some tests that show this in the later patches of this set. As per the subthread with Nikolay, they are not submitted for inclusion in Wine, but to ease review of the first three patches (1/7 through 3/7). You can see at https://testbot.winehq.org/JobDetails.pl?Key=97478 that the tests are correct on Windows (except for Windows 7, where the test program crashed to to a bug in Windows 7 that was later solved). You can also check that some failures appear if you run the tests in 4/7 through 7/7 and revert this patch. The actual content of this patch is pretty simple: when GStreamer returns a buffer, the PTS of that buffer is recovered in wg_parser.c (using the macro GST_BUFFER_PTS) and stored in the field event->u.buffer.pts. The previous implementation used to store also the timestamp at which the file was last restarted (stored in the start_time field of struct media_source), and subtracted that value from the PTS returned by GStreamer before passing it to SetSampleTime. With this change the subtraction is removed, as is the start_time field which is now useless. Giovanni. Il 06/09/21 17:11, Giovanni Mascellani ha scritto:
Signed-off-by: Giovanni Mascellani <gmascellani(a)codeweavers.com> --- dlls/winegstreamer/media_source.c | 5 +---- 1 file changed, 1 insertion(+), 4 deletions(-)
diff --git a/dlls/winegstreamer/media_source.c b/dlls/winegstreamer/media_source.c index 01ab626254a..cd8957db7b1 100644 --- a/dlls/winegstreamer/media_source.c +++ b/dlls/winegstreamer/media_source.c @@ -100,8 +100,6 @@ struct media_source SOURCE_SHUTDOWN, } state;
- LONGLONG start_time; - HANDLE read_thread; bool read_thread_shutdown; }; @@ -274,7 +272,6 @@ static void start_pipeline(struct media_source *source, struct source_async_comm position->vt = VT_I8; position->hVal.QuadPart = 0; } - source->start_time = position->hVal.QuadPart;
for (i = 0; i < source->stream_count; i++) { @@ -427,7 +424,7 @@ static void send_buffer(struct media_stream *stream, const struct wg_parser_even goto out; }
- if (FAILED(hr = IMFSample_SetSampleTime(sample, event->u.buffer.pts - stream->parent_source->start_time))) + if (FAILED(hr = IMFSample_SetSampleTime(sample, event->u.buffer.pts))) { ERR("Failed to set sample time, hr %#x.\n", hr); goto out;
participants (4)
-
Giovanni Mascellani -
Huw Davies -
Marvin -
Nikolay Sivov