Investigation into unifying the playback backends (Linux/MPV, Android/ExoPlayer, Windows/webview) onto one engine with hardware acceleration. Conclusion: video cannot be unified onto a native engine; audio can. The blocker is not mpv-specific. WebKitGTK, WebView2 and Android WebView each draw into their own compositor surface, so a native video surface sits either entirely above or entirely below the webview and cannot interleave with HTML. GStreamer and libVLC fail identically. mpv would additionally regress streaming: it has no adaptive bitrate, while the current hls.js path does. Six specs added: - playback-backend-unification: the analysis and decision, with evidence - android-audio-settings-parity: set_audio_settings on ExoPlayerBackend - android-native-video-spike: timeboxed test of SurfaceView compositing - windows-native-audio-backend: replace the webview <audio> shim with libmpv - libmpv2-migration: dead libmpv git pin -> libmpv2, plus a LICENSE file - playback-docs-corrections: the requirement-status fixes applied here Corrections to requirements.md, all verified against source: - UR-031/DR-034 claimed crossfade was "Done (Linux only)". It is implemented nowhere (mpv_backend.rs has a bare TODO) and is architecturally blocked on mpv, whose single-stream audio chain cannot feed acrossfade's two inputs. - Parity matrix listed crossfade as a Linux/Android gap; it is neither. - The matrix omitted the equalizer, which has the same Linux-only shape. - The suggested ConcatenatingMediaSource is deprecated in current Media3. nativeAdapter.ts cited tauri#10152 as an upstream blocker for native Android video. That issue is a stale feature request, dead since 2024-07-01; the capability shipped in tauri 27d01834 (2024-09-02), and the related black-screen bug was fixed in wry 0.39.4 (we ship 0.55.x). What is genuinely unproven is SurfaceView-behind-WebView compositing, which the spike now tracks.
210 lines
11 KiB
Markdown
210 lines
11 KiB
Markdown
# Spec: Android audio settings parity (EQ, normalization, gapless)
|
||
|
||
**Status:** Proposed
|
||
**Requirements:** UR-031, UR-032, UR-033, UR-027 → DR-034, DR-035, DR-036, DR-030; IR-004
|
||
**UX spec:** n/a — no UI change; Settings › Audio already renders these controls
|
||
**Supersedes / revises:** closes the audio half of the parity gap recorded in [playback-backend-unification.md](playback-backend-unification.md)
|
||
|
||
## Summary
|
||
|
||
Implement `set_audio_settings` / `audio_settings` on `ExoPlayerBackend` so the
|
||
equalizer, volume normalization, and gapless playback settings actually take
|
||
effect on Android. Today the Settings › Audio panel renders these controls on
|
||
Android and they silently do nothing — `ExoPlayerBackend` is the only backend
|
||
that does not override the trait's no-op defaults.
|
||
|
||
Crossfade is explicitly **not** included; see Out of scope.
|
||
|
||
## Motivation
|
||
|
||
`PlayerBackend` declares `set_audio_settings` with a default `Ok(())` body.
|
||
`MpvBackend`, `NullBackend`, and `WebviewAudioBackend` all override it;
|
||
`ExoPlayerBackend` does not. The settings are persisted, pushed to the backend on
|
||
every track load, and displayed in the UI — and then dropped on the floor.
|
||
|
||
This is the single most user-visible platform divergence in the app: a user who
|
||
sets a "Rock" EQ preset on Android sees the sliders move and hears no change.
|
||
|
||
The backend-unification investigation ruled out fixing this by swapping engines
|
||
(video cannot be unified; see the sibling spec), so the fix is to implement the
|
||
trait methods where they are missing.
|
||
|
||
## Layer assignment
|
||
|
||
| Logic / responsibility | Layer | Why it belongs there |
|
||
|------------------------|-------|----------------------|
|
||
| Band count, centre frequencies, gain range, preset→curve map | Rust (existing) | Already domain-owned in `settings.rs` per [audio-equalizer.md](audio-equalizer.md). Android must consume the same `AudioSettings`, not define its own bands. Duplicating the band layout in Kotlin would be a taxonomy leak of exactly the kind `check:boundary` guards against. |
|
||
| Mapping `AudioSettings` → Android audio-effect parameters | Rust → JNI boundary | Platform playback detail, the direct analogue of `build_af_filter` in `mpv_backend.rs`. Belongs with the other `set_audio_settings` code. |
|
||
| Attaching/detaching `Equalizer` and `LoudnessEnhancer` to the ExoPlayer audio session | Kotlin (`JellyTauPlayer.kt`) | Android platform API mechanics; needs the live `audioSessionId`, which only the Kotlin layer holds. |
|
||
| Normalization preset (Loud/Normal/Quiet) → target gain | Rust (existing) | `VolumeLevel` is domain vocabulary; the same preset must mean the same loudness on every platform. |
|
||
| Rendering sliders / preset chips | Frontend (existing) | Pure presentation; unchanged by this spec. |
|
||
|
||
Borderline row: attaching the effects could arguably be driven entirely from
|
||
Rust via JNI property calls. It goes to Kotlin because `AudioEffect` construction
|
||
requires the audio session id and must be re-attached when ExoPlayer rebuilds its
|
||
audio sink — lifecycle state that lives in `JellyTauPlayer.kt`. Rust still owns
|
||
*what* the values are; Kotlin owns *when* the effect objects exist.
|
||
|
||
## Design
|
||
|
||
### Rust — `ExoPlayerBackend` (`src-tauri/src/player/android/mod.rs`)
|
||
|
||
Override the two defaulted methods, mirroring the shape of the existing
|
||
`set_audio_track` JNI call:
|
||
|
||
```rust
|
||
fn set_audio_settings(&mut self, settings: &AudioSettings) -> Result<(), PlayerError> {
|
||
let s = settings.clone().with_crossfade_clamped().with_equalizer_normalised();
|
||
// Serialize as JSON — the same pattern load() already uses for subtitles,
|
||
// avoiding a 6-arg JNI signature that has to change every time a field lands.
|
||
let json = serde_json::to_string(&s).map_err(|e| PlayerError { message: e.to_string() })?;
|
||
// Kotlin: fun setAudioSettings(json: String)
|
||
self.call_player_method_string("setAudioSettings", &json)?;
|
||
self.shared_state.lock_safe().audio_settings = s;
|
||
Ok(())
|
||
}
|
||
|
||
fn audio_settings(&self) -> AudioSettings {
|
||
self.shared_state.lock_safe().audio_settings.clone()
|
||
}
|
||
```
|
||
|
||
`ExoPlayerState` gains an `audio_settings: AudioSettings` field. Note
|
||
`ExoPlayerBackend` currently holds no such state — `position`/`state`/`volume` are
|
||
all pushed in by JNI callbacks — so this is the first *pull*-side field. That is
|
||
correct: audio settings are commanded downward, never reported upward.
|
||
|
||
### Kotlin — `JellyTauPlayer.kt`
|
||
|
||
```kotlin
|
||
fun setAudioSettings(json: String) {
|
||
val s = JSONObject(json)
|
||
applyEqualizer(s.getBoolean("equalizerEnabled"), s.getJSONArray("equalizerBands"))
|
||
applyNormalization(s.getBoolean("normalizeVolume"), s.getString("volumeLevel"))
|
||
exoPlayer.pauseAtEndOfMediaItems = !s.getBoolean("gaplessPlayback")
|
||
}
|
||
```
|
||
|
||
Three independent mechanisms:
|
||
|
||
- **Gapless** — nearly free. ExoPlayer is gapless by default for compatible
|
||
formats; honouring the setting means *disabling* it when the user turns it off,
|
||
via `pauseAtEndOfMediaItems`. Note this only applies within a loaded playlist;
|
||
our queue loads one item at a time, so verify behaviour before claiming DR-035
|
||
on Android (see Testing).
|
||
- **Equalizer** — `android.media.audiofx.Equalizer` bound to
|
||
`exoPlayer.audioSessionId`. Android's EQ exposes a device-dependent band count
|
||
(commonly 5) at fixed centre frequencies, which will **not** match our 10-band
|
||
ISO layout. Rust owns the canonical 10 bands; Kotlin resamples them onto the
|
||
device's bands by nearest-centre-frequency interpolation. Gains are in
|
||
millibels (`setBandLevel` takes mB, we store dB → ×100), clamped to the
|
||
device's reported `getBandLevelRange()`.
|
||
- **Normalization** — `android.media.audiofx.LoudnessEnhancer`, also bound to the
|
||
audio session, `setTargetGain(mB)` derived from `VolumeLevel`. This is a gain
|
||
booster, not a true EBU R128 normalizer like MPV's `dynaudnorm`; parity is
|
||
approximate and should be documented as such rather than overclaimed.
|
||
|
||
Lifecycle: build the effects lazily on first use, release them in `release()`,
|
||
and re-attach on `onAudioSessionIdChanged` — ExoPlayer can rebuild its audio sink
|
||
(e.g. on a format change), which invalidates effects bound to the old session.
|
||
|
||
### Make the silent-failure mode impossible
|
||
|
||
The trait's default is the root cause of this whole class of bug:
|
||
|
||
```rust
|
||
// backend.rs:85 — reports success while doing nothing
|
||
fn set_audio_settings(&mut self, _settings: &AudioSettings) -> Result<(), PlayerError> {
|
||
Ok(())
|
||
}
|
||
```
|
||
|
||
Android inherits this, so every EQ/normalization change on Android returns `Ok`
|
||
and silently does nothing — the UI ships and has no effect, with no error anywhere.
|
||
|
||
Once `ExoPlayerBackend` implements the methods, **change the trait default to
|
||
`Err(PlayerError::not_implemented())`**, matching how `set_audio_track` /
|
||
`set_subtitle_track` already behave. Any future backend that forgets to implement
|
||
audio settings then fails loudly instead of lying.
|
||
|
||
Check the call sites before flipping it: `NullBackend` overrides both methods, so
|
||
the graceful-degradation path is unaffected, but confirm nothing treats a
|
||
`set_audio_settings` error as fatal to playback.
|
||
|
||
### Re-application on track load
|
||
|
||
`PlayerController` already re-pushes `AudioSettings` per track on the platforms
|
||
that implement it; the Android path inherits that for free once the trait methods
|
||
exist. No controller change.
|
||
|
||
### 🔴 Threading note
|
||
|
||
`setAudioSettings` is invoked from Rust on whatever thread the command lands on.
|
||
`AudioEffect` construction must not happen on the ExoPlayer application thread
|
||
from inside a player callback — that is the re-entrancy hazard CLAUDE.md warns
|
||
about, and the same shape as the `AutoplayDecision` deadlock. Post the work to
|
||
the player's handler rather than doing it inline in a listener.
|
||
|
||
## Out of scope
|
||
|
||
- **Crossfade (UR-031 / DR-034).** Not implemented on *any* platform today, and
|
||
architecturally blocked on MPV (single-stream audio chain; `acrossfade` needs
|
||
two inputs). Implementing it on Android alone would invert the parity gap. It
|
||
needs its own spec and probably two player instances.
|
||
- True EBU R128 normalization. `LoudnessEnhancer` is a gain stage; matching
|
||
`dynaudnorm` exactly is out of reach without a custom `AudioProcessor`.
|
||
- Windows audio settings — see [windows-native-audio-backend.md](windows-native-audio-backend.md).
|
||
|
||
## Acceptance criteria
|
||
|
||
- [ ] `ExoPlayerBackend` overrides `set_audio_settings` and `audio_settings`.
|
||
- [ ] EQ preset change on Android audibly changes playback; setting persists across track changes and app restart.
|
||
- [ ] Normalization toggle audibly changes level; the three presets are ordered Loud > Normal > Quiet.
|
||
- [ ] Disabling gapless produces a gap between consecutive tracks; enabling it does not.
|
||
- [ ] Effects are released on `release()` and survive an audio-session rebuild.
|
||
- [ ] `requirements.md` parity matrix updated: EQ and normalization ✅ Android.
|
||
- [ ] `bun run check` and `bun run test` pass.
|
||
- [ ] `cargo fmt` clean, `cargo clippy` clean, `bun run test:rust` passes.
|
||
- [ ] `bun run check:boundary` passes.
|
||
- [ ] New requirement-implementing code carries `// TRACES:` comments.
|
||
- [ ] `bindings.ts` regenerated if Rust types changed.
|
||
|
||
## Testing
|
||
|
||
**Rust** (`cargo test`): `set_audio_settings` stores the sanitized settings and
|
||
`audio_settings()` returns them — assert clamping/normalisation is applied
|
||
(crossfade clamped to 12s, band vector normalised to `EQ_BANDS.len()`). The JNI
|
||
call itself is not unit-testable; extract the JSON serialization into a pure
|
||
function and test that its shape matches what the Kotlin parser expects. That
|
||
serialization contract is the part most likely to silently break.
|
||
|
||
**Kotlin**: the band-resampling function (10 canonical bands → N device bands) is
|
||
pure arithmetic — extract it and unit-test it, including the degenerate cases of
|
||
a 5-band device and a device reporting 10 bands.
|
||
|
||
**Manual, on device** (these are the ones that actually prove it):
|
||
1. Set Bass Boost, play a track, confirm audible change.
|
||
2. Toggle normalization mid-track; confirm level change without a playback stall.
|
||
3. Queue two gapless-encoded tracks, toggle the setting, confirm the gap appears/disappears.
|
||
4. Force a format change (44.1kHz → 48kHz track) and confirm the EQ still applies afterwards — this exercises the session-rebuild re-attach.
|
||
|
||
## TRACES
|
||
|
||
- `ExoPlayerBackend::set_audio_settings` → `// TRACES: UR-027, UR-032, UR-033 | DR-030, DR-035, DR-036`
|
||
- Kotlin `setAudioSettings` / `applyEqualizer` / `applyNormalization` → same IDs
|
||
- Band-resampling helper + its tests → `DR-030 | UT-xxx`
|
||
|
||
## Notes for the implementer
|
||
|
||
- Read [audio-equalizer.md](audio-equalizer.md) first — it defines the canonical
|
||
band layout and the preset→curve rule this spec consumes. Do not redefine bands
|
||
in Kotlin.
|
||
- Android source edits go in `src-tauri/android/src` (canonical tree), then run
|
||
`scripts/sync-android-sources.sh`. Never edit the `gen/` tree.
|
||
- There is a **stale duplicate** `JellyTauPlayer.kt` (285 lines) at
|
||
`src-tauri/android/app/src/main/java/com/dtourolle/jellytau/player/` alongside
|
||
the real 1103-line file at `src-tauri/android/src/main/java/...`. Edit the
|
||
latter. Consider deleting the former as a separate change.
|
||
- A parallel Claude session may be active — `git diff` before "repairing"
|
||
unexpected changes.
|