fix(player): a duration of zero is not a duration
DR-251. Scrubbing was dead on Android because the seek bar had no scale: every position tick read `<position> / 0.0`. ExoPlayer reports C.TIME_UNSET until it has resolved a duration, and JellyTauPlayer.getDuration() maps that to 0.0. So the engine answered Some(0.0) rather than None, which satisfied every "unknown duration" fallback in the controller — `observed_duration()` was never consulted, and neither was the runtime the catalog had carried since long before anything started decoding. Zero is now read as "does not know yet" at each step, with a final fallback to the item's own duration. That fixes it for any engine that cannot answer, rather than for ExoPlayer specifically. Red first: the test asserts a controller whose engine reports nothing usable still reports the queued item's 1800s, and failed with None before the change. 790 Rust tests, clippy clean both ways.
This commit is contained in:
@@ -444,6 +444,7 @@ Internal architecture, components, and application logic.
|
|||||||
| DR-246 | The seek strategy turns on an ability the engine declares, not on the container the stream arrives in. `Capabilities::seeks_transcoded_in_place` is stated by each engine — true for hls.js, which seeks within the VOD playlist it was handed; false for mpv, which cannot make the server transcode from a new offset — and the command asks the engine currently rendering instead of inferring from `is_hls` and `use_html5`. The item's transport is no longer read at the seek site at all. Re-negotiating a stream needs the repository, which sits above the engine, so the engine states the capability and the caller acts on it rather than the engine owning the whole decision | Player | UR-040, UR-081 | Done |
|
| DR-246 | The seek strategy turns on an ability the engine declares, not on the container the stream arrives in. `Capabilities::seeks_transcoded_in_place` is stated by each engine — true for hls.js, which seeks within the VOD playlist it was handed; false for mpv, which cannot make the server transcode from a new offset — and the command asks the engine currently rendering instead of inferring from `is_hls` and `use_html5`. The item's transport is no longer read at the seek site at all. Re-negotiating a stream needs the repository, which sits above the engine, so the engine states the capability and the caller acts on it rather than the engine owning the whole decision | Player | UR-040, UR-081 | Done |
|
||||||
| DR-247 | ExoPlayer can be told where to start. `JellyTauPlayer.load(url, mediaId)` had no way to express a start position, so every caller loaded and then seeked; the position is now handed to ExoPlayer with the media item via `setMediaItem(item, startPositionMs)`, and the two-argument form delegates to it. Running the conformance cases on a device also settled which half of DR-241 was engine-specific: ExoPlayer already queues a seek issued before `prepare()` completes, so it never had the lost-seek defect mpv did — only the missing vocabulary for a start position | Player | UR-081, UR-005 | Done |
|
| DR-247 | ExoPlayer can be told where to start. `JellyTauPlayer.load(url, mediaId)` had no way to express a start position, so every caller loaded and then seeked; the position is now handed to ExoPlayer with the media item via `setMediaItem(item, startPositionMs)`, and the two-argument form delegates to it. Running the conformance cases on a device also settled which half of DR-241 was engine-specific: ExoPlayer already queues a seek issued before `prepare()` completes, so it never had the lost-seek defect mpv did — only the missing vocabulary for a start position | Player | UR-081, UR-005 | Done |
|
||||||
| DR-250 | Stopping means nothing is playing, from any renderer — not "whatever we believe owns playback has been asked to stop". A background-audio handoff swaps which renderer that is, and the swap is bookkeeping that can be mid-flight: `exit_background_audio` marks the webview element the player again the moment it is called, while the element has not reloaded. The teardown's stop was gated on flags describing what the component started, so after a handoff it described a player that was no longer making sound and the stop was skipped — the audio stream kept running and the mini player adopted it, which is why a movie reappeared as an audio track. The stop is now unconditional (it is idempotent) and clears the handoff base and flag, so a later position read cannot be interpreted against a handoff that no longer exists | Player | UR-040, UR-005 | Done |
|
| DR-250 | Stopping means nothing is playing, from any renderer — not "whatever we believe owns playback has been asked to stop". A background-audio handoff swaps which renderer that is, and the swap is bookkeeping that can be mid-flight: `exit_background_audio` marks the webview element the player again the moment it is called, while the element has not reloaded. The teardown's stop was gated on flags describing what the component started, so after a handoff it described a player that was no longer making sound and the stop was skipped — the audio stream kept running and the mini player adopted it, which is why a movie reappeared as an audio track. The stop is now unconditional (it is idempotent) and clears the handoff base and flag, so a later position read cannot be interpreted against a handoff that no longer exists | Player | UR-040, UR-005 | Done |
|
||||||
|
| DR-251 | A duration of zero is treated as "the engine does not know yet", and falls back to the runtime the item already carries. ExoPlayer reports `C.TIME_UNSET` until it resolves one and `JellyTauPlayer.getDuration()` maps that to `0.0`, so the engine answered `Some(0.0)` rather than `None` — which satisfied every "unknown duration" fallback and left the seek bar with no scale. It presented as scrubbing being broken rather than as a duration that never arrived, and the catalog had the runtime the whole time | Player | UR-005, UR-040 | Done |
|
||||||
| DR-198 | The webview runs under a real Content-Security-Policy, and the asset protocol is scoped to the one directory it still serves. `csp` was `null`, which disables CSP entirely: any script that reached the web layer — through a future `{@html}`, a dependency, or a devtools paste — would have inherited the whole IPC surface, and with it the user's session. `script-src 'self'` (Tauri injects a nonce for SvelteKit's inline bootstrap script at build time, so no `'unsafe-inline'` is needed) plus `object-src`/`frame-src 'none'` and `base-uri 'self'` is the part that is genuinely restrictive. `img-src`/`media-src`/`connect-src` cannot be: the Jellyfin origin is typed in by the user at run time and is commonly plain `http` on a LAN, so they allow `http:`/`https:` — a wide grant for *data*, but one that still bars `file:`, `filesystem:` and scripting schemes, and leaves `script-src` untouched. `style-src` keeps `'unsafe-inline'` because Svelte compiles `style="…"` attributes (including `app.html`'s `display: contents` wrapper) into markup; this is safe only while no `<style>` element survives into `index.html`, since a nonce there would make Tauri's injection outrank — and therefore void — `'unsafe-inline'`. `worker-src blob:` and `media-src blob:` are hls.js: it demuxes in a worker built from a blob and attaches MSE through `URL.createObjectURL`. `asset:` and `http://asset.localhost` are the same protocol under the two naming schemes `convertFileSrc` emits (custom scheme on Linux/macOS, `http` host on Windows/Android); `ipc:`/`http://ipc.localhost` is the invoke transport, which would otherwise be blocked by `connect-src`. A run-time CSP naming the server origin exactly was rejected: Tauri computes the header from immutable config when it serves the HTML, so it would mean rebuilding config and reloading the webview on every server change, for a policy the user can already point anywhere. The asset-protocol scope narrows from `$APPDATA/**` to `$APPDATA/thumbnails/**` — since DR-137 moved downloaded media to the loopback server, `imageCache` is the only `convertFileSrc` caller left, so the database and the encrypted-token fallback file no longer sit inside the grant | Security | UR-012, UR-071 | Done |
|
| DR-198 | The webview runs under a real Content-Security-Policy, and the asset protocol is scoped to the one directory it still serves. `csp` was `null`, which disables CSP entirely: any script that reached the web layer — through a future `{@html}`, a dependency, or a devtools paste — would have inherited the whole IPC surface, and with it the user's session. `script-src 'self'` (Tauri injects a nonce for SvelteKit's inline bootstrap script at build time, so no `'unsafe-inline'` is needed) plus `object-src`/`frame-src 'none'` and `base-uri 'self'` is the part that is genuinely restrictive. `img-src`/`media-src`/`connect-src` cannot be: the Jellyfin origin is typed in by the user at run time and is commonly plain `http` on a LAN, so they allow `http:`/`https:` — a wide grant for *data*, but one that still bars `file:`, `filesystem:` and scripting schemes, and leaves `script-src` untouched. `style-src` keeps `'unsafe-inline'` because Svelte compiles `style="…"` attributes (including `app.html`'s `display: contents` wrapper) into markup; this is safe only while no `<style>` element survives into `index.html`, since a nonce there would make Tauri's injection outrank — and therefore void — `'unsafe-inline'`. `worker-src blob:` and `media-src blob:` are hls.js: it demuxes in a worker built from a blob and attaches MSE through `URL.createObjectURL`. `asset:` and `http://asset.localhost` are the same protocol under the two naming schemes `convertFileSrc` emits (custom scheme on Linux/macOS, `http` host on Windows/Android); `ipc:`/`http://ipc.localhost` is the invoke transport, which would otherwise be blocked by `connect-src`. A run-time CSP naming the server origin exactly was rejected: Tauri computes the header from immutable config when it serves the HTML, so it would mean rebuilding config and reloading the webview on every server change, for a policy the user can already point anywhere. The asset-protocol scope narrows from `$APPDATA/**` to `$APPDATA/thumbnails/**` — since DR-137 moved downloaded media to the loopback server, `imageCache` is the only `convertFileSrc` caller left, so the database and the encrypted-token fallback file no longer sit inside the grant | Security | UR-012, UR-071 | Done |
|
||||||
|
|
||||||
---
|
---
|
||||||
@@ -756,6 +757,7 @@ Internal architecture, components, and application logic.
|
|||||||
| UT-218 | Every property name matched by the mpv event loop also appears in an `observe_property` call, asserted against the source because the registration cannot be observed at runtime without a live mpv | DR-239 | Done |
|
| UT-218 | Every property name matched by the mpv event loop also appears in an `observe_property` call, asserted against the source because the registration cannot be observed at runtime without a live mpv | DR-239 | Done |
|
||||||
| UT-219 | A fullscreen toggle moves the document only when an in-document `<video>` renders, and moves the OS window as well when a native surface does | DR-240 | Done |
|
| UT-219 | A fullscreen toggle moves the document only when an in-document `<video>` renders, and moves the OS window as well when a native surface does | DR-240 | Done |
|
||||||
| UT-220 | The conformance suite: opening at a position starts there and never at zero, a seek issued while opening is honoured and overrides the start it overtook, pause and play are observable, close is silent and idempotent, and an open cancelled by close never begins playing | DR-242, DR-243 | In Progress |
|
| UT-220 | The conformance suite: opening at a position starts there and never at zero, a seek issued while opening is honoured and overrides the start it overtook, pause and play are observable, close is silent and idempotent, and an open cancelled by close never begins playing | DR-242, DR-243 | In Progress |
|
||||||
|
| UT-221 | An engine that cannot report a duration does not erase the one the item carries: with the queue holding a 1800s item and the engine answering nothing usable, the controller still reports 1800s | DR-251 | Done |
|
||||||
|
|
||||||
### Integration Tests
|
### Integration Tests
|
||||||
|
|
||||||
|
|||||||
@@ -1102,12 +1102,34 @@ impl PlayerController {
|
|||||||
///
|
///
|
||||||
/// TRACES: UR-005 | DR-178
|
/// TRACES: UR-005 | DR-178
|
||||||
pub fn duration(&self) -> Option<f64> {
|
pub fn duration(&self) -> Option<f64> {
|
||||||
|
// Zero is not a duration, it is an engine saying it does not know yet.
|
||||||
|
//
|
||||||
|
// ExoPlayer reports `C.TIME_UNSET` until it has resolved one, and
|
||||||
|
// `JellyTauPlayer.getDuration()` maps that to `0.0` — so the engine
|
||||||
|
// answers `Some(0.0)`, every "unknown duration" fallback below is
|
||||||
|
// skipped, and the seek bar is left with no scale. That presents as
|
||||||
|
// scrubbing being broken rather than as a duration that never arrived.
|
||||||
|
//
|
||||||
|
// The item usually knows: the catalog carried a runtime long before
|
||||||
|
// anything started decoding.
|
||||||
|
//
|
||||||
|
// TRACES: UR-005, UR-040 | DR-251
|
||||||
|
let usable = |d: f64| (d > 0.0).then_some(d);
|
||||||
|
|
||||||
self.backend
|
self.backend
|
||||||
.lock_safe()
|
.lock_safe()
|
||||||
.snapshot()
|
.snapshot()
|
||||||
.duration
|
.duration
|
||||||
.map(|d| d.as_secs_f64())
|
.map(|d| d.as_secs_f64())
|
||||||
.or_else(|| self.observed_duration())
|
.and_then(usable)
|
||||||
|
.or_else(|| self.observed_duration().and_then(usable))
|
||||||
|
.or_else(|| {
|
||||||
|
self.queue
|
||||||
|
.lock_safe()
|
||||||
|
.current()
|
||||||
|
.and_then(|item| item.duration)
|
||||||
|
.and_then(usable)
|
||||||
|
})
|
||||||
}
|
}
|
||||||
|
|
||||||
/// Get queue reference
|
/// Get queue reference
|
||||||
@@ -2290,6 +2312,35 @@ impl Default for PlayerController {
|
|||||||
|
|
||||||
#[cfg(test)]
|
#[cfg(test)]
|
||||||
mod tests {
|
mod tests {
|
||||||
|
|
||||||
|
/// A duration the engine does not know must fall back to the one the item
|
||||||
|
/// carries, and zero must count as "does not know".
|
||||||
|
///
|
||||||
|
/// ExoPlayer reports `C.TIME_UNSET` for a duration it has not resolved;
|
||||||
|
/// `JellyTauPlayer.getDuration()` maps that to `0.0`, so the engine answers
|
||||||
|
/// `Some(0.0)` rather than `None` and every "unknown duration" fallback is
|
||||||
|
/// skipped. The seek bar then has no scale, which presents as scrubbing
|
||||||
|
/// being dead rather than as a missing duration.
|
||||||
|
///
|
||||||
|
/// TRACES: UR-005, UR-040 | DR-251 | UT-221
|
||||||
|
#[test]
|
||||||
|
fn test_duration_falls_back_to_the_item_when_the_engine_does_not_know() {
|
||||||
|
let controller = PlayerController::default();
|
||||||
|
let mut item = MediaItem::sample("item-1", "https://example.invalid/a.mp4");
|
||||||
|
item.duration = Some(1800.0);
|
||||||
|
|
||||||
|
{
|
||||||
|
let queue_arc = controller.queue();
|
||||||
|
let mut queue = queue_arc.lock_safe();
|
||||||
|
queue.set_queue(vec![item], 0);
|
||||||
|
}
|
||||||
|
|
||||||
|
assert_eq!(
|
||||||
|
controller.duration(),
|
||||||
|
Some(1800.0),
|
||||||
|
"an engine that cannot report a duration should not erase the one the item carries"
|
||||||
|
);
|
||||||
|
}
|
||||||
use super::*;
|
use super::*;
|
||||||
|
|
||||||
/// Test emitter that captures events for asserting the HTML5 report methods
|
/// Test emitter that captures events for asserting the HTML5 report methods
|
||||||
|
|||||||
Reference in New Issue
Block a user