[PATCH 1/2] xaudio2: Fix support for FAudio 19.06 and newer

Ethan Lee elee at codeweavers.com
Thu May 2 20:45:16 CDT 2019


I’m really glad you butted in because this is actually a very good point (sorry I didn’t see it earlier, I only get wine-devel messages as digests). So maybe what I can do is add something totally different like `FAudio_CommitOperationSet` and in the documentation deliberately put the phrase “CommitChanges” in so users wondering where that function went can go find it quickly. I still think my bogus CommitChanges needs to be scrapped somehow but if we can agree that any use of CommitChanges is a bug (yes, even retroactively), maybe I can just make it do something extremely unpleasant that still allows the program to run, but make it so anyone using it can see something is wrong (stderr messages maybe?).

The main thing that is a concern is that CommitChanges affects a HUGE amount of how the library works, which is why I want to destroy it as soon as I possibly can. The next release of FAudio is going to support operation sets, meaning any call that uses an operation set value other than `NOW` is going to be put in a queue somewhere, and will not be fired until Commit is called. We can’t ignore the old CommitChanges because doing so means leaving a stub, meaning commands will never be fired and will halt lots of audio activity and basically introduce a massive memory leak into the program. We also can’t make any assumptions about a default operation set because that breaks timing accuracy, meaning data can go missing and crash, or deadlocks can occur if callbacks contain threading activity in them.

So the question is: Can we agree that use of CommitChanges is an _application_ bug and tell users to migrate to CommitOperationSet, or do people expect me to somehow make CommitChanges not break as described above? If the former, cool, I can maybe make that work and deprecate the old function (I still really want to delete it at some point, though). If the latter, I’d rather people get mad at me over something that, based on the feedback presented, was a good decision compared to letting users deliberately do something that breaks programs for “compatibility”, if that phrase makes _any_ sense.

-Ethan

> Apologies for butting in, but I'm not sure this is any better. Code 
> that's written to call FAudio_CommitChanges would expect this function 
> to have the same specified behavior after an update. If code needs 
> updating to use the new version, having a separate 
> FAudio_CommitChangesBADAPI function seems a bit pointless; for the same 
> effort of changing the call to restore old behavior, you could just 
> ensure FAudio_CommitChanges is used correctly (unless the behavior 
> change is that significant, in which case it really should be a new 
> function).
> 
> When it comes to ABI changes, it should represent a massive change for 
> the interface and be a complete refresh. If you have libfaudio.so.0 and 
> libfaudio.so.1, for example, what happens if a process tries to pull in 
> both (e.g. the main app links against libfaudio.so.1, but the app also 
> links against libfoo.so which links to libfaudio.so.0)? Now you have two 
> separate FAudio_CommitChanges symbols with two different behaviors, and 
> something will break. I've actually run into this problem with SDL, 
> where I link against SDL2 and SDL_sound, but on some distros SDL_sound 
> links against SDL1; it all compiles and links fine, but misbehaves at 
> runtime for no immediately obvious reason (if I didn't know SDL_sound 
> might be linked against SDL1, while I link against SDL2, I'd be baffled 
> as to why seemingly correct calls are just failing for some people and 
> not others).



More information about the wine-devel mailing list