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.

Sign up for GitHub

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

Jump to bottom

[Merged by Bors] - document file formats for bytes field of AudioSource #6619

Closed
wants to merge 9 commits into from
Closed

[Merged by Bors] - document file formats for bytes field of AudioSource #6619

wants to merge 9 commits into from

Conversation

dis-da-mor
Copy link
Contributor

Objective

Fixes #6299

Solution

Change one line of documentation.

@alice-i-cecile alice-i-cecile added C-Docs An addition or correction to our documentation A-Audio Sounds playback and modification labels Nov 14, 2022
@alice-i-cecile
Copy link
Member

Change LGTM :) Can you update your PR title to explain what was done so it's easier to skim in the commit history?

@mockersf
Copy link
Member

👋
Could you change your PR title to something that describe the change? The PR title will end up as the commit in git history and it will improve readability of the git log.

Also, do you know if the formats supported depend on the feature flag enabled, or if those are only used when decoding files?

@dis-da-mor dis-da-mor changed the title Fix #6299 document file formats for bytes field of AudioSource Nov 14, 2022
@dis-da-mor
Copy link
Contributor Author

do you know if the formats supported depend on the feature flag enabled, or if those are only used when decoding files?

As far as I know you could store any bytes you would like in the bytes field, but playback won't work if the bytes aren't one of the supported formats and that format is enabled in the feature flag.

harudagondi
harudagondi reviewed Nov 14, 2022
View reviewed changes
crates/bevy_audio/src/audio_source.rs Outdated
@@ -8,7 +8,7 @@ use std::{io::Cursor, sync::Arc};
#[derive(Debug, Clone, TypeUuid)]
#[uuid = "7a14806a-672b-443b-8d16-4f18afefa463"]
pub struct AudioSource {
/// Raw data of the audio source
/// Raw data of the audio source, which must be in one of the file formats supported by Bevy (wav, ogg, flac or mp3).
Copy link
Member

@harudagondi harudagondi Nov 14, 2022 •
edited
Loading

Choose a reason for hiding this comment

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

Probably indicate that this uses rodio::Decoder internally, which has specific functions compiled depending on the features enabled. This means that if there are certain features disabled (mp3, flac, and wav are disabled by default), then it would not work.

@harudagondi
Copy link
Member

You should run cargo fmt --all. Also check if that link to rodio::decoder::Decoder is working in cargo doc.

@james7132 james7132 added this to the 0.10 milestone Dec 9, 2022
@james7132 james7132 added the S-Ready-For-Final-Review This PR has been approved by the community. It's ready for a maintainer to consider merging it label Dec 9, 2022
@alice-i-cecile
Copy link
Member

bors r+

bors bot pushed a commit that referenced this pull request Dec 11, 2022
@dis-da-mor
document file formats for bytes field of AudioSource (#6619)
81153a8 
# Objective

Fixes #6299 

## Solution

Change one line of documentation.

Co-authored-by: dis-da-moe <[email protected]>
@bors
Copy link
Contributor

bors bot commented Dec 11, 2022

Pull request successfully merged into main.

Build succeeded:

@bors bors bot changed the title document file formats for bytes field of AudioSource [Merged by Bors] - document file formats for bytes field of AudioSource Dec 11, 2022
@bors bors bot closed this Dec 11, 2022
alradish pushed a commit to alradish/bevy that referenced this pull request Jan 22, 2023
@dis-da-mor @alradish
document file formats for bytes field of AudioSource (bevyengine#…
fe73dd0 
…6619)

# Objective

Fixes bevyengine#6299 

## Solution

Change one line of documentation.

Co-authored-by: dis-da-moe <[email protected]>
ItsDoot pushed a commit to ItsDoot/bevy that referenced this pull request Feb 1, 2023
@dis-da-mor @ItsDoot
document file formats for bytes field of AudioSource (bevyengine#…
c65c08b 
…6619)

# Objective

Fixes bevyengine#6299 

## Solution

Change one line of documentation.

Co-authored-by: dis-da-moe <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@harudagondi harudagondi harudagondi approved these changes

@james7132 james7132 james7132 approved these changes

Assignees
No one assigned
Labels
A-Audio Sounds playback and modification C-Docs An addition or correction to our documentation S-Ready-For-Final-Review This PR has been approved by the community. It's ready for a maintainer to consider merging it
Projects
Audio
Status: Done
Milestone
0.10
Development

Successfully merging this pull request may close these issues.

Indicate AudioSource's bytes field must follow a supported audio file format
5 participants
@dis-da-mor @alice-i-cecile @mockersf @harudagondi @james7132