Skip to content

Conversation

@Starbuck5
Copy link
Member

@Starbuck5 Starbuck5 commented May 9, 2023

Continued from #2146

Problem: pygame-ce 2.2.1 has a regression where XM music doesn't work on Windows.

Regression caused by SDL_mixer, switching back down to 2.6.2 fixes it. Reported upstream: libsdl-org/SDL_mixer#518.

Solution: go back to using SDL_mixer 2.6.2 (on all platforms, for consistency), and add a unit test to ensure support on all platforms.

Problem 2: apparently xm music wasn't working on Mac? -> fixed by adjusting the build flags to disable shared libmodplug like all the other audio codec libraries.

Problem 3: manylinux build was doing something with docker that fails on PR branches unless they are direct branches of pygame-ce, so had to move my stuff to this new branch.

@Starbuck5 Starbuck5 requested a review from a team as a code owner May 9, 2023 07:22
@Starbuck5 Starbuck5 marked this pull request as draft May 9, 2023 07:45
@Starbuck5 Starbuck5 marked this pull request as ready for review May 10, 2023 04:19
@Starbuck5 Starbuck5 changed the title Fixes for tracker music support, unit test Fixes for tracker music support, add test May 10, 2023
Copy link
Member

@ankith26 ankith26 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

So if I understand what's going on here as of now, just removing mp3 tests somehow makes everything work again including xm loading? This is so suspicious hehe

@Starbuck5
Copy link
Member Author

So if I understand what's going on here as of now, just removing mp3 tests somehow makes everything work again including xm loading? This is so suspicious hehe

Just to be clear, I removed mp3 tests after adding them also in this PR. In the overall diff of this PR, no mp3 tests were removed. It's not that this PR removes mp3 tests.

I opened an issue about the mp3 loading failing (#2154), and cool-guy-dev confirmed this problem does not occur on real built pygame wheels. It seems to only be a problem when getting SDL_mixer from apt on the Ubuntu sdist runs.

This PR is totally ready for review, just focusing on xm music. (As a proxy for all the different tracker music formats, like mod and it).

@andrewhong04
Copy link
Member

LGTM, all tests pass on my machine.
image

Copy link
Member

@MyreMylar MyreMylar left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM 👍

The code changes fixes the failing xm tests on Windows for me.

@MyreMylar MyreMylar merged commit 9aed969 into main May 14, 2023
@Starbuck5 Starbuck5 added this to the 2.3 milestone May 14, 2023
@Starbuck5 Starbuck5 deleted the starbuck5-tracker-music branch May 15, 2023 00:59
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

buildconfig mixer.music pygame.mixer.music

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants