From e3b69a25ef024a9383f0981a972318e712913e58 Mon Sep 17 00:00:00 2001 From: zarzet <42882290+zarzet@users.noreply.github.com> Date: Tue, 13 Oct 2026 18:27:16 +0700 Subject: [PATCH] fix(download): probe M4A streams before preserving requested FLAC Provider and decryption extensions describe the downloaded container, not its audio codec. A decrypted M4A/MP4 can still carry FLAC, so a requested FLAC output now reaches the codec probe instead of being preserved early. Lossy streams keep their native container and still receive metadata. --- .../spotiflac/NativeDownloadContainerTest.kt | 54 +++++ .../zarz/spotiflac/NativeDownloadFinalizer.kt | 4 +- .../zarz/spotiflac/NativeFinalizerFFmpeg.kt | 8 - .../download_lossless_container_test.dart | 216 ++++++++++++++++++ .../download_queue_provider_single_item.dart | 27 ++- .../download_container_finalizer.dart | 18 +- test/download_container_finalizer_test.dart | 64 ++++-- 7 files changed, 341 insertions(+), 50 deletions(-) create mode 100644 integration_test/download_lossless_container_test.dart diff --git a/android/app/src/androidTest/kotlin/com/zarz/spotiflac/NativeDownloadContainerTest.kt b/android/app/src/androidTest/kotlin/com/zarz/spotiflac/NativeDownloadContainerTest.kt index 4015b17e..e0b51f11 100644 --- a/android/app/src/androidTest/kotlin/com/zarz/spotiflac/NativeDownloadContainerTest.kt +++ b/android/app/src/androidTest/kotlin/com/zarz/spotiflac/NativeDownloadContainerTest.kt @@ -202,6 +202,60 @@ class NativeDownloadContainerTest { } } + @Test + fun requestedFlacUsesTheActualStreamDespiteM4aProviderAndDecryptionHints() { + val context = InstrumentationRegistry.getInstrumentation().targetContext + val root = File(context.cacheDir, "lossless-container-${System.nanoTime()}").apply { mkdirs() } + try { + for ((codec, requested, expected) in listOf( + Triple("flac", ".flac", "flac"), + Triple("flac", ".m4a", "m4a"), + Triple("aac", ".flac", "m4a"), + )) { + val input = File(root, "track-$codec-$expected.m4a") + val fixture = NativeDownloadFinalizer.runFFmpegArguments(arrayOf( + "-v", "error", "-f", "lavfi", "-i", "sine=frequency=997:sample_rate=48000", + "-t", "0.3", "-c:a", codec, "-strict", "-2", "-f", "mp4", input.path, + )) + assertTrue(fixture.second, fixture.first) + assertTrue(isMP4ContainerFile(input.path)) + fun decodedAudio(file: File, name: String): ByteArray { + val pcm = File(root, name) + val decoded = NativeDownloadFinalizer.runFFmpegArguments(arrayOf( + "-v", "error", "-i", file.path, "-map", "0:a:0", + "-c:a", "pcm_s32le", "-f", "s32le", "-y", pcm.path, + )) + assertTrue(decoded.second, decoded.first) + return pcm.readBytes() + } + val before = decodedAudio(input, "before.pcm") + val request = JSONObject() + .put("contract_version", 1).put("item_id", "example-track") + .put("service", "example-provider").put("track_name", "Lossless container test") + .put("artist_name", "Example artist").put("album_name", "Example album") + .put("quality", "LOSSLESS").put("storage_mode", "app") + .put("output_ext", requested).put("embed_metadata", true) + val result = NativeDownloadFinalizer.finalize( + context, "example-track", request.toString(), "{}", + JSONObject().put("success", true).put("file_path", input.path) + .put("file_name", input.name).put("audio_codec", "m4a") + .put("output_extension", ".m4a") + .put("decryption", JSONObject().put("output_extension", ".m4a")), + "{\"save_download_history\":false}", + ) + assertTrue(result.toString(), result.getBoolean("success")) + assertTrue(result.toString(), result.getBoolean("native_finalized")) + val output = File(result.getString("file_path")) + assertEquals(expected, output.extension) + assertEquals(expected == "flac", NativeDownloadFinalizer.isNativeFlacFile(output.path)) + assertEquals(codec, NativeDownloadFinalizer.probePrimaryAudioCodec(output.path)) + assertTrue("$codec -> $expected changed decoded audio", before.contentEquals(decodedAudio(output, "after.pcm"))) + val tags = JSONObject(backend.readFileMetadata(output.path, output.name)) + assertEquals("Lossless container test", tags.getString("title")) + } + } finally { root.deleteRecursively() } + } + @Test fun misnamedMp4IsTaggedAndPublishedAsM4aWithoutOverwritingExistingAudio() { val context = InstrumentationRegistry.getInstrumentation().targetContext 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 76f8dd1e..cac6c25f 100644 --- a/android/app/src/main/kotlin/com/zarz/spotiflac/NativeDownloadFinalizer.kt +++ b/android/app/src/main/kotlin/com/zarz/spotiflac/NativeDownloadFinalizer.kt @@ -646,9 +646,9 @@ object NativeDownloadFinalizer { shouldCancel: () -> Boolean, ) { if (outputExt(input) != ".flac") return - val requestedDecryptionExt = requestedDecryptionOutputExt(input) val forceContainerConversion = shouldForceContainerConversion(input, state) - if (!forceContainerConversion && requestedDecryptionExt.isNotBlank() && requestedDecryptionExt != ".flac") return + // The decrypted container may still contain FLAC. Only the measured + // stream codec can decide whether an M4A/MP4 should be preserved. val mayNeedContainerConversion = forceContainerConversion || looksLikeM4a(state.filePath, state.fileName) || isMP4ContainerFile(state.filePath) || diff --git a/android/app/src/main/kotlin/com/zarz/spotiflac/NativeFinalizerFFmpeg.kt b/android/app/src/main/kotlin/com/zarz/spotiflac/NativeFinalizerFFmpeg.kt index 53287ed2..42016c11 100644 --- a/android/app/src/main/kotlin/com/zarz/spotiflac/NativeFinalizerFFmpeg.kt +++ b/android/app/src/main/kotlin/com/zarz/spotiflac/NativeFinalizerFFmpeg.kt @@ -242,11 +242,3 @@ internal fun NativeDownloadFinalizer.isNativeFlacFile(path: String): Boolean { false } } - -internal fun NativeDownloadFinalizer.requestedDecryptionOutputExt(input: NativeDownloadFinalizer.FinalizeInput): String { - val descriptor = input.result.optJSONObject("decryption") - return normalizeExt( - descriptor?.optString("output_extension", "") - ?.ifBlank { input.result.optString("output_extension", "") } - ) -} diff --git a/integration_test/download_lossless_container_test.dart b/integration_test/download_lossless_container_test.dart new file mode 100644 index 00000000..c2192276 --- /dev/null +++ b/integration_test/download_lossless_container_test.dart @@ -0,0 +1,216 @@ +import 'dart:async'; +import 'dart:convert'; +import 'dart:io'; + +import 'package:ffmpeg_kit_flutter_new_audio/ffmpeg_kit.dart'; +import 'package:ffmpeg_kit_flutter_new_audio/return_code.dart'; +import 'package:flutter/widgets.dart'; +import 'package:flutter_riverpod/flutter_riverpod.dart'; +import 'package:flutter_test/flutter_test.dart'; +import 'package:integration_test/integration_test.dart'; +// ignore: depend_on_referenced_packages +import 'package:path_provider_platform_interface/path_provider_platform_interface.dart'; +import 'package:shared_preferences/shared_preferences.dart'; +import 'package:spotiflac_android/models/download_item.dart'; +import 'package:spotiflac_android/models/settings.dart'; +import 'package:spotiflac_android/models/track.dart'; +import 'package:spotiflac_android/providers/download_queue_provider.dart'; +import 'package:spotiflac_android/providers/extension_provider.dart'; +import 'package:spotiflac_android/providers/settings_provider.dart'; +import 'package:spotiflac_android/services/app_state_database.dart'; +import 'package:spotiflac_android/services/ffmpeg_service.dart'; +import 'package:spotiflac_android/services/platform_bridge.dart'; + +class _Paths extends PathProviderPlatform { + _Paths(this._path); + final String _path; + @override + Future getApplicationDocumentsPath() async => _path; +} + +class _Settings extends SettingsNotifier { + _Settings(this._path); + final String _path; + @override + AppSettings build() => AppSettings( + downloadDirectory: _path, + nativeDownloadWorkerEnabled: false, + autoFallback: false, + embedLyrics: false, + embedReplayGain: false, + saveDownloadHistory: false, + ); +} + +class _Extensions extends ExtensionNotifier { + @override + ExtensionState build() => const ExtensionState( + isInitialized: true, + extensions: [ + Extension( + id: 'example.audio', + name: 'example.audio', + displayName: 'Example', + version: '1', + description: '', + enabled: true, + status: 'loaded', + hasDownloadProvider: true, + skipMetadataEnrichment: true, + skipLyrics: true, + ), + ], + ); +} + +void main() { + final binding = IntegrationTestWidgetsFlutterBinding.ensureInitialized(); + + testWidgets( + 'foreground queue finalizes lossless M4A without provider hints', + (tester) async { + final root = await Directory.systemTemp.createTemp('lossless-queue-'); + final originalPaths = PathProviderPlatform.instance; + PathProviderPlatform.instance = _Paths(root.path); + SharedPreferences.setMockInitialValues({ + 'app_state_migrated_queue_to_sqlite_v1': true, + }); + final db = await AppStateDatabase.instance.database; + final output = await Directory('${root.path}/output').create(); + final sources = await Directory('${root.path}/extensions').create(); + final data = await Directory('${root.path}/data').create(); + final scope = ProviderContainer( + overrides: [ + settingsProvider.overrideWith(() => _Settings(output.path)), + extensionProvider.overrideWith(_Extensions.new), + ], + ); + try { + await PlatformBridge.initExtensionSystem( + sources.path, + data.path, + masterKey: base64Encode(List.filled(32, 1)), + lyricsProviders: const [], + lyricsFetchOptions: const {}, + allowedDirectories: [output.path], + ); + final samples = {}; + for (final codec in ['flac', 'aac', 'alac']) { + final source = File('${root.path}/$codec.m4a'); + final session = await FFmpegKit.executeWithArguments([ + '-v', + 'error', + '-f', + 'lavfi', + '-i', + 'sine=frequency=997:sample_rate=48000', + '-t', + '0.3', + '-c:a', + codec, + '-strict', + '-2', + '-f', + 'mp4', + source.path, + ]); + expect( + ReturnCode.isSuccess(await session.getReturnCode()), + true, + reason: await session.getOutput(), + ); + samples[codec] = base64Encode(await source.readAsBytes()); + } + final extension = await Directory( + '${sources.path}/example.audio', + ).create(); + await File('${extension.path}/manifest.json').writeAsString( + jsonEncode({ + 'name': 'example.audio', + 'version': '1', + 'description': 'Container fixture', + 'type': ['download_provider'], + 'permissions': {'file': true}, + 'skipMetadataEnrichment': true, + }), + ); + await File('${extension.path}/index.js').writeAsString(''' +const samples = ${jsonEncode(samples)}; +registerExtension({ + checkAvailability(track) { return { available: true, track_id: track.id }; }, + download(trackId, quality, outputPath) { + const codec = quality === 'ALAC' ? 'alac' : (quality === 'LOSSY_FALLBACK' ? 'aac' : 'flac'); + const path = outputPath.slice(0, outputPath.lastIndexOf('.')) + '.m4a'; + file.writeBytes(path, samples[codec]); + return { success: true, file_path: path, output_extension: '.m4a' }; + } +}); +'''); + await PlatformBridge.loadExtensionsFromDir(sources.path); + await PlatformBridge.setExtensionEnabled('example.audio', true); + binding.handleAppLifecycleStateChanged(AppLifecycleState.resumed); + final queue = scope.read(downloadQueueProvider.notifier); + queue.pauseQueue(persistAcrossRestarts: false); + await queue.flushQueuePersistence(); + for (final (quality, codec, extension) in [ + ('LOSSLESS', 'flac', '.flac'), + ('LOSSY_FALLBACK', 'aac', '.m4a'), + ('ALAC', 'alac', '.m4a'), + ]) { + final done = Completer(); + final id = queue.addToQueue( + Track( + id: 'example.audio:$codec', + source: 'example.audio', + name: 'Track $codec', + artistName: 'Example artist', + albumName: 'Example album', + duration: 1, + ), + 'example.audio', + qualityOverride: quality, + ); + DownloadItem? finished; + final subscription = scope.listen(downloadQueueProvider, (_, next) { + final item = next.items.where((item) => item.id == id).firstOrNull; + if (item != null && + (item.status == DownloadStatus.completed || + item.status == DownloadStatus.failed)) { + finished = item; + } + // Successful items leave the queue before processing becomes idle. + if (!next.isProcessing && finished != null && !done.isCompleted) { + done.complete(finished); + } + }); + try { + queue.resumeQueue(); + final item = await done.future.timeout(const Duration(seconds: 45)); + expect(item.status, DownloadStatus.completed, reason: item.error); + expect(item.filePath, endsWith(extension)); + expect( + await FFmpegService.probePrimaryAudioCodec(item.filePath!), + codec, + ); + expect( + await FFmpegService.isNativeFlacFile(item.filePath!), + codec == 'flac', + ); + final tags = await PlatformBridge.readFileMetadata(item.filePath!); + expect(tags['title'], 'Track $codec'); + expect(tags['artist'], 'Example artist'); + } finally { + subscription.close(); + } + } + await queue.flushQueuePersistence(); + } finally { + scope.dispose(); + await PlatformBridge.cleanupExtensions(); + await db.close(); + PathProviderPlatform.instance = originalPaths; + await root.delete(recursive: true); + } + }, + ); +} diff --git a/lib/providers/download_queue_provider_single_item.dart b/lib/providers/download_queue_provider_single_item.dart index 222a1ff4..e3cd980d 100644 --- a/lib/providers/download_queue_provider_single_item.dart +++ b/lib/providers/download_queue_provider_single_item.dart @@ -116,6 +116,7 @@ class _DownloadRun { String actualQuality = ''; String? resultOutputExt; bool shouldPreserveNativeM4a = false; + bool forceContainerConversion = false; /// Filled by the SAF embed op from the local temp so the final quality /// probe doesn't have to copy the published file back out of SAF. @@ -650,13 +651,16 @@ class _DownloadRun { final resultIsKnownLossyAudio = isLossyAudioFormat(resultAudioFormat) && !isInconclusiveAudioCodec(resultAudioFormat); - final requiresContainerConversion = + forceContainerConversion = result.requiresContainerConversion || (!resultIsKnownLossyAudio && n._shouldRequestContainerConversion(actualService, safOutputExt)); final preferredOutputExt = n._extensionPreferredOutputExt(actualService); shouldPreserveNativeM4a = - !requiresContainerConversion && + // M4A/MP4 may contain FLAC. A requested FLAC output must reach the + // codec probe even if the extension did not ask for conversion. + safOutputExt != '.flac' && + !forceContainerConversion && (resultOutputExt == '.m4a' || resultOutputExt == '.mp4' || preferredOutputExt == '.m4a' || @@ -1141,8 +1145,7 @@ class _DownloadRun { } Future _preserveSafNativeM4a(String currentFilePath) async { - // Decrypted streams are already in their final format. - // Converting e.g. eac3 M4A to FLAC would produce fake upscaled output. + // Preserve explicitly requested M4A/MP4 output, including spatial audio. _log.d('M4A/MP4 file detected (SAF), preserving native container...'); final preserveExt = currentFilePath.toLowerCase().endsWith('.mp4') ? '.mp4' @@ -1204,7 +1207,7 @@ class _DownloadRun { codec == 'flac' && await FFmpegService.isNativeFlacFile(tempPath); final shouldAttemptConversion = shouldAttemptLosslessContainerConversion( - forceConversion: true, + forceConversion: forceContainerConversion, probedCodec: codec, ); if (!shouldAttemptConversion) { @@ -1216,6 +1219,13 @@ class _DownloadRun { final preserveExt = resultOutputExt == '.mp4' ? '.mp4' : '.m4a'; final newFileName = '${safBaseName ?? 'track'}$preserveExt'; producedFileName = newFileName; + if (metadataEmbeddingEnabled) { + await _embedFinalMetadata( + tempPath, + format: 'm4a', + writeExternalLrc: false, + ); + } return (tempPath, newFileName); } else if (isAlreadyNativeFlac) { _log.d( @@ -1267,7 +1277,9 @@ class _DownloadRun { if (newUri != null) { filePath = newUri; finalSafFileName = producedFileName; - _markFinalOutputAsFlac(); + if (branch == 'nativeFlac' || branch == 'convert') { + _markFinalOutputAsFlac(); + } } else if (branch == 'nativeFlac') { _log.w('Failed to write native FLAC to SAF'); } else if (branch == 'convert') { @@ -1339,7 +1351,7 @@ class _DownloadRun { await FFmpegService.isNativeFlacFile(currentFilePath); final shouldAttemptConversion = shouldAttemptLosslessContainerConversion( - forceConversion: true, + forceConversion: forceContainerConversion, probedCodec: codec, ); if (!shouldAttemptConversion) { @@ -1347,6 +1359,7 @@ class _DownloadRun { 'Preserving native container; audio codec is ${codec ?? 'unknown'}, ' 'no FLAC container conversion needed.', ); + await _preserveLocalNativeM4a(currentFilePath); } else if (isAlreadyNativeFlac) { _log.d( 'Native FLAC payload detected; ensuring .flac ' diff --git a/lib/services/download_container_finalizer.dart b/lib/services/download_container_finalizer.dart index 34bf716f..cda2db5b 100644 --- a/lib/services/download_container_finalizer.dart +++ b/lib/services/download_container_finalizer.dart @@ -34,13 +34,6 @@ class DownloadContainerFinalizer { }) async { if (requestedExtension != '.flac') return filePath; forceConversion = forceConversion || result.requiresContainerConversion; - final reportedCodec = normalizeAudioFormatValue(result.audioCodec); - if (!forceConversion && isLossyAudioFormat(reportedCodec)) { - debug( - 'Native-worker output is $reportedCodec; preserving native container.', - ); - return filePath; - } final outputExtension = result.outputExtension(path: filePath); final lowerPath = filePath.toLowerCase(); final isSafSource = filePath.startsWith('content://'); @@ -50,15 +43,8 @@ class DownloadContainerFinalizer { outputExtension == '.m4a' || outputExtension == '.mp4'; if (!forceConversion && !pathLooksLikeM4a && !isSafSource) return filePath; - final decryptionExtension = result.decryption?.normalizedOutputExtension; - if (!forceConversion && - decryptionExtension != null && - decryptionExtension != '.flac') { - debug( - 'Native-worker decrypted output requested $decryptionExtension; preserving native container.', - ); - return filePath; - } + // Provider/decryption extensions describe the input container, not its + // audio codec. Inspect the stream before preserving an M4A/MP4 download. bool shouldConvert(String? codec) { if (shouldAttemptLosslessContainerConversion( forceConversion: forceConversion, diff --git a/test/download_container_finalizer_test.dart b/test/download_container_finalizer_test.dart index 0265d51e..8db9f8c4 100644 --- a/test/download_container_finalizer_test.dart +++ b/test/download_container_finalizer_test.dart @@ -60,27 +60,47 @@ void main() { embedMetadata: embed, ); - test( - 'native output and explicit non-FLAC decryption bypass conversion', - () async { - for (final raw in >[ - {'audio_codec': 'aac'}, - {'decryption_key': 'abcd', 'output_extension': '.mp4'}, - ]) { - expect( - await finalize(finalizer(), DownloadResult.fromMap(raw)), - '/Song.m4a', - ); - } + test('explicit non-FLAC requests bypass conversion', () async { + for (final extension in ['.m4a', '.mp4', '.opus']) { expect( await finalize( finalizer(), - DownloadResult.fromMap({}), - extension: '.opus', + DownloadResult.fromMap({'audio_codec': 'flac'}), + extension: extension, force: true, ), '/Song.m4a', ); + } + }); + + test( + 'container hints cannot override measured FLAC or lossy audio', + () async { + final source = await File( + '${directory.path}/Song.m4a', + ).writeAsString('MP4'); + for (final codec in ['flac', 'aac', 'eac3']) { + for (final raw in >[ + {'audio_codec': 'm4a'}, + {'audio_codec': 'aac'}, + {'decryption_key': 'abcd', 'output_extension': '.mp4'}, + ]) { + final result = DownloadResult.fromMap(raw); + var converted = false; + final stage = finalizer( + probe: (_) async => codec, + convert: (_) async { + converted = true; + return '${directory.path}/Song.flac'; + }, + ); + final path = await finalize(stage, result, path: source.path); + expect(converted, codec == 'flac', reason: '$codec / $raw'); + expect(path, converted ? '${directory.path}/Song.flac' : source.path); + if (converted) expect(result.outputExtension(), '.flac'); + } + } }, ); @@ -146,7 +166,12 @@ void main() { return output.path; }, ); - final result = DownloadResult.fromMap({'file_name': 'Song.m4a'}); + final result = DownloadResult.fromMap({ + 'file_name': 'Song.m4a', + 'audio_codec': 'm4a', + 'decryption_key': 'abcd', + 'output_extension': '.mp4', + }); final path = await finalize( stage, result, @@ -192,7 +217,12 @@ void main() { }, deleteSource: (_) async => deleted = true, ); - final result = DownloadResult.fromMap({'file_name': 'Song.m4a'}); + final result = DownloadResult.fromMap({ + 'file_name': 'Song.m4a', + 'audio_codec': 'm4a', + 'decryption_key': 'abcd', + 'output_extension': '.mp4', + }); final stage = finalizer( probe: (_) async => 'flac', replaceSaf: replacer.replace, @@ -206,7 +236,7 @@ void main() { ); expect(path, published ? 'content://final' : null); expect(result.fileName, published ? 'Song.flac' : 'Song.m4a'); - expect(result.audioCodec, published ? 'flac' : null); + expect(result.audioCodec, published ? 'flac' : 'm4a'); expect(deleted, published); expect(await directory.list().length, 0); }