From 772bd97f7dbacc1dc3e9efd72f3b5beeb5316877 Mon Sep 17 00:00:00 2001 From: zarzet <42882290+zarzet@users.noreply.github.com> Date: Tue, 6 Oct 2026 00:04:14 +0700 Subject: [PATCH] fix(downloads): preserve filenames after decryption and conversion --- .../zarz/spotiflac/NativeDownloadFinalizer.kt | 33 +-- .../zarz/spotiflac/NativeFinalizerFFmpeg.kt | 18 +- .../spotiflac/NativeConversionOutputTest.kt | 47 ++++ .../download_queue_provider_finalization.dart | 44 +--- lib/services/ffmpeg_decryption.dart | 118 +++++----- lib/services/ffmpeg_service.dart | 2 + test/ffmpeg_decryption_output_test.dart | 215 ++++++++++++++++++ 7 files changed, 347 insertions(+), 130 deletions(-) create mode 100644 android/app/src/test/kotlin/com/zarz/spotiflac/NativeConversionOutputTest.kt create mode 100644 test/ffmpeg_decryption_output_test.dart 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 ae7b8a70..a1881308 100644 --- a/android/app/src/main/kotlin/com/zarz/spotiflac/NativeDownloadFinalizer.kt +++ b/android/app/src/main/kotlin/com/zarz/spotiflac/NativeDownloadFinalizer.kt @@ -613,7 +613,7 @@ object NativeDownloadFinalizer { ) replaceStatePath(context, input, state, decryptedPath, deleteOld = true) } finally { - if (successPath == null) { + if (successPath == null && outputPath != localInput) { File(outputPath).delete() } if (originalPath != successPath && originalPath.startsWith(context.cacheDir.absolutePath)) { @@ -694,15 +694,13 @@ object NativeDownloadFinalizer { if (!conversion.first || !File(stagedOutput).exists()) { throw IllegalStateException("automatic conversion failed: ${conversion.second}") } + val metadataFormat = if (target.codec == "aac") "m4a" else target.codec + embedBasicMetadata(context, stagedOutput, input, metadataFormat) if (!promoteStagedConversion(stagedOutput, output)) { throw IllegalStateException("failed to promote automatic conversion output") } - val metadataFormat = if (target.codec == "aac") "m4a" else target.codec - embedBasicMetadata(context, output, input, metadataFormat) - if (sameLocalExtension) { - replaceSameFormatLocalOutput(localInput, output) state.filePath = localInput state.fileName = File(localInput).name } else { @@ -712,7 +710,7 @@ object NativeDownloadFinalizer { } finally { if (!adoptedOutput) { File(stagedOutput).delete() - File(output).delete() + if (output != localInput) File(output).delete() } if (sourceWasSaf) File(localInput).delete() } @@ -724,24 +722,6 @@ object NativeDownloadFinalizer { state.audioCodec = target.codec } - private fun replaceSameFormatLocalOutput(inputPath: String, convertedPath: String) { - val source = File(inputPath) - val converted = File(convertedPath) - val backup = File("$inputPath.spotiflac-backup-${System.nanoTime()}") - if (!source.renameTo(backup)) { - throw IllegalStateException("failed to stage original for same-format conversion") - } - try { - if (!converted.renameTo(source)) { - throw IllegalStateException("failed to publish same-format conversion") - } - backup.delete() - } catch (e: Exception) { - if (!source.exists()) backup.renameTo(source) - throw e - } - } - private fun uniqueAutoConversionOutputPath(inputPath: String, extension: String): String { val source = File(inputPath) val requested = File(source.parentFile, "${source.nameWithoutExtension}$extension") @@ -819,13 +799,12 @@ object NativeDownloadFinalizer { execute = { arguments -> runFFmpegArguments(arguments, shouldCancel) }, checkCancelled = { checkCancelled(shouldCancel) }, ) + // Tag the staged file before replacing a same-suffix source. + embedBasicMetadata(context, stagedOutput, input, "flac") if (!promoteStagedConversion(stagedOutput, output)) { throw IllegalStateException("failed to publish container conversion output") } createdOutput = true - // Keep metadata failures before adoption so the source survives - // and the unsuccessful output is removed by the local cleanup. - embedBasicMetadata(context, output, input, "flac") replaceStatePath(context, input, state, output, deleteOld = true) adoptedOutput = true } finally { 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 24077a0c..9c30757f 100644 --- a/android/app/src/main/kotlin/com/zarz/spotiflac/NativeFinalizerFFmpeg.kt +++ b/android/app/src/main/kotlin/com/zarz/spotiflac/NativeFinalizerFFmpeg.kt @@ -171,19 +171,27 @@ internal fun NativeDownloadFinalizer.stagedConversionPath(finalPath: String): St internal fun NativeDownloadFinalizer.promoteStagedConversion(stagedPath: String, finalPath: String): Boolean { val staged = File(stagedPath) + if (!staged.isFile || staged.length() == 0L) return false fsyncQuietly(staged) val final = File(finalPath) - if (staged.renameTo(final)) return true - return final.delete() && staged.renameTo(final) + val backup = File("$finalPath.spotiflac-backup-${System.nanoTime()}") + val backedUp = final.exists() + if (backedUp && (!final.isFile || !final.renameTo(backup))) return false + if (staged.renameTo(final)) { + if (backedUp) backup.delete() + return true + } + if (backedUp) check(backup.renameTo(final)) { "failed to restore conversion source" } + return false } internal fun NativeDownloadFinalizer.buildOutputPath(inputPath: String, extension: String): String { val ext = normalizeExt(extension).ifBlank { ".tmp" } val file = File(inputPath) val base = file.nameWithoutExtension.ifBlank { "track" } - val candidate = File(file.parentFile, "$base$ext").absolutePath - if (candidate != inputPath) return candidate - return File(file.parentFile, "${base}_converted$ext").absolutePath + // This is the final name. FFmpeg writes via stagedConversionPath even + // when the source and final extensions match. + return File(file.parentFile, "$base$ext").absolutePath } internal fun NativeDownloadFinalizer.desiredFileName(input: NativeDownloadFinalizer.FinalizeInput, state: NativeDownloadFinalizer.FinalizeState, extension: String): String { diff --git a/android/app/src/test/kotlin/com/zarz/spotiflac/NativeConversionOutputTest.kt b/android/app/src/test/kotlin/com/zarz/spotiflac/NativeConversionOutputTest.kt new file mode 100644 index 00000000..aee00dc7 --- /dev/null +++ b/android/app/src/test/kotlin/com/zarz/spotiflac/NativeConversionOutputTest.kt @@ -0,0 +1,47 @@ +package com.zarz.spotiflac + +import java.io.File +import java.nio.file.Files +import org.junit.Assert.assertEquals +import org.junit.Assert.assertFalse +import org.junit.Assert.assertTrue +import org.junit.Test + +class NativeConversionOutputTest { + @Test + fun sameSuffixOutputKeepsItsNameAndPublishesOnlyWhenComplete() { + val directory = Files.createTempDirectory("native-conversion-output-").toFile() + try { + for (name in listOf("Song.flac", "Song.m4a", "Song.mp4", "Song_converted.flac")) { + val input = File(directory, name).apply { writeText("source") } + val output = NativeDownloadFinalizer.buildOutputPath(input.path, input.extension) + assertEquals(input.absolutePath, output) + val staged = File(NativeDownloadFinalizer.stagedConversionPath(output)) + staged.writeText("complete") + assertEquals("source", input.readText()) + assertTrue(NativeDownloadFinalizer.promoteStagedConversion(staged.path, output)) + assertEquals("complete", input.readText()) + assertFalse(staged.exists()) + input.delete() + assertEquals(0, directory.listFiles()!!.size) + } + } finally { + directory.deleteRecursively() + } + } + + @Test + fun emptyOrMissingStageCannotReplaceTheSource() { + val directory = Files.createTempDirectory("native-conversion-output-").toFile() + try { + val input = File(directory, "Song.flac").apply { writeText("source") } + val staged = File(NativeDownloadFinalizer.stagedConversionPath(input.path)) + assertFalse(NativeDownloadFinalizer.promoteStagedConversion(staged.path, input.path)) + staged.createNewFile() + assertFalse(NativeDownloadFinalizer.promoteStagedConversion(staged.path, input.path)) + assertEquals("source", input.readText()) + } finally { + directory.deleteRecursively() + } + } +} diff --git a/lib/providers/download_queue_provider_finalization.dart b/lib/providers/download_queue_provider_finalization.dart index e815bb69..c644d468 100644 --- a/lib/providers/download_queue_provider_finalization.dart +++ b/lib/providers/download_queue_provider_finalization.dart @@ -1025,42 +1025,22 @@ extension _DownloadQueueFinalization on DownloadQueueNotifier { return _DecryptOutcome(newUri, newFileName: producedFileName); } - if (repairAc4) { - final rawDecryptedPath = await FFmpegService.decryptWithDescriptor( - inputPath: filePath, - descriptor: descriptor, - deleteOriginal: false, - ); - if (rawDecryptedPath == null) { - try { - await deleteFile(filePath); - } catch (_) {} - return const _DecryptOutcome( - null, - failStage: DownloadQueueNotifier._decryptStageDecrypt, - ); - } - final decryptedPath = await _normalizeDecryptedIsoBmffAudioPath( - rawDecryptedPath, - result, - ); - if (_isMp4Container(decryptedPath)) { - try { - await PlatformBridge.ensureAC4Config(decryptedPath, filePath); - } catch (e) { - _log.w('AC-4 container repair skipped: $e'); - } - } - try { - await deleteFile(filePath); - } catch (_) {} - return _DecryptOutcome(decryptedPath); - } - final rawDecryptedPath = await FFmpegService.decryptWithDescriptor( inputPath: filePath, descriptor: descriptor, deleteOriginal: true, + // Repair while the encrypted source still exists, then publish under + // its original name instead of keeping a conversion suffix. + prepareOutput: repairAc4 + ? (path) async { + if (!_isMp4Container(path)) return; + try { + await PlatformBridge.ensureAC4Config(path, filePath); + } catch (e) { + _log.w('AC-4 container repair skipped: $e'); + } + } + : null, ); final decryptedPath = rawDecryptedPath == null ? null diff --git a/lib/services/ffmpeg_decryption.dart b/lib/services/ffmpeg_decryption.dart index 82189458..8ddeccd4 100644 --- a/lib/services/ffmpeg_decryption.dart +++ b/lib/services/ffmpeg_decryption.dart @@ -2,7 +2,6 @@ import 'dart:convert'; import 'dart:io'; import 'package:spotiflac_android/services/ffmpeg_models.dart'; -import 'package:spotiflac_android/services/ffmpeg_probe.dart'; import 'package:spotiflac_android/services/ffmpeg_support.dart'; import 'package:spotiflac_android/utils/logger.dart'; @@ -14,6 +13,8 @@ abstract final class FFmpegDecryption { required String inputPath, required DownloadDecryptionDescriptor descriptor, bool deleteOriginal = true, + Future Function(String)? execute, + Future Function(String)? prepareOutput, }) async { if (descriptor.normalizedStrategy != 'ffmpeg.mov_key') { _log.e( @@ -29,6 +30,8 @@ abstract final class FFmpegDecryption { inputFormat: descriptor.inputFormat, outputExtension: descriptor.outputExtension, deleteOriginal: deleteOriginal, + execute: execute ?? FFmpegExecution.command, + prepareOutput: prepareOutput, ); } @@ -70,26 +73,16 @@ abstract final class FFmpegDecryption { return '.flac'; } - static String _outputPath(String inputPath, String extension) { - final file = File(inputPath); - final name = file.uri.pathSegments.last; - final dot = name.lastIndexOf('.'); - final base = dot > 0 ? name.substring(0, dot) : name; - final path = '${file.parent.path}${Platform.pathSeparator}$base$extension'; - return path == inputPath - ? '${file.parent.path}${Platform.pathSeparator}${base}_converted$extension' - : path; - } - static Future _decryptMovKeyFile({ required String inputPath, required String decryptionKey, String? inputFormat, String? outputExtension, required bool deleteOriginal, + required Future Function(String) execute, + Future Function(String)? prepareOutput, }) async { final preferredExt = _preferredExtension(inputPath, outputExtension); - var tempOutput = _outputPath(inputPath, preferredExt); final demuxer = (inputFormat ?? '').trim().isNotEmpty ? inputFormat!.trim() : 'mov'; @@ -117,67 +110,60 @@ abstract final class FFmpegDecryption { return null; } FFmpegResult? lastResult; - var succeeded = false; - for (final key in candidates) { - _log.d('Executing FFmpeg decrypt command (key length: ${key.length})'); - var result = await FFmpegExecution.command( - command(tempOutput, mapAudioOnly: preferredExt == '.flac', key: key), + FFmpegOutputPlan? plan; + var published = false; + Future attempt( + String extension, + String key, { + bool forceMov = false, + }) async { + if (plan != null) await FFmpegOutput.cleanup(plan!.workingPath); + final output = await FFmpegOutput.plan( + inputPath, + extension, + deleteOriginal: deleteOriginal, + ); + plan = output; + final result = await execute( + command( + output.workingPath, + mapAudioOnly: extension == '.flac', + key: key, + forceMov: forceMov, + ), ); - if (!result.success && preferredExt == '.flac') { - final fallback = _outputPath(inputPath, '.m4a'); - final attempt = await FFmpegExecution.command( - command(fallback, mapAudioOnly: false, key: key), - ); - if (attempt.success) { - tempOutput = fallback; - result = attempt; - } - } - if (!result.success && - (preferredExt == '.flac' || preferredExt == '.m4a')) { - final fallback = _outputPath(inputPath, '.mp4'); - final attempt = await FFmpegExecution.command( - command(fallback, mapAudioOnly: false, key: key), - ); - if (attempt.success) { - tempOutput = fallback; - result = attempt; - } - } - if (!result.success) { - final fallback = _outputPath(inputPath, '.mp4'); - final attempt = await FFmpegExecution.command( - command(fallback, mapAudioOnly: false, key: key, forceMov: true), - ); - if (attempt.success) { - tempOutput = fallback; - result = attempt; - } - } lastResult = result; - if (result.success) { - succeeded = true; - break; - } - await FFmpegOutput.cleanup(tempOutput); + final file = File(output.workingPath); + return result.success && await file.exists() && await file.length() > 0; } - if (!succeeded) { + + try { + for (final key in candidates) { + _log.d('Executing FFmpeg decrypt command (key length: ${key.length})'); + if (await attempt(preferredExt, key) || + (preferredExt == '.flac' && await attempt('.m4a', key)) || + ((preferredExt == '.flac' || preferredExt == '.m4a') && + await attempt('.mp4', key)) || + await attempt('.mp4', key, forceMov: true)) { + await prepareOutput?.call(plan!.workingPath); + final path = await FFmpegOutput.finalize( + plan: plan!, + inputPath: inputPath, + deleteOriginal: deleteOriginal, + ); + published = path != null; + return path; + } + } _log.e('FFmpeg decrypt failed: ${lastResult?.output ?? 'unknown error'}'); return null; - } - try { - if (!await File(tempOutput).exists()) { - _log.e('Decrypted output file not found: $tempOutput'); - return null; - } - final source = File(inputPath); - if (deleteOriginal && await source.exists()) await source.delete(); - FFmpegProbe.invalidate(inputPath); - FFmpegProbe.invalidate(tempOutput); - return tempOutput; } catch (e) { _log.e('Failed to finalize decrypted file: $e'); return null; + } finally { + if (!published && plan != null) { + await FFmpegOutput.cleanup(plan!.workingPath); + } } } } diff --git a/lib/services/ffmpeg_service.dart b/lib/services/ffmpeg_service.dart index 13c7ec05..b80243a0 100644 --- a/lib/services/ffmpeg_service.dart +++ b/lib/services/ffmpeg_service.dart @@ -72,10 +72,12 @@ class FFmpegService { required String inputPath, required DownloadDecryptionDescriptor descriptor, bool deleteOriginal = true, + Future Function(String)? prepareOutput, }) => FFmpegDecryption.decrypt( inputPath: inputPath, descriptor: descriptor, deleteOriginal: deleteOriginal, + prepareOutput: prepareOutput, ); /// Measures EBU R128 loudness and true peak using a -18 LUFS reference. diff --git a/test/ffmpeg_decryption_output_test.dart b/test/ffmpeg_decryption_output_test.dart new file mode 100644 index 00000000..454c88a2 --- /dev/null +++ b/test/ffmpeg_decryption_output_test.dart @@ -0,0 +1,215 @@ +import 'dart:io'; + +import 'package:ffmpeg_kit_flutter_new_audio/ffmpeg_kit_config.dart'; +import 'package:flutter_test/flutter_test.dart'; +import 'package:spotiflac_android/services/ffmpeg_decryption.dart'; +import 'package:spotiflac_android/services/ffmpeg_models.dart'; + +void main() { + late Directory directory; + setUp(() async { + directory = await Directory.systemTemp.createTemp('decryption-output-'); + }); + tearDown(() async => directory.delete(recursive: true)); + + DownloadDecryptionDescriptor descriptor(String extension) => + DownloadDecryptionDescriptor( + strategy: 'ffmpeg.mov_key', + key: '00112233445566778899aabbccddeeff', + outputExtension: extension, + ); + + String outputPath(String command) => + FFmpegKitConfig.parseArguments(command).reversed.skip(1).first; + + Future succeed(String command) async { + await File(outputPath(command)).writeAsString('decrypted'); + return FFmpegResult(success: true, returnCode: 0, output: ''); + } + + for (final name in [ + 'Song.flac', + 'Song.m4a', + 'Song.mp4', + 'Song_converted.flac', + ]) { + test('decryption retains the original name $name', () async { + final source = await File( + '${directory.path}/$name', + ).writeAsString('encrypted'); + final result = await FFmpegDecryption.decrypt( + inputPath: source.path, + descriptor: descriptor( + '.${source.uri.pathSegments.last.split('.').last}', + ), + execute: (command) async { + expect(outputPath(command), isNot(source.path)); + expect(await source.readAsString(), 'encrypted'); + return succeed(command); + }, + ); + expect(result, source.path); + expect(await source.readAsString(), 'decrypted'); + expect(await directory.list().length, 1); + }); + } + + test('failed FLAC attempt falls back to the original M4A name', () async { + final source = await File( + '${directory.path}/Song.m4a', + ).writeAsString('encrypted'); + String? failedPath; + final result = await FFmpegDecryption.decrypt( + inputPath: source.path, + descriptor: descriptor('.flac'), + execute: (command) async { + expect(await source.readAsString(), 'encrypted'); + if (outputPath(command).endsWith('.flac')) { + failedPath = outputPath(command); + await File(failedPath!).writeAsString('partial'); + return FFmpegResult( + success: false, + returnCode: 1, + output: 'unsupported codec', + ); + } + expect(await File(failedPath!).exists(), isFalse); + return succeed(command); + }, + ); + expect(result, source.path); + expect(await source.readAsString(), 'decrypted'); + expect(await directory.list().length, 1); + }); + + test( + 'decryption failures and exceptions retain the source and clean outputs', + () async { + final source = await File( + '${directory.path}/Song.flac', + ).writeAsString('encrypted'); + for (final execute in Function(String)>[ + (command) async { + await File(outputPath(command)).writeAsString('partial'); + return FFmpegResult(success: false, returnCode: 1, output: 'bad key'); + }, + (_) async => throw StateError('execution failed'), + (_) async => FFmpegResult(success: true, returnCode: 0, output: ''), + ]) { + expect( + await FFmpegDecryption.decrypt( + inputPath: source.path, + descriptor: descriptor('.flac'), + execute: execute, + ), + isNull, + ); + expect(await source.readAsString(), 'encrypted'); + expect(await directory.list().length, 1); + } + }, + ); + + test( + 'decryption preserves an existing output and a requested original', + () async { + final source = await File( + '${directory.path}/Song.m4a', + ).writeAsString('encrypted'); + final sibling = await File( + '${directory.path}/Song.flac', + ).writeAsString('existing'); + final result = await FFmpegDecryption.decrypt( + inputPath: source.path, + descriptor: descriptor('.flac'), + deleteOriginal: false, + execute: succeed, + ); + expect(result, '${directory.path}/Song (2).flac'); + expect(await source.readAsString(), 'encrypted'); + expect(await sibling.readAsString(), 'existing'); + expect(await File(result!).readAsString(), 'decrypted'); + }, + ); + + test( + 'container preparation completes while the original is available', + () async { + final source = await File( + '${directory.path}/Song.mp4', + ).writeAsString('encrypted'); + final result = await FFmpegDecryption.decrypt( + inputPath: source.path, + descriptor: descriptor('.mp4'), + execute: succeed, + prepareOutput: (path) async { + expect(await source.readAsString(), 'encrypted'); + await File(path).writeAsString('repaired'); + }, + ); + expect(result, source.path); + expect(await source.readAsString(), 'repaired'); + expect(await directory.list().length, 1); + }, + ); + + test( + 'encrypted audio round trip keeps its name and remains decodable', + () async { + final source = File('${directory.path}/Song.m4a'); + final key = descriptor('.m4a').key; + ProcessResult generated; + try { + generated = await Process.run('ffmpeg', [ + '-v', + 'error', + '-f', + 'lavfi', + '-i', + 'sine=frequency=440:duration=0.2', + '-c:a', + 'aac', + '-encryption_scheme', + 'cenc-aes-ctr', + '-encryption_key', + key, + '-encryption_kid', + 'ffeeddccbbaa99887766554433221100', + source.path, + ]); + } on ProcessException { + markTestSkipped('Host FFmpeg is unavailable'); + return; + } + expect(generated.exitCode, 0, reason: '${generated.stderr}'); + final result = await FFmpegDecryption.decrypt( + inputPath: source.path, + descriptor: descriptor('.m4a'), + execute: (command) async { + final result = await Process.run( + 'ffmpeg', + FFmpegKitConfig.parseArguments(command), + ); + return FFmpegResult( + success: result.exitCode == 0, + returnCode: result.exitCode, + output: '${result.stderr}', + ); + }, + ); + expect(result, source.path); + final decoded = await Process.run('ffmpeg', [ + '-v', + 'error', + '-xerror', + '-i', + source.path, + '-f', + 'null', + '-', + ]); + expect(decoded.exitCode, 0, reason: '${decoded.stderr}'); + expect(await directory.list().length, 1); + }, + ); +}