Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Force the Apple Video/Audio Toolbox in GStreamer transforms #4

Closed
wants to merge 3 commits into from

Conversation

101arrowz
Copy link

I noticed some situations in which GStreamer fails to initialize when launching Steam because it can't find a way to decode H.264 byte-stream video via Quartz or MF (not sure). I'm don't know why Steam needs to decode H.264 at launch (maybe to probe Windows features?) In any case, this patch fixes the issue and should make H.264 and AAC decoding more robust.

Whisky Wine doesn't bundle libgstlibav.dylib, which links to FFmpeg and is what is typically used to actually decode AV streams with GStreamer. As a result, wg_transform_create with these kinds of streams always fails. Whisky Wine does bundle libgstapplemedia.dylib, which includes integrations with Apple Video Toolbox and Audio Toolbox. The Video Toolbox (vtdec or vtdec_hw in GStreamer) expects a packetized H.264 stream, i.e. stream-format = avc and not byte-stream; likewise, the Audio Toolbox expects a pre-framed input stream. We can use h264parse and aacparse for these, and this patch essentially forces that pre-parsing to happen.

This doesn't seem to always be the codepath for general video decoding; Wine seems to use GStreamer's autoplug for that in some situations via wg_parser.c. I'm not actually sure what situations would trigger this patch specifically, but given that this causes GStreamer transform initializations to actually work; it might help.

Note: this change is ported from this patch for CX24; it was only tested on CX24, not Whisky Wine

@101arrowz
Copy link
Author

P.S. You may want to consider symlinking mfplat and winegstreamer together, since it seems they're the same.

@JoshuaBrest
Copy link

Note: this change is ported from this patch for CX24; it was only tested on CX24, not Whisky Wine

So you haven't tested it? I'm confused.

@101arrowz
Copy link
Author

I have not tested this change with Whisky but it works fine on CX24. There isn't much to test for what it's worth; just verifying the transforms initialize properly when booting Steam. I can do it after the CI build completes.

@Gcenx
Copy link

Gcenx commented May 16, 2024

I have not tested this change with Whisky

That’s the first thing you should have done.

There isn't much to test for what it's worth; just verifying the transforms initialize properly when booting Steam.

That’s really not a good test case.

I can do it after the CI build completes.

You should have at least built & tested this locally.

@101arrowz
Copy link
Author

If you insist that I test locally before approving CI, I'll try to test this in a few days. I don't know of any specific way to test the behavior of wg_transform_create beyond the Steam launcher because I haven't actually encountered any situations that call it directly where this change alone fixes all issues. I tried using TopoEdit from the Media Foundation SDK, but it doesn't seem to ever invoke this function directly, instead prefering functions in wg_parser.c. If you have a better suggestion let me know.

@101arrowz
Copy link
Author

FYI your build script is broken by this update to GCC 14 in the default MinGW bottle in Homebrew, which makes incompatible pointer conversions an error by default. You may want to update your $CROSSCFLAGS.

@JoshuaBrest
Copy link

JoshuaBrest commented May 16, 2024 via email

@101arrowz
Copy link
Author

101arrowz commented May 16, 2024

I am not building the Unix side of Wine with GCC. I am building Unix-side with the LLVM provided in the cx-llvm bottle. However the build script installs MinGW, which cross-compiles the NT-side programs and DLLs within Wine with GCC, and I am pointing out that the MinGW Homebrew bottle has been updated to GCC 14, which breaks things. You'll need to add -Wno-incompatible-pointer-types (and -Wno-int-conversion) to CROSSCFLAGS, or forcibly install GCC 13 (probably need to build from source for that rather than using a Homebrew bottle).

@101arrowz
Copy link
Author

Tested the previous commit and it worked fine on Steam. I made more changes to resolve some issues with Media Foundation's TopoEdit; things still aren't fully working in TopoEdit but they're better than before. This change essentially allows any program that uses CLSID_CMSH264DecoderMFT to function properly.

@Gcenx
Copy link

Gcenx commented May 16, 2024

Does this change make any games playable as that's what anyone will care about for Whisky, I know upstream wine has had expanded applemedia plugin support (that's mainly irrelevant for official Winehq packages) that's part of CrossOver Preview.

@101arrowz
Copy link
Author

I don't know if this makes any games work better because I don't play any games that use the Media Foundation API. I would expect that these changes would help reduce bugs in those games, but I can't say for sure. All the games I do play work OK on Whisky (but are slower than CrossOver).

@Gcenx
Copy link

Gcenx commented May 16, 2024

Without a viable test case I won’t approve this PR, that’s not to say that @IsaacMarovitz will share my view.

As there’s now Artifacts anyone is free to test this and report back any games that now work with this over the current WhiskyWine release.

@IsaacMarovitz
Copy link
Member

@101arrowz Would a better solution not be to bundle libgstlibav.dylib and its dependencies?

@Gcenx
Copy link

Gcenx commented May 17, 2024

@IsaacMarovitz yes it would be better to just bundle libgstlibav.dylib with FFmpeg.

@101arrowz
Copy link
Author

I assumed you had a reason for not bundling it (maybe size or initialization time) but yes, it's probably better to just use FFmpeg here.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants