Skip to content

Update Rodio to 0.22#20323

Open
mnmaita wants to merge 33 commits intobevyengine:mainfrom
mnmaita:mnmaita/dependabot/cargo/cpal-0.16
Open

Update Rodio to 0.22#20323
mnmaita wants to merge 33 commits intobevyengine:mainfrom
mnmaita:mnmaita/dependabot/cargo/cpal-0.16

Conversation

@mnmaita
Copy link
Member

@mnmaita mnmaita commented Jul 29, 2025 •
edited
Loading

Objective

Solution

  • Updated both cpal and rodio to their latest versions.
  • Updated code to address rodio's breaking changes.
  • Reworked audio related feature flags. NOTE: symphonia will only be the default backend for formats with no alternative fallback.
  • Added audio-all-formats feature collection to easily enable all the available audio formats using their default backends.
  • Replaced aarch64-apple-ios-sim target with arm64-apple-ios-simulator.

Testing

  • Tested audio related examples.
  • CI checks passing.

@mnmaita
Copy link
Member Author

mnmaita commented Jul 29, 2025 •
edited
Loading

Outstanding items:

  • Fix tools/example-showcase/disable-audio.patch
  • Figure out why the soundtrack and spatial examples are not working.
  • Do we want to make symphonia the default?
  • Is it ok to have lewton as the default ogg backend due to issues with the symphonia implementation when using looped/buffered sources?
  • Is the target change for iOS simulator sensible here or should we postpone that?
  • Review feature flag names.
  • Add a migration guide.
  • Remove minimp3.
  • Bump cpal once it releases a new version with the Send + Sync fix and remove the patch in Cargo.toml.
  • Keep symphonia backend as opt-in features.
  • Bump rodio once 0.22 is released.
  • Remove rodio patch from Cargo.toml.
  • Update PR name and migration guide to reflect the latest rodio version.

@alice-i-cecile alice-i-cecile added A-Audio Sounds playback and modification C-Dependencies A change to the crates that Bevy depends on S-Waiting-on-Author The author needs to make changes or address concerns before this can be merged labels Jul 29, 2025
@alice-i-cecile alice-i-cecile added the M-Migration-Guide A breaking change to Bevy's public API that needs to be noted in a migration guide label Jul 29, 2025
@github-actions
Copy link
Contributor

github-actions bot commented Jul 29, 2025

It looks like your PR is a breaking change, but you didn't provide a migration guide.

Please review the instructions for writing migration guides, then expand or revise the content in the migration guides directory to reflect your changes.

@mnmaita mnmaita mentioned this pull request Aug 11, 2025
Closed
@nyfair
Copy link

nyfair commented Oct 16, 2025

Symphonia released 0.5.5 last week, and bevy_audio picks it up by default after cargo update. However, the combination of rodio 0.20 and symphonia 0.5.5 causes a panic when decoding MPEG-4 streams.

This bug can be verified by running this rodio example directly
https://github.com/RustAudio/rodio/blob/v0.20.1/examples/music_m4a.rs

I suggest either merging this PR or pinning the symphonia dependency to version 0.5.4 in Cargo.toml.

@mnmaita
Copy link
Member Author

mnmaita commented Oct 22, 2025

@nyfair this PR is quite broken so I wouldn't merge it yet. We could do the version pinning as a stopgap and leave a comment on said change to fix it later. I might have some time to update this PR in a couple of days so I'll try my best to finalize it soon. @alice-i-cecile in the meantime, would you be able to help me figuring out the 3rd, 4th and 5th items that I left in my comment?

@mnmaita mnmaita force-pushed the mnmaita/dependabot/cargo/cpal-0.16 branch from 7111175 to 51b8311 Compare October 22, 2025 12:57
@mnmaita mnmaita marked this pull request as ready for review October 22, 2025 13:47
mnmaita
mnmaita commented Oct 22, 2025
View reviewed changes
crates/bevy_audio/src/audio_source.rs
/// which iterates over samples of type [`rodio::Sample`].
/// Must be a [`rodio::Source`] so that it can provide information on the audio it is iterating over.
type Decoder: rodio::Source + Send + Iterator<Item = Self::DecoderItem>;
type Decoder: rodio::Source + Send + Iterator<Item = rodio::Sample>;
Copy link
Member Author

@mnmaita mnmaita Oct 22, 2025

Choose a reason for hiding this comment

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

rodio::Source has Iterator<Item = rodio::Sample> as a bound as far as I can see, should we drop this redundant bound or should we keep it?

@mnmaita mnmaita mentioned this pull request Oct 27, 2025
Closed
@mnmaita mnmaita force-pushed the mnmaita/dependabot/cargo/cpal-0.16 branch from b85a940 to 6248b2e Compare October 27, 2025 23:41
alice-i-cecile
alice-i-cecile requested changes Oct 29, 2025
View reviewed changes
Copy link
Member

@alice-i-cecile alice-i-cecile left a comment

Choose a reason for hiding this comment

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

