fix(player): clamp volume at the command boundary
player_set_volume passed `volume` through untouched. Each backend clamps to 0.0..=1.0 for itself, so local playback was already safe, but the remote branch reaches no backend: it converts with `(volume * 100.0) as i32`, which turns infinity into i32::MAX. NaN is handled explicitly since f32::clamp returns NaN for a NaN input and it then survives every comparison downstream. TRACES: DR-212 | UT-206
This commit is contained in:
@@ -1705,6 +1705,23 @@ pub async fn player_set_subtitle_track(
|
||||
Ok(get_player_status(&controller))
|
||||
}
|
||||
|
||||
/// Normalise a volume arriving over IPC to the 0.0..=1.0 range every backend
|
||||
/// works in.
|
||||
///
|
||||
/// NaN is handled before the clamp rather than by it: `f32::clamp` returns NaN
|
||||
/// for a NaN input (it only panics on NaN *bounds*), and NaN then survives every
|
||||
/// comparison downstream, so a backend clamp cannot catch it either. It is
|
||||
/// treated as "no volume asked for" and floored to 0.0.
|
||||
///
|
||||
/// TRACES: DR-212 | UT-206
|
||||
fn normalize_volume(volume: f32) -> f32 {
|
||||
if volume.is_nan() {
|
||||
0.0
|
||||
} else {
|
||||
volume.clamp(0.0, 1.0)
|
||||
}
|
||||
}
|
||||
|
||||
#[tauri::command]
|
||||
#[specta::specta]
|
||||
pub async fn player_set_volume(
|
||||
@@ -1712,6 +1729,12 @@ pub async fn player_set_volume(
|
||||
playback_mode: State<'_, super::playback_mode::PlaybackModeManagerWrapper>,
|
||||
volume: f32,
|
||||
) -> Result<PlayerStatus, String> {
|
||||
// Clamp at the boundary as well as in each backend: the remote branch below
|
||||
// never reaches a backend clamp, and `(f32::INFINITY * 100.0) as i32` would
|
||||
// hand the server i32::MAX as a volume percentage.
|
||||
// TRACES: DR-212 | UT-206
|
||||
let volume = normalize_volume(volume);
|
||||
|
||||
// Check if we're in remote mode
|
||||
let mode = playback_mode.0.get_mode();
|
||||
|
||||
@@ -2769,6 +2792,44 @@ pub async fn player_disable_jellyfin(player: State<'_, PlayerStateWrapper>) -> R
|
||||
mod tests {
|
||||
use crate::utils::lock::MutexSafe;
|
||||
|
||||
/// UT-206 — the volume the command hands on is always a real number in
|
||||
/// 0.0..=1.0.
|
||||
///
|
||||
/// Every backend clamps for itself, but the remote branch of
|
||||
/// `player_set_volume` reaches no backend at all: it does
|
||||
/// `(volume * 100.0) as i32`, which turns infinity into `i32::MAX` and NaN
|
||||
/// into 0. NaN also survives `f32::clamp` unchanged, so clamping alone is
|
||||
/// not enough — it has to be tested for.
|
||||
///
|
||||
/// TRACES: DR-212 | UT-206
|
||||
#[test]
|
||||
fn test_normalize_volume_clamps_and_rejects_nan() {
|
||||
use super::normalize_volume;
|
||||
|
||||
// In-range values pass through untouched.
|
||||
assert_eq!(normalize_volume(0.0), 0.0);
|
||||
assert_eq!(normalize_volume(0.5), 0.5);
|
||||
assert_eq!(normalize_volume(1.0), 1.0);
|
||||
|
||||
// Out of range clamps to the same 0.0..=1.0 the backends use.
|
||||
assert_eq!(normalize_volume(-0.5), 0.0);
|
||||
assert_eq!(normalize_volume(42.0), 1.0);
|
||||
assert_eq!(normalize_volume(f32::INFINITY), 1.0);
|
||||
assert_eq!(normalize_volume(f32::NEG_INFINITY), 0.0);
|
||||
|
||||
// NaN is not a volume; it must not reach the Jellyfin percentage
|
||||
// conversion or a backend.
|
||||
let from_nan = normalize_volume(f32::NAN);
|
||||
assert!(!from_nan.is_nan(), "NaN must not pass through the boundary");
|
||||
assert_eq!(from_nan, 0.0);
|
||||
|
||||
// Whatever comes out survives the remote branch's percentage cast.
|
||||
for input in [-1.0, 0.25, 9.0, f32::INFINITY, f32::NAN] {
|
||||
let percent = (normalize_volume(input) * 100.0) as i32;
|
||||
assert!((0..=100).contains(&percent), "input {input} gave {percent}");
|
||||
}
|
||||
}
|
||||
|
||||
/// The subtitle list the frontend resolved must survive the IPC hop and end
|
||||
/// up on the `MediaItem` the native backend loads.
|
||||
///
|
||||
|
||||
Reference in New Issue
Block a user