diff --git a/lib/providers/download_queue_provider_finalization.dart b/lib/providers/download_queue_provider_finalization.dart index 0db4eb10..fda710e8 100644 --- a/lib/providers/download_queue_provider_finalization.dart +++ b/lib/providers/download_queue_provider_finalization.dart @@ -1276,15 +1276,7 @@ extension _DownloadQueueFinalization on DownloadQueueNotifier { return filePath; } if (isAlreadyNativeFlac) { - var flacPath = filePath; - if (!filePath.toLowerCase().endsWith('.flac')) { - final renamedPath = filePath.replaceAll(RegExp(r'\.[^.]+$'), '.flac'); - final targetPath = renamedPath == filePath - ? '$filePath.flac' - : renamedPath; - await File(filePath).rename(targetPath); - flacPath = targetPath; - } + final flacPath = await FFmpegService.ensureNativeFlacExtension(filePath); 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 44ddf5ed..db25a6df 100644 --- a/lib/providers/download_queue_provider_single_item.dart +++ b/lib/providers/download_queue_provider_single_item.dart @@ -1459,19 +1459,10 @@ class _DownloadRun { 'Native FLAC payload detected; ensuring .flac ' 'extension and embedding metadata.', ); - var flacPath = currentFilePath; - if (!currentFilePath.toLowerCase().endsWith('.flac')) { - final renamedPath = currentFilePath.replaceAll( - RegExp(r'\.[^.]+$'), - '.flac', - ); - final targetPath = renamedPath == currentFilePath - ? '$currentFilePath.flac' - : renamedPath; - await File(currentFilePath).rename(targetPath); - flacPath = targetPath; - filePath = targetPath; - } + final flacPath = await FFmpegService.ensureNativeFlacExtension( + currentFilePath, + ); + filePath = flacPath; await _embedFinalMetadata(flacPath, format: 'flac'); _markFinalOutputAsFlac(); diff --git a/lib/services/ffmpeg_service.dart b/lib/services/ffmpeg_service.dart index ed838b55..890d3006 100644 --- a/lib/services/ffmpeg_service.dart +++ b/lib/services/ffmpeg_service.dart @@ -103,17 +103,27 @@ class FFmpegService { : firstPath == secondPath; } - static Future _uniqueConversionPath(String requestedPath) async { - if (!await File(requestedPath).exists()) return requestedPath; + // Reserve names atomically: concurrent conversions may choose the same + // sibling basename before either FFmpeg process has created its output. + static Future _reserveConversionPath(String requestedPath) async { final file = File(requestedPath); final fileName = file.uri.pathSegments.last; final dotIndex = fileName.lastIndexOf('.'); final baseName = dotIndex > 0 ? fileName.substring(0, dotIndex) : fileName; final extension = dotIndex > 0 ? fileName.substring(dotIndex) : ''; - for (var index = 2; ; index++) { - final candidate = - '${file.parent.path}${Platform.pathSeparator}$baseName ($index)$extension'; - if (!await File(candidate).exists()) return candidate; + for (var index = 1; ; index++) { + final candidate = index == 1 + ? requestedPath + : '${file.parent.path}${Platform.pathSeparator}$baseName ($index)$extension'; + try { + await File(candidate).create(exclusive: true); + return candidate; + } on FileSystemException { + if (await FileSystemEntity.type(candidate, followLinks: false) == + FileSystemEntityType.notFound) { + rethrow; + } + } } } @@ -133,13 +143,14 @@ class FFmpegService { if (_sameLocalPath(requestedPath, inputPath) && deleteOriginal) { final token = DateTime.now().microsecondsSinceEpoch; return _ConversionOutputPlan( - workingPath: - '${inputFile.parent.path}${Platform.pathSeparator}.$baseName.spotiflac-$token$normalizedExt', + workingPath: await _reserveConversionPath( + '${inputFile.parent.path}${Platform.pathSeparator}.$baseName.spotiflac-$token$normalizedExt', + ), finalPath: inputPath, ); } - final finalPath = await _uniqueConversionPath(requestedPath); + final finalPath = await _reserveConversionPath(requestedPath); return _ConversionOutputPlan(workingPath: finalPath, finalPath: finalPath); } @@ -159,8 +170,10 @@ class FFmpegService { required String inputPath, required bool deleteOriginal, }) async { - if (!await File(plan.workingPath).exists()) { - _log.e('Converted output is missing: ${plan.workingPath}'); + final workingFile = File(plan.workingPath); + if (!await workingFile.exists() || await workingFile.length() == 0) { + _log.e('Converted output is missing or empty: ${plan.workingPath}'); + await _cleanupConversionOutput(plan); return null; } @@ -665,25 +678,61 @@ class FFmpegService { ..add('aresample=${options.join(':')}'); } - static Future convertM4aToFlac(String inputPath) async { - final outputPath = _buildOutputPath(inputPath, '.flac'); - - final command = - '-v error -xerror -i "$inputPath" -c:a flac -compression_level 8 "$outputPath" -y'; - - final result = await _execute(command); - - if (result.success) { - try { - await File(inputPath).delete(); - } catch (_) {} - return outputPath; + static Future convertM4aToFlac( + String inputPath, { + @visibleForTesting Future Function(List)? execute, + }) async { + final plan = await _conversionOutputPlan( + inputPath, + '.flac', + deleteOriginal: true, + ); + try { + final result = await (execute ?? _executeWithArguments)([ + '-v', + 'error', + '-xerror', + '-i', + inputPath, + '-c:a', + 'flac', + '-compression_level', + '8', + plan.workingPath, + '-y', + ]); + if (result.success) { + return await _finalizeConversionOutput( + plan: plan, + inputPath: inputPath, + deleteOriginal: true, + ); + } + _log.e('M4A to FLAC conversion failed: ${result.output}'); + } catch (e) { + _log.e('M4A to FLAC conversion failed: $e'); } - - _log.e('M4A to FLAC conversion failed: ${result.output}'); + await _cleanupConversionOutput(plan); return null; } + /// Corrects a native FLAC payload's suffix without replacing a sibling file. + static Future ensureNativeFlacExtension(String inputPath) async { + if (inputPath.toLowerCase().endsWith('.flac')) return inputPath; + final plan = await _conversionOutputPlan( + inputPath, + '.flac', + deleteOriginal: true, + ); + try { + await File(inputPath).rename(plan.finalPath); + return plan.finalPath; + } catch (_) { + await _cleanupConversionOutput(plan); + rethrow; + } + } + static Future convertM4aToLossy( String inputPath, { required String format, diff --git a/test/ffmpeg_conversion_output_test.dart b/test/ffmpeg_conversion_output_test.dart new file mode 100644 index 00000000..d214531f --- /dev/null +++ b/test/ffmpeg_conversion_output_test.dart @@ -0,0 +1,139 @@ +import 'dart:async'; +import 'dart:io'; + +import 'package:flutter_test/flutter_test.dart'; +import 'package:spotiflac_android/services/ffmpeg_service.dart'; + +void main() { + late Directory directory; + setUp(() async { + directory = await Directory.systemTemp.createTemp('conversion-output-'); + }); + tearDown(() async => directory.delete(recursive: true)); + + Future input(String name) => + File('${directory.path}/$name').writeAsString('source'); + + Future succeed(List arguments) async { + await File(arguments[arguments.length - 2]).writeAsString('converted'); + return FFmpegResult(success: true, returnCode: 0, output: ''); + } + + test( + 'conversion preserves existing sibling and handles quoted paths', + () async { + final source = await input('Song "live".m4a'); + final sibling = await File( + '${directory.path}/Song "live".flac', + ).writeAsString('existing'); + final result = await FFmpegService.convertM4aToFlac( + source.path, + execute: (arguments) async { + expect(arguments[arguments.indexOf('-i') + 1], source.path); + expect(await source.exists(), isTrue); + return succeed(arguments); + }, + ); + expect(result, '${directory.path}/Song "live" (2).flac'); + expect(await sibling.readAsString(), 'existing'); + expect(await File(result!).readAsString(), 'converted'); + expect(await source.exists(), isFalse); + }, + ); + + test('failed conversion removes partial output and retains source', () async { + final source = await input('Song.m4a'); + final result = await FFmpegService.convertM4aToFlac( + source.path, + execute: (arguments) async { + await File(arguments[arguments.length - 2]).writeAsString('partial'); + return FFmpegResult(success: false, returnCode: 1, output: 'failure'); + }, + ); + expect(result, isNull); + expect(await source.readAsString(), 'source'); + expect(await directory.list().length, 1); + }); + + test( + 'exceptions and empty successful outputs preserve the original', + () async { + final source = await input('Song.m4a'); + for (final execute in Function(List)>[ + (_) async => throw StateError('execution failed'), + (_) async => FFmpegResult(success: true, returnCode: 0, output: ''), + ]) { + expect( + await FFmpegService.convertM4aToFlac(source.path, execute: execute), + isNull, + ); + expect(await source.readAsString(), 'source'); + expect(await directory.list().length, 1); + } + }, + ); + + test('same-suffix conversion stages output until it is complete', () async { + final source = await input('Song.flac'); + final result = await FFmpegService.convertM4aToFlac( + source.path, + execute: (arguments) async { + expect(arguments[arguments.length - 2], isNot(source.path)); + expect(await source.readAsString(), 'source'); + return succeed(arguments); + }, + ); + expect(result, source.path); + expect(await source.readAsString(), 'converted'); + expect(await directory.list().length, 1); + }); + + test('concurrent conversions reserve distinct output paths', () async { + final first = await input('Song.m4a'); + final second = await input('Song.mp4'); + final bothStarted = Completer(); + final outputPaths = {}; + Future execute(List arguments) async { + outputPaths.add(arguments[arguments.length - 2]); + if (outputPaths.length == 2) bothStarted.complete(); + await bothStarted.future.timeout(const Duration(seconds: 3)); + return succeed(arguments); + } + + final results = await Future.wait([ + FFmpegService.convertM4aToFlac(first.path, execute: execute), + FFmpegService.convertM4aToFlac(second.path, execute: execute), + ]); + expect(results.toSet(), hasLength(2)); + expect(results, everyElement(isNotNull)); + expect(await directory.list().length, 2); + }); + + test( + 'native FLAC rename preserves sibling files and adds missing suffix', + () async { + final source = await input('Song.m4a'); + final sibling = await File( + '${directory.path}/Song.flac', + ).writeAsString('existing'); + final result = await FFmpegService.ensureNativeFlacExtension(source.path); + expect(result, '${directory.path}/Song (2).flac'); + expect(await sibling.readAsString(), 'existing'); + expect(await File(result).readAsString(), 'source'); + expect(await FFmpegService.ensureNativeFlacExtension(result), result); + final noSuffix = await input('Other'); + expect( + await FFmpegService.ensureNativeFlacExtension(noSuffix.path), + '${noSuffix.path}.flac', + ); + }, + ); + + test('failed native rename releases its reserved destination', () async { + await expectLater( + FFmpegService.ensureNativeFlacExtension('${directory.path}/missing.m4a'), + throwsA(isA()), + ); + expect(await directory.list().length, 0); + }); +}