From 4ab9e53e43fe9d836feab5a487a2df741bdf37e1 Mon Sep 17 00:00:00 2001 From: zarzet <42882290+zarzet@users.noreply.github.com> Date: Thu, 8 Oct 2026 15:49:48 +0700 Subject: [PATCH] fix(downloads): finalize album ReplayGain after history is saved --- .../album_replaygain_completion_test.dart | 463 ++++++++++++++++++ ...download_queue_provider_native_worker.dart | 5 +- .../download_queue_provider_replaygain.dart | 76 ++- .../download_queue_provider_single_item.dart | 32 +- 4 files changed, 551 insertions(+), 25 deletions(-) create mode 100644 integration_test/album_replaygain_completion_test.dart diff --git a/integration_test/album_replaygain_completion_test.dart b/integration_test/album_replaygain_completion_test.dart new file mode 100644 index 00000000..ae4d303a --- /dev/null +++ b/integration_test/album_replaygain_completion_test.dart @@ -0,0 +1,463 @@ +import 'dart:async'; +import 'dart:convert'; +import 'dart:io'; +import 'dart:math' as math; + +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/history_database.dart'; +import 'package:spotiflac_android/services/platform_bridge.dart'; +import 'package:spotiflac_android/utils/logger.dart'; + +class _Paths extends PathProviderPlatform { + _Paths(this._path); + final String _path; + @override + Future getApplicationDocumentsPath() async => _path; + @override + Future getTemporaryPath() async => _path; +} + +class _Settings extends SettingsNotifier { + _Settings(this._directory, this._concurrency, this._nativeWorker); + final String _directory; + final int _concurrency; + final bool _nativeWorker; + @override + AppSettings build() => AppSettings( + downloadDirectory: _directory, + filenameFormat: '{title}', + singleFilenameFormat: '{title}', + embedMetadata: true, + embedLyrics: false, + embedReplayGain: true, + nativeDownloadWorkerEnabled: _nativeWorker, + concurrentDownloads: _concurrency, + autoFallback: false, + lyricsProviders: const [], + motionArtworkEnabled: false, + ); +} + +class _SimultaneousHistory extends DownloadHistoryNotifier { + final _bothSaved = Completer(); + var _saved = 0; + + @override + Future addToHistory( + DownloadHistoryItem item, { + bool preserveTrackVariant = false, + }) async { + await super.addToHistory(item, preserveTrackVariant: preserveTrackVariant); + if (++_saved == 2) _bothSaved.complete(); + // Both real SQLite writes finish before either caller publishes completion. + // This deliberately exposes concurrent finalization, independently of I/O speed. + await _bothSaved.future; + } +} + +class _FailOnceHistory extends DownloadHistoryNotifier { + var _failed = false; + + @override + Future addToHistory( + DownloadHistoryItem item, { + bool preserveTrackVariant = false, + }) async { + if (!_failed && item.trackName.endsWith('-1')) { + _failed = true; + throw StateError('Fixture: history temporarily unavailable'); + } + await super.addToHistory(item, preserveTrackVariant: preserveTrackVariant); + } +} + +class _Extensions extends ExtensionNotifier { + @override + ExtensionState build() => const ExtensionState( + isInitialized: true, + extensions: [ + Extension( + id: 'example.audio', + name: 'example.audio', + displayName: 'Example Audio', + version: '1', + description: 'Local audio fixture', + enabled: true, + status: 'loaded', + hasDownloadProvider: true, + skipMetadataEnrichment: true, + ), + ], + ); +} + +Future _ffmpeg(List arguments) async { + final session = await FFmpegKit.executeWithArguments(arguments); + final output = await session.getOutput() ?? ''; + expect( + ReturnCode.isSuccess(await session.getReturnCode()), + true, + reason: output, + ); + return output; +} + +Future _pcmHash(String path) async { + // FFmpegKit's log callback does not capture muxer stdout on every platform. + final checksum = File('$path.pcm.md5'); + try { + await _ffmpeg([ + '-y', + '-v', + 'error', + '-i', + path, + '-map', + '0:a:0', + '-f', + 'md5', + checksum.path, + ]); + final output = await checksum.readAsString(); + expect(output, matches(r'MD5=[a-f0-9]+')); + return output.trim(); + } finally { + if (await checksum.exists()) await checksum.delete(); + } +} + +void main() { + final binding = IntegrationTestWidgetsFlutterBinding.ensureInitialized(); + const baseline = bool.fromEnvironment('REPLAYGAIN_COMPLETION_BASELINE'); + final results = >[]; + late Directory root; + late Directory output; + late PathProviderPlatform originalPaths; + final hashes = []; + setUpAll(() async { + root = await Directory.systemTemp.createTemp('album-rg-completion-'); + output = await Directory('${root.path}/output').create(); + originalPaths = PathProviderPlatform.instance; + PathProviderPlatform.instance = _Paths(root.path); + SharedPreferences.setMockInitialValues({ + 'app_state_migrated_queue_to_sqlite_v1': true, + }); + final extensions = await Directory('${root.path}/extensions').create(); + final data = await Directory('${root.path}/data').create(); + await PlatformBridge.initExtensionSystem( + extensions.path, + data.path, + masterKey: base64Encode(List.filled(32, 1)), + lyricsProviders: const [], + lyricsFetchOptions: const {}, + allowedDirectories: [output.path], + ); + final audio = []; + for (var i = 0; i < 2; i++) { + final path = '${root.path}/fixture-$i.flac'; + await _ffmpeg([ + '-y', + '-f', + 'lavfi', + '-i', + 'sine=frequency=${440 + i * 220}:sample_rate=44100', + '-t', + '${2 + i}', + '-af', + 'volume=${i == 0 ? 1 : 0.25}', + '-c:a', + 'flac', + path, + ]); + audio.add(base64Encode(await File(path).readAsBytes())); + hashes.add(await _pcmHash(path)); + } + final provider = await Directory( + '${extensions.path}/example.audio', + ).create(); + await File('${provider.path}/manifest.json').writeAsString( + jsonEncode({ + 'name': 'example.audio', + 'version': '1', + 'description': 'Local album ReplayGain completion fixture', + 'type': ['download_provider'], + 'permissions': {'file': true}, + 'skipMetadataEnrichment': true, + }), + ); + await File('${provider.path}/index.js').writeAsString(''' +registerExtension({ + checkAvailability(id) { return { available: true, track_id: id }; }, + download(trackId, quality, outputPath) { + const audio = ${jsonEncode(audio)}; + const index = String(trackId).endsWith('one') ? 0 : 1; + file.writeBytes(outputPath, audio[index]); + return { success: true, file_path: outputPath, actual_extension: '.flac' }; + } +}); +'''); + await PlatformBridge.loadExtensionsFromDir(extensions.path); + await PlatformBridge.setExtensionEnabled('example.audio', true); + LogBuffer.loggingEnabled = true; + }); + tearDownAll(() async { + await PlatformBridge.cleanupExtensions(); + LogBuffer.loggingEnabled = false; + await HistoryDatabase.instance.close(); + await (await AppStateDatabase.instance.database).close(); + PathProviderPlatform.instance = originalPaths; + await root.delete(recursive: true); + binding.reportData ??= {}; + binding.reportData!['album_replaygain_completion'] = { + 'platform': Platform.operatingSystem, + 'baseline_observation': baseline, + 'completed_cases': results.length, + 'cases': results, + }; + }); + setUp( + () => binding.handleAppLifecycleStateChanged(AppLifecycleState.resumed), + ); + + final configurations = [ + for (final nativeWorker in [false, if (Platform.isAndroid) true]) + for (final concurrency in [1, 2]) + ( + nativeWorker: nativeWorker, + concurrency: concurrency, + lateTrack: false, + retry: false, + ), + (nativeWorker: false, concurrency: 2, lateTrack: true, retry: false), + (nativeWorker: false, concurrency: 1, lateTrack: false, retry: true), + ]; + for (final config in configurations) { + final concurrency = config.concurrency; + final name = + '${config.nativeWorker ? 'native' : 'foreground'}-$concurrency' + '${config.lateTrack ? '-late' : ''}${config.retry ? '-retry' : ''}'; + testWidgets('full queue album completion, $name', (tester) async { + final container = ProviderContainer( + overrides: [ + settingsProvider.overrideWith( + () => _Settings(output.path, concurrency, config.nativeWorker), + ), + extensionProvider.overrideWith(_Extensions.new), + if (concurrency == 2 && !config.nativeWorker) + downloadHistoryProvider.overrideWith(_SimultaneousHistory.new), + if (config.retry) + downloadHistoryProvider.overrideWith(_FailOnceHistory.new), + ], + ); + final files = {}; + final states = >{}; + final tracks = [ + for (var i = 0; i < 2; i++) + Track( + id: 'example.audio:$name-${i == 0 ? 'one' : 'two'}', + name: 'Track $name-$i', + artistName: 'Artist', + albumArtist: 'Artist', + albumName: 'Album $name', + albumId: 'album-$name', + source: 'example.audio', + duration: 2 + i, + trackNumber: i + 1, + totalTracks: 2, + ), + ]; + final albumMessage = 'Album ReplayGain for "id:album-$name":'; + void addDuringWrite() { + if (!config.lateTrack || + tracks.length == 3 || + !LogBuffer().entries.any((e) => e.message.contains(albumMessage))) { + return; + } + final extra = tracks.first.copyWith( + id: 'example.audio:$name-extra-one', + name: 'Track $name-extra', + ); + tracks.add(extra); + container.read(downloadQueueProvider.notifier).addMultipleToQueue([ + extra, + ], 'example.audio'); + } + + LogBuffer().addListener(addDuringWrite); + try { + final queue = container.read(downloadQueueProvider.notifier); + queue.pauseQueue(persistAcrossRestarts: false); + queue.addMultipleToQueue(tracks, 'example.audio'); + await queue.flushQueuePersistence(); + var done = Completer(); + var retried = false; + String? failedId; + final subscription = container.listen(downloadQueueProvider, (_, next) { + for (final item in next.items) { + final transitions = states.putIfAbsent(item.track.id, () => []); + if (transitions.lastOrNull != item.status.name) { + transitions.add(item.status.name); + } + if (item.status == DownloadStatus.completed) { + files[item.track.id] = item.filePath!; + } else if (item.status == DownloadStatus.failed && + !done.isCompleted) { + if (config.retry && !retried) { + expect(item.error, contains('history temporarily unavailable')); + failedId = item.id; + } else { + done.completeError( + StateError('${item.track.id}: ${item.error}'), + ); + } + } + } + if (failedId != null && + !retried && + !next.isProcessing && + !done.isCompleted) { + done.complete(); + } + if (files.length == tracks.length && + next.items.isEmpty && + !next.isProcessing && + !done.isCompleted) { + done.complete(); + } + }); + try { + queue.resumeQueue(); + await done.future.timeout(const Duration(seconds: 60)); + if (config.retry) { + expect(files, hasLength(1)); + expect(container.read(downloadQueueProvider).failedCount, 1); + final savedTags = await PlatformBridge.readFileMetadata( + files.values.single, + ); + expect(savedTags['replaygain_album_gain'], anyOf(isNull, isEmpty)); + expect( + LogBuffer().entries.where( + (e) => e.message.contains(albumMessage), + ), + isEmpty, + ); + retried = true; + done = Completer(); + await queue.retryItem(failedId!); + await done.future.timeout(const Duration(seconds: 60)); + } + await queue.flushQueuePersistence(); + } finally { + subscription.close(); + } + final metadata = >[]; + for (var i = 0; i < tracks.length; i++) { + final path = files[tracks[i].id]!; + final tags = await PlatformBridge.readFileMetadata(path); + expect( + tags['replaygain_track_gain'], + isNotEmpty, + reason: '$name track gain', + ); + expect( + tags['replaygain_track_peak'], + isNotEmpty, + reason: '$name track peak', + ); + expect(await _pcmHash(path), hashes[i == 2 ? 0 : i]); + final history = await container + .read(downloadHistoryProvider.notifier) + .getByFilePathAsync(path); + expect(history?.filePath, path); + if (!baseline) { + expect( + tags['replaygain_album_gain'], + isNotEmpty, + reason: '$name album gain', + ); + expect( + tags['replaygain_album_peak'], + isNotEmpty, + reason: '$name album peak', + ); + } + metadata.add({ + 'track_gain': tags['replaygain_track_gain'], + 'track_peak': tags['replaygain_track_peak'], + 'album_gain': tags['replaygain_album_gain'], + 'album_peak': tags['replaygain_album_peak'], + 'pcm_unchanged': true, + 'history_persisted': true, + }); + } + if (!baseline) { + var power = 0.0; + var duration = 0; + var peak = 0.0; + for (var i = 0; i < tracks.length; i++) { + final gain = double.parse( + (metadata[i]['track_gain'] as String).replaceAll(' dB', ''), + ); + power += math.pow(10, (-18 - gain) / 10) * tracks[i].duration; + duration += tracks[i].duration; + peak = math.max( + peak, + double.parse(metadata[i]['track_peak'] as String), + ); + } + final gain = -18 - 10 * math.log(power / duration) / math.ln10; + final expectedGain = + '${gain >= 0 ? '+' : ''}${gain.toStringAsFixed(2)} dB'; + for (final tags in metadata) { + expect(tags['album_gain'], expectedGain); + expect(tags['album_peak'], peak.toStringAsFixed(6)); + } + expect(tracks.length, config.lateTrack ? 3 : 2); + expect( + LogBuffer().entries.where((e) => e.message.contains(albumMessage)), + hasLength(config.nativeWorker ? 0 : (config.lateTrack ? 2 : 1)), + ); + if (config.nativeWorker) { + expect( + LogBuffer().entries.any( + (e) => e.message.contains( + 'Starting Android native download worker', + ), + ), + true, + ); + } + } + results.add({ + 'name': name, + 'concurrency': concurrency, + 'tags': metadata, + 'transitions': states, + }); + } finally { + LogBuffer().removeListener(addDuringWrite); + final deadline = DateTime.now().add(const Duration(seconds: 10)); + while (container.read(downloadQueueProvider).isProcessing && + DateTime.now().isBefore(deadline)) { + await Future.delayed(const Duration(milliseconds: 20)); + } + container.dispose(); + } + }); + } +} diff --git a/lib/providers/download_queue_provider_native_worker.dart b/lib/providers/download_queue_provider_native_worker.dart index a9b1d6c0..373306c3 100644 --- a/lib/providers/download_queue_provider_native_worker.dart +++ b/lib/providers/download_queue_provider_native_worker.dart @@ -1597,6 +1597,9 @@ extension _DownloadQueueNativeWorker on DownloadQueueNotifier { ); }, ); + // Quality-variant renaming may have changed the path since the scan. + _updateAlbumRgFilePath(trackToDownload, completedFilePath); + await _checkAndWriteAlbumReplayGain(trackToDownload); removeItem(item.id); } @@ -1623,8 +1626,6 @@ extension _DownloadQueueNativeWorker on DownloadQueueNotifier { return; } _storeTrackReplayGainForAlbum(track, filePath, rgResult); - _updateAlbumRgFilePath(track, filePath); - await _checkAndWriteAlbumReplayGain(track); _log.d( 'Native-worker ReplayGain written: gain=${rgResult.trackGain}, peak=${rgResult.trackPeak}', ); diff --git a/lib/providers/download_queue_provider_replaygain.dart b/lib/providers/download_queue_provider_replaygain.dart index 330eb5b6..a94d417a 100644 --- a/lib/providers/download_queue_provider_replaygain.dart +++ b/lib/providers/download_queue_provider_replaygain.dart @@ -2,7 +2,7 @@ part of 'download_queue_provider.dart'; class _AlbumRgTrackEntry { - String filePath; + final String filePath; final String trackId; final double integratedLufs; final double truePeakLinear; @@ -19,6 +19,8 @@ class _AlbumRgTrackEntry { class _AlbumRgAccumulator { final List<_AlbumRgTrackEntry> entries = []; + int revision = 0; + Future? writing; } extension _DownloadQueueReplayGain on DownloadQueueNotifier { @@ -31,6 +33,7 @@ extension _DownloadQueueReplayGain on DownloadQueueNotifier { final accumulator = _albumRgData[key]; if (accumulator == null) return; accumulator.entries.removeWhere((e) => e.trackId == track.id); + accumulator.revision++; if (accumulator.entries.isEmpty) { _albumRgData.remove(key); } @@ -60,6 +63,7 @@ extension _DownloadQueueReplayGain on DownloadQueueNotifier { durationSecs: track.duration.toDouble(), ), ); + _albumRgData[key]!.revision++; } /// Replace the temp path stored in the accumulator with the final output @@ -69,9 +73,18 @@ extension _DownloadQueueReplayGain on DownloadQueueNotifier { final key = _albumRgKey(track); final accumulator = _albumRgData[key]; if (accumulator == null) return; - for (final entry in accumulator.entries) { + for (var i = 0; i < accumulator.entries.length; i++) { + final entry = accumulator.entries[i]; if (entry.trackId == track.id) { - entry.filePath = finalPath; + if (entry.filePath == finalPath) return; + accumulator.entries[i] = _AlbumRgTrackEntry( + filePath: finalPath, + trackId: entry.trackId, + integratedLufs: entry.integratedLufs, + truePeakLinear: entry.truePeakLinear, + durationSecs: entry.durationSecs, + ); + accumulator.revision++; break; } } @@ -81,6 +94,7 @@ extension _DownloadQueueReplayGain on DownloadQueueNotifier { /// in the current queue are done. If so, compute album gain and write it /// to every track's file. Future _checkAndWriteAlbumReplayGain(Track track) async { + if (!ref.mounted) return; final settings = ref.read(settingsProvider); if (!settings.embedReplayGain) return; @@ -90,12 +104,12 @@ extension _DownloadQueueReplayGain on DownloadQueueNotifier { if (albumReplayGainBlocked(state.items, key)) return; - // The accumulator entries represent successfully scanned tracks. Entries - // are only added after a successful ReplayGain scan, removed on retry or - // when a non-completed item is removed from the queue, so every entry - // here corresponds to a track that completed (or is about to complete) - // its download. - await _computeAndWriteAlbumRg(key, accumulator); + // A tag failure must not turn a durable download back into a failed item. + try { + await _computeAndWriteAlbumRg(key, accumulator); + } catch (e) { + _log.w('Album ReplayGain check failed: $e'); + } } /// Write album ReplayGain tags to a single file. @@ -140,11 +154,50 @@ extension _DownloadQueueReplayGain on DownloadQueueNotifier { Future _computeAndWriteAlbumRg( String key, _AlbumRgAccumulator accumulator, + ) { + // Concurrent completions/dismissals can observe the same ready album. + // Share the write so they cannot edit the same audio files concurrently. + final pending = accumulator.writing; + if (pending != null) return pending; + late final Future writing; + writing = _writeReadyAlbumRg(key, accumulator).whenComplete(() { + if (identical(accumulator.writing, writing)) accumulator.writing = null; + }); + accumulator.writing = writing; + return writing; + } + + Future _writeReadyAlbumRg( + String key, + _AlbumRgAccumulator accumulator, + ) async { + while (ref.mounted && identical(_albumRgData[key], accumulator)) { + // The caller already checked readiness. Only rescan if the queue changes + // during I/O; a bulk retrigger must not become one full scan per album. + final queueItems = state.items; + final revision = accumulator.revision; + await _writeAlbumRgSnapshot(key, accumulator); + if (!ref.mounted || !identical(_albumRgData[key], accumulator)) return; + // New requests/scans or final-path changes may arrive during tag I/O. + // Retain their statistics until those requests complete, then recompute. + if (!identical(queueItems, state.items) && + albumReplayGainBlocked(state.items, key)) { + return; + } + if (revision == accumulator.revision) { + _albumRgData.remove(key); + return; + } + } + } + + Future _writeAlbumRgSnapshot( + String key, + _AlbumRgAccumulator accumulator, ) async { final validEntries = accumulator.entries.toList(); // Single-track albums already have their track gain, so need no write. if (validEntries.length <= 1) { - _albumRgData.remove(key); return; } @@ -174,13 +227,12 @@ extension _DownloadQueueReplayGain on DownloadQueueNotifier { ); for (final entry in validEntries) { + if (!ref.mounted || !identical(_albumRgData[key], accumulator)) return; try { await _writeAlbumReplayGain(entry.filePath, albumGain, albumPeak); } catch (e) { _log.w('Failed to write album ReplayGain to ${entry.filePath}: $e'); } } - - _albumRgData.remove(key); } } diff --git a/lib/providers/download_queue_provider_single_item.dart b/lib/providers/download_queue_provider_single_item.dart index 2d24b8d6..222a1ff4 100644 --- a/lib/providers/download_queue_provider_single_item.dart +++ b/lib/providers/download_queue_provider_single_item.dart @@ -849,6 +849,22 @@ class _DownloadRun { final rgPath = filePath; stageCompleted('external lyrics'); + if (wasExisting && + rgPath != null && + !isNetworkDownload && + settings.embedReplayGain) { + // A retry after history persistence failed may reuse the finished file. + // retryItem purges the old scan, and normal metadata embedding is skipped + // for existing files, so recover the measurement for this attempt. + try { + final scan = await ReplayGainService.scanAndApplyToFile(rgPath); + if (scan != null) { + n._storeTrackReplayGainForAlbum(trackToDownload, rgPath, scan); + } + } catch (e) { + _log.w('Could not recover ReplayGain for existing download: $e'); + } + } // Album ReplayGain: update the accumulator path to the final file // location. For SAF downloads the metadata was embedded on a temp // copy, so the stored path still points there. Replace it with the @@ -857,17 +873,6 @@ class _DownloadRun { if (rgPath != null && !isNetworkDownload) { n._updateAlbumRgFilePath(trackToDownload, rgPath); } - // Album ReplayGain: check if all album tracks are now complete and, - // if so, compute and write album gain/peak to every track file. - try { - if (!isNetworkDownload) { - await n._checkAndWriteAlbumReplayGain(trackToDownload); - } - } catch (e) { - _log.w('Album ReplayGain check failed: $e'); - } - stageCompleted('album ReplayGain'); - await _persistCompletionAndNotify(); stageCompleted('quality probe, history and notification'); return true; @@ -1694,6 +1699,11 @@ class _DownloadRun { ); }, ); + // Only durable completions can unblock an album. Checking while this + // item is still finalizing makes the final track block its own album. + if (!isNetworkDownload) { + await n._checkAndWriteAlbumReplayGain(trackToDownload); + } try { await n._notificationService.showDownloadComplete( trackName: item.track.name,