fix(player): never skip the gapless target offline + bound the skip cascade (#467)
The offline-skip recursion walked the entire remaining queue synchronously, desyncing the logical queue from the still-playing engine. Fixes: - The gapless-transition target is the track the engine is already playing; never run the availability-skip branch for it. - isTrackAvailable now counts the in-memory/disk cache (new v2_is_track_cached command), not just downloaded copies. - Cap consecutive offline skips and play-failure auto-skips at 5 in both +page.svelte and playbackService.ts. cc
This commit is contained in:
4 files changed
+83
-6
No files matched your search
@@ -73,6 +73,41 @@ fn v2_prefetch_count(quality: qbz_models::Quality) -> usize {
|
|||||||
throttled_cap
|
throttled_cap
|
||||||
}
|
}
|
||||||
|
|
||||||
|
/// Lightweight offline-availability check used by the frontend queue guard
|
||||||
|
/// (issue #467). A track counts as available offline if it is in the
|
||||||
|
/// in-memory playback cache (covers the currently-playing, fully-buffered
|
||||||
|
/// track and any prefetched upcoming tracks) OR has a ready file in the
|
||||||
|
/// persistent offline cache. This stops the offline queue walk from skipping
|
||||||
|
/// tracks that are actually playable.
|
||||||
|
#[tauri::command]
|
||||||
|
pub async fn v2_is_track_cached(
|
||||||
|
track_id: u64,
|
||||||
|
offline_cache: State<'_, OfflineCacheState>,
|
||||||
|
app_state: State<'_, AppState>,
|
||||||
|
) -> Result<bool, RuntimeError> {
|
||||||
|
// In-memory playback cache: the currently-playing track and prefetched
|
||||||
|
// upcoming tracks live here.
|
||||||
|
if app_state.audio_cache.contains(track_id) {
|
||||||
|
return Ok(true);
|
||||||
|
}
|
||||||
|
|
||||||
|
// Persistent offline cache: a downloaded track whose file is still on disk.
|
||||||
|
let cached_path = {
|
||||||
|
let db_opt = offline_cache.db.lock().await;
|
||||||
|
match db_opt.as_ref() {
|
||||||
|
Some(db) => db.get_file_path(track_id).ok().flatten(),
|
||||||
|
None => None,
|
||||||
|
}
|
||||||
|
};
|
||||||
|
if let Some(path) = cached_path {
|
||||||
|
if std::path::Path::new(&path).exists() {
|
||||||
|
return Ok(true);
|
||||||
|
}
|
||||||
|
}
|
||||||
|
|
||||||
|
Ok(false)
|
||||||
|
}
|
||||||
|
|
||||||
/// How far ahead to look for tracks to prefetch (to handle mixed playlists
|
/// How far ahead to look for tracks to prefetch (to handle mixed playlists
|
||||||
/// with local/offline tracks interspersed with Qobuz tracks)
|
/// with local/offline tracks interspersed with Qobuz tracks)
|
||||||
const V2_PREFETCH_LOOKAHEAD: usize = 15;
|
const V2_PREFETCH_LOOKAHEAD: usize = 15;
|
||||||
|
|||||||
@@ -1968,6 +1968,7 @@ pub fn run(qconnect_cli_override: Option<bool>) {
|
|||||||
commands_v2::v2_set_media_metadata,
|
commands_v2::v2_set_media_metadata,
|
||||||
commands_v2::v2_play_next_gapless,
|
commands_v2::v2_play_next_gapless,
|
||||||
commands_v2::v2_prefetch_track,
|
commands_v2::v2_prefetch_track,
|
||||||
|
commands_v2::v2_is_track_cached,
|
||||||
commands_v2::v2_reinit_audio_device,
|
commands_v2::v2_reinit_audio_device,
|
||||||
commands_v2::v2_check_audio_device_presence,
|
commands_v2::v2_check_audio_device_presence,
|
||||||
commands_v2::v2_get_audio_output_status,
|
commands_v2::v2_get_audio_output_status,
|
||||||
|
|||||||
@@ -171,9 +171,20 @@ async function handoffPlayTrackToRemoteRenderer(trackId: number): Promise<boolea
|
|||||||
* on one bad row. Returns true when a next track was dispatched,
|
* on one bad row. Returns true when a next track was dispatched,
|
||||||
* false when the queue is empty.
|
* false when the queue is empty.
|
||||||
*/
|
*/
|
||||||
|
// Bound the auto-skip cascade (issue #467): a run of failing tracks must not
|
||||||
|
// walk the queue. The counter is reset by the next successful play.
|
||||||
|
const MAX_CONSECUTIVE_SKIPS = 5;
|
||||||
|
let consecutiveSkips = 0;
|
||||||
|
|
||||||
async function autoSkipToNext(): Promise<boolean> {
|
async function autoSkipToNext(): Promise<boolean> {
|
||||||
|
if (consecutiveSkips >= MAX_CONSECUTIVE_SKIPS) {
|
||||||
|
console.warn(`[playback] Auto-skip cap (${MAX_CONSECUTIVE_SKIPS}) reached — stopping queue walk (#467)`);
|
||||||
|
showToast(get(t)('toast.noAvailableTracks'), 'info');
|
||||||
|
return false;
|
||||||
|
}
|
||||||
const next = await nextTrack();
|
const next = await nextTrack();
|
||||||
if (!next) return false;
|
if (!next) return false;
|
||||||
|
consecutiveSkips++;
|
||||||
console.log('Auto-skipping to next track:', next.title);
|
console.log('Auto-skipping to next track:', next.title);
|
||||||
// Brief delay so the "unavailable" toast is readable before we
|
// Brief delay so the "unavailable" toast is readable before we
|
||||||
// replace it with the next track's loading toast.
|
// replace it with the next track's loading toast.
|
||||||
@@ -350,6 +361,8 @@ export async function playTrack(
|
|||||||
|
|
||||||
setIsPlaying(true);
|
setIsPlaying(true);
|
||||||
dismissBuffering();
|
dismissBuffering();
|
||||||
|
// A track actually started playing — reset the auto-skip cascade guard (#467).
|
||||||
|
consecutiveSkips = 0;
|
||||||
if (showSuccessToast) {
|
if (showSuccessToast) {
|
||||||
showToast(`Playing: ${track.title}`, 'success');
|
showToast(`Playing: ${track.title}`, 'success');
|
||||||
}
|
}
|
||||||
@@ -472,9 +485,16 @@ export async function playTrack(
|
|||||||
}, 500);
|
}, 500);
|
||||||
return false;
|
return false;
|
||||||
} else if (behavior === 'always_skip') {
|
} else if (behavior === 'always_skip') {
|
||||||
|
if (consecutiveSkips >= MAX_CONSECUTIVE_SKIPS) {
|
||||||
|
console.warn(`[playback] Auto-skip cap (${MAX_CONSECUTIVE_SKIPS}) reached — stopping queue walk (#467)`);
|
||||||
|
showToast(get(t)('toast.noAvailableTracks'), 'info');
|
||||||
|
setIsPlaying(false);
|
||||||
|
return false;
|
||||||
|
}
|
||||||
showToast(get(t)('qualityFallback.autoSkipping'), 'info');
|
showToast(get(t)('qualityFallback.autoSkipping'), 'info');
|
||||||
const next = await nextTrack();
|
const next = await nextTrack();
|
||||||
if (next) {
|
if (next) {
|
||||||
|
consecutiveSkips++;
|
||||||
setTimeout(() => {
|
setTimeout(() => {
|
||||||
const nextSource = resolvePlaybackSource(next);
|
const nextSource = resolvePlaybackSource(next);
|
||||||
const nextIsLocal = isPlaybackSourceLocal(nextSource, next.is_local ?? false);
|
const nextIsLocal = isPlaybackSourceLocal(nextSource, next.is_local ?? false);
|
||||||
|
|||||||
+27
-6
@@ -3099,7 +3099,18 @@
|
|||||||
// Local tracks are always available
|
// Local tracks are always available
|
||||||
if (isLocalTrack(track.id)) return true;
|
if (isLocalTrack(track.id)) return true;
|
||||||
|
|
||||||
// Check if Qobuz track has a local copy
|
// A track in the playback/disk cache is playable offline — this covers the
|
||||||
|
// currently-playing, fully-buffered track and prefetched upcoming tracks,
|
||||||
|
// not just explicitly downloaded copies (issue #467).
|
||||||
|
try {
|
||||||
|
if (await invoke<boolean>('v2_is_track_cached', { trackId: track.id })) {
|
||||||
|
return true;
|
||||||
|
}
|
||||||
|
} catch {
|
||||||
|
// Fall through to the downloaded-copy check below.
|
||||||
|
}
|
||||||
|
|
||||||
|
// Check if Qobuz track has a downloaded local copy
|
||||||
try {
|
try {
|
||||||
const localIds = await invoke<number[]>('v2_playlist_get_tracks_with_local_copies', {
|
const localIds = await invoke<number[]>('v2_playlist_get_tracks_with_local_copies', {
|
||||||
trackIds: [track.id]
|
trackIds: [track.id]
|
||||||
@@ -3110,18 +3121,28 @@
|
|||||||
}
|
}
|
||||||
}
|
}
|
||||||
|
|
||||||
|
// Maximum consecutive offline skips before we stop walking the queue.
|
||||||
|
// Prevents the synchronous skip cascade from blasting the logical queue
|
||||||
|
// pointer to the end while the engine is still playing (issue #467).
|
||||||
|
const MAX_OFFLINE_SKIPS = 5;
|
||||||
|
|
||||||
// Helper to play a track from the queue (with offline skip support)
|
// Helper to play a track from the queue (with offline skip support)
|
||||||
async function playQueueTrack(track: BackendQueueTrack, skippedIds = new Set<number>(), gaplessTransition = false) {
|
async function playQueueTrack(track: BackendQueueTrack, skippedIds = new Set<number>(), gaplessTransition = false) {
|
||||||
const source = resolvePlaybackSource(track);
|
const source = resolvePlaybackSource(track);
|
||||||
const isLocal = isPlaybackSourceLocal(source, isLocalTrack(track.id));
|
const isLocal = isPlaybackSourceLocal(source, isLocalTrack(track.id));
|
||||||
|
|
||||||
// In offline mode, check if track is available
|
// In offline mode, check if track is available — but NEVER for the gapless
|
||||||
if (offlineStatus.isOffline && !isLocal) {
|
// target. By definition it is the track the engine is already playing, so
|
||||||
|
// entering the skip branch here would desync the logical queue from the
|
||||||
|
// audio engine and walk the queue to the end (issue #467). Fall through to
|
||||||
|
// update metadata only.
|
||||||
|
if (offlineStatus.isOffline && !isLocal && !gaplessTransition) {
|
||||||
const available = await isTrackAvailable(track);
|
const available = await isTrackAvailable(track);
|
||||||
if (!available) {
|
if (!available) {
|
||||||
// Skip to next track (prevent infinite loop)
|
// Bound the cascade: a run of undownloaded tracks must not walk the
|
||||||
if (skippedIds.has(track.id)) {
|
// whole queue synchronously. Stop on a revisit OR once we hit the
|
||||||
// Already tried this track, stop to prevent infinite loop
|
// consecutive-skip cap (issue #467).
|
||||||
|
if (skippedIds.has(track.id) || skippedIds.size >= MAX_OFFLINE_SKIPS) {
|
||||||
setQueueEnded(true);
|
setQueueEnded(true);
|
||||||
showToast($t('toast.noAvailableTracks'), 'info');
|
showToast($t('toast.noAvailableTracks'), 'info');
|
||||||
return;
|
return;
|
||||||
|
|||||||
Reference in new issue
Block a user