-
-
Notifications
You must be signed in to change notification settings - Fork 421
Fix download metadata / video UI cards, fix dash downloads #586
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: main
Are you sure you want to change the base?
Changes from 2 commits
a2be340
b6b0748
312e0ad
b77df88
1c73c29
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change | ||||
|---|---|---|---|---|---|---|
|
|
@@ -289,6 +289,16 @@ | |||||
| normalized = { ...normalized, audioQuality: derivedQuality }; | ||||||
| } | ||||||
|
|
||||||
| const copyrightText = | ||||||
| typeof normalized.copyright === 'string' | ||||||
| ? normalized.copyright | ||||||
| : normalized.copyright && typeof normalized.copyright === 'object' | ||||||
| ? normalized.copyright.text | ||||||
| : normalized.copyright; | ||||||
| if (copyrightText !== normalized.copyright) { | ||||||
| normalized = { ...normalized, copyright: copyrightText ?? '' }; | ||||||
| } | ||||||
|
|
||||||
| normalized.isUnavailable = isTrackUnavailable(normalized); | ||||||
|
|
||||||
| return normalized.type == 'video' ? new PreparedVideo(normalized) : new PreparedTrack(normalized); | ||||||
|
|
@@ -312,6 +322,20 @@ | |||||
| normalized.artist = video.artists[0]; | ||||||
| } | ||||||
|
|
||||||
| if (!normalized.imageId) { | ||||||
| const imageCandidate = video.imageId || video.squareImage || video.image || video.cover; | ||||||
| if (typeof imageCandidate === 'string' || typeof imageCandidate === 'number') { | ||||||
| normalized.imageId = imageCandidate; | ||||||
| } | ||||||
| } | ||||||
|
|
||||||
| if (!normalized.image) { | ||||||
| const imageCandidate = video.image || video.squareImage || video.cover || normalized.imageId; | ||||||
| if (typeof imageCandidate === 'string' || typeof imageCandidate === 'number') { | ||||||
| normalized.image = imageCandidate; | ||||||
| } | ||||||
| } | ||||||
|
|
||||||
| return normalized; | ||||||
| } | ||||||
|
|
||||||
|
|
@@ -1614,7 +1638,36 @@ | |||||
| } | ||||||
|
|
||||||
| const id = input?.id || input; | ||||||
| const track = typeof input === 'object' ? input : await this.getTrack(id, downloadQuality); | ||||||
| const hasMissingDownloadMetadata = (candidate) => | ||||||
| candidate?.trackNumber == null || | ||||||
| (candidate?.volumeNumber == null && candidate?.discNumber == null) || | ||||||
| candidate?.album?.numberOfTracks == null; | ||||||
|
|
||||||
| let track = typeof input === 'object' ? this.prepareTrack(input) : await this.getTrack(id, downloadQuality); | ||||||
| if ( | ||||||
| typeof input === 'object' && | ||||||
| !track?.type?.toLowerCase?.().includes('video') && | ||||||
| hasMissingDownloadMetadata(track) | ||||||
| ) { | ||||||
| try { | ||||||
| const fullTrack = await this.getTrackMetadata(id); | ||||||
| track = this.prepareTrack({ | ||||||
| ...fullTrack, | ||||||
| ...track, | ||||||
| trackNumber: track?.trackNumber ?? fullTrack?.trackNumber, | ||||||
| volumeNumber: track?.volumeNumber ?? fullTrack?.volumeNumber, | ||||||
| discNumber: track?.discNumber ?? fullTrack?.discNumber, | ||||||
| album: { | ||||||
| ...(fullTrack?.album || {}), | ||||||
| ...(track?.album || {}), | ||||||
| }, | ||||||
| artist: track?.artist || fullTrack?.artist, | ||||||
| artists: track?.artists?.length ? track.artists : fullTrack?.artists, | ||||||
| }); | ||||||
| } catch (e) { | ||||||
| console.warn('Failed to hydrate full track metadata for download:', e); | ||||||
| } | ||||||
| } | ||||||
| const isVideo = track?.type?.toLowerCase().includes('video'); | ||||||
|
||||||
| const isVideo = track?.type?.toLowerCase().includes('video'); | |
| const isVideo = (track?.type?.toLowerCase?.() || '').includes('video'); |
Check failure on line 1962 in js/api.js
GitHub Actions / test
[chromium] js/api.test.ts > Track Downloads > 'Low'
Error: Download failed. The stream may require a proxy.
❯ LosslessAPI.downloadTrack js/api.js:1962:18
❯ downloadTrack js/api.test.ts:109:15
❯ js/api.test.ts:282:21
Check failure on line 1962 in js/api.js
GitHub Actions / test
[chromium] js/api.test.ts > Track Downloads > 'High'
Error: Download failed. The stream may require a proxy.
❯ LosslessAPI.downloadTrack js/api.js:1962:18
❯ downloadTrack js/api.test.ts:109:15
❯ js/api.test.ts:282:21
Check failure on line 1962 in js/api.js
GitHub Actions / test
[chromium] js/api.test.ts > Track Downloads > 'Lossless, but not really'
Error: Download failed. The stream may require a proxy.
❯ LosslessAPI.downloadTrack js/api.js:1962:18
❯ downloadTrack js/api.test.ts:109:15
❯ js/api.test.ts:282:21
Check failure on line 1962 in js/api.js
GitHub Actions / test
[chromium] js/api.test.ts > Track Downloads > 'Lossless (Unchanged)'
Error: Download failed. The stream may require a proxy.
❯ LosslessAPI.downloadTrack js/api.js:1962:18
❯ downloadTrack js/api.test.ts:109:15
❯ js/api.test.ts:282:21
Check failure on line 1962 in js/api.js
GitHub Actions / test
[chromium] js/api.test.ts > Track Downloads > 'HD Lossless (Unchanged)'
Error: Download failed. The stream may require a proxy.
❯ LosslessAPI.downloadTrack js/api.js:1962:18
❯ downloadTrack js/api.test.ts:109:15
❯ js/api.test.ts:282:21
Check failure on line 1962 in js/api.js
GitHub Actions / test
[chromium] js/api.test.ts > Track Downloads > 'Lossless (ALAC)'
Error: Download failed. The stream may require a proxy.
❯ LosslessAPI.downloadTrack js/api.js:1962:18
❯ downloadTrack js/api.test.ts:109:15
❯ js/api.test.ts:282:21
Check failure on line 1962 in js/api.js
GitHub Actions / test
[chromium] js/api.test.ts > Track Downloads > 'HD Lossless (ALAC)'
Error: Download failed. The stream may require a proxy.
❯ LosslessAPI.downloadTrack js/api.js:1962:18
❯ downloadTrack js/api.test.ts:109:15
❯ js/api.test.ts:282:21
Check failure on line 1962 in js/api.js
GitHub Actions / test
[chromium] js/api.test.ts > Track Downloads > 'Lossless (FLAC)'
Error: Download failed. The stream may require a proxy.
❯ LosslessAPI.downloadTrack js/api.js:1962:18
❯ downloadTrack js/api.test.ts:109:15
❯ js/api.test.ts:282:21
Check failure on line 1962 in js/api.js
GitHub Actions / test
[chromium] js/api.test.ts > Track Downloads > 'HD Lossless (FLAC)'
Error: Download failed. The stream may require a proxy.
❯ LosslessAPI.downloadTrack js/api.js:1962:18
❯ downloadTrack js/api.test.ts:109:15
❯ js/api.test.ts:282:21
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -315,8 +315,19 @@ function removeBulkDownloadTask(notifEl) { | |
| } | ||
|
|
||
| async function downloadTrackBlob(track, quality, api, signal = null, onProgress = null) { | ||
| const tidalAPI = api.tidalAPI || api; | ||
| let downloadTrack = track; | ||
| try { | ||
| if (typeof tidalAPI.enrichTrack === 'function') { | ||
| const { enrichedTrack } = await tidalAPI.enrichTrack(track, { downloadQuality: quality }); | ||
| if (enrichedTrack) downloadTrack = enrichedTrack; | ||
| } | ||
| } catch (e) { | ||
| console.warn('Failed to enrich track metadata before bulk download:', e); | ||
| } | ||
|
|
||
| const blob = await api.downloadTrack(track.id, quality, undefined, { | ||
| track, | ||
| track: downloadTrack, | ||
| signal, | ||
| onProgress, | ||
| triggerDownload: false, | ||
|
|
@@ -326,7 +337,7 @@ async function downloadTrackBlob(track, quality, api, signal = null, onProgress | |
| // Detect actual format from blob signature BEFORE adding metadata | ||
| const extension = await getExtensionFromBlob(blob); | ||
|
|
||
| return { blob, extension }; | ||
| return { blob, extension, track: downloadTrack }; | ||
|
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Apply the enriched-track return value to discography downloads too.
🔧 Proposed follow-up fix for the discography caller- const { blob, extension } = await downloadTrackBlob(track, quality, api, signal, null);
- const filename = buildTrackFilename(track, quality, extension);
+ const {
+ blob,
+ extension,
+ track: enrichedTrack,
+ } = await downloadTrackBlob(track, quality, api, signal, null);
+ const effectiveTrack = enrichedTrack || track;
+ const filename = buildTrackFilename(effectiveTrack, quality, extension);- const lyricsData = await lyricsManager.fetchLyrics(track.id, track);
+ const lyricsData = await lyricsManager.fetchLyrics(effectiveTrack.id, effectiveTrack);
if (lyricsData) {
- const lrcContent = lyricsManager.generateLRCContent(lyricsData, track);
+ const lrcContent = lyricsManager.generateLRCContent(lyricsData, effectiveTrack);🤖 Prompt for AI Agents |
||
| } | ||
|
|
||
| async function bulkDownload({ | ||
|
|
@@ -365,7 +376,11 @@ async function bulkDownload({ | |
| updateBulkDownloadProgress(notification, i, tracks.length, trackTitle); | ||
|
|
||
| try { | ||
| const { blob, extension } = await downloadTrackBlob(track, quality, api, signal, (p) => { | ||
| const { | ||
| blob, | ||
| extension, | ||
| track: enrichedTrack, | ||
| } = await downloadTrackBlob(track, quality, api, signal, (p) => { | ||
|
Comment on lines
+379
to
+383
|
||
| if (p instanceof DownloadProgress && p.totalBytes && p.receivedBytes) { | ||
| fileFraction = p.receivedBytes / p.totalBytes; | ||
| } else if (p instanceof SegmentedDownloadProgress && p.currentSegment && p.totalSegments) { | ||
|
|
@@ -375,7 +390,8 @@ async function bulkDownload({ | |
| fileFraction = Math.min(fileFraction, 0.99); // Cap at 99% to avoid showing 100% before finalization | ||
| updateBulkDownloadProgress(notification, i + fileFraction, tracks.length, trackTitle, p); | ||
| }); | ||
| const filename = buildTrackFilename(track, quality, extension); | ||
| const effectiveTrack = enrichedTrack || track; | ||
| const filename = buildTrackFilename(effectiveTrack, quality, extension); | ||
|
Comment on lines
+393
to
+394
|
||
| const discNumber = discLayout.resolveDiscNumber(i); | ||
| const discPath = separateByDisc ? `${getDiscFolderName(discNumber)}/${filename}` : filename; | ||
|
|
||
|
|
@@ -389,9 +405,9 @@ async function bulkDownload({ | |
|
|
||
| if (lyricsManager && lyricsSettings.shouldDownloadLyrics()) { | ||
| try { | ||
| const lyricsData = await lyricsManager.fetchLyrics(track.id, track); | ||
| const lyricsData = await lyricsManager.fetchLyrics(effectiveTrack.id, effectiveTrack); | ||
| if (lyricsData) { | ||
| const lrcContent = lyricsManager.generateLRCContent(lyricsData, track); | ||
| const lrcContent = lyricsManager.generateLRCContent(lyricsData, effectiveTrack); | ||
| if (lrcContent) { | ||
| const lrcFilename = filename.replace(/\.[^.]+$/, '.lrc'); | ||
| yield { | ||
|
|
@@ -1079,7 +1095,7 @@ export async function downloadTrackWithMetadata( | |
| triggerDownload: false, | ||
| }); | ||
|
|
||
| const finalFilename = buildTrackFilename(track, quality, await getExtensionFromBlob(blob)) | ||
| const finalFilename = buildTrackFilename(enrichedTrack, quality, await getExtensionFromBlob(blob)) | ||
| .split('/') | ||
| .pop(); | ||
|
|
||
|
|
@@ -1102,13 +1118,13 @@ export async function downloadTrackWithMetadata( | |
|
|
||
| if (lyricsManager && lyricsSettings.shouldDownloadLyrics()) { | ||
| try { | ||
| const lyricsData = await lyricsManager.fetchLyrics(track.id, track); | ||
| const lyricsData = await lyricsManager.fetchLyrics(enrichedTrack.id, enrichedTrack); | ||
| if (lyricsData) { | ||
| await folderWriter.write( | ||
| singleWriterEntry({ | ||
| name: [...entryName.split('.').slice(0, -1), 'lrc'].join('.'), | ||
| lastModified: new Date(), | ||
| input: lyricsManager.getLRC(lyricsData, track), | ||
| input: lyricsManager.getLRC(lyricsData, enrichedTrack), | ||
| }) | ||
| ); | ||
| } | ||
|
|
||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Preserve hydrated album fields when the original value is nullish.
Because
track.albumis spread afterfullTrack.album, an incomplete source withalbum.numberOfTracks: nulloverwrites the hydrated value. That leaves the exact missing metadata from Lines 1641-1644 unresolved.🔧 Proposed fix
album: { ...(fullTrack?.album || {}), ...(track?.album || {}), + numberOfTracks: track?.album?.numberOfTracks ?? fullTrack?.album?.numberOfTracks, },📝 Committable suggestion
🤖 Prompt for AI Agents