Updated VolumeLevel to use Decibel and Amplitude Ratio Units#9582
Updated VolumeLevel to use Decibel and Amplitude Ratio Units#9582bushrat011899 wants to merge 7 commits intobevyengine:mainfrom
VolumeLevel to use Decibel and Amplitude Ratio Units#9582Conversation
VolumeLevel to use Decibel and Amplitude Ratio Units
|
This comment got me thinking, I can actually remove the bounds checks entirely. Both variants take a single
So I've adjusted the access and comparison methods to reflect the true statement that the amplitude of a |
hymm
left a comment
There was a problem hiding this comment.
Do we really need 2 variants for volume? Unity, Unreal, and Godot all just use decibels. That might just be because they don't have sum types, but the current pr feels a little awkward when you want to do math on the volume.
These are not appropriate due to the possibility of `NaN` values being passed directly into the `enum`.
I think using an enum is a good way to force understanding around what the volume type represents. The An alternative could be to new-type |
| /// phase inversion. If `a` and `b` have the same magnitude (`a.abs() == b.abs()`), but opposite | ||
| /// sign (`a == -b`), then `Self::Amplitude(a) == Self::Amplitude(b)`, as this phase information | ||
| /// is currently ignored. As such, you should use positive values for this variant to avoid confusion. | ||
| Amplitude(f32), |
There was a problem hiding this comment.
At the risk of some bikeshedding (and probably pedantry), "amplitude" refers to the volume itself, not a specific "unit" -- Linear would be a better term, as it is the term that is most used for the unitless gain ratio (this applies to the method name line 26 too). I can imagine some users being confused when looking at this variant by itself and not knowing whether we are talking about the linear gain or the logarithmic one (in dB), and having to look up the enum definition to clear the confusion.
| /// Create a new volume level relative to the global volume. | ||
| pub fn new_relative(volume: f32) -> Self { | ||
| Self::Relative(VolumeLevel::new(volume)) | ||
| pub fn new_relative(amplitude: f32) -> Self { |
There was a problem hiding this comment.
It's been kept as-is to not introduce breaking changes, but I do think it would be better to have new_relative and new_absolute use the VolumeLevel. Or we can deprecate these ones and introduce new methods (meaning 4 methods total for the cartesian product of (relative,absolute)x(amplitude,decibel)?
# Objective - Prework for reviving #9582. ## Solution - Move the two types to volume.rs and made it compile. - Also `#[reflect(Debug)]` on `Volume` while I'm here. ## Testing - Ran example locally. - Rely on CI.
# Objective - Prework for reviving bevyengine#9582. ## Solution - Move the two types to volume.rs and made it compile. - Also `#[reflect(Debug)]` on `Volume` while I'm here. ## Testing - Ran example locally. - Rely on CI.
# Objective - Prework for reviving bevyengine#9582. ## Solution - Move the two types to volume.rs and made it compile. - Also `#[reflect(Debug)]` on `Volume` while I'm here. ## Testing - Ran example locally. - Rely on CI.
# Objective - Allow users to configure volume using decibels by changing the `Volume` type from newtyping an `f32` to an enum with `Linear` and `Decibels` variants. - Fixes #9507. - Alternative reworked version of closed #9582. ## Solution Compared to #9582, this PR has the following main differences: 1. It uses the term "linear scale" instead of "amplitude" per https://github.com/bevyengine/bevy/pull/9582/files#r1513529491. 2. Supports `ops` for doing `Volume` arithmetic. Can add two volumes, e.g. to increase/decrease the current volume. Can multiply two volumes, e.g. to get the “effective” volume of an audio source considering global volume. [requested and blessed on Discord]: https://discord.com/channels/691052431525675048/749430447326625812/1318272597003341867 ## Testing - Ran `cargo run --example soundtrack`. - Ran `cargo run --example audio_control`. - Ran `cargo run --example spatial_audio_2d`. - Ran `cargo run --example spatial_audio_3d`. - Ran `cargo run --example pitch`. - Ran `cargo run --example decodable`. - Ran `cargo run --example audio`. --- ## Migration Guide Audio volume can now be configured using decibel values, as well as using linear scale values. To enable this, some types and functions in `bevy_audio` have changed. - `Volume` is now an enum with `Linear` and `Decibels` variants. Before: ```rust let v = Volume(1.0); ``` After: ```rust let volume = Volume::Linear(1.0); let volume = Volume::Decibels(0.0); // or now you can deal with decibels if you prefer ``` - `Volume::ZERO` has been renamed to the more semantically correct `Volume::SILENT` because `Volume` now supports decibels and "zero volume" in decibels actually means "normal volume". - The `AudioSinkPlayback` trait's volume-related methods now deal with `Volume` types rather than `f32`s. `AudioSinkPlayback::volume()` now returns a `Volume` rather than an `f32`. `AudioSinkPlayback::set_volume` now receives a `Volume` rather than an `f32`. This affects the `AudioSink` and `SpatialAudioSink` implementations of the trait. The previous `f32` values are equivalent to the volume converted to linear scale so the `Volume:: Linear` variant should be used to migrate between `f32`s and `Volume`. - The `GlobalVolume::new` function now receives a `Volume` instead of an `f32`. --------- Co-authored-by: Zachary Harrold <zac@harrold.com.au>
Objective
Solution
AudioSinkPlaybacktraitandVolumeLeveltype.VolumeLevelwith anenumrepresenting either the amplitude ratio or decibel level. Names and types were chosen to be consistent with 3rd party crates likeKira, and the internally used types in the rest ofbevy_audioChangelog
VolumeLevelinto its own filevolume_level.rsto reduce clutter inaudio.rs.VolumeLevel, as both variantsAmplitudeandDecibelsare isomorphic and internally possess both properties.amplitude()anddecibels()methods to get the desired unit type fromVolumeLeveldirectly without amatchstatement.debug_assert!statements to ensure calculated values are within their appropriate bounds.AmplitudeandDecibels, with a truth table obtained from WikipediaVolumeLeveltype, or explicitly request an amplitude which is then wrapped in aVolumeLevel::Amplitudewhere appropriate.VolumeLevel::getandVolumeLevel::newdue to ambiguity in the unit to return.Migration Guide
VolumeLevel::new(volume)->VolumeLevel::Amplitude(volume)VolumeLevel::get()->VolumeLevel::amplitude()AudioSinkPlayback::volume()->AudioSinkPlayback::volume().amplitude()AudioSinkPlayback::set_volume(volume)->AudioSinkPlayback::set_volume(VolumeLevel::Amplitude(volume))