We shouldn't be exposing minimp3 at all, per #20183

@github-project-automation github-project-automation bot moved this to Needs Update in Audio (Old) Oct 29, 2025
@mnmaita mnmaita force-pushed the mnmaita/dependabot/cargo/cpal-0.16 branch from 6248b2e to 9ef8982 Compare October 29, 2025 19:14
@github-actions
Copy link
Contributor

github-actions bot commented Oct 29, 2025

You added a new feature but didn't update the readme. Please run cargo run -p build-templated-pages -- update features to update it, and commit the file change.

@alice-i-cecile alice-i-cecile added this to the 0.18 milestone Nov 23, 2025
@alice-i-cecile alice-i-cecile added S-Needs-Review Needs reviewer attention (from anyone!) to move forward S-Waiting-on-Author The author needs to make changes or address concerns before this can be merged and removed S-Waiting-on-Author The author needs to make changes or address concerns before this can be merged S-Needs-Review Needs reviewer attention (from anyone!) to move forward labels Dec 10, 2025
@alice-i-cecile
Copy link
Member

alice-i-cecile commented Dec 10, 2025

Can you please resolve merge conflicts and get CI passing?

@alice-i-cecile alice-i-cecile removed this from the 0.18 milestone Dec 10, 2025
@mnmaita
Copy link
Member Author

mnmaita commented Dec 10, 2025

Can you please resolve merge conflicts and get CI passing?

Hey @alice-i-cecile, I can fix the conflicts but I don't think I can fix the CI, we'd need a new version of cpal to be cut. From what I can see in the check-sources job, it's not desirable to patch cpal version with an arbitrary revision from their main branch (which contains the fix for Send+Sync issues in some of their types). How should we proceed here? I held off on writing the migration guide for this reason, but I could work on finishing that too if desirable.

@alice-i-cecile
Copy link
Member

alice-i-cecile commented Dec 11, 2025

Can you track down the cpal authors and pester them (politely and patiently!) for a release? You're correct that we can't ship this with a git dependency.

@alice-i-cecile alice-i-cecile added S-Blocked This cannot move forward until something else changes and removed S-Waiting-on-Author The author needs to make changes or address concerns before this can be merged labels Dec 11, 2025
@mnmaita mnmaita force-pushed the mnmaita/dependabot/cargo/cpal-0.16 branch 2 times, most recently from 1eb562d to 1282031 Compare March 23, 2026 04:07
@mnmaita mnmaita force-pushed the mnmaita/dependabot/cargo/cpal-0.16 branch from 1282031 to 306b9aa Compare March 23, 2026 04:10
@alice-i-cecile alice-i-cecile requested a review from rparrett March 23, 2026 04:15
@alice-i-cecile alice-i-cecile added X-Uncontroversial This work is generally agreed upon D-Modest A "normal" level of difficulty; suitable for simple features or challenging fixes labels Mar 23, 2026
@rparrett
Copy link
Contributor

rparrett commented Mar 23, 2026

Were you able to test this on Android? The mobile example should work as long as the setup_scene system is removed. (see #23208)

On my end, in a simulator, I have working audio in that example with Bevy 0.18.1, but not with this PR. I'm not seeing any useful debug logging, just not hearing the music.

It seems that adding -P 26 to the cargo ndk invocation is required to build, so mobile example build instructions may need to be updated?

@rparrett
Copy link
Contributor

rparrett commented Mar 23, 2026 •
edited
Loading

Scratch that. Resorted to clean build / cargo update / etc, and it seems to work fine for me now. It's possible that the emulator didn't like me switching audio devices or something.

rparrett
rparrett reviewed Mar 23, 2026
View reviewed changes
rparrett
rparrett reviewed Mar 23, 2026
View reviewed changes
@mnmaita
Copy link
Member Author

mnmaita commented Mar 23, 2026

Were you able to test this on Android? The mobile example should work as long as the setup_scene system is removed. (see #23208)

I only tested this on Windows, Linux and Mac so thanks for testing it on Android. 🙏🏻

@mnmaita @rparrett
Apply suggestions from code review
e1c3727 
Co-authored-by: Rob Parrett <robparrett@gmail.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@alice-i-cecile alice-i-cecile alice-i-cecile approved these changes

+1 more reviewer

@rparrett rparrett rparrett approved these changes

Reviewers whose approvals may not affect merge requirements

Assignees

No one assigned

Labels

A-Audio Sounds playback and modification C-Dependencies A change to the crates that Bevy depends on D-Modest A "normal" level of difficulty; suitable for simple features or challenging fixes M-Migration-Guide A breaking change to Bevy's public API that needs to be noted in a migration guide S-Needs-Review Needs reviewer attention (from anyone!) to move forward X-Uncontroversial This work is generally agreed upon

Projects

Audio
Status: Needs SME Triage
Audio (Old)
Status: Needs Update

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

7 participants

@mnmaita @nyfair @alice-i-cecile @omninonsense @yara-blue @rparrett @cart