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

Update softmixer.rs to use AtomicU64 from atomic_shim, so that librespot works with MIPS and MIPSEL #1461

Merged
merged 3 commits into from
Jan 31, 2025

Conversation

humaita-github
Copy link
Contributor

MIPS and MIPSEL [and PowerPC (?)] don't support AtomicU64, since they are 32 bit. However, there is a crate that provides shims for these platforms. When it detects it is running on unsupported platforms, it fallbacks to the shim implementation, using crossbeam Mutex. For supported platforms, there should be no change.

See initial discussion in #1460

Use AtomicU64 from atomic_shim, so that librespot works with MIPS and MIPSEL
@photovoltex
Copy link
Member

Would you be so kind to fix the compilation issues?

@humaita-github
Copy link
Contributor Author

Hi, sorry for the back and forth -- I am new to rust. All should be fixed now.

@photovoltex
Copy link
Member

No probs, everyone starts at some point :). Just try to check next time that at least it compiles before submitting a PR^^;

@photovoltex photovoltex merged commit 471735a into librespot-org:dev Jan 31, 2025
13 checks passed
@humaita-github
Copy link
Contributor Author

Hi, it turns out that atomic_shim works for MIPS and MIPSEL, but not for ARM. However, portable-atomic seems to work well for all targets. I will create a new pull request

@photovoltex
Copy link
Member

But shouldn't that have been caught by the CI? And what do you mean exactly by it does "not work" for ARM?

@humaita-github
Copy link
Contributor Author

Hi, yes it should have been caught, but it didn't. I am compiling for ~ 15 different ARM flavours, and unfortunately I didn't take note on which one was failing, it might have been ARMv5 or ARMv7. The error is the same as MIPS show without atomic_shim or similar:

PS: The problems with ARM, MIPS and PowerPC are mentioned here: https://doc.rust-lang.org/core/sync/atomic/#portability

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.

2 participants