mirror of
https://github.com/nimbold/Firelink.git
synced 2026-08-11 03:59:09 +00:00
fix(media): stabilize YouTube size estimates
This commit is contained in:
@@ -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.');
|
||||
|
||||
@@ -207,6 +207,86 @@ describe('useDownloadProgressStore', () => {
|
||||
release();
|
||||
});
|
||||
|
||||
it('drops a persisted temporary media estimate when fragmented progress has no total', async () => {
|
||||
const handlers: Record<string, (event: any) => 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<string, (event: any) => 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<string, (event: any) => void> = {};
|
||||
vi.mocked(ipc.listenEvent).mockImplementation((event, handler) => {
|
||||
|
||||
@@ -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);
|
||||
}
|
||||
|
||||
@@ -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'];
|
||||
|
||||
@@ -447,6 +447,64 @@ const normalizeQueuePositions = (downloads: DownloadItem[]): DownloadItem[] => {
|
||||
});
|
||||
};
|
||||
|
||||
const TEMPORARY_MEDIA_ESTIMATE_MAX_BYTES = 1024;
|
||||
const DISPLAYED_SIZE_UNIT_MULTIPLIERS: Record<string, number> = {
|
||||
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<DownloadItem, 'isMedia' | 'downloadedBytes' | 'totalBytes' | 'totalIsEstimate' | 'size'>
|
||||
): 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<DownloadItem, 'status' | 'queueId' | 'hasBeenDispatched'>;
|
||||
export type DownloadDraft = Omit<DownloadItem, 'status' | 'queueId' | 'hasBeenDispatched'> & {
|
||||
/** 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<DownloadState>((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<DownloadState>((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 => ({
|
||||
|
||||
Reference in New Issue
Block a user