feat(android): implement audio settings (EQ, normalization, gapless)
🏗️ Build and Test JellyTau / Android Compile Check (push) Has been cancelled
Publish Documentation / Build & publish docs to gitea-pages (push) Has been cancelled
Traceability Validation / Check Requirement Traces (push) Has been cancelled
🏗️ Build and Test JellyTau / Run Tests (push) Has been cancelled
🏗️ Build and Test JellyTau / Android Compile Check (push) Has been cancelled
Publish Documentation / Build & publish docs to gitea-pages (push) Has been cancelled
Traceability Validation / Check Requirement Traces (push) Has been cancelled
🏗️ Build and Test JellyTau / Run Tests (push) Has been cancelled
ExoPlayerBackend was the only backend not overriding the PlayerBackend trait's set_audio_settings/audio_settings defaults, so the Settings > Audio controls rendered on Android and silently did nothing — the default returns Ok(()) while applying nothing, so the failure was invisible. Rust owns what the values are (canonical 10-band ISO layout, preset curves, normalization presets); Kotlin owns when the AudioEffect objects exist, since that needs the live audio session id. - settings.rs: audio_settings_jni_payload() sanitises (crossfade clamped, band vector normalised) before serialising, so a malformed vector cannot reach the Kotlin parser. JSON rather than a wide JNI signature, matching how load() already passes subtitles — adding a field will not change the signature. - ExoPlayerBackend: set_audio_settings/audio_settings over JNI; ExoPlayerState gains the first command-side field (settings are pushed out, never reported). - JellyTauPlayer.kt: Equalizer, LoudnessEnhancer, and gapless via pauseAtEndOfMediaItems. Three details that are easy to get wrong: - Effects re-attach on onAudioSessionIdChanged. ExoPlayer rebuilds its audio sink on a format change, which invalidates effects bound to the old session; without this the EQ silently stops applying mid-queue. - All effect work is posted to mainHandler rather than run inline. AudioEffect construction from a player callback can re-enter the player and deadlock — the same shape as the AutoplayDecision lock-scrutinee bug. - Device equalizers expose a device-dependent band count (commonly 5) at fixed centres, so the canonical 10 bands are resampled by nearest centre frequency. resampleBands() is a pure @JvmStatic function so that mapping is testable without a device. Normalization is approximate, not parity: LoudnessEnhancer is a gain stage, not a true EBU R128 normalizer like MPV's dynaudnorm. Recorded as such rather than claimed as equivalent. Crossfade is deliberately excluded — unimplemented on every platform and blocked on mpv, so building it on Android alone would invert the parity gap. Tests written first and observed failing (cannot find function audio_settings_jni_payload) before the implementation: the payload contract is pinned by tests because a serde rename would otherwise silently break the Kotlin parser. Not yet verified on a physical device — AudioEffect availability and band layouts are device-specific. Requirements matrix marks these rows accordingly, and flipping the trait default to Err(not_implemented()) is deferred until that verification lands.
This commit is contained in:
@@ -18,6 +18,7 @@ use super::events::{PlayerStatusEvent, SharedEventEmitter};
|
||||
use super::media::{MediaItem, MediaType};
|
||||
use super::state::PlayerState;
|
||||
use crate::playback_reporting::{EventThrottler, PlaybackOperation, PlaybackReporter};
|
||||
use crate::settings::{audio_settings_jni_payload, AudioSettings};
|
||||
use crate::utils::conversions::seconds_to_ticks;
|
||||
|
||||
/// Global reference to the JavaVM for JNI callbacks
|
||||
@@ -148,6 +149,10 @@ struct ExoPlayerState {
|
||||
volume: f32,
|
||||
is_loaded: bool,
|
||||
current_media: Option<MediaItem>,
|
||||
/// Last applied audio settings. Unlike the fields above (which JNI callbacks
|
||||
/// push *in*), this is commanded *out* — audio settings are never reported
|
||||
/// by the player, so this is the authoritative copy for `audio_settings()`.
|
||||
audio_settings: AudioSettings,
|
||||
}
|
||||
|
||||
impl ExoPlayerState {
|
||||
@@ -159,6 +164,7 @@ impl ExoPlayerState {
|
||||
volume: 1.0,
|
||||
is_loaded: false,
|
||||
current_media: None,
|
||||
audio_settings: AudioSettings::default(),
|
||||
}
|
||||
}
|
||||
}
|
||||
@@ -529,6 +535,56 @@ impl PlayerBackend for ExoPlayerBackend {
|
||||
self.shared_state.lock_safe().volume
|
||||
}
|
||||
|
||||
/// Apply audio settings to ExoPlayer (equalizer, normalization, gapless).
|
||||
///
|
||||
/// Sent as JSON rather than a wide JNI signature so new fields do not change
|
||||
/// the method signature — the same approach `load()` uses for subtitles. The
|
||||
/// Kotlin side owns the *mechanics* (attaching AudioEffects to the audio
|
||||
/// session); the canonical band layout and preset curves stay in Rust.
|
||||
///
|
||||
/// TRACES: UR-027, UR-032, UR-033 | DR-030, DR-035, DR-036
|
||||
fn set_audio_settings(&mut self, settings: &AudioSettings) -> Result<(), PlayerError> {
|
||||
let json = audio_settings_jni_payload(settings).map_err(|e| {
|
||||
PlayerError::playback_failed(format!("Failed to serialize audio settings: {}", e))
|
||||
})?;
|
||||
|
||||
let vm = JAVA_VM
|
||||
.get()
|
||||
.ok_or_else(|| PlayerError::playback_failed("JavaVM not initialized"))?;
|
||||
|
||||
let mut env = vm
|
||||
.attach_current_thread()
|
||||
.map_err(|e| PlayerError::playback_failed(format!("Failed to attach thread: {}", e)))?;
|
||||
|
||||
let json_jstring = env.new_string(&json).map_err(|e| {
|
||||
PlayerError::playback_failed(format!("Failed to create settings string: {}", e))
|
||||
})?;
|
||||
|
||||
env.call_method(
|
||||
&self.player_ref,
|
||||
"setAudioSettings",
|
||||
"(Ljava/lang/String;)V",
|
||||
&[JValue::Object(&json_jstring)],
|
||||
)
|
||||
.map_err(|e| {
|
||||
PlayerError::playback_failed(format!("Failed to call setAudioSettings: {}", e))
|
||||
})?;
|
||||
|
||||
// Store the sanitised form so audio_settings() reflects what was applied,
|
||||
// not what was requested.
|
||||
self.shared_state.lock_safe().audio_settings = settings
|
||||
.clone()
|
||||
.with_crossfade_clamped()
|
||||
.with_equalizer_normalised();
|
||||
|
||||
Ok(())
|
||||
}
|
||||
|
||||
/// TRACES: UR-027, UR-032, UR-033 | DR-030, DR-035, DR-036
|
||||
fn audio_settings(&self) -> AudioSettings {
|
||||
self.shared_state.lock_safe().audio_settings.clone()
|
||||
}
|
||||
|
||||
fn set_audio_track(&mut self, stream_index: i32) -> Result<(), PlayerError> {
|
||||
let vm = JAVA_VM
|
||||
.get()
|
||||
|
||||
@@ -179,10 +179,80 @@ impl VideoSettings {
|
||||
}
|
||||
}
|
||||
|
||||
/// Serialise `AudioSettings` into the JSON payload handed to the Android player
|
||||
/// over JNI.
|
||||
///
|
||||
/// Sanitises first (crossfade clamped, band vector normalised) so a malformed
|
||||
/// vector can never reach the Kotlin parser. JSON is used rather than a wide JNI
|
||||
/// signature so that adding a field does not change the method signature — the
|
||||
/// same approach `load()` already uses for subtitles.
|
||||
///
|
||||
/// The emitted keys are camelCase (serde) and `volumeLevel` is lowercase; the
|
||||
/// Kotlin side matches on those literals. Both are pinned by tests.
|
||||
///
|
||||
/// TRACES: UR-027, UR-032, UR-033 | DR-030, DR-035, DR-036
|
||||
pub fn audio_settings_jni_payload(settings: &AudioSettings) -> Result<String, serde_json::Error> {
|
||||
let sanitised = settings
|
||||
.clone()
|
||||
.with_crossfade_clamped()
|
||||
.with_equalizer_normalised();
|
||||
serde_json::to_string(&sanitised)
|
||||
}
|
||||
|
||||
#[cfg(test)]
|
||||
mod tests {
|
||||
use super::*;
|
||||
|
||||
/// The JNI payload must sanitise before serialising: an over-long crossfade
|
||||
/// is clamped and a wrong-length band vector is normalised to EQ_BANDS.len().
|
||||
/// Sending raw values would let a malformed vector reach the Kotlin parser.
|
||||
///
|
||||
/// TRACES: UR-027, UR-032, UR-033 | DR-030, DR-035, DR-036 | UT-AUDIO-JNI-1
|
||||
#[test]
|
||||
fn test_audio_settings_jni_payload_is_sanitised() {
|
||||
let settings = AudioSettings {
|
||||
crossfade_duration: 30.0,
|
||||
equalizer_bands: vec![20.0, -30.0],
|
||||
..AudioSettings::default()
|
||||
};
|
||||
|
||||
let json = audio_settings_jni_payload(&settings).expect("serialises");
|
||||
let v: serde_json::Value = serde_json::from_str(&json).expect("valid JSON");
|
||||
|
||||
assert_eq!(v["crossfadeDuration"], 12.0, "crossfade clamped to 12s");
|
||||
|
||||
let bands = v["equalizerBands"].as_array().expect("bands array");
|
||||
assert_eq!(bands.len(), EQ_BANDS.len(), "band vector normalised to 10");
|
||||
assert_eq!(bands[0], EQ_GAIN_MAX as f64, "gain clamped to +12dB");
|
||||
assert_eq!(bands[1], EQ_GAIN_MIN as f64, "gain clamped to -12dB");
|
||||
}
|
||||
|
||||
/// The Kotlin side parses these exact keys. camelCase is what serde emits
|
||||
/// for AudioSettings; a rename here silently breaks the Android parser,
|
||||
/// which is why the contract is pinned by a test rather than by convention.
|
||||
///
|
||||
/// TRACES: UR-027, UR-032, UR-033 | DR-030, DR-035, DR-036 | UT-AUDIO-JNI-2
|
||||
#[test]
|
||||
fn test_audio_settings_jni_payload_key_contract() {
|
||||
let json = audio_settings_jni_payload(&AudioSettings::default()).expect("serialises");
|
||||
let v: serde_json::Value = serde_json::from_str(&json).expect("valid JSON");
|
||||
|
||||
for key in [
|
||||
"crossfadeDuration",
|
||||
"gaplessPlayback",
|
||||
"normalizeVolume",
|
||||
"volumeLevel",
|
||||
"equalizerEnabled",
|
||||
"equalizerBands",
|
||||
] {
|
||||
assert!(v.get(key).is_some(), "JNI payload must carry `{key}`");
|
||||
}
|
||||
|
||||
// VolumeLevel is #[serde(rename_all = "lowercase")]; Kotlin matches on
|
||||
// these literals.
|
||||
assert_eq!(v["volumeLevel"], "normal");
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn test_default_settings() {
|
||||
let settings = AudioSettings::default();
|
||||
|
||||
Reference in New Issue
Block a user