On Fri Sep 6 07:34:21 2024 +0000, Rémi Bernon wrote:
I'm trying to ask for details because I've seen two reported
deadlocks, both of which were application bugs, and according to my analyses, changing the threads we make those requests from isn't actually going to fix anything. Doesn't help much to say things are application bugs. if they work on Windows it's a Wine bug, regardless of how theoretically incorrect the application is.
Rewriting an entire component is a waste of time if we don't fully
understand a deadlock and why Windows avoids it, and why we can be sure we will also avoid it. At least it would be frontend API specific problems to solve, which can be solved without worrying about breaking other frontends with some backend threading changes.
And, again, we can change the threads we make read requests from
without needing to rewrite anything. You can handle pending read requests from the same thread that you call wg_parser_stream_get_buffer() from. Sure through very convoluted ways we maybe can. FFmpeg is simply better there because it doesn't force any threading pattern upon us. Anyway, having experimented with FFmpeg up to the point where it mostly passes all our tests, I can say that it's definitely a better fit for us as a backend.
- It doesn't force any unix side threads upon us, it can still use
internal decoding threads but hides them and we don't need to care, 2) it is performing better in every decoding scenario I tried with (20% faster on a given H264 4K video), 3) it is simpler, the whole branch is ~6K LoC shorter than winegstreamer, while passing most of the tests (with some additional successes, and a couple of remaining failures), 4) most of the code is moved to the frontend modules, IMO much better for maintainability, given how hard it has been to get reviews for the MF media source, I'd say that's a huge win. Now if you claim that all the winegtreamer bugs are solved, very good, it's only a strategical choice left. Up to julliard to decide what he thinks is best.
As a mostly impartial party between you two:
zf is right that short-term, rewriting such a big piece involves huge regression risk and unclear benefits, especially with how many apps do bizarre things; it would be easier to hack around whatever issues still exist in winegstreamer. winetest will catch many possible regressions, but some craziness never made its way there.
However, long-term, rbernon is right that if we get past the regressions, it will closer match native, simplify future maintenance, get rid of a bunch of hacks, and reduce the amount of cross-thread communication. I don't know which (if any) immediate app compat benefits would manifest, but simplifying the codebase is benefit enough that I'd vote yes.