quartz: Fixes for the BURIKO visual novel engine's videos
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.
Merge request reports
Activity
added 90 commits
-
afea37c0...a2c20d0e - 85 commits from branch
wine:master
- e8a06746 - quartz: Permit compressed output from CLSID_decodebin_parser.
- 2f878aca - quartz/tests: Test the new compressed output support.
- c7acd6c0 - quartz: Fix error code on empty filename.
- 62fd03f0 - quartz/tests: Test the new error codes.
- 0b3d931a - quartz: Fix memory leak on failure path.
Toggle commit list-
afea37c0...a2c20d0e - 85 commits from branch
added 15 commits
-
0b3d931a...8568848b - 10 commits from branch
wine:master
- a10c8371 - quartz: Permit compressed output from CLSID_decodebin_parser.
- a46a321a - tests/quartz: Test the new compressed output support.
- ea252d81 - quartz: Fix error code on empty filename.
- 0c25da83 - tests/quartz: Test the new error codes.
- 5023503d - quartz: Fix memory leak on failure path.
Toggle commit list-
0b3d931a...8568848b - 10 commits from branch
added 38 commits
-
5023503d...bc52693f - 33 commits from branch
wine:master
- 8d9dcc9b - quartz: Permit compressed output from CLSID_decodebin_parser.
- 32189c67 - quartz/tests: Test the new compressed output support.
- c05bd587 - quartz: Fix error code on empty filename.
- d2704a08 - quartz/tests: Test the new error codes.
- 797918ae - quartz: Fix memory leak on failure path.
Toggle commit list-
5023503d...bc52693f - 33 commits from branch
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 5c24b7e5. 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.
Sure, done. Except that autoplug-continue thing you mentioned; the avisplit tests still pass, so I think it already supports everything it should. If not, the CI will remind me.
I also did the same change to wavparse, then deleted the now-single-member wg_parser_type enum - if every caller passes the same value, it's just a waste of space. (I can drop those commits if you don't like them, I don't think they yield any functional changes. They're just cleanups.)
Maybe it's possible to add GstAllocator support and tell GStreamer to write directly into IMediaSample; that'd reduce the copies to just a few thread sync ops. But that's out of scope for this MR.
And if not (and until then), computers have gotten faster in the 13 years since decodebin_parser was created (a2916f3a - though the name is newer), the overhead is probably less noticable than it would've been back then.
added 8 commits
- 8b589335 - winegstreamer: Create decodebin instead of avidemux.
- 54318c0d - winegstreamer: Use decodebin instead of wavparse.
- 32f5073c - winegstreamer: Delete now-meaningless wg_parser_type enum.
- a0fa9fcb - winegstreamer: Reduce CLSID_decodebin_parser merit, so the MPEG splitter is...
- 978ef86e - quartz/tests: Test that compressed formats are offered for MPEGs.
- 616407a1 - quartz: Fix error code on empty filename.
- 0e887273 - quartz/tests: Test the new error codes.
- 74c20245 - quartz: Fix memory leak on failure path.
Toggle commit listLooks good, thanks. I'm seeing some test failures however: https://testbot.winehq.org/JobDetails.pl?Key=144685
The timeout is probably because 100 ms just isn't enough; I'd turn that up to at least 1 second.
The Windows 11 failures are weirder. I've never seen the filter graph use IBaseFilter::FindPin() before and I don't know what it's being used for, but implementing it does make things basically work, at least.
A baffling error indeed. Especially considering it only shows up on that AMD VM.
I guess the AMD GPU drivers are involved somehow, and doing something silly. Didn't want to deal with refcounts and potential ref cycles, so they use the pin name, maybe. Or rather, they use the pin ID, which, for some reason, is distinct from the name.
Well, no point guessing. Let's just implement it, it's the Right Thing to do. (And let's fill in actual names and IDs, to avoid the risk of FindPin(L"") returning wrong thing on a filter with multiple pins.)
e: mixed up a few words
Edited by Alfred Agrell
added 78 commits
-
5cae8276...00198c40 - 70 commits from branch
wine:master
- 64d3fce3 - winegstreamer: Create decodebin instead of avidemux.
- fa673b67 - winegstreamer: Use decodebin instead of wavparse.
- 3250345a - winegstreamer: Delete now-meaningless wg_parser_type enum.
- a4b17cb5 - winegstreamer: Reduce CLSID_decodebin_parser merit, so the MPEG splitter is...
- d41b6b6a - quartz/tests: Test that compressed formats are offered for MPEGs.
- 0dcbccea - quartz: Fix error code on empty filename.
- 30c2af58 - quartz/tests: Test the new error codes.
- e610e984 - quartz: Fix memory leak on failure path.
Toggle commit list-
5cae8276...00198c40 - 70 commits from branch
added 62 commits
-
e610e984...b85fff5a - 54 commits from branch
wine:master
- 5b24cc1a - winegstreamer: Create decodebin instead of avidemux.
- 460c28c4 - winegstreamer: Use decodebin instead of wavparse.
- d71846d0 - winegstreamer: Delete now-meaningless wg_parser_type enum.
- e870f31d - winegstreamer: Reduce CLSID_decodebin_parser merit, so the MPEG splitter is...
- 16579ffb - quartz/tests: Test that compressed formats are offered for MPEGs.
- 6a3bec8e - quartz: Fix error code on empty filename.
- 56dfb5bb - quartz/tests: Test the new error codes.
- 917bd3ca - quartz: Fix memory leak on failure path.
Toggle commit list-
e610e984...b85fff5a - 54 commits from branch