From 5dac33dfa4cc6f1b73e68f3bf88a672699cf2a2f Mon Sep 17 00:00:00 2001 From: zarzet Date: Fri, 28 Aug 2026 22:35:48 +0700 Subject: [PATCH] fix(download): preserve finalization metadata and library labels --- .../zarz/spotiflac/NativeDownloadFinalizer.kt | 8 +- .../spotiflac/NativeFinalizationPolicy.kt | 16 ++++ .../spotiflac/NativeFinalizationPolicyTest.kt | 34 ++++++++ .../download_queue_provider_finalization.dart | 35 +++++++-- .../download_queue_provider_single_item.dart | 27 ++++++- lib/screens/queue_library_refresh_policy.dart | 33 ++++++++ lib/screens/queue_tab_collection_items.dart | 12 ++- lib/screens/queue_tab_filter_widgets.dart | 25 +++--- lib/utils/audio_format_utils.dart | 15 ++++ test/models_and_utils_test.dart | 8 ++ test/queue_library_refresh_policy_test.dart | 77 +++++++++++++++++++ 11 files changed, 264 insertions(+), 26 deletions(-) diff --git a/android/app/src/main/kotlin/com/zarz/spotiflac/NativeDownloadFinalizer.kt b/android/app/src/main/kotlin/com/zarz/spotiflac/NativeDownloadFinalizer.kt index 98b01a30..f3875647 100644 --- a/android/app/src/main/kotlin/com/zarz/spotiflac/NativeDownloadFinalizer.kt +++ b/android/app/src/main/kotlin/com/zarz/spotiflac/NativeDownloadFinalizer.kt @@ -18,8 +18,6 @@ import com.zarz.spotiflac.SafDownloadHandler.normalizeExt import com.zarz.spotiflac.NativeFinalizationPolicy.applyQualityVariantFilenameLabel import com.zarz.spotiflac.NativeFinalizationPolicy.displayAudioQuality import com.zarz.spotiflac.NativeFinalizationPolicy.formatIndexTag -import com.zarz.spotiflac.NativeFinalizationPolicy.isLosslessAudioCodec -import com.zarz.spotiflac.NativeFinalizationPolicy.isLossyAudioCodec import com.zarz.spotiflac.NativeFinalizationPolicy.normalizeAudioCodec import com.zarz.spotiflac.NativeFinalizationPolicy.resolvePreferredDecryptionExtension import gobackend.Gobackend @@ -808,7 +806,11 @@ object NativeDownloadFinalizer { try { val codec = probePrimaryAudioCodec(localInput, shouldCancel) val isAlreadyNativeFlac = codec == "flac" && isNativeFlacFile(localInput) - if (!isLosslessAudioCodec(codec)) { + if (!NativeFinalizationPolicy.shouldAttemptLosslessContainerConversion( + forceContainerConversion, + codec, + ) + ) { Log.d(TAG, "Preserving native container; audio codec is ${codec.ifBlank { "unknown" }}") // The preserved stream is not FLAC but still carries the // requested .flac name. Rename to the real container so the diff --git a/android/app/src/main/kotlin/com/zarz/spotiflac/NativeFinalizationPolicy.kt b/android/app/src/main/kotlin/com/zarz/spotiflac/NativeFinalizationPolicy.kt index f2ad7b08..16ddf015 100644 --- a/android/app/src/main/kotlin/com/zarz/spotiflac/NativeFinalizationPolicy.kt +++ b/android/app/src/main/kotlin/com/zarz/spotiflac/NativeFinalizationPolicy.kt @@ -158,6 +158,22 @@ internal object NativeFinalizationPolicy { ) } + /** + * Decides whether a stream should enter the lossless container conversion + * path. A missing or generic M4A/MP4 codec is inconclusive rather than + * proof of lossy audio; an extension capability may explicitly request + * that FFmpeg attempt the conversion. Known lossy codecs remain native. + */ + fun shouldAttemptLosslessContainerConversion( + forceConversion: Boolean, + probedCodec: String?, + ): Boolean { + if (isLosslessAudioCodec(probedCodec)) return true + val normalized = normalizeAudioCodec(probedCodec) + val inconclusive = normalized == null || normalized == "m4a" + return forceConversion && inconclusive + } + fun displayAudioQuality( filePath: String, fileName: String, diff --git a/android/app/src/test/kotlin/com/zarz/spotiflac/NativeFinalizationPolicyTest.kt b/android/app/src/test/kotlin/com/zarz/spotiflac/NativeFinalizationPolicyTest.kt index 30bf4f6c..f78c02da 100644 --- a/android/app/src/test/kotlin/com/zarz/spotiflac/NativeFinalizationPolicyTest.kt +++ b/android/app/src/test/kotlin/com/zarz/spotiflac/NativeFinalizationPolicyTest.kt @@ -101,6 +101,40 @@ class NativeFinalizationPolicyTest { assertFalse(NativeFinalizationPolicy.isLosslessAudioCodec("aac")) } + @Test + fun forcedContainerConversionAttemptsInconclusiveCodecButPreservesLossy() { + assertTrue( + NativeFinalizationPolicy.shouldAttemptLosslessContainerConversion( + forceConversion = true, + probedCodec = null, + ), + ) + assertTrue( + NativeFinalizationPolicy.shouldAttemptLosslessContainerConversion( + forceConversion = true, + probedCodec = "m4a", + ), + ) + assertTrue( + NativeFinalizationPolicy.shouldAttemptLosslessContainerConversion( + forceConversion = false, + probedCodec = "flac", + ), + ) + assertFalse( + NativeFinalizationPolicy.shouldAttemptLosslessContainerConversion( + forceConversion = true, + probedCodec = "aac", + ), + ) + assertFalse( + NativeFinalizationPolicy.shouldAttemptLosslessContainerConversion( + forceConversion = false, + probedCodec = null, + ), + ) + } + @Test fun isoBmffAudioUsesM4aExceptForAc4Passthrough() { assertEquals( diff --git a/lib/providers/download_queue_provider_finalization.dart b/lib/providers/download_queue_provider_finalization.dart index 3c4c1ee5..0db4eb10 100644 --- a/lib/providers/download_queue_provider_finalization.dart +++ b/lib/providers/download_queue_provider_finalization.dart @@ -1114,15 +1114,22 @@ extension _DownloadQueueFinalization on DownloadQueueNotifier { result['audio_codec']?.toString() ?? result['actual_audio_codec']?.toString(), ); - if (isLossyAudioFormat(resultAudioFormat)) { + final requiresContainerConversion = + result['requires_container_conversion'] == true || + result['requiresContainerConversion'] == true || + _shouldRequestContainerConversion( + context.item.service, + context.outputExt, + ); + // M4A/MP4 identifies a container, not necessarily a lossy codec. When an + // extension explicitly requests conversion, probe the stream below rather + // than treating the container label as proof that its audio is lossy. + if (!requiresContainerConversion && isLossyAudioFormat(resultAudioFormat)) { _log.d( 'Native-worker output is $resultAudioFormat; preserving native container.', ); return filePath; } - final requiresContainerConversion = - result['requires_container_conversion'] == true || - result['requiresContainerConversion'] == true; final resultOutputExt = _downloadResultOutputExt( result, filePath: filePath, @@ -1180,6 +1187,13 @@ extension _DownloadQueueFinalization on DownloadQueueNotifier { ); } + void markFinalOutputAsFlac() { + result['audio_codec'] = 'flac'; + result['format'] = 'flac'; + result['actual_extension'] = '.flac'; + result['output_extension'] = '.flac'; + } + if (context.storageMode == 'saf' && isContentUri(filePath)) { final treeUri = context.downloadTreeUri; if (treeUri == null || treeUri.isEmpty) { @@ -1195,7 +1209,10 @@ extension _DownloadQueueFinalization on DownloadQueueNotifier { final codec = await FFmpegService.probePrimaryAudioCodec(tempPath); final isAlreadyNativeFlac = codec == 'flac' && await FFmpegService.isNativeFlacFile(tempPath); - if (!FFmpegService.isLosslessAudioCodec(codec)) { + final shouldAttemptConversion = + FFmpegService.isLosslessAudioCodec(codec) || + (requiresContainerConversion && isInconclusiveAudioCodec(codec)); + if (!shouldAttemptConversion) { _log.d( 'Preserving native container; audio codec is ${codec ?? 'unknown'}, ' 'no FLAC container conversion needed.', @@ -1241,13 +1258,17 @@ extension _DownloadQueueFinalization on DownloadQueueNotifier { return null; } result['file_name'] = producedFileName; + markFinalOutputAsFlac(); return newUri; } final codec = await FFmpegService.probePrimaryAudioCodec(filePath); final isAlreadyNativeFlac = codec == 'flac' && await FFmpegService.isNativeFlacFile(filePath); - if (!FFmpegService.isLosslessAudioCodec(codec)) { + final shouldAttemptConversion = + FFmpegService.isLosslessAudioCodec(codec) || + (requiresContainerConversion && isInconclusiveAudioCodec(codec)); + if (!shouldAttemptConversion) { _log.d( 'Preserving native container; audio codec is ${codec ?? 'unknown'}, ' 'no FLAC container conversion needed.', @@ -1265,6 +1286,7 @@ extension _DownloadQueueFinalization on DownloadQueueNotifier { flacPath = targetPath; } await embedFlacMetadata(flacPath); + markFinalOutputAsFlac(); return flacPath; } final flacPath = await FFmpegService.convertM4aToFlac(filePath); @@ -1272,6 +1294,7 @@ extension _DownloadQueueFinalization on DownloadQueueNotifier { return null; } await embedFlacMetadata(flacPath); + markFinalOutputAsFlac(); return flacPath; } diff --git a/lib/providers/download_queue_provider_single_item.dart b/lib/providers/download_queue_provider_single_item.dart index 705052c2..572d68be 100644 --- a/lib/providers/download_queue_provider_single_item.dart +++ b/lib/providers/download_queue_provider_single_item.dart @@ -691,11 +691,13 @@ class _DownloadRun { result['audio_codec']?.toString() ?? result['actual_audio_codec']?.toString(), ); - final resultIsLossyAudio = isLossyAudioFormat(resultAudioFormat); + final resultIsKnownLossyAudio = + isLossyAudioFormat(resultAudioFormat) && + !isInconclusiveAudioCodec(resultAudioFormat); final requiresContainerConversion = result['requires_container_conversion'] == true || result['requiresContainerConversion'] == true || - (!resultIsLossyAudio && + (!resultIsKnownLossyAudio && n._shouldRequestContainerConversion(actualService, safOutputExt)); final preferredOutputExt = n._extensionPreferredOutputExt(actualService); shouldPreserveNativeM4a = @@ -987,6 +989,14 @@ class _DownloadRun { } } + void _markFinalOutputAsFlac() { + result['audio_codec'] = 'flac'; + result['format'] = 'flac'; + result['actual_extension'] = '.flac'; + result['output_extension'] = '.flac'; + resultOutputExt = '.flac'; + } + Future _publishDeferredSafOutputOnce() async { final localPath = filePath; if (localPath == null || isContentUri(localPath)) return true; @@ -1252,7 +1262,10 @@ class _DownloadRun { final codec = await FFmpegService.probePrimaryAudioCodec(tempPath); final isAlreadyNativeFlac = codec == 'flac' && await FFmpegService.isNativeFlacFile(tempPath); - if (!FFmpegService.isLosslessAudioCodec(codec)) { + final shouldAttemptConversion = + FFmpegService.isLosslessAudioCodec(codec) || + isInconclusiveAudioCodec(codec); + if (!shouldAttemptConversion) { _log.d( 'Preserving native container; audio codec is ${codec ?? 'unknown'}, ' 'no FLAC container conversion needed.', @@ -1309,6 +1322,7 @@ class _DownloadRun { if (newUri != null) { filePath = newUri; finalSafFileName = producedFileName; + _markFinalOutputAsFlac(); } else if (branch == 'nativeFlac') { _log.w('Failed to write native FLAC to SAF'); } else if (branch == 'convert') { @@ -1437,7 +1451,10 @@ class _DownloadRun { final isAlreadyNativeFlac = codec == 'flac' && await FFmpegService.isNativeFlacFile(currentFilePath); - if (!FFmpegService.isLosslessAudioCodec(codec)) { + final shouldAttemptConversion = + FFmpegService.isLosslessAudioCodec(codec) || + isInconclusiveAudioCodec(codec); + if (!shouldAttemptConversion) { _log.d( 'Preserving native container; audio codec is ${codec ?? 'unknown'}, ' 'no FLAC container conversion needed.', @@ -1462,6 +1479,7 @@ class _DownloadRun { } await _embedFinalMetadata(flacPath, format: 'flac'); + _markFinalOutputAsFlac(); } else { n.updateItemStatus( item.id, @@ -1474,6 +1492,7 @@ class _DownloadRun { if (flacPath != null) { filePath = flacPath; + _markFinalOutputAsFlac(); _log.d('Converted to FLAC: $flacPath'); _log.d('Embedding metadata and cover to converted FLAC...'); diff --git a/lib/screens/queue_library_refresh_policy.dart b/lib/screens/queue_library_refresh_policy.dart index 45b308cd..2ccf403c 100644 --- a/lib/screens/queue_library_refresh_policy.dart +++ b/lib/screens/queue_library_refresh_policy.dart @@ -1,6 +1,8 @@ import 'dart:async'; import 'package:flutter/foundation.dart'; +import 'package:spotiflac_android/utils/audio_format_utils.dart'; +import 'package:spotiflac_android/utils/audio_quality_badge_policy.dart'; import 'package:spotiflac_android/services/downloaded_embedded_cover_resolver.dart'; import 'package:spotiflac_android/services/library_database.dart'; import 'package:spotiflac_android/utils/file_access.dart'; @@ -31,6 +33,37 @@ bool shouldRetainQueueLibraryPageSnapshot({ required bool activeDownloadFallbackAvailable, }) => currentIsEmpty && cachedHasContent && activeDownloadFallbackAvailable; +/// Keeps a completed queue card visible until the matching Library row has +/// actually landed. A completed item with an in-memory History row must not +/// expire merely because the rest of its album batch was cancelled: the +/// paged Library query can still be one refresh behind that persisted row. +bool shouldRetainCompletionBridge({ + required bool isRequeued, + required bool hasActiveDownloads, + required bool libraryRowLanded, + required bool hasHistoryItem, + required bool expired, +}) { + if (isRequeued || libraryRowLanded) return false; + if (hasActiveDownloads || hasHistoryItem) return true; + return !expired; +} + +/// Builds a mode-aware label while a just-completed item is waiting for its +/// History row. This avoids pinning the card to the track's old quality text +/// when the user changes the Library label setting during an album download. +String? buildCompletionBridgeFallbackQualityLabel({ + required String mode, + required String? completedItemFilePath, + required String? storedQuality, +}) { + return buildLibraryAudioQualityLabel( + mode: mode, + format: audioFormatForPath(completedItemFilePath), + storedQuality: storedQuality, + ); +} + /// Returns distinct final-path candidates for a just-completed download. /// History is authoritative after conversion/SAF publication, while the /// completed queue item remains a safe fallback if the matched history row is diff --git a/lib/screens/queue_tab_collection_items.dart b/lib/screens/queue_tab_collection_items.dart index 3b47204e..46bd4f75 100644 --- a/lib/screens/queue_tab_collection_items.dart +++ b/lib/screens/queue_tab_collection_items.dart @@ -121,7 +121,11 @@ extension _QueueTabCollectionItemWidgets on _QueueTabState { : UnifiedLibraryItem.fromDownloadHistory(historyItem); final quality = unifiedItem?.qualityForMode(_libraryQualityLabelMode) ?? - track.audioQuality; + buildCompletionBridgeFallbackQualityLabel( + mode: _libraryQualityLabelMode, + completedItemFilePath: item.filePath, + storedQuality: track.audioQuality, + ); final playablePath = resolveCompletionBridgePlayablePath( historyFilePath: historyItem?.filePath, completedItemFilePath: item.filePath, @@ -258,7 +262,11 @@ extension _QueueTabCollectionItemWidgets on _QueueTabState { : UnifiedLibraryItem.fromDownloadHistory(historyItem); final quality = unifiedItem?.qualityForMode(_libraryQualityLabelMode) ?? - track.audioQuality; + buildCompletionBridgeFallbackQualityLabel( + mode: _libraryQualityLabelMode, + completedItemFilePath: item.filePath, + storedQuality: track.audioQuality, + ); final playablePath = resolveCompletionBridgePlayablePath( historyFilePath: historyItem?.filePath, completedItemFilePath: item.filePath, diff --git a/lib/screens/queue_tab_filter_widgets.dart b/lib/screens/queue_tab_filter_widgets.dart index d2e5a003..e79cd7d5 100644 --- a/lib/screens/queue_tab_filter_widgets.dart +++ b/lib/screens/queue_tab_filter_widgets.dart @@ -81,23 +81,26 @@ extension _QueueTabFilterWidgets on _QueueTabState { final pending = []; final hasActiveDownloads = activeDownloadIds.isNotEmpty; _completionBridge.forEach((id, _) { - final historyId = bridgeHistoryById[id]?.id ?? id; + final historyItem = bridgeHistoryById[id]; + final historyId = historyItem?.id ?? id; final landed = libIdSet.contains('dl_$historyId'); final addedAt = _completionBridgeAt[id]; final expired = addedAt == null || now.difference(addedAt).inSeconds >= 6; - if (activeDownloadIds.contains(id)) { - // Re-queued (retry): the live row takes over from the bridge. - stale.add(id); - } else if (hasActiveDownloads) { - // Keep just-completed tracks pinned in the lead zone while the - // rest of the batch is still downloading, so they don't jump - // below the remaining queue the moment they finish. + if (shouldRetainCompletionBridge( + isRequeued: activeDownloadIds.contains(id), + hasActiveDownloads: hasActiveDownloads, + libraryRowLanded: landed, + hasHistoryItem: historyItem != null, + expired: expired, + )) { + // Keep completed tracks pinned while their batch is active or their + // persisted History row is still waiting for the paged Library. pending.add(id); - } else if (landed || expired) { - stale.add(id); } else { - pending.add(id); + // Re-queued items are represented by the live row; landed items by + // the normal Library row; unpersisted bridges retain a short grace. + stale.add(id); } }); bridgeIds = pending; diff --git a/lib/utils/audio_format_utils.dart b/lib/utils/audio_format_utils.dart index 7b26739a..5bfb6fe6 100644 --- a/lib/utils/audio_format_utils.dart +++ b/lib/utils/audio_format_utils.dart @@ -69,6 +69,10 @@ String? audioFormatForPath(String? filePath, {String? fileName}) { final candidates = [?filePath, ?fileName]; for (final candidate in candidates) { final lower = candidate.trim().toLowerCase(); + if (lower.endsWith('.flac')) return 'FLAC'; + if (lower.endsWith('.alac')) return 'ALAC'; + if (lower.endsWith('.wav')) return 'WAV'; + if (lower.endsWith('.aiff') || lower.endsWith('.aif')) return 'AIFF'; if (lower.endsWith('.opus') || lower.endsWith('.ogg')) return 'OPUS'; if (lower.endsWith('.mp3')) return 'MP3'; if (lower.endsWith('.aac')) return 'AAC'; @@ -153,6 +157,17 @@ bool isLossyAudioFormat(String? value) { }.contains(normalizeAudioFormatValue(value)); } +/// Whether a reported codec value only identifies the ISO-BMFF container. +/// `m4a`/`mp4` cannot tell callers whether the audio stream is AAC, ALAC, +/// FLAC, or another codec, so an explicit conversion request must probe or +/// attempt conversion instead of classifying it as known-lossy audio. +bool isInconclusiveAudioCodec(String? value) { + final raw = normalizeOptionalString(value); + if (raw == null) return true; + final normalized = raw.toLowerCase().replaceAll('-', '_'); + return normalized == 'm4a' || normalized == 'mp4'; +} + /// Returns a provider-independent quality label suitable for a filename. /// /// Requested labels such as LOSSLESS and HI_RES are intentionally ignored: diff --git a/test/models_and_utils_test.dart b/test/models_and_utils_test.dart index c29f071a..5d196694 100644 --- a/test/models_and_utils_test.dart +++ b/test/models_and_utils_test.dart @@ -1305,6 +1305,14 @@ void main() { ); }); + test('treats generic M4A container reports as inconclusive codecs', () { + expect(isInconclusiveAudioCodec(null), isTrue); + expect(isInconclusiveAudioCodec('m4a'), isTrue); + expect(isInconclusiveAudioCodec('MP4'), isTrue); + expect(isInconclusiveAudioCodec('flac'), isFalse); + expect(isInconclusiveAudioCodec('aac'), isFalse); + }); + test('uses m4a for ISO-BMFF audio except AC-4 passthrough', () { expect(isoBmffAudioExtensionForCodec('opus'), '.m4a'); expect(isoBmffAudioExtensionForCodec('ec-3'), '.m4a'); diff --git a/test/queue_library_refresh_policy_test.dart b/test/queue_library_refresh_policy_test.dart index d3bfedb8..6c2442bc 100644 --- a/test/queue_library_refresh_policy_test.dart +++ b/test/queue_library_refresh_policy_test.dart @@ -2,6 +2,7 @@ import 'dart:io'; import 'package:flutter_test/flutter_test.dart'; +import 'package:spotiflac_android/models/settings.dart'; import 'package:spotiflac_android/screens/queue_library_refresh_policy.dart'; import 'package:spotiflac_android/services/library_database.dart'; @@ -108,6 +109,82 @@ void main() { ); }); + test( + 'completion bridge survives partial album cancellation until landing', + () { + expect( + shouldRetainCompletionBridge( + isRequeued: false, + hasActiveDownloads: false, + libraryRowLanded: false, + hasHistoryItem: true, + expired: true, + ), + isTrue, + ); + expect( + shouldRetainCompletionBridge( + isRequeued: false, + hasActiveDownloads: false, + libraryRowLanded: true, + hasHistoryItem: true, + expired: true, + ), + isFalse, + ); + }, + ); + + test('unpersisted completion bridge retains only its short grace period', () { + expect( + shouldRetainCompletionBridge( + isRequeued: false, + hasActiveDownloads: false, + libraryRowLanded: false, + hasHistoryItem: false, + expired: false, + ), + isTrue, + ); + expect( + shouldRetainCompletionBridge( + isRequeued: false, + hasActiveDownloads: false, + libraryRowLanded: false, + hasHistoryItem: false, + expired: true, + ), + isFalse, + ); + }); + + test('completion bridge fallback follows the current label mode', () { + expect( + buildCompletionBridgeFallbackQualityLabel( + mode: AppSettings.libraryQualityLabelFileFormat, + completedItemFilePath: '/music/completed.flac', + storedQuality: '24-bit/96kHz', + ), + 'FLAC', + ); + expect( + buildCompletionBridgeFallbackQualityLabel( + mode: AppSettings.libraryQualityLabelBitDepthOnly, + completedItemFilePath: '/music/completed.flac', + storedQuality: '24-bit/96kHz', + ), + '24-bit', + ); + expect( + buildCompletionBridgeFallbackQualityLabel( + mode: AppSettings.libraryQualityLabelBitrate, + completedItemFilePath: '/music/completed.flac', + storedQuality: '24-bit/96kHz', + ), + isNull, + ); + }); + test( 'completion probe falls back from stale history to completed path', () async {