The BURIKO visual novel engine (as seen in, for example, https://store.steampowered.com/app/1200720/MakingLovers/) demands that the upstream filter tries to connect with a MPEG format type.
Then it memorizes the resolution, rejects the connection, and expects upstream to try RGB32 or RGB24 instead.
It also passes an empty string as filename, and demands this exact error code.
I have no idea why.
Wine-Bug: https://bugs.winehq.org/show_bug.cgi?id=56491
-- v4: quartz: Fix memory leak on failure path. quartz/tests: Test the new error codes. quartz: Fix error code on empty filename. quartz/tests: Test the new compressed output support. quartz: Permit compressed output from CLSID_decodebin_parser.
From: Alfred Agrell floating@muncher.se
Wine-Bug: https://bugs.winehq.org/show_bug.cgi?id=56491 --- dlls/winegstreamer/quartz_parser.c | 71 ++++++++++++++++++++++++++++++ 1 file changed, 71 insertions(+)
diff --git a/dlls/winegstreamer/quartz_parser.c b/dlls/winegstreamer/quartz_parser.c index 6c126fa13e2..7e8fc6f6098 100644 --- a/dlls/winegstreamer/quartz_parser.c +++ b/dlls/winegstreamer/quartz_parser.c @@ -1402,6 +1402,8 @@ static void parser_destroy(struct strmbase_filter *iface) free(filter); }
+static HRESULT parser_reinit_stream(struct parser *filter, bool compressed); + static HRESULT parser_init_stream(struct strmbase_filter *iface) { struct parser *filter = impl_from_strmbase_filter(iface); @@ -1414,6 +1416,25 @@ static HRESULT parser_init_stream(struct strmbase_filter *iface)
filter->streaming = true;
+ for (i = 0; i < filter->source_count; ++i) + { + struct parser_source *source = filter->sources[i]; + struct wg_format format; + bool ret; + + if (source->pin.pin.peer) + { + ret = amt_to_wg_format(&source->pin.pin.mt, &format); + assert(ret); + if (format.major_type != WG_MAJOR_TYPE_VIDEO && format.major_type != WG_MAJOR_TYPE_AUDIO) + { + HRESULT hr = parser_reinit_stream(filter, true); + if (FAILED(hr)) + return hr; + break; + } + } + } for (i = 0; i < filter->source_count; ++i) { struct parser_source *source = filter->sources[i]; @@ -1461,6 +1482,45 @@ static HRESULT parser_init_stream(struct strmbase_filter *iface) return S_OK; }
+static HRESULT parser_reinit_stream(struct parser *filter, bool compressed) +{ + LONGLONG file_size, unused; + unsigned int i; + HRESULT hr; + + wg_parser_disconnect(filter->wg_parser); + + /* read_thread() needs to stay alive to service any read requests GStreamer + * sends, so we can only shut it down after GStreamer stops. */ + filter->sink_connected = false; + WaitForSingleObject(filter->read_thread, INFINITE); + CloseHandle(filter->read_thread); + + wg_parser_destroy(filter->wg_parser); + filter->wg_parser = 0; + + if (!(filter->wg_parser = wg_parser_create(WG_PARSER_DECODEBIN, compressed))) + return E_OUTOFMEMORY; + + filter->sink_connected = true; + filter->read_thread = CreateThread(NULL, 0, read_thread, filter, 0, NULL); + + IAsyncReader_Length(filter->reader, &file_size, &unused); + + if (FAILED(hr = wg_parser_connect(filter->wg_parser, file_size))) + return hr; + + assert(wg_parser_get_stream_count(filter->wg_parser) ==filter->source_count); + + for (i = 0; i < filter->source_count; ++i) + { + struct parser_source *source = filter->sources[i]; + source->wg_stream = wg_parser_get_stream(filter->wg_parser, i); + } + + return S_OK; +} + static HRESULT parser_cleanup_stream(struct strmbase_filter *iface) { struct parser *filter = impl_from_strmbase_filter(iface); @@ -1618,6 +1678,17 @@ static HRESULT decodebin_parser_source_get_media_type(struct parser_source *pin, WG_VIDEO_FORMAT_RGB15, };
+ wg_parser_stream_get_codec_format(pin->wg_stream, &format); + + memset(mt, 0, sizeof(AM_MEDIA_TYPE)); + + if (amt_from_wg_format(mt, &format, false)) + { + if (!index--) + return S_OK; + FreeMediaType(mt); + } + wg_parser_stream_get_preferred_format(pin->wg_stream, &format);
memset(mt, 0, sizeof(AM_MEDIA_TYPE));
From: Alfred Agrell floating@muncher.se
--- dlls/quartz/tests/filtergraph.c | 200 +++++++++++++++++++++++++++++++- 1 file changed, 194 insertions(+), 6 deletions(-)
diff --git a/dlls/quartz/tests/filtergraph.c b/dlls/quartz/tests/filtergraph.c index 92baa0e74e8..32fa57b9469 100644 --- a/dlls/quartz/tests/filtergraph.c +++ b/dlls/quartz/tests/filtergraph.c @@ -817,6 +817,7 @@ static void test_render_with_multithread(void) struct testpin { IPin IPin_iface; + IMemInputPin IMemInputPin_iface; LONG ref; PIN_DIRECTION dir; struct testfilter *filter; @@ -836,6 +837,10 @@ struct testpin HRESULT Connect_hr; HRESULT EnumMediaTypes_hr; HRESULT QueryInternalConnections_hr; + + BYTE input[64]; + ULONG input_size; + HANDLE on_input_full; };
struct testfilter @@ -870,7 +875,6 @@ struct testfilter IReferenceClock IReferenceClock_iface;
IFileSourceFilter IFileSourceFilter_iface; - WCHAR filename[MAX_PATH]; };
static inline struct testpin *impl_from_IEnumMediaTypes(IEnumMediaTypes *iface) @@ -955,6 +959,11 @@ static inline struct testpin *impl_from_IPin(IPin *iface) return CONTAINING_RECORD(iface, struct testpin, IPin_iface); }
+static inline struct testpin *impl_from_IMemInputPin(IMemInputPin *iface) +{ + return CONTAINING_RECORD(iface, struct testpin, IMemInputPin_iface); +} + static HRESULT WINAPI testpin_QueryInterface(IPin *iface, REFIID iid, void **out) { struct testpin *pin = impl_from_IPin(iface); @@ -966,6 +975,12 @@ static HRESULT WINAPI testpin_QueryInterface(IPin *iface, REFIID iid, void **out IPin_AddRef(*out); return S_OK; } + if (pin->IMemInputPin_iface.lpVtbl && IsEqualGUID(iid, &IID_IMemInputPin)) + { + *out = &pin->IMemInputPin_iface; + IMemInputPin_AddRef(*out); + return S_OK; + }
*out = NULL; return E_NOINTERFACE; @@ -1970,11 +1985,7 @@ static ULONG WINAPI testfilesource_Release(IFileSourceFilter *iface) static HRESULT WINAPI testfilesource_Load(IFileSourceFilter *iface, const WCHAR *filename, const AM_MEDIA_TYPE *mt) { - struct testfilter *filter = impl_from_IFileSourceFilter(iface); - if (winetest_debug > 1) trace("%p->Load()\n", iface); - - wcscpy(filter->filename, filename); - + if (winetest_debug > 1) trace("%p->Load(%ls)\n", iface, filename); return S_OK; }
@@ -4193,6 +4204,182 @@ static void test_ec_complete(void) ok(filter3.ref == 1, "Got outstanding refcount %ld.\n", filter3.ref); }
+static HRESULT WINAPI mpegtestsink_ReceiveConnection(IPin *iface, IPin *peer, const AM_MEDIA_TYPE *mt) +{ + struct testpin *pin = impl_from_IPin(iface); + if (winetest_debug > 1) trace("%p->ReceiveConnection(%p)\n", pin, peer); + + if (!IsEqualGUID(&mt->majortype, &MEDIATYPE_Video)) + return VFW_E_TYPE_NOT_ACCEPTED; + if (!IsEqualGUID(&mt->subtype, &MEDIASUBTYPE_MPEG1Payload)) + return VFW_E_TYPE_NOT_ACCEPTED; + if (!IsEqualGUID(&mt->formattype, &FORMAT_MPEGVideo)) + return VFW_E_TYPE_NOT_ACCEPTED; + + pin->peer = peer; + IPin_AddRef(peer); + return S_OK; +} + +static HRESULT WINAPI mpegtestpin_EndOfStream(IPin *iface) +{ + return S_OK; +} + +static HRESULT WINAPI mpegtestpin_NewSegment(IPin *iface, REFERENCE_TIME start, REFERENCE_TIME stop, double rate) +{ + return S_OK; +} + +static const IPinVtbl mpegtestsink_vtbl = +{ + testpin_QueryInterface, + testpin_AddRef, + testpin_Release, + no_Connect, + mpegtestsink_ReceiveConnection, + testpin_Disconnect, + testpin_ConnectedTo, + testpin_ConnectionMediaType, + testpin_QueryPinInfo, + testpin_QueryDirection, + testpin_QueryId, + testpin_QueryAccept, + testpin_EnumMediaTypes, + testpin_QueryInternalConnections, + mpegtestpin_EndOfStream, + testpin_BeginFlush, + testpin_EndFlush, + mpegtestpin_NewSegment +}; + +static HRESULT WINAPI meminput_QueryInterface(IMemInputPin *iface, REFIID iid, void **out) +{ + struct testpin *pin = impl_from_IMemInputPin(iface); + if (winetest_debug > 1) trace("%p->QueryInterface(%s)\n", pin, wine_dbgstr_guid(iid)); + + *out = NULL; + return E_NOINTERFACE; +} + +static ULONG WINAPI meminput_AddRef(IMemInputPin *iface) +{ + struct testpin *pin = impl_from_IMemInputPin(iface); + return InterlockedIncrement(&pin->ref); +} + +static ULONG WINAPI meminput_Release(IMemInputPin *iface) +{ + struct testpin *pin = impl_from_IMemInputPin(iface); + return InterlockedDecrement(&pin->ref); +} + +static HRESULT STDMETHODCALLTYPE meminput_GetAllocator(IMemInputPin *iface, IMemAllocator **allocator) +{ + return VFW_E_NO_ALLOCATOR; +} + +static HRESULT STDMETHODCALLTYPE meminput_NotifyAllocator(IMemInputPin *iface, IMemAllocator *allocator, BOOL readonly) +{ + return S_OK; +} + +static HRESULT STDMETHODCALLTYPE meminput_GetAllocatorRequirements(IMemInputPin *iface, ALLOCATOR_PROPERTIES *props) +{ + return E_NOTIMPL; +} + +static HRESULT STDMETHODCALLTYPE meminput_Receive(IMemInputPin *iface, IMediaSample *sample) +{ + struct testpin *pin = impl_from_IMemInputPin(iface); + size_t new_bytes; + HRESULT hr; + BYTE *ptr; + long len; + + len = IMediaSample_GetActualDataLength(sample); + hr = IMediaSample_GetPointer(sample, &ptr); + + ok(hr == S_OK, "Got hr %#lx.\n", hr); + ok(len > 0, "Got %ld.\n", len); + + new_bytes = min(sizeof(pin->input) - pin->input_size, len); + if (new_bytes) + { + memcpy(pin->input + pin->input_size, ptr, new_bytes); + pin->input_size += new_bytes; + if (pin->input_size == sizeof(pin->input)) + SetEvent(pin->on_input_full); + } + + return S_OK; +} + +static HRESULT STDMETHODCALLTYPE meminput_ReceiveMultiple(IMemInputPin *iface, IMediaSample **samples, LONG n_samples, LONG *n_samples_processed) +{ + *n_samples_processed = 1; + return IMemInputPin_Receive(iface, samples[0]); +} + +static HRESULT STDMETHODCALLTYPE meminput_ReceiveCanBlock(IMemInputPin *iface) { return S_OK; } + +static const IMemInputPinVtbl mpegtestsink_meminput_vtbl = +{ + meminput_QueryInterface, + meminput_AddRef, + meminput_Release, + + meminput_GetAllocator, + meminput_NotifyAllocator, + meminput_GetAllocatorRequirements, + meminput_Receive, + meminput_ReceiveMultiple, + meminput_ReceiveCanBlock +}; + +static void test_renderfile_compressed(void) +{ + WCHAR *filename = load_resource(L"test.mpg"); + struct testpin sink_pin; + struct testfilter sink; + IMediaControl *control; + IFilterGraph2 *graph; + HRESULT hr; + DWORD ret; + + graph = create_graph(); + testpin_init(&sink_pin, &mpegtestsink_vtbl, PINDIR_INPUT); + sink_pin.IMemInputPin_iface.lpVtbl = &mpegtestsink_meminput_vtbl; + testfilter_init(&sink, &sink_pin, 1); + + hr = IFilterGraph2_AddFilter(graph, &sink.IBaseFilter_iface, L"sink"); + ok(hr == S_OK, "Got hr %#lx.\n", hr); + + hr = IFilterGraph2_RenderFile(graph, filename, NULL); + ok(hr == S_OK, "Got hr %#lx.\n", hr); + + ok(sink_pin.peer != NULL, "Expected connection.\n"); + + sink_pin.on_input_full = CreateEventW(NULL, FALSE, FALSE, NULL); + + hr = IFilterGraph2_QueryInterface(graph, &IID_IMediaControl, (void **)&control); + ok(hr == S_OK, "Got hr %#lx.\n", hr); + hr = IMediaControl_Pause(control); + ok(SUCCEEDED(hr), "Got hr %#lx.\n", hr); + + ret = WaitForSingleObject(sink_pin.on_input_full, 100); + ok(ret == WAIT_OBJECT_0, "Got %#lx.\n", ret); + + ok(sink_pin.input[0] == 0x00, "Expected MPEG sequence header.\n"); + ok(sink_pin.input[1] == 0x00, "Expected MPEG sequence header.\n"); + ok(sink_pin.input[2] == 0x01, "Expected MPEG sequence header.\n"); + ok(sink_pin.input[3] == 0xB3, "Expected MPEG sequence header.\n"); + + IMediaControl_Stop(control); + IFilterGraph2_Release(graph); + DeleteFileW(filename); +} + static void test_renderfile_failure(void) { static const char bogus_data[20] = {0xde, 0xad, 0xbe, 0xef}; @@ -5751,6 +5938,7 @@ START_TEST(filtergraph) test_sync_source(); test_filter_state(); test_ec_complete(); + test_renderfile_compressed(); test_renderfile_failure(); test_graph_seeking(); test_default_sync_source();
From: Alfred Agrell floating@muncher.se
Wine-Bug: https://bugs.winehq.org/show_bug.cgi?id=56491 --- dlls/quartz/filtergraph.c | 3 +++ 1 file changed, 3 insertions(+)
diff --git a/dlls/quartz/filtergraph.c b/dlls/quartz/filtergraph.c index f2c44498109..995c77d89b9 100644 --- a/dlls/quartz/filtergraph.c +++ b/dlls/quartz/filtergraph.c @@ -1516,6 +1516,9 @@ static HRESULT WINAPI FilterGraph2_AddSourceFilter(IFilterGraph2 *iface, TRACE("graph %p, filename %s, filter_name %s, ret_filter %p.\n", graph, debugstr_w(filename), debugstr_w(filter_name), ret_filter);
+ if (!*filename) + return VFW_E_NOT_FOUND; + if (!get_media_type(filename, NULL, NULL, &clsid)) clsid = CLSID_AsyncReader; TRACE("Using source filter %s.\n", debugstr_guid(&clsid));
From: Alfred Agrell floating@muncher.se
--- dlls/quartz/tests/filtergraph.c | 13 +++++++++++++ 1 file changed, 13 insertions(+)
diff --git a/dlls/quartz/tests/filtergraph.c b/dlls/quartz/tests/filtergraph.c index 32fa57b9469..dc68226ec94 100644 --- a/dlls/quartz/tests/filtergraph.c +++ b/dlls/quartz/tests/filtergraph.c @@ -5353,6 +5353,19 @@ static void test_add_source_filter(void) else skip("Not enough permission to register media types.\n");
+ /* Test some failure cases. */ + + filter = (IBaseFilter *)0xdeadbeef; + hr = IFilterGraph2_AddSourceFilter(graph, L"", NULL, &filter); + /* the BURIKO visual novel engine requires this exact error code */ + ok(hr == VFW_E_NOT_FOUND, "Got hr %#lx.\n", hr); + ok(filter == (IBaseFilter *)0xdeadbeef, "Got %p.\n", filter); + + filter = (IBaseFilter *)0xdeadbeef; + hr = IFilterGraph2_AddSourceFilter(graph, L"doesnt_exist.mp3", NULL, &filter); + ok(hr == HRESULT_FROM_WIN32(ERROR_FILE_NOT_FOUND), "Got hr %#lx.\n", hr); + ok(filter == (IBaseFilter *)0xdeadbeef, "Got %p.\n", filter); + ref = IFilterGraph2_Release(graph); ok(!ref, "Got outstanding refcount %ld.\n", ref); }
From: Alfred Agrell floating@muncher.se
--- dlls/quartz/filtergraph.c | 1 + 1 file changed, 1 insertion(+)
diff --git a/dlls/quartz/filtergraph.c b/dlls/quartz/filtergraph.c index 995c77d89b9..4696d5ced01 100644 --- a/dlls/quartz/filtergraph.c +++ b/dlls/quartz/filtergraph.c @@ -1542,6 +1542,7 @@ static HRESULT WINAPI FilterGraph2_AddSourceFilter(IFilterGraph2 *iface, if (FAILED(hr)) { WARN("Failed to load file, hr %#lx.\n", hr); + IBaseFilter_Release(filter); return hr; }
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=144609
Your paranoid android.
=== w11pro64_amd (64 bit report) ===
quartz: filtergraph.c:1494: Test failed: Unexpected call. filtergraph.c:4359: Test failed: Got hr 0x80040218. filtergraph.c:4361: Test failed: Expected connection. filtergraph.c:4371: Test failed: Got 0x102. filtergraph.c:4375: Test failed: Expected MPEG sequence header. filtergraph.c:4376: Test failed: Expected MPEG sequence header.
I don't hate 2/5, but I'm not sure I really want to add more code to work around this, instead of just preferring the real MPEG-1 splitter. We have all the infrastructure in place to output compressed samples like on Windows, we just need to hook it up here.
That would mean adding the relevant formats to autoplug-continue, and then lowering the priority of the decodebin parser so that it's below that of the MPEG splitter, probably to MERIT_NORMAL - 1. That also puts it below the AVI splitter, and in order to avoid regressions from unknown formats, that means we should port the AVI splitter over to the same scheme the MPEG splitter uses, i.e. an equivalent of 5c24b7e56. Fortunately I think that should all be pretty easy.
On Thu Apr 4 00:34:41 2024 +0000, Elizabeth Figura wrote:
I don't hate 2/5, but I'm not sure I really want to add more code to work around this, instead of just preferring the real MPEG-1 splitter. We have all the infrastructure in place to output compressed samples like on Windows, we just need to hook it up here. That would mean adding the relevant formats to autoplug-continue, and then lowering the priority of the decodebin parser so that it's below that of the MPEG splitter, probably to MERIT_NORMAL - 1. That also puts it below the AVI splitter, and in order to avoid regressions from unknown formats, that means we should port the AVI splitter over to the same scheme the MPEG splitter uses, i.e. an equivalent of 5c24b7e56. Fortunately I think that should all be pretty easy.
(2/5 is just adding a test, I'll assume you mean 1/5.)
Yeah, I'm not sure about that part either. I'm not sure about any of this, other than what BURIKO needs, and I have no idea about _why_ any of that.
As mentioned in bugzilla, using the MPEG splitter and decoders may have a performance impact, even in applications that don't care about compressed MPEG; that's why I chose this approach. And, as you said, changing filter priorities yields a regression risk.
But if you feel that the improved do-it-right-ness outweighs said drawbacks, then so be it. Please confirm which way you want it.
(2/5 is just adding a test, I'll assume you mean 1/5.)
Yes, sorry.
Yeah, I'm not sure about that part either. I'm not sure about any of this, other than what BURIKO needs, and I have no idea about _why_ any of that.
As mentioned in bugzilla, using the MPEG splitter and decoders may have a performance impact, even in applications that don't care about compressed MPEG; that's why I chose this approach. And, as you said, changing filter priorities yields a regression risk.
But if you feel that the improved do-it-right-ness outweighs said drawbacks, then so be it. Please confirm which way you want it.
Yes, it makes performance worse and debugging harder, but at this point the ship has sailed; that's a problem we just need to deal with. That was one of the motivations for delaying doing the work to implement compressed output support, but now we've reached the point where we need it. I think the tradeoffs at this point are such that always using the real MPEG-1 splitter and AVI splitter is the right option.
-
Alfred Agrell -
Alfred Agrell (＠Alcaro)
-
Elizabeth Figura (＠zfigura)
-
Marvin