dtourolle
b11188e9dd
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.
199 lines
8.2 KiB
Markdown
199 lines
8.2 KiB
Markdown
# Spec: Playback documentation corrections
|
||
|
||
**Status:** Proposed
|
||
**Requirements:** revises the status of DR-034; corrects the parity matrix in [requirements.md](../requirements.md)
|
||
**UX spec:** n/a
|
||
**Supersedes / revises:** implements the "Corrections to existing docs" section of [playback-backend-unification.md](playback-backend-unification.md)
|
||
|
||
## Summary
|
||
|
||
Fix four factual errors in the requirements doc and the player source comments,
|
||
all found while investigating backend unification. Each claims something the code
|
||
does not do. Small change, but they are actively misleading: two of them assert a
|
||
feature is implemented when it is implemented nowhere, and one cites an upstream
|
||
blocker that no longer exists.
|
||
|
||
Documentation and comments only — no behaviour change.
|
||
|
||
## Motivation
|
||
|
||
These errors compound. DR-034 reads "Done (Linux only)", so a future session
|
||
planning Android parity would reasonably assume crossfade exists on Linux and
|
||
only needs porting — when in fact it is unimplemented everywhere *and*
|
||
architecturally blocked on the engine it supposedly runs on. Likewise the
|
||
tauri#10152 comment has been discouraging work on Android native video since the
|
||
upstream capability shipped in September 2024.
|
||
|
||
## The corrections
|
||
|
||
### 1. DR-034 status is wrong
|
||
|
||
`requirements.md` line ~196:
|
||
|
||
```
|
||
| DR-034 | Crossfade engine with configurable duration (0-12s) | Player | UR-031 | Done (Linux only) |
|
||
```
|
||
|
||
The code:
|
||
|
||
```rust
|
||
// src-tauri/src/player/mpv_backend.rs, in set_audio_settings
|
||
// TODO: Implement crossfade via MPV audio filters if needed
|
||
```
|
||
|
||
That is the entire crossfade implementation. `crossfade_duration` is plumbed
|
||
through `AudioSettings` and clamped to 0–12s, but no backend ever acts on it.
|
||
|
||
**Change to:** `Not implemented (blocked on MPV — see playback-backend-unification.md)`
|
||
|
||
Worth stating *why* in the requirements entry, because it is not a scheduling
|
||
gap: mpv's audio chain is single-stream, and FFmpeg's `acrossfade` is an `N→A`
|
||
filter needing two inputs. Real crossfade requires two libmpv instances with
|
||
manually ramped volumes. Upstream declined the feature (mpv#4512).
|
||
|
||
### 1b. UR-031 status is wrong for the same reason
|
||
|
||
`requirements.md` line ~44:
|
||
|
||
```
|
||
| UR-031 | Crossfade between audio tracks | Low | Done (Linux only) |
|
||
```
|
||
|
||
Same error one level up: the *user* requirement is also marked done. Since no
|
||
backend implements crossfade, UR-031 is not satisfied on any platform.
|
||
|
||
**Change to:** `Not implemented (blocked — see DR-034)`
|
||
|
||
Note line ~517 of the same file (`UR-031 (Crossfade), UR-032 (Gapless),
|
||
UR-033 (Normalization) only work on Linux`) inherits the error — crossfade works
|
||
nowhere, so it should read UR-032/UR-033 only.
|
||
|
||
### 2. Parity matrix crossfade row is wrong
|
||
|
||
```
|
||
| Crossfade | ✅ | ❌ | Gap |
|
||
```
|
||
|
||
**Change to** `| Crossfade | ❌ | ❌ | Not implemented |` — it is not a
|
||
platform-parity gap, it is an unbuilt feature.
|
||
|
||
### 3. Parity matrix is missing the equalizer
|
||
|
||
The matrix lists crossfade, gapless, and normalization but omits the EQ, which
|
||
has the same Linux-only shape and the same root cause (`ExoPlayerBackend` not
|
||
overriding `set_audio_settings`). `build_af_filter` and `eq_filter_entries` exist
|
||
only in `mpv_backend.rs`; there is no equalizer code in the Android tree.
|
||
|
||
**Add:** `| Equalizer (10-band) | ✅ | ❌ | Gap |`
|
||
|
||
### 4. `nativeAdapter.ts` cites a stale blocker
|
||
|
||
`src/lib/player/adapters/nativeAdapter.ts:11-14` states native Android video is
|
||
blocked upstream by tauri#10152 (transparent webview / SurfaceView compositing).
|
||
|
||
tauri#10152 is open but **dead since 2024-07-01**, and it is a *feature request*
|
||
that `WebviewWindowBuilder::transparent` was desktop-only — not a report that
|
||
compositing is broken. The capability shipped in tauri commit `27d01834`
|
||
(2024-09-02), which moved `transparent()` into the cross-platform impl block with
|
||
only the tao call `#[cfg(desktop)]`-fenced. It landed as a clippy cleanup, so the
|
||
issue was never closed. Separately, the black/white-screen bug (tauri#8381,
|
||
tauri#9408) was a broken JNI signature for `setBackgroundColor`, fixed in wry
|
||
0.39.4; we ship wry 0.55.x.
|
||
|
||
**Change to:** a comment stating the adapter is currently unreachable because
|
||
`createAdapter` hardcodes the HTML5 kind, that transparency is no longer an
|
||
upstream blocker, and that
|
||
[android-native-video-spike.md](android-native-video-spike.md) tracks whether
|
||
SurfaceView compositing actually works. Be explicit that *nobody has
|
||
demonstrated* SurfaceView-behind-WebView on Tauri Android — nothing upstream
|
||
blocks it, and nothing upstream proves it.
|
||
|
||
### 5. Platform capability is signalled three incompatible ways
|
||
|
||
Not a doc error — a real inconsistency found during the same investigation, worth
|
||
recording here even though fixing it needs its own change.
|
||
|
||
Which backend a platform uses is currently expressed three ways:
|
||
|
||
1. Rust `#[cfg]` gates in `player/mod.rs` and `create_player_backend` — the truth.
|
||
2. The `useHtml5Element` / `VideoBackend` value from `get_player_status` — which
|
||
the frontend discards (see the spike spec).
|
||
3. **Frontend user-agent sniffing** in `src/lib/services/webviewAudio.ts:30-41`:
|
||
|
||
```ts
|
||
const ua = navigator.userAgent.toLowerCase();
|
||
const isAndroid = ua.includes("android");
|
||
const isLinux = ua.includes("linux") && !isAndroid;
|
||
return !isAndroid && !isLinux;
|
||
```
|
||
|
||
The comment says it is "matching the Rust cfg gate" — i.e. the frontend
|
||
re-derives a backend decision from the user-agent string and hopes it stays in
|
||
sync. That is the frontend deciding *which backend exists*, which is domain
|
||
knowledge, not presentation. It also breaks silently the moment a new target is
|
||
added or a webview's UA changes.
|
||
|
||
**This is a boundary leak of the same family the spec-review checklist exists to
|
||
catch**, even though `check:boundary`'s tripwire (item-type arrays) does not
|
||
match it. Rust already computes the answer; the frontend should consume it.
|
||
|
||
Not fixed by this spec — it is behavioural, not documentation. It should be
|
||
folded into the spike spec's factory rework, where the same
|
||
"consume Rust's decision instead of re-deriving it" change is already in scope.
|
||
|
||
### Also worth fixing while here
|
||
|
||
`requirements.md` IR-004 reads "In Progress (basic playback works, audio settings
|
||
missing)". That stays accurate until
|
||
[android-audio-settings-parity.md](android-audio-settings-parity.md) lands, but
|
||
the "Future Fix" list in the parity issue proposes
|
||
`ConcatenatingMediaSource` for crossfade — **deprecated in current Media3**. Drop
|
||
that suggestion; the modern approach is a custom `AudioProcessor`.
|
||
|
||
## Layer assignment
|
||
|
||
No logic. Documentation and comments only.
|
||
|
||
| Logic / responsibility | Layer | Why it belongs there |
|
||
|------------------------|-------|----------------------|
|
||
| — | — | No logic introduced or moved by this spec. |
|
||
|
||
## Out of scope
|
||
|
||
- Implementing crossfade. This spec only stops claiming it exists.
|
||
- Implementing Android audio settings — see the parity spec.
|
||
- Running the Android video spike — see that spec.
|
||
- Rewriting the architecture docs. `docs/architecture/05-platform-backends.md`
|
||
should be re-read for the same class of error, but that is a larger pass.
|
||
|
||
## Acceptance criteria
|
||
|
||
- [ ] DR-034 status corrected, with the blocking reason stated.
|
||
- [ ] UR-031 status corrected (line ~44), and the "only work on Linux" line (~517) no longer lists crossfade.
|
||
- [ ] Parity matrix: crossfade ❌/❌; equalizer row added.
|
||
- [ ] `ConcatenatingMediaSource` suggestion removed from the "Future Fix" list.
|
||
- [ ] `nativeAdapter.ts` comment corrected and pointing at the spike spec.
|
||
- [ ] `bun run check` and `bun run test` pass (a comment change still touches TS).
|
||
- [ ] `bun run traces:markdown` re-run if requirement text changed.
|
||
|
||
No Rust changes, so the `cargo` gates do not apply.
|
||
|
||
## Testing
|
||
|
||
None beyond the standard gates — no behaviour changes. Confirm
|
||
`bun run traces:markdown` regenerates cleanly, since DR-034's row is referenced
|
||
by the traceability matrix.
|
||
|
||
## TRACES
|
||
|
||
No code implementing requirements changes; no TRACES comments to add or update.
|
||
The DR-034 row in `docs/traceability.md` will regenerate with the corrected text.
|
||
|
||
## Notes for the implementer
|
||
|
||
- Do **not** silently delete DR-034. The requirement (UR-031 crossfade) is still
|
||
wanted; it is the *status* that is wrong. Keeping the row with an honest status
|
||
and a reason is the point.
|
||
- A parallel Claude session may be active — `git diff` before "repairing"
|
||
unexpected changes.
|