[PATCH 0/4] MR3792: Fill in some VMR9 stubs
This resolves 2 of 8 missing features needed by the Kirikiri visual novel engine. -- https://gitlab.winehq.org/wine/wine/-/merge_requests/3792
From: Alfred Agrell <floating(a)muncher.se> --- dlls/quartz/vmr9.c | 10 ++++++++-- 1 file changed, 8 insertions(+), 2 deletions(-) diff --git a/dlls/quartz/vmr9.c b/dlls/quartz/vmr9.c index 8eee6587182..9ea3374ca96 100644 --- a/dlls/quartz/vmr9.c +++ b/dlls/quartz/vmr9.c @@ -2118,9 +2118,15 @@ static HRESULT WINAPI VMR9SurfaceAllocatorNotify_AllocateSurfaceHelper(IVMRSurfa static HRESULT WINAPI VMR9SurfaceAllocatorNotify_NotifyEvent(IVMRSurfaceAllocatorNotify9 *iface, LONG code, LONG_PTR param1, LONG_PTR param2) { struct quartz_vmr *This = impl_from_IVMRSurfaceAllocatorNotify9(iface); + IMediaEventSink *sink; + HRESULT hr; - FIXME("(%p/%p)->(...) stub\n", iface, This); - return E_NOTIMPL; + hr = IFilterGraph_QueryInterface(This->renderer.filter.graph, &IID_IMediaEventSink, (void **)&sink); + if (FAILED(hr)) + return hr; + hr = IMediaEventSink_Notify(sink, code, param1, param2); + IMediaEventSink_Release(sink); + return hr; } static const IVMRSurfaceAllocatorNotify9Vtbl VMR9_SurfaceAllocatorNotify_Vtbl = -- GitLab https://gitlab.winehq.org/wine/wine/-/merge_requests/3792
From: Alfred Agrell <floating(a)muncher.se> --- dlls/quartz/vmr9.c | 29 ++++++++++++++++++++++++++++- 1 file changed, 28 insertions(+), 1 deletion(-) diff --git a/dlls/quartz/vmr9.c b/dlls/quartz/vmr9.c index 9ea3374ca96..b278bfa8ce9 100644 --- a/dlls/quartz/vmr9.c +++ b/dlls/quartz/vmr9.c @@ -95,6 +95,7 @@ struct quartz_vmr */ VMR9Mode mode; BITMAPINFOHEADER bmiheader; + const GUID *current_subtype; HMODULE hD3d9; @@ -441,6 +442,7 @@ static HRESULT allocate_surfaces(struct quartz_vmr *filter, const AM_MEDIA_TYPE if (IsEqualGUID(&mt->subtype, formats[i].subtype)) { info.Format = formats[i].format; + filter->current_subtype = formats[i].subtype; if (formats[i].flags & VMR9AllocFlag_TextureSurface) { @@ -2003,12 +2005,37 @@ static HRESULT WINAPI VMR9SurfaceAllocatorNotify_ChangeD3DDevice(IVMRSurfaceAllo { struct quartz_vmr *This = impl_from_IVMRSurfaceAllocatorNotify9(iface); - FIXME("(%p/%p)->(...) semi-stub\n", iface, This); if (This->allocator_d3d9_dev) IDirect3DDevice9_Release(This->allocator_d3d9_dev); This->allocator_d3d9_dev = device; IDirect3DDevice9_AddRef(This->allocator_d3d9_dev); + if (This->num_surfaces) + { + VIDEOINFOHEADER hd = { + { 0, 0, This->bmiheader.biWidth, This->bmiheader.biHeight }, + { 0, 0, This->bmiheader.biWidth, This->bmiheader.biHeight }, + This->bmiheader.biSizeImage, + 0, + 0, + This->bmiheader + }; + AM_MEDIA_TYPE mt = { + MEDIATYPE_Video, + *This->current_subtype, + TRUE, + TRUE, + This->bmiheader.biSizeImage, + FORMAT_VideoInfo, + NULL, + sizeof(hd), + (BYTE *)&hd + }; + + vmr_disconnect(&This->renderer); + vmr_connect(&This->renderer, &mt); + } + return S_OK; } -- GitLab https://gitlab.winehq.org/wine/wine/-/merge_requests/3792
From: Alfred Agrell <floating(a)muncher.se> --- dlls/quartz/tests/vmr9.c | 37 +++++++++++++++++++++++++++++++++++++ 1 file changed, 37 insertions(+) diff --git a/dlls/quartz/tests/vmr9.c b/dlls/quartz/tests/vmr9.c index 9d88afb295d..a859673ceb6 100644 --- a/dlls/quartz/tests/vmr9.c +++ b/dlls/quartz/tests/vmr9.c @@ -4267,6 +4267,42 @@ static void test_unconnected_eos(void) ok(!ref, "Got outstanding refcount %ld.\n", ref); } +static void test_notifyevent(void) +{ + IFilterGraph2 *graph = create_graph(); + IBaseFilter *filter = create_vmr9(VMR9Mode_Renderless); + IVMRSurfaceAllocatorNotify9 *notify; + IMediaEvent *eventsrc; + unsigned int ret; + HRESULT hr; + ULONG ref; + + hr = IFilterGraph2_AddFilter(graph, filter, NULL); + ok(hr == S_OK, "Got hr %#lx.\n", hr); + + hr = IFilterGraph2_QueryInterface(graph, &IID_IMediaEvent, (void **)&eventsrc); + ok(hr == S_OK, "Got hr %#lx.\n", hr); + + ret = check_event_code(eventsrc, 0, 0x12345678, 0x9ABC, 0xDEF0); + ok(ret == 0, "Got %u custom events.\n", ret); + + hr = IBaseFilter_QueryInterface(filter, &IID_IVMRSurfaceAllocatorNotify9, (void **)¬ify); + ok(hr == S_OK, "Got hr %#lx.\n", hr); + hr = IVMRSurfaceAllocatorNotify9_NotifyEvent(notify, 0x12345678, 0x9ABC, 0xDEF0); + ok(hr == S_OK, "Got hr %#lx.\n", hr); + + ret = check_event_code(eventsrc, 0, 0x12345678, 0x9ABC, 0xDEF0); + ok(ret == 1, "Got %u custom events.\n", ret); + + IMediaEvent_Release(eventsrc); + ref = IFilterGraph2_Release(graph); + ok(!ref, "Got outstanding refcount %ld.\n", ref); + ref = IBaseFilter_Release(filter); + ok(!ref, "Got outstanding refcount %ld.\n", ref); + ref = IVMRSurfaceAllocatorNotify9_Release(notify); + ok(!ref, "Got outstanding refcount %ld.\n", ref); +} + START_TEST(vmr9) { IBaseFilter *filter; @@ -4303,6 +4339,7 @@ START_TEST(vmr9) test_windowless_size(); test_mixing_prefs(); test_unconnected_eos(); + test_notifyevent(); CoUninitialize(); } -- GitLab https://gitlab.winehq.org/wine/wine/-/merge_requests/3792
From: Alfred Agrell <floating(a)muncher.se> --- dlls/quartz/tests/vmr9.c | 95 ++++++++++++++++++++++++++++++++++++++++ dlls/quartz/vmr9.c | 3 +- 2 files changed, 97 insertions(+), 1 deletion(-) diff --git a/dlls/quartz/tests/vmr9.c b/dlls/quartz/tests/vmr9.c index a859673ceb6..411fc7b2c15 100644 --- a/dlls/quartz/tests/vmr9.c +++ b/dlls/quartz/tests/vmr9.c @@ -4303,6 +4303,100 @@ static void test_notifyevent(void) ok(!ref, "Got outstanding refcount %ld.\n", ref); } +static void test_changed3ddevice(void) +{ + VIDEOINFOHEADER vih = + { + .bmiHeader.biSize = sizeof(BITMAPINFOHEADER), + .bmiHeader.biBitCount = 32, + .bmiHeader.biWidth = 32, + .bmiHeader.biHeight = 16, + .bmiHeader.biPlanes = 1, + .bmiHeader.biCompression = BI_RGB, + }; + AM_MEDIA_TYPE req_mt = + { + .majortype = MEDIATYPE_Video, + .subtype = MEDIASUBTYPE_RGB32, + .formattype = FORMAT_VideoInfo, + .cbFormat = sizeof(vih), + .pbFormat = (BYTE *)&vih, + }; + struct presenter presenter = + { + .IVMRSurfaceAllocator9_iface.lpVtbl = &allocator_vtbl, + .IVMRImagePresenter9_iface.lpVtbl = &presenter_vtbl, + .refcount = 1, + .accept_flags = VMR9AllocFlag_TextureSurface, + }; + IBaseFilter *filter = create_vmr9(VMR9Mode_Renderless); + IFilterGraph2 *graph = create_graph(); + IVMRSurfaceAllocatorNotify9 *notify; + IDirect3DDevice9 *device, *device2; + RECT rect = {0, 0, 640, 480}; + struct testfilter source; + HWND window; + HRESULT hr; + ULONG ref; + IPin *pin; + + testfilter_init(&source); + + AdjustWindowRect(&rect, WS_OVERLAPPEDWINDOW, FALSE); + window = CreateWindowA("static", "quartz_test", WS_OVERLAPPEDWINDOW, 0, 0, + rect.right - rect.left, rect.bottom - rect.top, NULL, NULL, NULL, NULL); + if (!(device = create_device(window))) + { + IBaseFilter_Release(filter); + DestroyWindow(window); + return; + } + + IBaseFilter_QueryInterface(filter, &IID_IVMRSurfaceAllocatorNotify9, (void **)¬ify); + presenter.notify = notify; + + hr = IVMRSurfaceAllocatorNotify9_AdviseSurfaceAllocator(notify, 0xabacab, + &presenter.IVMRSurfaceAllocator9_iface); + ok(hr == S_OK, "Got hr %#lx.\n", hr); + + hr = IVMRSurfaceAllocatorNotify9_SetD3DDevice(notify, device, MonitorFromWindow(window, MONITOR_DEFAULTTOPRIMARY)); + if (hr == E_NOINTERFACE) + { + win_skip("Direct3D does not support video rendering.\n"); + goto out; + } + ok(hr == S_OK, "Got hr %#lx.\n", hr); + + IFilterGraph2_AddFilter(graph, &source.filter.IBaseFilter_iface, NULL); + IFilterGraph2_AddFilter(graph, filter, NULL); + + IBaseFilter_FindPin(filter, L"VMR Input0", &pin); + hr = IFilterGraph2_ConnectDirect(graph, &source.source.pin.IPin_iface, pin, &req_mt); + ok(hr == S_OK, "Got hr %#lx.\n", hr); + + device2 = create_device(window); + ok(device2 != NULL, "Out of memory\n"); + + ok(presenter.got_TerminateDevice == 0, "got %d\n", presenter.got_TerminateDevice); + + hr = IVMRSurfaceAllocatorNotify9_ChangeD3DDevice(notify, device, MonitorFromWindow(window, MONITOR_DEFAULTTOPRIMARY)); + ok(hr == S_OK, "Got hr %#lx.\n", hr); + ok(presenter.got_TerminateDevice == 1, "got %d\n", presenter.got_TerminateDevice); + + IDirect3DDevice9_Release(device2); + +out: + IPin_Release(pin); + ref = IFilterGraph2_Release(graph); + ok(!ref, "Got outstanding refcount %ld.\n", ref); + IVMRSurfaceAllocatorNotify9_Release(notify); + ref = IBaseFilter_Release(filter); + ok(!ref, "Got outstanding refcount %ld.\n", ref); + ref = IDirect3DDevice9_Release(device); + ok(!ref, "Got outstanding refcount %ld.\n", ref); + DestroyWindow(window); +} + START_TEST(vmr9) { IBaseFilter *filter; @@ -4340,6 +4434,7 @@ START_TEST(vmr9) test_mixing_prefs(); test_unconnected_eos(); test_notifyevent(); + test_changed3ddevice(); CoUninitialize(); } diff --git a/dlls/quartz/vmr9.c b/dlls/quartz/vmr9.c index b278bfa8ce9..8cbbcd24f79 100644 --- a/dlls/quartz/vmr9.c +++ b/dlls/quartz/vmr9.c @@ -2001,7 +2001,8 @@ static HRESULT WINAPI VMR9SurfaceAllocatorNotify_SetD3DDevice(IVMRSurfaceAllocat return S_OK; } -static HRESULT WINAPI VMR9SurfaceAllocatorNotify_ChangeD3DDevice(IVMRSurfaceAllocatorNotify9 *iface, IDirect3DDevice9 *device, HMONITOR monitor) +static HRESULT WINAPI VMR9SurfaceAllocatorNotify_ChangeD3DDevice(IVMRSurfaceAllocatorNotify9 *iface, + IDirect3DDevice9 *device, HMONITOR monitor) { struct quartz_vmr *This = impl_from_IVMRSurfaceAllocatorNotify9(iface); -- GitLab https://gitlab.winehq.org/wine/wine/-/merge_requests/3792
Hi, It looks like your patch introduced the new failures shown below. Please investigate and fix them before resubmitting your patch. If they are not new, fixing them anyway would help a lot. Otherwise please ask for the known failures list to be updated. The tests also ran into some preexisting test failures. If you know how to fix them that would be helpful. See the TestBot job for the details: The full results can be found at: https://testbot.winehq.org/JobDetails.pl?Key=137242 Your paranoid android. === w8 (32 bit report) === quartz: 0c84:vmr9: unhandled exception c0000005 at 0049F344 === w8adm (32 bit report) === quartz: 0a9c:vmr9: unhandled exception c0000005 at 0049F344 === w864 (32 bit report) === quartz: 0920:vmr9: unhandled exception c0000005 at 0049F344 === w1064v1507 (32 bit report) === quartz: 05c8:vmr9: unhandled exception c0000005 at 0049F349 === w1064v1809 (32 bit report) === quartz: 1d84:vmr9: unhandled exception c0000005 at 0049F349 === w1064_tsign (32 bit report) === quartz: 1f60:vmr9: unhandled exception c0000005 at 0049F344 === w10pro64 (32 bit report) === quartz: 21d4:vmr9: unhandled exception c0000005 at 0049F344 === w864 (64 bit report) === quartz: 0b78:vmr9: unhandled exception c0000005 at 000000000048911D === w1064v1507 (64 bit report) === quartz: 0dd0:vmr9: unhandled exception c0000005 at 0000000000489120 === w1064v1809 (64 bit report) === quartz: 1d24:vmr9: unhandled exception c0000005 at 0000000000489120 === w1064_2qxl (64 bit report) === quartz: 1dcc:vmr9: unhandled exception c0000005 at 000000000048911D === w1064_adm (64 bit report) === quartz: 1da0:vmr9: unhandled exception c0000005 at 0000000000489120 === w1064_tsign (64 bit report) === quartz: 1dc0:vmr9: unhandled exception c0000005 at 0000000000489120 === w10pro64 (64 bit report) === quartz: 0e0c:vmr9: unhandled exception c0000005 at 000000000048911D === w10pro64_en_AE_u8 (64 bit report) === quartz: 23e4:vmr9: unhandled exception c0000005 at 000000000048911D === w10pro64_ar (64 bit report) === quartz: 0cb4:vmr9: unhandled exception c0000005 at 000000000048911D === w10pro64_ja (64 bit report) === quartz: 21d8:vmr9: unhandled exception c0000005 at 0000000000489120 === w10pro64_zh_CN (64 bit report) === quartz: 2250:vmr9: unhandled exception c0000005 at 000000000048911D === w11pro64_amd (64 bit report) === quartz: vmr9.c:4384: Test failed: got 0
Thanks for the patches! I have a few comments: * In 1/4, let's please replace the FIXME with a TRACE instead of deleting it. And because I'm particular about trace format, I'd like to prescribe the following: TRACE("filter %p, code %#x, param1 %#Ix, param2 %#Ix.\n", filter, code, param1, param2); * 2/4 could use a more descriptive title, along the lines of "recreate surfaces". * I don't like the idea of calling vmr_connect() / vmr_disconnect() directly. Also, I wouldn't guess we need to mess with the window position and size since it should be the same. In practice I think all we should need to do is call allocate_surfaces(), as well as factoring out and calling an equivalent function from vmr_disconnect(). * We shouldn't need to save or reconstruct the connection media type; it's stored in the pin (filter->renderer.sink.pin.mt). Yes, this means that filter->bmiheader is redundant, especially when we have get_bitmap_header(). * In 4/4, you create a second device but don't actually use it, I assume by accident? Also, "out of memory" is... a bit odd; I'd just write "Failed to create device." -- https://gitlab.winehq.org/wine/wine/-/merge_requests/3792#note_44929
- Good point. It's not a common function (as evidenced by it being unimplemented thus far), but more trace always helps. - Sure, can do - Yeah, I'm not sure either. I usually aim for the smallest change I can find, especially when I'm new to a codebase, but that isn't always the best solution. - Oh, so it does exist. I tried looking for it, but there's so many nested objects I got lost. Should I refactor out that bmiheader field while I'm at it? - Yeah, that's a typo. Good catch. And yeah, better unspecific than risking an incorrect guess. Will fix as soon as I've figured out how to convince Git to do what I want. -- https://gitlab.winehq.org/wine/wine/-/merge_requests/3792#note_44933
Oh, so it does exist. I tried looking for it, but there's so many nested objects I got lost.
Yeah, strmbase is... probably a little overdesigned. The helpers are great if you know the code, but not exactly friendly to a new developer :-/
Should I refactor out that bmiheader field while I'm at it?
That would be welcome. It should be a separate patch, though. -- https://gitlab.winehq.org/wine/wine/-/merge_requests/3792#note_44934
participants (4)
-
Alfred Agrell -
Alfred Agrell (＠Alcaro)
-
Marvin -
Zebediah Figura (＠zfigura)