Fix sleep bug, fix menu return
🏗️ Build and Test JellyTau / Run Tests (push) Successful in 4m1s
Traceability Validation / Check Requirement Traces (push) Successful in 23s
Build & Release / Run Tests (push) Successful in 4m7s
🏗️ Build and Test JellyTau / Build Android APK (push) Successful in 19m5s
Build & Release / Build Linux (push) Successful in 16m20s
Build & Release / Build Android (push) Successful in 19m12s
Build & Release / Create Release (push) Successful in 8s
🏗️ Build and Test JellyTau / Run Tests (push) Successful in 4m1s
Traceability Validation / Check Requirement Traces (push) Successful in 23s
Build & Release / Run Tests (push) Successful in 4m7s
🏗️ Build and Test JellyTau / Build Android APK (push) Successful in 19m5s
Build & Release / Build Linux (push) Successful in 16m20s
Build & Release / Build Android (push) Successful in 19m12s
Build & Release / Create Release (push) Successful in 8s
This commit is contained in:
@@ -1087,7 +1087,7 @@ pub async fn enqueue_video_downloads(
|
||||
/// (FIFO within a priority), registers each, flips it to `downloading`, and
|
||||
/// spawns a worker. Each spawned worker calls this again on completion/failure,
|
||||
/// so the queue drains itself without any frontend involvement.
|
||||
async fn pump_download_queue(
|
||||
pub(crate) async fn pump_download_queue(
|
||||
app: tauri::AppHandle,
|
||||
db_service: Arc<crate::storage::db_service::RusqliteService>,
|
||||
active_downloads: Arc<Mutex<std::collections::HashSet<i64>>>,
|
||||
|
||||
@@ -378,6 +378,65 @@ pub(super) async fn check_for_local_download(
|
||||
}
|
||||
}
|
||||
|
||||
/// Re-point queued streaming items at completed local downloads.
|
||||
///
|
||||
/// Sources are resolved once when the queue is built, so downloads that finish
|
||||
/// while it plays (preloaded upcoming tracks) — or that existed before the
|
||||
/// connection dropped — would otherwise keep streaming. Called before advancing
|
||||
/// so the next track always prefers the on-disk copy.
|
||||
///
|
||||
/// Returns the number of items switched to a local source.
|
||||
pub(super) async fn refresh_queue_local_sources(
|
||||
controller: &PlayerController,
|
||||
db: &DatabaseWrapper,
|
||||
) -> Result<usize, String> {
|
||||
// Collect remote item IDs first; the queue lock must not be held across awaits.
|
||||
let remote_ids: Vec<String> = {
|
||||
let queue = controller.queue();
|
||||
let queue_lock = queue.lock().map_err(|e| e.to_string())?;
|
||||
queue_lock
|
||||
.items()
|
||||
.iter()
|
||||
.filter_map(|item| match &item.source {
|
||||
MediaSource::Remote { jellyfin_item_id, .. } => Some(jellyfin_item_id.clone()),
|
||||
_ => None,
|
||||
})
|
||||
.collect()
|
||||
};
|
||||
|
||||
if remote_ids.is_empty() {
|
||||
return Ok(0);
|
||||
}
|
||||
|
||||
let mut local_paths: Vec<(String, String)> = Vec::new();
|
||||
for id in remote_ids {
|
||||
if let Some(path) = check_for_local_download(db, &id).await? {
|
||||
local_paths.push((id, path));
|
||||
}
|
||||
}
|
||||
|
||||
if local_paths.is_empty() {
|
||||
return Ok(0);
|
||||
}
|
||||
|
||||
let queue = controller.queue();
|
||||
let mut queue_lock = queue.lock().map_err(|e| e.to_string())?;
|
||||
let mut switched = 0;
|
||||
for item in queue_lock.items_mut() {
|
||||
if let MediaSource::Remote { jellyfin_item_id, .. } = &item.source {
|
||||
if let Some((id, path)) = local_paths.iter().find(|(id, _)| id == jellyfin_item_id) {
|
||||
info!("[Player] Switching queued track {} to local download: {}", id, path);
|
||||
item.source = MediaSource::Local {
|
||||
file_path: PathBuf::from(path),
|
||||
jellyfin_item_id: Some(id.clone()),
|
||||
};
|
||||
switched += 1;
|
||||
}
|
||||
}
|
||||
}
|
||||
Ok(switched)
|
||||
}
|
||||
|
||||
/// Play a single media item (audio or video)
|
||||
///
|
||||
/// Accepts a PlayItemRequest with all optional fields properly defaulted.
|
||||
@@ -679,6 +738,7 @@ pub async fn player_next(
|
||||
player: State<'_, PlayerStateWrapper>,
|
||||
session: State<'_, MediaSessionManagerWrapper>,
|
||||
playback_mode: State<'_, super::playback_mode::PlaybackModeManagerWrapper>,
|
||||
db: State<'_, DatabaseWrapper>,
|
||||
) -> Result<PlayerStatus, String> {
|
||||
debug!("[player_next] Command called from frontend");
|
||||
|
||||
@@ -697,6 +757,10 @@ pub async fn player_next(
|
||||
} else {
|
||||
// Local playback
|
||||
let controller = player.0.lock().await;
|
||||
// Prefer downloads that completed since the queue was built
|
||||
if let Err(e) = refresh_queue_local_sources(&controller, &db).await {
|
||||
warn!("[player_next] Failed to refresh local sources: {}", e);
|
||||
}
|
||||
controller.next().map_err(|e| e.to_string())?;
|
||||
controller.emit_queue_changed();
|
||||
|
||||
@@ -732,6 +796,7 @@ pub async fn player_previous(
|
||||
player: State<'_, PlayerStateWrapper>,
|
||||
session: State<'_, MediaSessionManagerWrapper>,
|
||||
playback_mode: State<'_, super::playback_mode::PlaybackModeManagerWrapper>,
|
||||
db: State<'_, DatabaseWrapper>,
|
||||
) -> Result<PlayerStatus, String> {
|
||||
// Check if we're in remote mode
|
||||
let mode = playback_mode.0.get_mode();
|
||||
@@ -748,6 +813,10 @@ pub async fn player_previous(
|
||||
} else {
|
||||
// Local playback
|
||||
let controller = player.0.lock().await;
|
||||
// Prefer downloads that completed since the queue was built
|
||||
if let Err(e) = refresh_queue_local_sources(&controller, &db).await {
|
||||
warn!("[player_previous] Failed to refresh local sources: {}", e);
|
||||
}
|
||||
controller.previous().map_err(|e| e.to_string())?;
|
||||
controller.emit_queue_changed();
|
||||
|
||||
@@ -1669,12 +1738,23 @@ pub async fn player_preload_upcoming(
|
||||
player: State<'_, PlayerStateWrapper>,
|
||||
db: State<'_, DatabaseWrapper>,
|
||||
smart_cache: State<'_, SmartCacheWrapper>,
|
||||
download_manager: State<'_, crate::commands::download::DownloadManagerWrapper>,
|
||||
app: tauri::AppHandle,
|
||||
user_id: String,
|
||||
_download_base_path: String,
|
||||
) -> Result<PreloadResult, String> {
|
||||
let db_service = {
|
||||
// The pump only starts rows that carry both a stream URL and a target dir,
|
||||
// so resolve the same storage root the user-initiated download paths use
|
||||
// (storage_get_path = the database's parent directory).
|
||||
let (db_service, target_dir) = {
|
||||
let database = db.0.lock().map_err(|e| e.to_string())?;
|
||||
Arc::new(database.service())
|
||||
let target_dir = database
|
||||
.path()
|
||||
.parent()
|
||||
.ok_or_else(|| "Database path has no parent directory".to_string())?
|
||||
.to_string_lossy()
|
||||
.to_string();
|
||||
(Arc::new(database.service()), target_dir)
|
||||
};
|
||||
|
||||
// Get cache settings
|
||||
@@ -1712,10 +1792,12 @@ pub async fn player_preload_upcoming(
|
||||
|
||||
// Process each upcoming item
|
||||
for item in upcoming_items {
|
||||
// Only process items with Remote source (not already local)
|
||||
let jellyfin_id = match &item.source {
|
||||
MediaSource::Remote { jellyfin_item_id, .. } => {
|
||||
jellyfin_item_id.clone()
|
||||
// Only process items with Remote source (not already local). The
|
||||
// source already carries the resolved stream URL — reuse it so the
|
||||
// pump can start the download without any extra resolution step.
|
||||
let (jellyfin_id, stream_url) = match &item.source {
|
||||
MediaSource::Remote { jellyfin_item_id, stream_url } => {
|
||||
(jellyfin_item_id.clone(), stream_url.clone())
|
||||
}
|
||||
MediaSource::Local { .. } => {
|
||||
already_downloaded += 1;
|
||||
@@ -1727,9 +1809,13 @@ pub async fn player_preload_upcoming(
|
||||
}
|
||||
};
|
||||
|
||||
// Check if already downloaded
|
||||
// Check if already downloaded or actively in flight. Stale pending rows
|
||||
// without a stream URL are NOT skipped here — the upsert below heals
|
||||
// them so the pump can finally start them.
|
||||
let query = Query::with_params(
|
||||
"SELECT file_path FROM downloads WHERE item_id = ? AND user_id = ? AND status IN ('completed', 'downloading', 'pending') LIMIT 1",
|
||||
"SELECT file_path FROM downloads WHERE item_id = ? AND user_id = ?
|
||||
AND (status IN ('completed', 'downloading')
|
||||
OR (status = 'pending' AND stream_url IS NOT NULL)) LIMIT 1",
|
||||
vec![
|
||||
QueryParam::String(jellyfin_id.clone()),
|
||||
QueryParam::String(user_id.clone()),
|
||||
@@ -1746,14 +1832,29 @@ pub async fn player_preload_upcoming(
|
||||
continue;
|
||||
}
|
||||
|
||||
// Queue for download with low priority (preload priority = -100)
|
||||
let file_path = format!("{}/{}.mp3", sanitize_filename(&item.album.clone().unwrap_or_default()), sanitize_filename(&item.title));
|
||||
// Queue for download with low priority (preload priority = -100) so
|
||||
// user-initiated downloads always win a pump slot first.
|
||||
let album_dir = item
|
||||
.album
|
||||
.as_deref()
|
||||
.filter(|a| !a.is_empty())
|
||||
.unwrap_or("Unknown Album");
|
||||
let file_path = format!(
|
||||
"downloads/{}/{}.mp3",
|
||||
sanitize_filename(album_dir),
|
||||
sanitize_filename(&item.title)
|
||||
);
|
||||
|
||||
// Insert download record with preload priority
|
||||
// Insert with the stream URL + target dir the pump needs to start it.
|
||||
// On conflict, heal pre-existing rows that were queued without a URL
|
||||
// (they could never start) instead of leaving them stuck.
|
||||
let insert_query = Query::with_params(
|
||||
"INSERT INTO downloads (item_id, user_id, file_path, status, priority, queued_at, item_name, artist_name, album_name)
|
||||
VALUES (?, ?, ?, 'pending', -100, CURRENT_TIMESTAMP, ?, ?, ?)
|
||||
ON CONFLICT(item_id, user_id) DO NOTHING",
|
||||
"INSERT INTO downloads (item_id, user_id, file_path, status, priority, queued_at, item_name, artist_name, album_name, download_source, media_type, stream_url, target_dir)
|
||||
VALUES (?, ?, ?, 'pending', -100, CURRENT_TIMESTAMP, ?, ?, ?, 'auto', 'audio', ?, ?)
|
||||
ON CONFLICT(item_id, user_id) DO UPDATE SET
|
||||
stream_url = excluded.stream_url,
|
||||
target_dir = excluded.target_dir
|
||||
WHERE downloads.status = 'pending' AND downloads.stream_url IS NULL",
|
||||
vec![
|
||||
QueryParam::String(jellyfin_id),
|
||||
QueryParam::String(user_id.clone()),
|
||||
@@ -1761,6 +1862,8 @@ pub async fn player_preload_upcoming(
|
||||
QueryParam::String(item.title.clone()),
|
||||
item.artist.clone().map(QueryParam::String).unwrap_or(QueryParam::Null),
|
||||
item.album.clone().map(QueryParam::String).unwrap_or(QueryParam::Null),
|
||||
QueryParam::String(stream_url),
|
||||
QueryParam::String(target_dir.clone()),
|
||||
],
|
||||
);
|
||||
|
||||
@@ -1780,6 +1883,16 @@ pub async fn player_preload_upcoming(
|
||||
}
|
||||
}
|
||||
|
||||
// Kick the pump so the queued preloads actually start; without this they'd
|
||||
// only begin once some other download activity pumps the queue.
|
||||
if queued_count > 0 {
|
||||
let active_downloads = {
|
||||
let manager = download_manager.0.lock().map_err(|e| e.to_string())?;
|
||||
manager.get_active_downloads()
|
||||
};
|
||||
crate::commands::download::pump_download_queue(app, db_service, active_downloads).await;
|
||||
}
|
||||
|
||||
info!("[Preload] Result: queued={}, already_downloaded={}, skipped={}", queued_count, already_downloaded, skipped);
|
||||
|
||||
Ok(PreloadResult {
|
||||
@@ -1886,6 +1999,97 @@ pub async fn player_disable_jellyfin(
|
||||
|
||||
#[cfg(test)]
|
||||
mod tests {
|
||||
/// Queue items enqueued as Remote must flip to Local once a completed
|
||||
/// download exists on disk — this is what makes preloaded tracks (and
|
||||
/// offline playback after a connection drop) actually use the cache.
|
||||
#[tokio::test]
|
||||
async fn test_refresh_queue_local_sources_switches_completed_downloads() {
|
||||
use super::{refresh_queue_local_sources, DatabaseWrapper};
|
||||
use crate::player::{MediaItem, MediaSource, MediaType, PlayerController};
|
||||
use crate::storage::Database;
|
||||
use std::sync::Mutex;
|
||||
|
||||
// A real file on disk for the completed download; a missing file for
|
||||
// the second entry to prove nonexistent files are not switched.
|
||||
let dir = std::env::temp_dir().join("jellytau-test-refresh-sources");
|
||||
std::fs::create_dir_all(&dir).unwrap();
|
||||
let existing = dir.join("track-a.mp3");
|
||||
std::fs::write(&existing, b"audio").unwrap();
|
||||
let missing = dir.join("track-b-missing.mp3");
|
||||
let _ = std::fs::remove_file(&missing);
|
||||
|
||||
let database = Database::open_in_memory().unwrap();
|
||||
{
|
||||
let conn = database.connection();
|
||||
let conn = conn.lock().unwrap();
|
||||
conn.execute_batch(&format!(
|
||||
r#"
|
||||
INSERT INTO servers (id, name, url) VALUES ('srv', 'Test', 'http://test');
|
||||
INSERT INTO users (id, server_id, username) VALUES ('user1', 'srv', 'tester');
|
||||
INSERT INTO downloads (item_id, user_id, file_path, status)
|
||||
VALUES ('track-a', 'user1', '{}', 'completed');
|
||||
INSERT INTO downloads (item_id, user_id, file_path, status)
|
||||
VALUES ('track-b', 'user1', '{}', 'completed');
|
||||
"#,
|
||||
existing.display(),
|
||||
missing.display()
|
||||
))
|
||||
.unwrap();
|
||||
}
|
||||
let db = DatabaseWrapper(Mutex::new(database));
|
||||
|
||||
let make_item = |id: &str| MediaItem {
|
||||
id: id.to_string(),
|
||||
title: id.to_string(),
|
||||
name: None,
|
||||
artist: None,
|
||||
album: None,
|
||||
album_name: None,
|
||||
album_id: None,
|
||||
artist_items: None,
|
||||
artists: None,
|
||||
primary_image_tag: None,
|
||||
item_type: None,
|
||||
playlist_id: None,
|
||||
duration: None,
|
||||
artwork_url: None,
|
||||
media_type: MediaType::Audio,
|
||||
source: MediaSource::Remote {
|
||||
stream_url: format!("http://test/Audio/{}/stream", id),
|
||||
jellyfin_item_id: id.to_string(),
|
||||
},
|
||||
video_codec: None,
|
||||
needs_transcoding: false,
|
||||
video_width: None,
|
||||
video_height: None,
|
||||
subtitles: vec![],
|
||||
series_id: None,
|
||||
server_id: None,
|
||||
};
|
||||
|
||||
let controller = PlayerController::default();
|
||||
controller
|
||||
.set_queue(vec![make_item("track-a"), make_item("track-b")], 0)
|
||||
.unwrap();
|
||||
|
||||
let switched = refresh_queue_local_sources(&controller, &db).await.unwrap();
|
||||
assert_eq!(switched, 1, "only the download whose file exists switches");
|
||||
|
||||
let queue = controller.queue();
|
||||
let queue_lock = queue.lock().unwrap();
|
||||
match &queue_lock.items()[0].source {
|
||||
MediaSource::Local { file_path, jellyfin_item_id } => {
|
||||
assert_eq!(file_path, &existing);
|
||||
assert_eq!(jellyfin_item_id.as_deref(), Some("track-a"));
|
||||
}
|
||||
other => panic!("track-a should be local, got {:?}", other),
|
||||
}
|
||||
assert!(
|
||||
matches!(queue_lock.items()[1].source, MediaSource::Remote { .. }),
|
||||
"track-b's file is missing, it must stay remote"
|
||||
);
|
||||
}
|
||||
|
||||
/// Test track index finding in album
|
||||
/// This reproduces the bug where clicking songs 1-5 always played song 13
|
||||
#[test]
|
||||
|
||||
@@ -347,10 +347,16 @@ pub async fn player_add_tracks_by_ids(
|
||||
#[specta::specta]
|
||||
pub async fn player_skip_to(
|
||||
player: State<'_, PlayerStateWrapper>,
|
||||
db: State<'_, DatabaseWrapper>,
|
||||
index: usize,
|
||||
) -> Result<PlayerStatus, String> {
|
||||
let controller = player.0.lock().await;
|
||||
|
||||
// Prefer downloads that completed since the queue was built
|
||||
if let Err(e) = super::refresh_queue_local_sources(&controller, &db).await {
|
||||
log::warn!("[player_skip_to] Failed to refresh local sources: {}", e);
|
||||
}
|
||||
|
||||
// Skip to the index and get the item to play
|
||||
let item = {
|
||||
let queue = controller.queue();
|
||||
|
||||
@@ -142,6 +142,7 @@ pub async fn player_play_next_episode(
|
||||
pub async fn player_on_playback_ended(
|
||||
player: State<'_, PlayerStateWrapper>,
|
||||
repository_manager: State<'_, crate::commands::repository::RepositoryManagerWrapper>,
|
||||
db: State<'_, DatabaseWrapper>,
|
||||
item_id: Option<String>,
|
||||
repository_handle: Option<String>,
|
||||
) -> Result<(), String> {
|
||||
@@ -174,7 +175,7 @@ pub async fn player_on_playback_ended(
|
||||
// Handle the decision
|
||||
match decision {
|
||||
AutoplayDecision::Stop => {
|
||||
log::debug!("[Autoplay] Decision: Stop playback");
|
||||
log::info!("[Autoplay] Decision: Stop playback");
|
||||
let controller = controller_arc.lock().await;
|
||||
// Clear the queue so the frontend's currentQueueItem becomes null and
|
||||
// the mini player hides. Without this, the queue still holds the last
|
||||
@@ -193,9 +194,14 @@ pub async fn player_on_playback_ended(
|
||||
}
|
||||
}
|
||||
AutoplayDecision::AdvanceToNext => {
|
||||
log::debug!("[Autoplay] Decision: Advance to next track");
|
||||
log::info!("[Autoplay] Decision: Advance to next track");
|
||||
// Advance to next track in queue
|
||||
let controller = controller_arc.lock().await;
|
||||
// Prefer downloads that completed since the queue was built (e.g.
|
||||
// preloaded upcoming tracks) over continuing to stream.
|
||||
if let Err(e) = super::refresh_queue_local_sources(&controller, &db).await {
|
||||
log::warn!("[Autoplay] Failed to refresh local sources: {}", e);
|
||||
}
|
||||
if let Err(e) = controller.next() {
|
||||
log::error!("[Autoplay] Failed to advance to next track: {}", e);
|
||||
// Emit PlaybackEnded event on error
|
||||
|
||||
@@ -52,6 +52,7 @@ pub struct RepositoryManagerWrapper(pub RepositoryManager);
|
||||
#[specta::specta]
|
||||
pub async fn repository_create(
|
||||
manager: State<'_, RepositoryManagerWrapper>,
|
||||
player: State<'_, crate::commands::player::PlayerStateWrapper>,
|
||||
db: State<'_, crate::commands::storage::DatabaseWrapper>,
|
||||
connectivity: State<'_, crate::commands::connectivity::ConnectivityMonitorWrapper>,
|
||||
server_url: String,
|
||||
@@ -115,6 +116,14 @@ pub async fn repository_create(
|
||||
manager.0.create(handle.clone(), hybrid);
|
||||
info!("[REPO] Repository stored successfully");
|
||||
|
||||
// Give the player controller a repository for next-episode lookups. The
|
||||
// Android playback-ended callback has no repository handle, so without
|
||||
// this the episode autoplay countdown never triggers there.
|
||||
if let Some(repo) = manager.0.get(&handle) {
|
||||
let controller = player.0.lock().await;
|
||||
controller.set_repository(repo);
|
||||
}
|
||||
|
||||
Ok(handle)
|
||||
}
|
||||
|
||||
|
||||
Reference in New Issue
Block a user