diff --git a/src-tauri/src/lib.rs b/src-tauri/src/lib.rs index 0c6f77d..0bd4a39 100644 --- a/src-tauri/src/lib.rs +++ b/src-tauri/src/lib.rs @@ -1080,10 +1080,17 @@ fn parse_media_progress_line(line: &str) -> Option { .map(|seconds| format!("{}s", seconds.round() as u64)) }) .unwrap_or_else(|| "-".to_string()); - let size = progress_json_string(&progress, "_total_bytes_str") - .or_else(|| progress_json_string(&progress, "_total_bytes_estimate_str")) - .or_else(|| (total > 0.0).then(|| crate::download::format_size(total))); let total_is_estimate = exact_total.is_none() && estimated_total.is_some(); + // yt-dlp's estimated total is a moving bitrate-based guess. Use it to + // derive a percentage when no better signal exists, but never expose + // it as the media byte denominator. The Add window supplies the + // stable format estimate, and an exact total is emitted at completion. + let size = if total_is_estimate { + None + } else { + progress_json_string(&progress, "_total_bytes_str") + .or_else(|| exact_total.map(|total| crate::download::format_size(total))) + }; return Some(MediaProgress { fraction: fraction.clamp(0.0, 1.0), @@ -1091,7 +1098,7 @@ fn parse_media_progress_line(line: &str) -> Option { eta, size, downloaded_bytes: (total > 0.0 || downloaded > 0.0).then_some(downloaded), - total_bytes: (total > 0.0).then_some(total), + total_bytes: exact_total.filter(|total| total.is_finite() && *total > 0.0), total_is_estimate, }); } @@ -1394,6 +1401,23 @@ fn aggregate_media_byte_progress( } } +fn media_progress_event_totals( + progress: &MediaProgress, + byte_progress: Option<(u64, u64, bool)>, + total_tracks: f64, +) -> (Option, Option) { + let fallback_total = (total_tracks <= 1.0) + .then_some(progress.total_bytes) + .flatten(); + let total_bytes = byte_progress + .map(|value| value.1 as f64) + .or(fallback_total); + let total_is_estimate = byte_progress + .map(|value| value.2) + .or_else(|| fallback_total.map(|_| progress.total_is_estimate)); + (total_bytes, total_is_estimate) +} + fn emit_media_progress( app_handle: &tauri::AppHandle, id: &str, @@ -1414,6 +1438,11 @@ fn emit_media_progress( } let byte_progress = aggregate_media_byte_progress(&progress, track_changed, state); let (speed, eta) = media_progress_speed(&progress, Instant::now(), &mut state.speed_sampler); + let (total_bytes, total_is_estimate) = + media_progress_event_totals(&progress, byte_progress, total_tracks); + let downloaded_bytes = byte_progress + .map(|value| value.0 as f64) + .or(progress.downloaded_bytes); let size = byte_progress .map(|(_, total, total_is_estimate)| { let prefix = if total_is_estimate { "~" } else { "" }; @@ -1432,9 +1461,9 @@ fn emit_media_progress( eta, size, size_is_final: false, - downloaded_bytes: byte_progress.map(|value| value.0 as f64), - total_bytes: byte_progress.map(|value| value.1 as f64), - total_is_estimate: byte_progress.map(|value| value.2), + downloaded_bytes, + total_bytes, + total_is_estimate, }, ); state.last_progress_at = now; @@ -6277,7 +6306,8 @@ mod tests { collect_download_uris, drain_media_output_lines, filename_from_content_disposition, filename_from_url_disposition_query, filename_from_url_path, is_excluded_yt_dlp_format, is_browser_cookie_extraction_error, json_lower, media_metadata_cache_key, - media_output_template, media_progress_args, media_progress_speed, + media_output_template, media_progress_args, media_progress_event_totals, + media_progress_speed, cookie_scope_for_url, metadata_authentication_error, metadata_cookie_header_present, metadata_headers, metadata_response_error, normalize_speed_limit_for_aria2, @@ -7528,7 +7558,7 @@ mod tests { } #[test] - fn marks_structured_estimated_total_when_exact_total_is_null() { + fn keeps_structured_estimated_total_out_of_stable_byte_progress() { let line = format!( "{MEDIA_PROGRESS_PREFIX}{{\"downloaded_bytes\":5242880,\"total_bytes\":null,\"total_bytes_estimate\":10485760,\"_total_bytes_estimate_str\":\"~10.00MiB\"}}" ); @@ -7539,14 +7569,28 @@ mod tests { fraction: 0.5, speed: "-".to_string(), eta: "-".to_string(), - size: Some("~10.00MiB".to_string()), + size: None, downloaded_bytes: Some(5242880.0), - total_bytes: Some(10485760.0), + total_bytes: None, total_is_estimate: true, }) ); } + #[test] + fn ignores_estimated_media_totals_without_fragment_metadata() { + let line = format!( + "{MEDIA_PROGRESS_PREFIX}{{\"downloaded_bytes\":512,\"total_bytes_estimate\":1024,\"_percent_str\":\"50.0%\"}}" + ); + + let progress = parse_media_progress_line(&line).expect("structured progress should parse"); + assert_eq!(progress.fraction, 0.5); + assert_eq!(progress.size, None); + assert_eq!(progress.downloaded_bytes, Some(512.0)); + assert_eq!(progress.total_bytes, None); + assert!(progress.total_is_estimate); + } + #[test] fn parses_chunked_structured_ytdlp_progress() { let mut buffer = String::new(); @@ -7599,10 +7643,11 @@ mod tests { "{MEDIA_PROGRESS_PREFIX}{{\"downloaded_bytes\":1024,\"total_bytes_estimate\":1024,\"fragment_index\":0,\"fragment_count\":354,\"_percent_str\":\"100.0%\"}}" ); - assert_eq!( - parse_media_progress_line(&line).map(|progress| progress.fraction), - Some(0.0) - ); + let progress = parse_media_progress_line(&line).expect("structured progress should parse"); + assert_eq!(progress.fraction, 0.0); + assert_eq!(progress.size, None); + assert_eq!(progress.total_bytes, None); + assert!(progress.total_is_estimate); } #[test] @@ -7673,6 +7718,32 @@ mod tests { ); } + #[test] + fn preserves_single_track_totals_when_byte_aggregation_is_unavailable() { + let progress = MediaProgress { + fraction: 0.5, + speed: "-".to_string(), + eta: "-".to_string(), + size: Some("~2.00 KB".to_string()), + downloaded_bytes: Some(1024.0), + total_bytes: Some(2048.0), + total_is_estimate: true, + }; + + assert_eq!( + media_progress_event_totals(&progress, None, 1.0), + (Some(2048.0), Some(true)) + ); + assert_eq!( + media_progress_event_totals(&progress, None, 2.0), + (None, None) + ); + assert_eq!( + media_progress_event_totals(&progress, Some((1024, 4096, false)), 1.0), + (Some(4096.0), Some(false)) + ); + } + #[test] fn freezes_a_media_track_estimate_across_progress_updates() { let mut state = MediaProgressEmitterState::new(); diff --git a/src/components/AddDownloadsModal.tsx b/src/components/AddDownloadsModal.tsx index fb76e4e..9ffc94e 100644 --- a/src/components/AddDownloadsModal.tsx +++ b/src/components/AddDownloadsModal.tsx @@ -968,7 +968,8 @@ export const AddDownloadsModal = () => { isMedia: item.isMedia, resumable: item.resumable, mediaFormatSelector: formatSelector, - size: item.size || (item.sizeBytes ? formatBytes(item.sizeBytes) : undefined) + size: item.size || (item.sizeBytes ? formatBytes(item.sizeBytes) : undefined), + sizeBytes: item.sizeBytes }, action); if (!added) { throw new Error('Backend rejected download start.'); diff --git a/src/store/downloadStore.test.ts b/src/store/downloadStore.test.ts index 1f136bd..636bcb3 100644 --- a/src/store/downloadStore.test.ts +++ b/src/store/downloadStore.test.ts @@ -207,6 +207,86 @@ describe('useDownloadProgressStore', () => { release(); }); + it('drops a persisted temporary media estimate when fragmented progress has no total', async () => { + const handlers: Record void> = {}; + vi.mocked(ipc.listenEvent).mockImplementation((event, handler) => { + handlers[event] = handler as (event: any) => void; + return Promise.resolve(vi.fn()); + }); + useDownloadStore.setState({ + downloads: [{ + id: 'stale-media-estimate', + url: 'https://youtube.com/watch?v=stale', + fileName: 'video.mkv', + status: 'downloading', + category: 'Movies', + dateAdded: '', + isMedia: true, + downloadedBytes: 11989, + totalBytes: 1024, + totalIsEstimate: true, + size: '~85.7 MB' + }] + }); + + const release = await initDownloadListener(); + handlers['download-progress']({ payload: { + id: 'stale-media-estimate', + fraction: 0.38, + speed: '2.7 MB/s', + eta: '7s', + size: null, + size_is_final: false, + downloaded_bytes: 13000, + total_bytes: null, + total_is_estimate: null + } }); + + expect(useDownloadStore.getState().downloads[0]).toMatchObject({ + downloadedBytes: 13000, + size: undefined + }); + expect(useDownloadStore.getState().downloads[0].totalBytes).toBeUndefined(); + expect(useDownloadStore.getState().downloads[0].totalIsEstimate).toBeUndefined(); + release(); + }); + + it('removes a stale tiny media size after restart when byte counters were volatile', async () => { + const handlers: Record void> = {}; + vi.mocked(ipc.listenEvent).mockImplementation((event, handler) => { + handlers[event] = handler as (event: any) => void; + return Promise.resolve(vi.fn()); + }); + useDownloadStore.setState({ + downloads: [{ + id: 'stale-media-size', + url: 'https://youtube.com/watch?v=stale-size', + fileName: 'video.mkv', + status: 'downloading', + category: 'Movies', + dateAdded: '', + isMedia: true, + size: '~1.00 KB' + }] + }); + + const release = await initDownloadListener(); + handlers['download-progress']({ payload: { + id: 'stale-media-size', + fraction: 0.01, + speed: '2.7 MB/s', + eta: '7s', + size: null, + size_is_final: false, + downloaded_bytes: 2048, + total_bytes: null, + total_is_estimate: null + } }); + + expect(useDownloadStore.getState().downloads[0].size).toBeUndefined(); + release(); + }); + it('ignores stale active state events after pause but accepts terminal reconciliation', async () => { const handlers: Record void> = {}; vi.mocked(ipc.listenEvent).mockImplementation((event, handler) => { diff --git a/src/store/downloadStore.ts b/src/store/downloadStore.ts index fde59bd..6834d1e 100644 --- a/src/store/downloadStore.ts +++ b/src/store/downloadStore.ts @@ -8,6 +8,7 @@ import { useDownloadProgressStore } from './downloadProgressStore'; import { clearDownloadControlIntent, downloadControlIntentFor, + hasStaleTemporaryMediaEstimate, useDownloadStore } from './useDownloadStore'; @@ -68,6 +69,25 @@ const startDownloadListeners = async () => { if (payload.total_is_estimate !== null && payload.total_is_estimate !== undefined) { updates.totalIsEstimate = payload.total_is_estimate; } + const observedDownloadedBytes = Math.max( + current.downloadedBytes ?? 0, + payload.downloaded_bytes ?? 0 + ); + // Older lifecycles may have persisted yt-dlp's temporary fragmented + // estimate (often 1 KiB). Once actual bytes exceed it and the current + // progress frame has no reliable total, discard that stale denominator + // so it cannot survive a pause, queue transition, or app restart. + if (payload.total_bytes == null && hasStaleTemporaryMediaEstimate({ + isMedia: current.isMedia, + downloadedBytes: observedDownloadedBytes, + totalBytes: current.totalBytes, + totalIsEstimate: current.totalIsEstimate, + size: current.size + })) { + updates.size = undefined; + updates.totalBytes = undefined; + updates.totalIsEstimate = undefined; + } if (Object.keys(updates).length > 0) { mainStore.updateDownload(payload.id, updates); } diff --git a/src/store/useDownloadStore.test.ts b/src/store/useDownloadStore.test.ts index 09084eb..b07a319 100644 --- a/src/store/useDownloadStore.test.ts +++ b/src/store/useDownloadStore.test.ts @@ -1,5 +1,5 @@ import { describe, it, expect, vi, beforeEach } from 'vitest'; -import { dispatchItem, getProxyArgs, getSiteLogin, normalizeCustomProxy, normalizePersistedQueueState, normalizePersistedQueues, useDownloadStore } from './useDownloadStore'; +import { dispatchItem, getProxyArgs, getSiteLogin, hasStaleTemporaryMediaEstimate, normalizeCustomProxy, normalizePersistedDownloadProgress, normalizePersistedQueueState, normalizePersistedQueues, useDownloadStore } from './useDownloadStore'; import { useDownloadProgressStore } from './downloadProgressStore'; import { useSettingsStore } from './useSettingsStore'; import * as ipc from '../ipc'; @@ -194,6 +194,95 @@ describe('useDownloadStore', () => { .toEqual(['00000000-0000-0000-0000-000000000001', '00000000-0000-0000-0000-000000000001']); }); + it('removes persisted temporary media estimates that are smaller than downloaded bytes', async () => { + vi.mocked(ipc.invokeCommand).mockImplementation(async (cmd: string) => { + if (cmd === 'db_get_all_queues') return []; + if (cmd === 'db_get_all_downloads') { + return [JSON.stringify({ + id: 'stale-media-estimate', + url: 'https://youtube.com/watch?v=stale', + fileName: 'video.mkv', + status: 'queued', + category: 'Movies', + dateAdded: '', + queueId: '00000000-0000-0000-0000-000000000001', + isMedia: true, + size: '~1.00 KB', + downloadedBytes: 11_989, + totalBytes: 1_024, + totalIsEstimate: true + })]; + } + return undefined; + }); + + await useDownloadStore.getState().initDB(); + + expect(useDownloadStore.getState().downloads[0]).toMatchObject({ + size: undefined, + downloadedBytes: 11_989, + totalBytes: undefined, + totalIsEstimate: undefined + }); + }); + + it('does not discard a legitimate large media estimate when downloaded bytes exceed it', () => { + const media = { + isMedia: true, + downloadedBytes: 90_000_000, + totalBytes: 89_817_907, + totalIsEstimate: true, + size: '~85.7 MB' + } as const; + + expect(hasStaleTemporaryMediaEstimate(media)).toBe(false); + expect(normalizePersistedDownloadProgress({ + id: 'large-estimate', + url: 'https://youtube.com/watch?v=large', + fileName: 'video.mkv', + status: 'queued', + category: 'Movies', + dateAdded: '', + ...media + })).toMatchObject({ + size: '~85.7 MB', + downloadedBytes: 90_000_000, + totalBytes: 89_817_907, + totalIsEstimate: true + }); + }); + + it('does not discard a legitimate small media estimate without contradictory progress', () => { + const media = { + isMedia: true, + size: '~500 B', + downloadedBytes: 500, + totalBytes: undefined, + totalIsEstimate: true + } as const; + + expect(hasStaleTemporaryMediaEstimate(media)).toBe(false); + expect(normalizePersistedDownloadProgress({ + id: 'small-media', + url: 'https://youtube.com/watch?v=small', + fileName: 'short.mkv', + status: 'queued', + category: 'Movies', + dateAdded: '', + ...media + })).toMatchObject(media); + }); + + it('recognizes IEC-formatted temporary media estimates', () => { + expect(hasStaleTemporaryMediaEstimate({ + isMedia: true, + size: '~1.00 KiB', + downloadedBytes: 2_048, + totalBytes: undefined, + totalIsEstimate: true + })).toBe(true); + }); + it('normalizes proxy settings for download dispatch', async () => { expect(normalizeCustomProxy('127.0.0.1', 8080)).toBe('http://127.0.0.1:8080'); expect(normalizeCustomProxy('http://proxy.local:9000', 8080)).toBe('http://proxy.local:9000'); @@ -582,6 +671,25 @@ describe('useDownloadStore', () => { expect(ipc.invokeCommand).not.toHaveBeenCalledWith('enqueue_download', expect.anything()); }); + it('carries a media format estimate into numeric progress state', async () => { + await useDownloadStore.getState().addDownload({ + id: 'media-estimate', + url: 'https://youtube.com/watch?v=estimate', + fileName: 'video.mkv', + category: 'Movies', + dateAdded: '', + isMedia: true, + size: '~85.7 MB', + sizeBytes: 89_817_907 + }, { type: 'add-to-queue', queueId: 'queue-b' }); + + expect(useDownloadStore.getState().downloads[0]).toMatchObject({ + totalBytes: 89_817_907, + totalIsEstimate: true + }); + expect(useDownloadStore.getState().downloads[0]).not.toHaveProperty('sizeBytes'); + }); + it('starts immediately in the main queue', async () => { vi.mocked(ipc.invokeCommand).mockImplementation(async (cmd: string) => { if (cmd === 'get_pending_order') return ['start-1']; diff --git a/src/store/useDownloadStore.ts b/src/store/useDownloadStore.ts index 43d7dac..eecde77 100644 --- a/src/store/useDownloadStore.ts +++ b/src/store/useDownloadStore.ts @@ -447,6 +447,64 @@ const normalizeQueuePositions = (downloads: DownloadItem[]): DownloadItem[] => { }); }; +const TEMPORARY_MEDIA_ESTIMATE_MAX_BYTES = 1024; +const DISPLAYED_SIZE_UNIT_MULTIPLIERS: Record = { + B: 1, + KB: 1024, + KIB: 1024, + MB: 1024 ** 2, + MIB: 1024 ** 2, + GB: 1024 ** 3, + GIB: 1024 ** 3, + TB: 1024 ** 4, + TIB: 1024 ** 4 +}; + +const displayedSizeBytes = (size: string | undefined): number | undefined => { + const match = size?.trim().match(/^~\s*([0-9]+(?:\.[0-9]+)?)\s*(B|KB|KIB|MB|MIB|GB|GIB|TB|TIB)$/i); + if (!match) return undefined; + + const bytes = Number(match[1]) * DISPLAYED_SIZE_UNIT_MULTIPLIERS[match[2].toUpperCase()]; + return Number.isFinite(bytes) ? bytes : undefined; +}; + +export const hasStaleTemporaryMediaEstimate = ( + download: Pick +): boolean => { + if (download.isMedia !== true) return false; + + const hasImpossibleNumericEstimate = download.totalIsEstimate === true && + typeof download.totalBytes === 'number' && + Number.isFinite(download.totalBytes) && + download.totalBytes > 0 && + download.totalBytes <= TEMPORARY_MEDIA_ESTIMATE_MAX_BYTES && + typeof download.downloadedBytes === 'number' && + Number.isFinite(download.downloadedBytes) && + download.downloadedBytes > download.totalBytes; + const visibleEstimateBytes = displayedSizeBytes(download.size); + const hasImpossibleVisibleEstimate = visibleEstimateBytes !== undefined && + visibleEstimateBytes <= TEMPORARY_MEDIA_ESTIMATE_MAX_BYTES && + typeof download.downloadedBytes === 'number' && + Number.isFinite(download.downloadedBytes) && + download.downloadedBytes > visibleEstimateBytes && + (download.totalBytes == null || download.totalBytes <= TEMPORARY_MEDIA_ESTIMATE_MAX_BYTES); + + return hasImpossibleNumericEstimate || hasImpossibleVisibleEstimate; +}; + +export const normalizePersistedDownloadProgress = (download: DownloadItem): DownloadItem => + hasStaleTemporaryMediaEstimate(download) + ? { + ...download, + // The old lifecycle could persist yt-dlp's temporary HLS estimate as + // both the numeric denominator and the visible size. Neither value is + // recoverable after the fact, so remove the false claim on startup. + size: undefined, + totalBytes: undefined, + totalIsEstimate: undefined + } + : download; + export type { DownloadStatus }; export const MAIN_QUEUE_ID = '00000000-0000-0000-0000-000000000001'; const DEFAULT_MAIN_QUEUE_NAME = 'Main Queue'; @@ -499,7 +557,10 @@ export type ExtensionDownloadRequest = ExtensionDownload; export type AddDownloadAction = | { type: 'start-now' } | { type: 'add-to-queue'; queueId: string }; -export type DownloadDraft = Omit; +export type DownloadDraft = Omit & { + /** Numeric format estimate supplied by the media Add window. */ + sizeBytes?: number; +}; export type PendingAddRequestContext = { version: number; referer: string; @@ -797,8 +858,13 @@ export const useDownloadStore = create((set, get) => ({ ); const maxPos = queueItems.reduce((max, d) => Math.max(max, d.queuePosition ?? 0), -1); const queuePosition = maxPos + 1; + const { sizeBytes, ...downloadDraft } = item; const ownedItem: DownloadItem = { - ...item, + ...downloadDraft, + totalBytes: item.totalBytes ?? sizeBytes, + totalIsEstimate: item.totalIsEstimate ?? ( + item.isMedia === true && item.size?.trim().startsWith('~') + ), connections: resolveDownloadConnections(item.connections, settings.perServerConnections), destination: destPath, status: action.type === 'add-to-queue' ? 'staged' : 'ready', @@ -1490,7 +1556,7 @@ export const useDownloadStore = create((set, get) => ({ const persistedQueueId = download.queueId || MAIN_QUEUE_ID; const queueId = normalizedQueueState.queueIdRemap.get(persistedQueueId) || (knownQueueIds.has(persistedQueueId) ? persistedQueueId : MAIN_QUEUE_ID); - return { ...download, queueId }; + return normalizePersistedDownloadProgress({ ...download, queueId }); }); set(state => ({