[PATCH v3 1/2] quartz: Cache IMediaSeeking for filters.
The Legend of Heroes: Trails of Cold Steel II crashes on its intro videos because it mistakenly destroys and frees its own filter when the IMediaSeeking object is destroyed, which is retrieved as a separate object when we query for it, with a refcount of 1 regardless of the filter's refcount. It does so with msvcrt operator delete, even if the filter had outstanding ref counts. It happens to work on Windows because Windows caches the object exposing the interface. Signed-off-by: Gabriel Ivăncescu <gabrielopcode(a)gmail.com> --- dlls/quartz/filtergraph.c | 45 +++++++++++++++++---------------------- 1 file changed, 20 insertions(+), 25 deletions(-) diff --git a/dlls/quartz/filtergraph.c b/dlls/quartz/filtergraph.c index c659cf9..1013087 100644 --- a/dlls/quartz/filtergraph.c +++ b/dlls/quartz/filtergraph.c @@ -153,6 +153,7 @@ struct filter { struct list entry, sorted_entry; IBaseFilter *filter; + IMediaSeeking *seeking; WCHAR *name; BOOL sorting; }; @@ -593,24 +594,19 @@ static BOOL has_output_pins(IBaseFilter *filter) return FALSE; } -static BOOL is_renderer(IBaseFilter *filter) +static BOOL is_renderer(struct filter *filter) { IAMFilterMiscFlags *flags; - IMediaSeeking *seeking; BOOL ret = FALSE; - if (SUCCEEDED(IBaseFilter_QueryInterface(filter, &IID_IAMFilterMiscFlags, (void **)&flags))) + if (SUCCEEDED(IBaseFilter_QueryInterface(filter->filter, &IID_IAMFilterMiscFlags, (void **)&flags))) { if (IAMFilterMiscFlags_GetMiscFlags(flags) & AM_FILTER_MISC_FLAGS_IS_RENDERER) ret = TRUE; IAMFilterMiscFlags_Release(flags); } - else if (SUCCEEDED(IBaseFilter_QueryInterface(filter, &IID_IMediaSeeking, (void **)&seeking))) - { - IMediaSeeking_Release(seeking); - if (!has_output_pins(filter)) - ret = TRUE; - } + else if (filter->seeking && !has_output_pins(filter->filter)) + ret = TRUE; return ret; } @@ -675,12 +671,17 @@ static HRESULT WINAPI FilterGraph2_AddFilter(IFilterGraph2 *iface, } IBaseFilter_AddRef(entry->filter = filter); + + /* Cache the IMediaSeeking object as some broken apps actually depend on this. */ + if (FAILED(IBaseFilter_QueryInterface(filter, &IID_IMediaSeeking, (void**)&entry->seeking))) + entry->seeking = NULL; + list_add_head(&graph->filters, &entry->entry); list_add_head(&graph->sorted_filters, &entry->sorted_entry); entry->sorting = FALSE; ++graph->version; - if (is_renderer(filter)) + if (is_renderer(entry)) ++graph->nRenderers; return duplicate_name ? VFW_S_DUPLICATE_NAME : hr; @@ -755,11 +756,13 @@ static HRESULT WINAPI FilterGraph2_RemoveFilter(IFilterGraph2 *iface, IBaseFilte hr = IBaseFilter_JoinFilterGraph(pFilter, NULL, NULL); if (SUCCEEDED(hr)) { - if (is_renderer(pFilter)) + if (is_renderer(entry)) --This->nRenderers; IBaseFilter_SetSyncSource(pFilter, NULL); IBaseFilter_Release(pFilter); + if (entry->seeking) + IMediaSeeking_Release(entry->seeking); list_remove(&entry->entry); list_remove(&entry->sorted_entry); CoTaskMemFree(entry->name); @@ -2219,13 +2222,9 @@ static HRESULT all_renderers_seek(IFilterGraphImpl *This, fnFoundSeek FoundSeek, LIST_FOR_EACH_ENTRY(filter, &This->filters, struct filter, entry) { - IMediaSeeking *seek = NULL; - - IBaseFilter_QueryInterface(filter->filter, &IID_IMediaSeeking, (void **)&seek); - if (!seek) + if (!filter->seeking) continue; - hr = FoundSeek(This, seek, arg); - IMediaSeeking_Release(seek); + hr = FoundSeek(This, filter->seeking, arg); if (hr_return != E_NOTIMPL) allnotimpl = FALSE; if (hr_return == S_OK || (FAILED(hr) && hr != E_NOTIMPL && SUCCEEDED(hr_return))) @@ -2414,7 +2413,6 @@ static HRESULT WINAPI MediaSeeking_GetStopPosition(IMediaSeeking *iface, LONGLON { IFilterGraphImpl *graph = impl_from_IMediaSeeking(iface); HRESULT hr = E_NOTIMPL, filter_hr; - IMediaSeeking *seeking; struct filter *filter; LONGLONG filter_stop; @@ -2429,11 +2427,10 @@ static HRESULT WINAPI MediaSeeking_GetStopPosition(IMediaSeeking *iface, LONGLON LIST_FOR_EACH_ENTRY(filter, &graph->filters, struct filter, entry) { - if (FAILED(IBaseFilter_QueryInterface(filter->filter, &IID_IMediaSeeking, (void **)&seeking))) + if (!filter->seeking) continue; - filter_hr = IMediaSeeking_GetStopPosition(seeking, &filter_stop); - IMediaSeeking_Release(seeking); + filter_hr = IMediaSeeking_GetStopPosition(filter->seeking, &filter_stop); if (SUCCEEDED(filter_hr)) { hr = S_OK; @@ -2507,7 +2504,6 @@ static HRESULT WINAPI MediaSeeking_SetPositions(IMediaSeeking *iface, LONGLONG * { IFilterGraphImpl *graph = impl_from_IMediaSeeking(iface); HRESULT hr = E_NOTIMPL, filter_hr; - IMediaSeeking *seeking; struct filter *filter; FILTER_STATE state; @@ -2539,12 +2535,11 @@ static HRESULT WINAPI MediaSeeking_SetPositions(IMediaSeeking *iface, LONGLONG * { LONGLONG current = current_ptr ? *current_ptr : 0, stop = stop_ptr ? *stop_ptr : 0; - if (FAILED(IBaseFilter_QueryInterface(filter->filter, &IID_IMediaSeeking, (void **)&seeking))) + if (!filter->seeking) continue; - filter_hr = IMediaSeeking_SetPositions(seeking, ¤t, + filter_hr = IMediaSeeking_SetPositions(filter->seeking, ¤t, current_flags | AM_SEEKING_ReturnTime, &stop, stop_flags); - IMediaSeeking_Release(seeking); if (SUCCEEDED(filter_hr)) { hr = S_OK; -- 2.21.0
Signed-off-by: Gabriel Ivăncescu <gabrielopcode(a)gmail.com> --- dlls/quartz/tests/filtergraph.c | 17 +++++++++++++++++ 1 file changed, 17 insertions(+) diff --git a/dlls/quartz/tests/filtergraph.c b/dlls/quartz/tests/filtergraph.c index 9988dca..af659a9 100644 --- a/dlls/quartz/tests/filtergraph.c +++ b/dlls/quartz/tests/filtergraph.c @@ -1165,6 +1165,7 @@ struct testfilter ULONG misc_flags; IMediaSeeking IMediaSeeking_iface; + LONG seeking_ref; DWORD seek_caps; BOOL support_testguid; LONGLONG seek_duration, seek_current, seek_stop; @@ -1525,12 +1526,14 @@ static HRESULT WINAPI testseek_QueryInterface(IMediaSeeking *iface, REFIID iid, static ULONG WINAPI testseek_AddRef(IMediaSeeking *iface) { struct testfilter *filter = impl_from_IMediaSeeking(iface); + InterlockedIncrement(&filter->seeking_ref); return InterlockedIncrement(&filter->ref); } static ULONG WINAPI testseek_Release(IMediaSeeking *iface) { struct testfilter *filter = impl_from_IMediaSeeking(iface); + InterlockedDecrement(&filter->seeking_ref); return InterlockedDecrement(&filter->ref); } @@ -3530,14 +3533,20 @@ static void test_ec_complete(void) hr = check_ec_complete(graph, &filter1.IBaseFilter_iface); ok(hr == S_OK, "Got hr %#x.\n", hr); + ok(filter1.seeking_ref > 0, "Unexpected seeking refcount %d.\n", filter1.seeking_ref); IFilterGraph2_RemoveFilter(graph, &filter1.IBaseFilter_iface); + ok(filter1.seeking_ref == 0, "Unexpected seeking refcount %d.\n", filter1.seeking_ref); + filter1_pin.dir = PINDIR_OUTPUT; IFilterGraph2_AddFilter(graph, &filter1.IBaseFilter_iface, NULL); hr = check_ec_complete(graph, &filter1.IBaseFilter_iface); ok(hr == E_ABORT, "Got hr %#x.\n", hr); + ok(filter1.seeking_ref > 0, "Unexpected seeking refcount %d.\n", filter1.seeking_ref); IFilterGraph2_RemoveFilter(graph, &filter1.IBaseFilter_iface); + ok(filter1.seeking_ref == 0, "Unexpected seeking refcount %d.\n", filter1.seeking_ref); + filter1.IMediaSeeking_iface.lpVtbl = NULL; filter1_pin.dir = PINDIR_INPUT; filter1.pin_count = 1; @@ -3769,6 +3778,8 @@ static void test_graph_seeking(void) hr = IMediaSeeking_GetCapabilities(seeking, &caps); ok(hr == S_OK, "Got hr %#x.\n", hr); ok(caps == AM_SEEKING_CanDoSegments, "Got caps %#x.\n", caps); + ok(filter1.seeking_ref > 0, "Unexpected seeking refcount %d.\n", filter1.seeking_ref); + ok(filter2.seeking_ref > 0, "Unexpected seeking refcount %d.\n", filter2.seeking_ref); caps = AM_SEEKING_CanDoSegments | AM_SEEKING_CanGetCurrentPos; hr = IMediaSeeking_CheckCapabilities(seeking, &caps); @@ -4082,10 +4093,16 @@ static void test_graph_seeking(void) IMediaFilter_Release(filter); IMediaControl_Release(control); IMediaSeeking_Release(seeking); + + ok(filter1.seeking_ref > 0, "Unexpected seeking refcount %d.\n", filter1.seeking_ref); + ok(filter2.seeking_ref > 0, "Unexpected seeking refcount %d.\n", filter2.seeking_ref); + ref = IFilterGraph2_Release(graph); ok(!ref, "Got outstanding refcount %d.\n", hr); ok(filter1.ref == 1, "Got outstanding refcount %d.\n", filter1.ref); ok(filter2.ref == 1, "Got outstanding refcount %d.\n", filter2.ref); + ok(filter1.seeking_ref == 0, "Unexpected seeking refcount %d.\n", filter1.seeking_ref); + ok(filter2.seeking_ref == 0, "Unexpected seeking refcount %d.\n", filter2.seeking_ref); } static void test_default_sync_source(void) -- 2.21.0
Signed-off-by: Zebediah Figura <zfigura(a)codeweavers.com>
participants (2)
-
Gabriel Ivăncescu -
Zebediah Figura