fix(player): change the audio track, and load subtitles at all
Two faults, both present since v0.0.1, both found and confirmed on a device. Audio track (DR-258). Jellyfin builds a transcode around one AudioStreamIndex, so the alternate tracks are not in the stream that arrives — but the native path only ever called setAudioTrack(n), which indexes ExoPlayer's audio track *groups*. On Android that is the common case, since any source whose default audio codec the device cannot decode is transcoded: logcat showed ExoPlayer holding `Audio tracks: 1` while the menu listed every track in the file, so each selection warned `Invalid audio track index` and was dropped, leaving the default track playing with nothing in the UI saying so. determine_audio_track_switch_strategy now decides by whether the stream in front of the engine carries the track at all — a direct play still selects in place, a transcode is re-negotiated at the chosen index and resumed. Where it resumes is the player's answer rather than the UI's: the native path has no <video> element to read, so it sends no position, and defaulting that to zero re-opened the film at the beginning (caught on device before it shipped). Subtitles (DR-259). The URL was missing its `Stream.` route segment, so every sideloaded subtitle 404ed; since media3 1.5 a sideloaded text track only becomes a track group once its file is parsed, so 42 failed fetches left ExoPlayer with no text tracks and selection warned `available: 0`. Verified against a live server: the built URL answers 404, the corrected one 200. The tests that should have caught this asserted the shape of a mock helper that restated the format string instead of the URL the app requests — so the new test drives the repository itself, and failed red on the old URL.
This commit is contained in:
@@ -23,6 +23,7 @@ pub mod session;
|
||||
pub mod sleep_timer;
|
||||
pub mod state;
|
||||
pub mod stream_end;
|
||||
pub mod track_switch;
|
||||
|
||||
#[cfg(test)]
|
||||
mod mpv_backend_test;
|
||||
@@ -71,6 +72,7 @@ pub use seek::{determine_video_seek_strategy, VideoSeekStrategy};
|
||||
pub use session::{MediaSessionManager, MediaSessionType};
|
||||
pub use sleep_timer::{SleepTimerMode, SleepTimerState};
|
||||
pub use state::{EndReason, PlayerState};
|
||||
pub use track_switch::{determine_audio_track_switch_strategy, AudioTrackSwitchStrategy};
|
||||
|
||||
// Re-export platform-specific backends
|
||||
#[cfg(target_os = "android")]
|
||||
|
||||
@@ -0,0 +1,155 @@
|
||||
//! Audio-track switch strategy decision logic.
|
||||
//!
|
||||
//! Pure logic, extracted from the command layer so it can be unit-tested in the
|
||||
//! player core — the sibling of [`super::seek`]. `player_switch_audio_track`
|
||||
//! turns the resulting [`AudioTrackSwitchStrategy`] into a concrete action.
|
||||
//!
|
||||
//! The rule this module exists to state: **an engine can only select a track
|
||||
//! the stream in front of it actually carries.** A Jellyfin transcode is built
|
||||
//! around one `AudioStreamIndex`, so the alternate tracks are not in the stream
|
||||
//! at all — the switch has to re-open it. Only a direct play/stream hands the
|
||||
//! engine the source file with every track present.
|
||||
|
||||
/// How a request to change audio track has to be carried out.
|
||||
#[derive(Debug, Clone, Copy, PartialEq, Eq)]
|
||||
pub enum AudioTrackSwitchStrategy {
|
||||
/// Re-open the stream pinned to the chosen track; the frontend reloads its
|
||||
/// `<video>` element. An HTML5 element cannot select an audio track at all,
|
||||
/// so this holds whether or not the current stream is a transcode.
|
||||
Html5ReloadStream,
|
||||
/// Re-open the stream pinned to the chosen track; the backend reloads
|
||||
/// itself and restores the position.
|
||||
BackendReloadStream,
|
||||
/// The engine already holds every track — select in place, no reload.
|
||||
BackendSelectInPlace,
|
||||
}
|
||||
|
||||
/// Decide how to honour an audio-track change.
|
||||
///
|
||||
/// # Arguments
|
||||
/// * `needs_transcoding` - Whether the stream now playing is a server-side
|
||||
/// transcode, which carries exactly the one audio track it was built around.
|
||||
/// * `use_html5` - Whether the frontend `<video>` element is rendering.
|
||||
///
|
||||
/// TRACES: UR-021 | IR-019, DR-024, DR-258 | UT-232
|
||||
pub fn determine_audio_track_switch_strategy(
|
||||
needs_transcoding: bool,
|
||||
use_html5: bool,
|
||||
) -> AudioTrackSwitchStrategy {
|
||||
if use_html5 {
|
||||
return AudioTrackSwitchStrategy::Html5ReloadStream;
|
||||
}
|
||||
|
||||
if needs_transcoding {
|
||||
AudioTrackSwitchStrategy::BackendReloadStream
|
||||
} else {
|
||||
AudioTrackSwitchStrategy::BackendSelectInPlace
|
||||
}
|
||||
}
|
||||
|
||||
/// Where to resume after re-opening the stream for a track change.
|
||||
///
|
||||
/// `requested` is what the caller supplied; `engine_position` is where the
|
||||
/// engine itself says it is. The caller wins when it has something real to say,
|
||||
/// and the engine answers otherwise — which is the whole point: **position is
|
||||
/// the player's to know**, not the UI's to remember.
|
||||
///
|
||||
/// The native path proved why. It has no `<video>` element, so the frontend
|
||||
/// sent `null`, the command defaulted to `0.0`, and switching audio track
|
||||
/// re-opened the stream at the beginning of the film — the track changed and
|
||||
/// the viewer lost their place. A non-finite or negative value is treated the
|
||||
/// same as absent rather than passed through to a backend that would reject it.
|
||||
///
|
||||
/// TRACES: UR-021, UR-005 | IR-019, DR-024, DR-258 | UT-233
|
||||
pub fn resume_position(requested: Option<f64>, engine_position: f64) -> f64 {
|
||||
let usable = requested.filter(|p| p.is_finite() && *p > 0.0);
|
||||
let fallback = if engine_position.is_finite() && engine_position > 0.0 {
|
||||
engine_position
|
||||
} else {
|
||||
0.0
|
||||
};
|
||||
|
||||
usable.unwrap_or(fallback)
|
||||
}
|
||||
|
||||
#[cfg(test)]
|
||||
mod tests {
|
||||
use super::*;
|
||||
|
||||
/// The reported bug, seen on a device: switching audio track changed the
|
||||
/// track but "restarts from zero". The native path has no `<video>`
|
||||
/// element, so the frontend passed `null` and the re-opened stream began at
|
||||
/// the start of the film — logcat: `Re-opened the stream on audio stream 2
|
||||
/// and resumed at 0` while playback was 22 minutes in.
|
||||
#[test]
|
||||
fn a_caller_with_no_position_resumes_where_the_engine_is() {
|
||||
assert_eq!(resume_position(None, 1337.5), 1337.5);
|
||||
}
|
||||
|
||||
/// The HTML5 path does have an element and its clock is the honest answer
|
||||
/// there, so what the caller supplies wins.
|
||||
#[test]
|
||||
fn a_caller_that_knows_its_position_is_believed() {
|
||||
assert_eq!(resume_position(Some(42.0), 1337.5), 42.0);
|
||||
}
|
||||
|
||||
/// A position that is not a position — NaN from an element with no
|
||||
/// metadata, or a negative from a clock read mid-teardown — is treated as
|
||||
/// absent. Passing it through re-opens at a place no backend accepts.
|
||||
#[test]
|
||||
fn a_nonsense_position_falls_back_to_the_engine() {
|
||||
assert_eq!(resume_position(Some(f64::NAN), 90.0), 90.0);
|
||||
assert_eq!(resume_position(Some(-5.0), 90.0), 90.0);
|
||||
assert_eq!(resume_position(None, f64::NAN), 0.0);
|
||||
}
|
||||
|
||||
/// Switching track in the first moments of playback resumes at the start,
|
||||
/// which is where the viewer actually is.
|
||||
#[test]
|
||||
fn the_very_beginning_stays_the_very_beginning() {
|
||||
assert_eq!(resume_position(None, 0.0), 0.0);
|
||||
}
|
||||
|
||||
/// The reported bug: on Android the audio-track menu did nothing and the
|
||||
/// default track kept playing.
|
||||
///
|
||||
/// Jellyfin had negotiated a transcode (`TranscodeReasons=AudioCodecNot
|
||||
/// Supported`) whose URL pins `AudioStreamIndex=1`, so ExoPlayer was handed
|
||||
/// a stream with exactly one audio track — logcat: `Audio tracks: 1`. The
|
||||
/// native path nonetheless only ever called `setAudioTrack(n)`, which
|
||||
/// indexes ExoPlayer's audio track *groups* and so found nothing to select:
|
||||
/// `Invalid audio track index: 1 (available: 1)`, warned and dropped. The
|
||||
/// track the viewer asked for is not in the stream; it has to be re-opened.
|
||||
#[test]
|
||||
fn a_transcode_is_re_opened_because_it_carries_only_one_track() {
|
||||
assert_eq!(
|
||||
determine_audio_track_switch_strategy(true, false),
|
||||
AudioTrackSwitchStrategy::BackendReloadStream
|
||||
);
|
||||
}
|
||||
|
||||
/// A direct play hands the engine the source file, every track included, so
|
||||
/// ExoPlayer selects in place — no reload, no re-buffer, no lost position.
|
||||
#[test]
|
||||
fn a_direct_play_switches_in_place() {
|
||||
assert_eq!(
|
||||
determine_audio_track_switch_strategy(false, false),
|
||||
AudioTrackSwitchStrategy::BackendSelectInPlace
|
||||
);
|
||||
}
|
||||
|
||||
/// An HTML5 `<video>` element has no track-selection API, so it reloads
|
||||
/// either way. This is the path that already worked, and it must keep
|
||||
/// working: the fix is about the native side only.
|
||||
#[test]
|
||||
fn html5_always_reloads_because_the_element_cannot_select() {
|
||||
assert_eq!(
|
||||
determine_audio_track_switch_strategy(true, true),
|
||||
AudioTrackSwitchStrategy::Html5ReloadStream
|
||||
);
|
||||
assert_eq!(
|
||||
determine_audio_track_switch_strategy(false, true),
|
||||
AudioTrackSwitchStrategy::Html5ReloadStream
|
||||
);
|
||||
}
|
||||
}
|
||||
Reference in New Issue
Block a user