From 3d35058b02bc693943ad8e95b30314fd96932f49 Mon Sep 17 00:00:00 2001 From: zarzet <42882290+zarzet@users.noreply.github.com> Date: Sun, 6 Sep 2026 14:04:04 +0700 Subject: [PATCH] fix(playback): retain ReplayGain for SAF descriptor leases Carry the original display name through playback leases to format-aware metadata reads. Cache successful normalization results by source identity, bound the cache, and retry transient read failures. Add regressions for descriptor hints, album gain fallback, and failed-read retries. --- .../kotlin/com/zarz/spotiflac/MainActivity.kt | 1 + lib/services/music_player_service.dart | 48 ++++------- lib/services/platform_bridge.dart | 14 +++- lib/services/playback_normalization.dart | 47 +++++++++++ test/playback_normalization_test.dart | 81 +++++++++++++++++++ 5 files changed, 155 insertions(+), 36 deletions(-) create mode 100644 lib/services/playback_normalization.dart create mode 100644 test/playback_normalization_test.dart diff --git a/android/app/src/main/kotlin/com/zarz/spotiflac/MainActivity.kt b/android/app/src/main/kotlin/com/zarz/spotiflac/MainActivity.kt index 4e2b59e2..21b05edd 100644 --- a/android/app/src/main/kotlin/com/zarz/spotiflac/MainActivity.kt +++ b/android/app/src/main/kotlin/com/zarz/spotiflac/MainActivity.kt @@ -838,6 +838,7 @@ class MainActivity: FlutterFragmentActivity() { return mapOf( "token" to token, "path" to "/proc/self/fd/${descriptor.fd}", + "display_name" to buildUriDisplayName(Uri.parse(uriStr)), ) } diff --git a/lib/services/music_player_service.dart b/lib/services/music_player_service.dart index f09a2ec1..368daf00 100644 --- a/lib/services/music_player_service.dart +++ b/lib/services/music_player_service.dart @@ -8,6 +8,7 @@ import 'package:audio_session/audio_session.dart' import 'package:audioplayers/audioplayers.dart'; import 'package:spotiflac_android/services/app_state_database.dart'; import 'package:spotiflac_android/services/platform_bridge.dart'; +import 'package:spotiflac_android/services/playback_normalization.dart'; import 'package:spotiflac_android/utils/int_utils.dart'; import 'package:spotiflac_android/utils/logger.dart'; import 'package:spotiflac_android/utils/string_utils.dart'; @@ -550,46 +551,23 @@ class MusicPlayerHandler extends BaseAudioHandler unawaited(_persistSession(position: position)); } - // ReplayGain normalization: resolved path -> volume multiplier. - final Map _normalizationVolumeCache = {}; + final _normalizationCache = PlaybackNormalizationCache( + readMetadata: PlatformBridge.readFileMetadata, + onReadError: (error) => + _log.w('Failed to read gain tags for normalization: $error'), + ); - /// Volume multiplier from the file's ReplayGain/R128 tags (track gain, - /// album gain fallback; Opus R128 tags are converted to ReplayGain dB by - /// the Go reader). 1.0 when disabled, untagged, or unreadable. Positive - /// gains clamp at 1.0 — setVolume can only attenuate. Future _normalizationVolumeFor( String path, { String? cacheKey, + String? displayName, }) async { if (!_playbackNormalizationEnabled) return 1.0; - final effectiveCacheKey = cacheKey ?? path; - final cached = _normalizationVolumeCache[effectiveCacheKey]; - if (cached != null) return cached; - - var volume = 1.0; - try { - final metadata = await PlatformBridge.readFileMetadata(path); - final gainDb = - _parseGainDb(metadata['replaygain_track_gain']) ?? - _parseGainDb(metadata['replaygain_album_gain']); - if (gainDb != null) { - volume = pow(10.0, gainDb / 20.0).toDouble().clamp(0.0, 1.0); - } - } catch (e) { - _log.w('Failed to read gain tags for normalization: $e'); - } - if (_normalizationVolumeCache.length > 128) { - _normalizationVolumeCache.clear(); - } - _normalizationVolumeCache[effectiveCacheKey] = volume; - return volume; - } - - static double? _parseGainDb(Object? raw) { - final text = raw?.toString(); - if (text == null || text.isEmpty) return null; - final match = RegExp(r'-?\d+(\.\d+)?').firstMatch(text); - return match == null ? null : double.tryParse(match.group(0)!); + return _normalizationCache.volumeFor( + path, + cacheKey: cacheKey, + displayName: displayName, + ); } /// Re-applies normalization to the playing track when the setting flips. @@ -616,6 +594,7 @@ class MusicPlayerHandler extends BaseAudioHandler final volume = await _normalizationVolumeFor( resolved, cacheKey: media.isContentUri ? media.source : null, + displayName: playbackLease?.displayName, ); if (playbackLease != null) { await PlatformBridge.closeContentUriPlaybackLease(playbackLease.token); @@ -1072,6 +1051,7 @@ class MusicPlayerHandler extends BaseAudioHandler var normalizationVolume = await _normalizationVolumeFor( resolved, cacheKey: media.isContentUri ? media.source : null, + displayName: playbackLease?.displayName, ); if (!_isCurrentPlayRequest(generation, media)) return; await _player.setAudioContext(_musicAudioContext); diff --git a/lib/services/platform_bridge.dart b/lib/services/platform_bridge.dart index 1fa24839..40fa1103 100644 --- a/lib/services/platform_bridge.dart +++ b/lib/services/platform_bridge.dart @@ -88,7 +88,13 @@ class ContentUriPlaybackLease { final String path; final String token; - const ContentUriPlaybackLease({required this.path, required this.token}); + final String? displayName; + + const ContentUriPlaybackLease({ + required this.path, + required this.token, + this.displayName, + }); } class InstallationState { @@ -845,7 +851,11 @@ class PlatformBridge { final path = map['path']?.toString() ?? ''; final token = map['token']?.toString() ?? ''; if (path.isEmpty || token.isEmpty) return null; - return ContentUriPlaybackLease(path: path, token: token); + return ContentUriPlaybackLease( + path: path, + token: token, + displayName: result['display_name'] as String?, + ); } static Future closeContentUriPlaybackLease(String token) async { diff --git a/lib/services/playback_normalization.dart b/lib/services/playback_normalization.dart new file mode 100644 index 00000000..6001da3e --- /dev/null +++ b/lib/services/playback_normalization.dart @@ -0,0 +1,47 @@ +import 'dart:math'; + +/// Caches successful tag reads by stable media identity. Descriptor paths are +/// short-lived and require their display-name hint to select the audio parser. +class PlaybackNormalizationCache { + final Future> Function(String, {String? displayName}) + readMetadata; + final void Function(Object)? onReadError; + final Map _volumes = {}; + static final _gainNumber = RegExp(r'-?\d+(\.\d+)?'); + + PlaybackNormalizationCache({required this.readMetadata, this.onReadError}); + + Future volumeFor( + String path, { + String? cacheKey, + String? displayName, + }) async { + final key = cacheKey ?? path; + final cached = _volumes[key]; + if (cached != null) return cached; + try { + final metadata = await readMetadata(path, displayName: displayName); + if (metadata['error'] != null) { + onReadError?.call(metadata['error'] as Object); + return 1.0; + } + final gain = + _gain(metadata['replaygain_track_gain']) ?? + _gain(metadata['replaygain_album_gain']); + final volume = gain == null + ? 1.0 + : pow(10.0, gain / 20.0).toDouble().clamp(0.0, 1.0); + if (_volumes.length >= 128) _volumes.remove(_volumes.keys.first); + _volumes[key] = volume; + return volume; + } catch (error) { + onReadError?.call(error); + return 1.0; + } + } + + static double? _gain(Object? value) { + final match = _gainNumber.firstMatch(value?.toString() ?? ''); + return match == null ? null : double.tryParse(match.group(0)!); + } +} diff --git a/test/playback_normalization_test.dart b/test/playback_normalization_test.dart new file mode 100644 index 00000000..20877884 --- /dev/null +++ b/test/playback_normalization_test.dart @@ -0,0 +1,81 @@ +import 'package:flutter_test/flutter_test.dart'; +import 'package:spotiflac_android/services/playback_normalization.dart'; + +void main() { + test( + 'descriptor normalization uses hint and caches by original URI', + () async { + var reads = 0; + final cache = PlaybackNormalizationCache( + readMetadata: (path, {displayName}) async { + reads++; + expect(path, '/proc/self/fd/42'); + expect(displayName, 'Song.flac'); + return { + 'replaygain_track_gain': '-6.00 dB', + 'replaygain_album_gain': '-3.00 dB', + }; + }, + ); + expect( + await cache.volumeFor( + '/proc/self/fd/42', + displayName: 'Song.flac', + cacheKey: 'content://music/song', + ), + closeTo(0.501187, 0.000001), + ); + expect( + await cache.volumeFor( + '/proc/self/fd/99', + displayName: 'Song.flac', + cacheKey: 'content://music/song', + ), + closeTo(0.501187, 0.000001), + ); + expect(reads, 1); + }, + ); + + test( + 'failed descriptor metadata does not poison local fallback cache', + () async { + var reads = 0; + final cache = PlaybackNormalizationCache( + readMetadata: (path, {displayName}) async { + reads++; + if (path.startsWith('/proc/')) { + return {'error': 'descriptor access denied'}; + } + return {'replaygain_album_gain': '-6.00 dB'}; + }, + ); + expect( + await cache.volumeFor( + '/proc/self/fd/42', + displayName: 'Song.mp3', + cacheKey: 'song', + ), + 1.0, + ); + expect( + await cache.volumeFor('/cache/Song.mp3', cacheKey: 'song'), + closeTo(0.501187, 0.000001), + ); + expect(reads, 2); + }, + ); + + test('exceptions are retried and positive gain only attenuates', () async { + var reads = 0; + final cache = PlaybackNormalizationCache( + readMetadata: (path, {displayName}) async { + if (++reads == 1) throw StateError('temporarily unavailable'); + return {'replaygain_track_gain': '+6.00 dB'}; + }, + ); + expect(await cache.volumeFor('song'), 1.0); + expect(await cache.volumeFor('song'), 1.0); + expect(reads, 2); + }); +}