From bfffb8da11d7fc9daf5d7e6c8a9c715befe6d131 Mon Sep 17 00:00:00 2001 From: zarzet <42882290+zarzet@users.noreply.github.com> Date: Sun, 6 Sep 2026 21:03:57 +0700 Subject: [PATCH] fix(replaygain): write and verify native Opus gain tags Preserve audio and artwork while replacing conflicting Opus gain tags with R128 comments. Verify manual, download, and album writes, handle SAF failures, and refresh playback normalization after saving. --- .../zarz/spotiflac/NativeDownloadFinalizer.kt | 42 +++- go_backend/audio_metadata_ogg.go | 7 +- go_backend/ogg_edit.go | 35 +++ go_backend/replaygain_opus_test.go | 216 ++++++++++++++++++ lib/providers/download_queue_provider.dart | 1 + .../download_queue_provider_embedding.dart | 36 ++- ...download_queue_provider_native_worker.dart | 15 +- .../download_queue_provider_replaygain.dart | 56 +---- lib/services/ffmpeg_service.dart | 43 ++-- lib/services/music_player_service.dart | 31 ++- lib/services/playback_normalization.dart | 13 +- lib/services/replaygain_service.dart | 156 ++++++++++--- test/playback_normalization_test.dart | 35 +++ test/replaygain_service_test.dart | 197 ++++++++++++++++ 14 files changed, 745 insertions(+), 138 deletions(-) create mode 100644 go_backend/replaygain_opus_test.go create mode 100644 test/replaygain_service_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 f3875647..1c9cf04c 100644 --- a/android/app/src/main/kotlin/com/zarz/spotiflac/NativeDownloadFinalizer.kt +++ b/android/app/src/main/kotlin/com/zarz/spotiflac/NativeDownloadFinalizer.kt @@ -30,6 +30,7 @@ import java.util.concurrent.CancellationException import java.util.concurrent.CountDownLatch import java.util.concurrent.TimeUnit import java.util.concurrent.atomic.AtomicBoolean +import kotlin.math.abs import kotlin.math.pow object NativeDownloadFinalizer { @@ -251,8 +252,17 @@ object NativeDownloadFinalizer { result.put("auto_conversion_warning", e.message ?: "conversion failed") } checkCancelled(shouldCancel) - val replayGain = writeReplayGain(context, effectiveInput, state, shouldCancel) - if (replayGain != null) result.put("replaygain", replayGain) + try { + val replayGain = writeReplayGain(context, effectiveInput, state, shouldCancel) + if (replayGain != null) result.put("replaygain", replayGain) + } catch (e: CancellationException) { + throw e + } catch (e: Exception) { + // Gain tagging is optional; keep the completed audio if + // its native editor or the verification step fails. + Log.w(TAG, "ReplayGain write failed: ${e.message}") + result.put("replaygain_warning", e.message ?: "ReplayGain write failed") + } checkCancelled(shouldCancel) try { refreshFinalAudioQualityMetadata(context, result, state) @@ -982,14 +992,14 @@ object NativeDownloadFinalizer { private fun writeReplayGainFields(context: Context, path: String, fields: JSONObject) { if (!path.startsWith("content://")) { - Gobackend.editFileMetadata(path, fields.toString()) + writeLocalReplayGainFields(path, fields) return } val tempPath = SafDownloadHandler.copyContentUriToTemp(context, path) ?: throw IllegalStateException("failed to copy SAF file for ReplayGain write") try { - Gobackend.editFileMetadata(tempPath, fields.toString()) + writeLocalReplayGainFields(tempPath, fields) val uri = Uri.parse(path) context.contentResolver.openOutputStream(uri, "wt")?.use { output -> File(tempPath).inputStream().use { input -> input.copyTo(output) } @@ -1000,6 +1010,30 @@ object NativeDownloadFinalizer { } } + private fun writeLocalReplayGainFields(path: String, fields: JSONObject) { + val result = parseObject(Gobackend.editFileMetadata(path, fields.toString())) + val method = result.optString("method", "") + check( + result.optBoolean("success", false) && + !result.has("error") && + (method == "native" || method.startsWith("native_")), + ) { "ReplayGain native write did not complete: $result" } + + val metadata = parseObject(Gobackend.readFileMetadata(path)) + check(!metadata.has("error")) { "ReplayGain verification failed: $metadata" } + val isOpus = metadata.optString("audio_codec", "") == "opus" + for (key in fields.keys()) { + // Opus stores only R128 gain, exposed by the Go reader as dB. + if (isOpus && key.endsWith("_peak")) continue + val expected = fields.optString(key, "").trim().removeSuffix("dB").trim().toDoubleOrNull() + val actual = metadata.optString(key, "").trim().removeSuffix("dB").trim().toDoubleOrNull() + val tolerance = if (key.endsWith("_gain")) 0.01 else 0.000001 + check(expected != null && actual != null && abs(actual - expected) <= tolerance) { + "ReplayGain verification failed for $key" + } + } + } + private fun refreshFinalAudioQualityMetadata(context: Context, result: JSONObject, state: FinalizeState) { if (!supportsAudioMetadataProbe(state.filePath, state.fileName)) return diff --git a/go_backend/audio_metadata_ogg.go b/go_backend/audio_metadata_ogg.go index 81afcb7c..2c8f701c 100644 --- a/go_backend/audio_metadata_ogg.go +++ b/go_backend/audio_metadata_ogg.go @@ -48,7 +48,8 @@ func ReadOggVorbisComments(filePath string) (*AudioMetadata, error) { } } - if metadata.Title == "" && metadata.Artist == "" { + if metadata.Title == "" && metadata.Artist == "" && + metadata.ReplayGainTrackGain == "" && metadata.ReplayGainAlbumGain == "" { return nil, fmt.Errorf("no Vorbis comments found") } @@ -314,9 +315,9 @@ func parseVorbisComments(data []byte, metadata *AudioMetadata) { // r128ToReplayGainDb converts an R128_*_GAIN value (integer, 1/256 dB steps, // -23 LUFS reference) to a ReplayGain 2 dB string (-18 LUFS reference): -// rg = q/256 + 5. Inverse of the writer's replayGainDbToR128. +// rg = q/256 + 5. Inverse of applyOpusReplayGainEdits. func r128ToReplayGainDb(raw string) (string, bool) { - q, err := strconv.Atoi(strings.TrimSpace(raw)) + q, err := strconv.ParseInt(strings.TrimSpace(raw), 10, 16) if err != nil { return "", false } diff --git a/go_backend/ogg_edit.go b/go_backend/ogg_edit.go index 6a619f82..b8f90c43 100644 --- a/go_backend/ogg_edit.go +++ b/go_backend/ogg_edit.go @@ -7,8 +7,10 @@ import ( "errors" "fmt" "io" + "math" "os" "path/filepath" + "strconv" "strings" flacvorbis "github.com/go-flac/flacvorbis/v2" @@ -350,6 +352,12 @@ func EditOggFields(filePath string, fields map[string]string) error { cmt.Vendor = vendor cmt.Comments = comments applyVorbisFieldEdits(cmt, fields) + if isOpus { + if err := applyOpusReplayGainEdits(cmt, fields); err != nil { + f.Close() + return err + } + } coverPath := strings.TrimSpace(fields["cover_path"]) if coverPath != "" && fileExists(coverPath) { @@ -446,3 +454,30 @@ func EditOggFields(filePath string, fields map[string]string) error { syncDir(filepath.Dir(filePath)) return nil } + +// Opus uses R128 gain comments relative to -23 LUFS, in signed Q7.8 units. +// The scan already includes OpusHead's output gain; preserve that header and +// store only the additional adjustment. Remove legacy tags for edited scopes +// so players cannot select conflicting gains or peaks (RFC 7845 section 5.2). +func applyOpusReplayGainEdits(cmt *flacvorbis.MetaDataBlockVorbisComment, fields map[string]string) error { + for _, scope := range []string{"track", "album"} { + raw, present := fields["replaygain_"+scope+"_gain"] + if !present { + continue + } + value := "" + if raw = strings.TrimSpace(raw); raw != "" { + db, err := strconv.ParseFloat(strings.TrimSpace(strings.TrimSuffix(raw, "dB")), 64) + q := math.Round((db - 5) * 256) + if err != nil || math.IsNaN(q) || math.IsInf(q, 0) || q < -32768 || q > 32767 { + return fmt.Errorf("invalid Opus %s ReplayGain: %q", scope, raw) + } + value = strconv.Itoa(int(q)) + } + upper := strings.ToUpper(scope) + setOrClearComment(cmt, "R128_"+upper+"_GAIN", value) + removeCommentKey(cmt, "REPLAYGAIN_"+upper+"_GAIN") + removeCommentKey(cmt, "REPLAYGAIN_"+upper+"_PEAK") + } + return nil +} diff --git a/go_backend/replaygain_opus_test.go b/go_backend/replaygain_opus_test.go new file mode 100644 index 00000000..ba4d8bc6 --- /dev/null +++ b/go_backend/replaygain_opus_test.go @@ -0,0 +1,216 @@ +package gobackend + +import ( + "bytes" + "encoding/base64" + "encoding/binary" + "encoding/json" + "os" + "os/exec" + "path/filepath" + "strings" + "testing" +) + +func readReplayGainOggPages(t *testing.T, path string) []oggEditPage { + t.Helper() + f, err := os.Open(path) + if err != nil { + t.Fatal(err) + } + defer f.Close() + pages, err := readAllOggEditPages(f) + if err != nil { + t.Fatal(err) + } + return pages +} + +func TestOpusReplayGainReplacesLegacyTagsAndPreservesAudio(t *testing.T) { + for _, ext := range []string{".opus", ".ogg"} { + t.Run(ext, func(t *testing.T) { + path := filepath.Join(t.TempDir(), "song"+ext) + buildTestOpus(t, path, []string{ + "TITLE=Example Song", "CUSTOM=Keep me", "METADATA_BLOCK_PICTURE=existing-picture", + "REPLAYGAIN_TRACK_GAIN=-4.00 dB", "REPLAYGAIN_TRACK_PEAK=0.800000", + "R128_TRACK_GAIN=-2304", "r128_track_gain=100", + "REPLAYGAIN_ALBUM_GAIN=-3.00 dB", "REPLAYGAIN_ALBUM_PEAK=0.900000", + "R128_ALBUM_GAIN=-2048", + }, 3) + // Preserve a nonzero OpusHead output gain as well as every audio page. + pages := readReplayGainOggPages(t, path) + binary.LittleEndian.PutUint16(pages[0].data[16:18], 256) + var original bytes.Buffer + for _, page := range pages { + if err := page.serialize(&original); err != nil { + t.Fatal(err) + } + } + if err := os.WriteFile(path, original.Bytes(), 0o600); err != nil { + t.Fatal(err) + } + + for _, scope := range []string{"track", "album"} { + fields, _ := json.Marshal(map[string]string{ + "replaygain_" + scope + "_gain": "-12.20 dB", + "replaygain_" + scope + "_peak": "1.258925", + }) + result, err := EditFileMetadata(path, string(fields)) + if err != nil || !strings.Contains(result, "native_ogg") { + t.Fatalf("write: %s, %v", result, err) + } + metadata, err := ReadFileMetadata(path) + if err != nil { + t.Fatal(err) + } + var readback map[string]any + if err := json.Unmarshal([]byte(metadata), &readback); err != nil { + t.Fatal(err) + } + if got := readback["replaygain_"+scope+"_gain"]; got != "-12.20 dB" { + t.Fatalf("%s gain = %v", scope, got) + } + raw := mustReadFile(t, path) + upper := strings.ToUpper(scope) + if bytes.Count(bytes.ToUpper(raw), []byte("R128_"+upper+"_GAIN=")) != 1 || + !bytes.Contains(raw, []byte("R128_"+upper+"_GAIN=-4403")) || + bytes.Contains(bytes.ToUpper(raw), []byte("REPLAYGAIN_"+upper+"_")) { + t.Fatalf("conflicting or incorrect %s comments", scope) + } + if scope == "track" && !bytes.Contains(raw, []byte("R128_ALBUM_GAIN=-2048")) { + t.Fatal("track update changed album gain") + } + } + after := readReplayGainOggPages(t, path) + if len(after) != len(pages) { + t.Fatalf("page count changed: %d -> %d", len(pages), len(after)) + } + for i := range pages { + if i == 1 { // Only OpusTags may change. + continue + } + var beforePage, afterPage bytes.Buffer + _ = pages[i].serialize(&beforePage) + _ = after[i].serialize(&afterPage) + if !bytes.Equal(beforePage.Bytes(), afterPage.Bytes()) { + t.Fatalf("header/audio page %d changed", i) + } + } + raw := mustReadFile(t, path) + for _, comment := range []string{"CUSTOM=Keep me", "METADATA_BLOCK_PICTURE=existing-picture"} { + if !bytes.Contains(raw, []byte(comment)) { + t.Fatalf("lost %s", comment) + } + } + if err := EditOggFields(path, map[string]string{"replaygain_track_gain": ""}); err != nil { + t.Fatal(err) + } + raw = mustReadFile(t, path) + if bytes.Contains(raw, []byte("R128_TRACK_GAIN=")) || !bytes.Contains(raw, []byte("R128_ALBUM_GAIN=-4403")) { + t.Fatal("clearing track gain affected the wrong scope") + } + }) + } +} + +func TestOpusReplayGainInvalidValuesLeaveFileUntouched(t *testing.T) { + for _, gain := range []string{"invalid", "NaN", "+Inf", "-Inf", "-124 dB", "133 dB"} { + t.Run(gain, func(t *testing.T) { + path := filepath.Join(t.TempDir(), "song.opus") + buildTestOpus(t, path, []string{"R128_TRACK_GAIN=-1280"}, 1) + before := mustReadFile(t, path) + if err := EditOggFields(path, map[string]string{"replaygain_track_gain": gain}); err == nil { + t.Fatal("invalid gain was accepted") + } + if !bytes.Equal(before, mustReadFile(t, path)) { + t.Fatal("failed write changed the original") + } + }) + } + for _, raw := range []string{"32768", "-32769", "1.5"} { + if _, ok := r128ToReplayGainDb(raw); ok { + t.Fatalf("invalid R128 value accepted: %s", raw) + } + } +} + +func TestOpusReplayGainWithoutTitleOrArtist(t *testing.T) { + path := filepath.Join(t.TempDir(), "song.opus") + buildTestOpus(t, path, nil, 1) + if err := EditOggFields(path, map[string]string{"replaygain_track_gain": "0.00 dB"}); err != nil { + t.Fatal(err) + } + metadata, err := ReadFileMetadata(path) + if err != nil || !strings.Contains(metadata, `"replaygain_track_gain":"0.00 dB"`) { + t.Fatalf("ReplayGain-only comments were not read: %s, %v", metadata, err) + } +} + +func TestOpusReplayGainMediaRoundTrip(t *testing.T) { + ffmpeg, err := exec.LookPath("ffmpeg") + if err != nil { + t.Skip("requires ffmpeg on PATH") + } + run := func(t *testing.T, args ...string) []byte { + t.Helper() + cmd := exec.Command(ffmpeg, append([]string{"-hide_banner", "-v", "error"}, args...)...) + var stderr bytes.Buffer + cmd.Stderr = &stderr + out, err := cmd.Output() + if err != nil { + t.Fatalf("ffmpeg: %v: %s", err, &stderr) + } + return out + } + for _, cover := range []bool{false, true} { + name := "without-cover" + if cover { + name = "with-cover" + } + t.Run(name, func(t *testing.T) { + dir := t.TempDir() + path := filepath.Join(dir, "song.opus") + run(t, "-f", "lavfi", "-i", "sine=frequency=440:duration=0.2", "-c:a", "libopus", + "-metadata", "title=Example Song", "-metadata", "REPLAYGAIN_TRACK_GAIN=-4.00 dB", + "-metadata", "R128_TRACK_GAIN=-2304", path) + var picture []byte + if cover { + picture, err = base64.StdEncoding.DecodeString("iVBORw0KGgoAAAANSUhEUgAAAAEAAAABCAQAAAC1HAwCAAAAC0lEQVR42mP8/x8AAwMCAO+aB1sAAAAASUVORK5CYII=") + if err != nil { + t.Fatal(err) + } + coverPath := filepath.Join(dir, "cover.png") + if err := os.WriteFile(coverPath, picture, 0o600); err != nil { + t.Fatal(err) + } + if err := EditOggFields(path, map[string]string{"cover_path": coverPath}); err != nil { + t.Fatal(err) + } + } + audioHash := func() []byte { + return run(t, "-i", path, "-map", "0:a:0", "-c:a", "copy", "-f", "hash", "-hash", "sha256", "-") + } + beforeHash := audioHash() + if err := EditOggFields(path, map[string]string{ + "replaygain_track_gain": "-12.20 dB", "replaygain_track_peak": "1.258925", + "replaygain_album_gain": "-10.00 dB", "replaygain_album_peak": "1.300000", + }); err != nil { + t.Fatal(err) + } + if !bytes.Equal(beforeHash, audioHash()) { + t.Fatal("encoded audio changed") + } + run(t, "-i", path, "-map", "0:a:0", "-f", "null", "-") + metadata, err := ReadOggVorbisComments(path) + if err != nil || metadata.ReplayGainTrackGain != "-12.20 dB" || metadata.ReplayGainAlbumGain != "-10.00 dB" { + t.Fatalf("gain reread: %+v, %v", metadata, err) + } + if cover { + got, _, err := extractOggCoverArt(path) + if err != nil || !bytes.Equal(got, picture) { + t.Fatalf("cover changed: %v", err) + } + } + }) + } +} diff --git a/lib/providers/download_queue_provider.dart b/lib/providers/download_queue_provider.dart index 6aabbcc1..45b885f0 100644 --- a/lib/providers/download_queue_provider.dart +++ b/lib/providers/download_queue_provider.dart @@ -23,6 +23,7 @@ import 'package:spotiflac_android/services/app_state_database.dart'; import 'package:spotiflac_android/services/platform_bridge.dart'; import 'package:spotiflac_android/services/download_request_payload.dart'; import 'package:spotiflac_android/services/ffmpeg_service.dart'; +import 'package:spotiflac_android/services/replaygain_service.dart'; import 'package:spotiflac_android/services/notification_service.dart'; import 'package:spotiflac_android/services/verification_notification.dart'; import 'package:spotiflac_android/utils/logger.dart' hide log; diff --git a/lib/providers/download_queue_provider_embedding.dart b/lib/providers/download_queue_provider_embedding.dart index eb32d9dc..09770869 100644 --- a/lib/providers/download_queue_provider_embedding.dart +++ b/lib/providers/download_queue_provider_embedding.dart @@ -563,16 +563,16 @@ extension _DownloadQueueEmbedding on DownloadQueueNotifier { final rgResult = await FFmpegService.scanReplayGain(filePath); if (rgResult != null) { scannedReplayGain = rgResult; - metadata['REPLAYGAIN_TRACK_GAIN'] = rgResult.trackGain; - metadata['REPLAYGAIN_TRACK_PEAK'] = rgResult.trackPeak; - if (format == 'opus') { - final r128 = FFmpegService.replayGainDbToR128(rgResult.trackGain); - if (r128 != null) metadata['R128_TRACK_GAIN'] = r128; + if (format != 'opus') { + metadata['REPLAYGAIN_TRACK_GAIN'] = rgResult.trackGain; + metadata['REPLAYGAIN_TRACK_PEAK'] = rgResult.trackPeak; } _log.d( 'ReplayGain for $format: gain=${rgResult.trackGain}, peak=${rgResult.trackPeak}', ); - _storeTrackReplayGainForAlbum(track, filePath, rgResult); + if (format != 'opus') { + _storeTrackReplayGainForAlbum(track, filePath, rgResult); + } } } catch (e) { _log.w('Failed to scan ReplayGain for $format: $e'); @@ -634,10 +634,10 @@ extension _DownloadQueueEmbedding on DownloadQueueNotifier { // audio through untouched — no FFmpeg spawn, no full container remux, // no temp-promote copy. The Go side answers method=ffmpeg for files it // can't handle natively, and any failure falls back to FFmpeg below. - // Scanned ReplayGain (opt-in, non-FLAC) keeps the FFmpeg path: its - // extra tags (e.g. Opus R128_TRACK_GAIN) ride the FFmpeg metadata map. + // Opus ReplayGain is written and verified separately below, through the + // same native R128 writer used by manual scans and album gain updates. var embeddedNatively = false; - if (scannedReplayGain == null) { + if (scannedReplayGain == null || format == 'opus') { try { final nativeFields = { 'title': track.name, @@ -686,7 +686,10 @@ extension _DownloadQueueEmbedding on DownloadQueueNotifier { filePath, nativeFields, ); - embeddedNatively = response['method'] != 'ffmpeg'; + embeddedNatively = + response['success'] == true && + response['error'] == null && + response['method'] != 'ffmpeg'; } catch (e) { _log.w('Native $format tag embed failed, falling back to FFmpeg: $e'); } @@ -731,6 +734,19 @@ extension _DownloadQueueEmbedding on DownloadQueueNotifier { } } + if (format == 'opus' && scannedReplayGain != null) { + final written = await ReplayGainService.writeTrackTags( + filePath, + scannedReplayGain.trackGain, + scannedReplayGain.trackPeak, + ); + if (written) { + _storeTrackReplayGainForAlbum(track, filePath, scannedReplayGain); + } else { + _log.w('Failed to write Opus ReplayGain'); + } + } + if (isM4a && settings.embedReplayGain && scannedReplayGain != null) { try { await PlatformBridge.editFileMetadata(filePath, { diff --git a/lib/providers/download_queue_provider_native_worker.dart b/lib/providers/download_queue_provider_native_worker.dart index 143a570b..21dbe7a2 100644 --- a/lib/providers/download_queue_provider_native_worker.dart +++ b/lib/providers/download_queue_provider_native_worker.dart @@ -1415,7 +1415,6 @@ extension _DownloadQueueNativeWorker on DownloadQueueNotifier { actualBitrate = autoConvertBitrateKbps(settings.autoConvertBitrate); } await _writeNativeWorkerReplayGain( - context: context, settings: settings, track: trackToDownload, filePath: filePath, @@ -1547,7 +1546,6 @@ extension _DownloadQueueNativeWorker on DownloadQueueNotifier { } Future _writeNativeWorkerReplayGain({ - required _NativeWorkerRequestContext context, required AppSettings settings, required Track track, required String filePath, @@ -1555,19 +1553,20 @@ extension _DownloadQueueNativeWorker on DownloadQueueNotifier { if (!settings.embedReplayGain) { return; } - if (context.outputExt != '.flac' && context.outputExt != '.m4a') { + final ext = audioFormatForPath(filePath)?.toLowerCase(); + if (ext != 'flac' && + ext != 'm4a' && + ext != 'mp3' && + ext != 'opus' && + !isContentUri(filePath)) { return; } try { - final rgResult = await FFmpegService.scanReplayGain(filePath); + final rgResult = await ReplayGainService.scanAndApplyToFile(filePath); if (rgResult == null) { return; } - await PlatformBridge.editFileMetadata(filePath, { - 'replaygain_track_gain': rgResult.trackGain, - 'replaygain_track_peak': rgResult.trackPeak, - }); _storeTrackReplayGainForAlbum(track, filePath, rgResult); _updateAlbumRgFilePath(track, filePath); await _checkAndWriteAlbumReplayGain(track); diff --git a/lib/providers/download_queue_provider_replaygain.dart b/lib/providers/download_queue_provider_replaygain.dart index 21978762..71241a38 100644 --- a/lib/providers/download_queue_provider_replaygain.dart +++ b/lib/providers/download_queue_provider_replaygain.dart @@ -172,55 +172,13 @@ extension _DownloadQueueReplayGain on DownloadQueueNotifier { String albumGain, String albumPeak, ) async { - final lower = filePath.toLowerCase(); - if (lower.endsWith('.flac') || - lower.endsWith('.ape') || - lower.endsWith('.wv') || - lower.endsWith('.mpc')) { - // Native writer — only touches the provided fields, preserves the rest. - await PlatformBridge.editFileMetadata(filePath, { - 'replaygain_album_gain': albumGain, - 'replaygain_album_peak': albumPeak, - }); - } else if (isContentUri(filePath)) { - // SAF content:// URI — FFmpeg can read it but can't write back directly. - // Get the temp output from FFmpeg, then copy it to the SAF URI. - String? tempPath; - final ok = await FFmpegService.writeAlbumReplayGainTags( - filePath, - albumGain, - albumPeak, - returnTempPath: true, - onTempReady: (path) => tempPath = path, - ); - if (ok && tempPath != null) { - try { - final safOk = await PlatformBridge.writeTempToSaf( - tempPath!, - filePath, - ); - if (!safOk) { - _log.w('SAF write-back failed for album RG: $filePath'); - } - } finally { - try { - final tmp = File(tempPath!); - if (await tmp.exists()) await tmp.delete(); - } catch (_) {} - } - } else { - _log.w('FFmpeg album ReplayGain write failed for SAF: $filePath'); - } - } else { - // Local MP3 / Opus — use FFmpeg copy-with-metadata approach. - final ok = await FFmpegService.writeAlbumReplayGainTags( - filePath, - albumGain, - albumPeak, - ); - if (!ok) { - _log.w('FFmpeg album ReplayGain write failed for: $filePath'); - } + final ok = await ReplayGainService.writeAlbumTags( + filePath, + albumGain, + albumPeak, + ); + if (!ok) { + _log.w('Album ReplayGain write failed for: $filePath'); } } diff --git a/lib/services/ffmpeg_service.dart b/lib/services/ffmpeg_service.dart index 61579172..f7ff0e0e 100644 --- a/lib/services/ffmpeg_service.dart +++ b/lib/services/ffmpeg_service.dart @@ -1057,22 +1057,6 @@ class FFmpegService { ); } - /// Convert a ReplayGain gain value (dB, referenced to -18 LUFS) into an Opus - /// R128 gain tag value (Q7.8 fixed point integer, referenced to -23 LUFS). - /// - /// Opus players read `R128_TRACK_GAIN` / `R128_ALBUM_GAIN` per RFC 7845, not - /// the `REPLAYGAIN_*` dB strings. The reference levels differ by exactly 5 dB - /// (-18 vs -23 LUFS), so the R128 gain equals the ReplayGain value minus 5 dB, - /// stored as `round(dB * 256)`. - static String? replayGainDbToR128(String replayGainDb) { - final match = RegExp(r'-?\d+\.?\d*').firstMatch(replayGainDb); - if (match == null) return null; - final rgDb = double.tryParse(match.group(0) ?? ''); - if (rgDb == null) return null; - final r128Db = rgDb - 5.0; - return (r128Db * 256).round().toString(); - } - /// Write album ReplayGain tags to a file via FFmpeg. /// /// For local files, replaces the file in-place and returns `true`. @@ -1097,10 +1081,9 @@ class FFmpegService { /// Write track ReplayGain tags to a file via FFmpeg, replacing it in place. /// - /// Used for formats that are not handled by the native tag writers - /// (MP3/Opus). All existing streams and metadata are preserved via - /// `-map 0 -c copy -map_metadata 0`; only the REPLAYGAIN_TRACK_* fields are - /// added/overwritten. Returns `true` when the file was rewritten in place. + /// Used as a fallback for formats other than Ogg/Opus. Copies streams and + /// metadata with `-map 0 -c copy -map_metadata 0`, setting track gain and peak. + /// The caller verifies the tags after a successful rewrite. static Future writeTrackReplayGainTags( String filePath, String trackGain, @@ -1108,7 +1091,7 @@ class FFmpegService { ) => _writeReplayGainTags(filePath, 'Track', trackGain, trackPeak); /// Shared implementation for album/track ReplayGain tagging. - /// [scope] is 'Album' or 'Track'; it selects the REPLAYGAIN_*/R128_* tags. + /// [scope] is 'Album' or 'Track'; it selects the REPLAYGAIN_* tags. static Future _writeReplayGainTags( String filePath, String scope, @@ -1120,6 +1103,10 @@ class FFmpegService { final ext = filePath.contains('.') ? '.${filePath.split('.').last}' : '.tmp'; + if (ext.toLowerCase() == '.opus' || ext.toLowerCase() == '.ogg') { + _log.e('Ogg/Opus ReplayGain requires the native tag writer'); + return false; + } final tempDir = await getTemporaryDirectory(); final tempOutput = _nextTempEmbedPath(tempDir.path, ext); final tag = scope.toUpperCase(); @@ -1141,15 +1128,6 @@ class FFmpegService { 'REPLAYGAIN_${tag}_PEAK=$peak', ]; - if (ext.toLowerCase() == '.opus') { - final r128 = replayGainDbToR128(gain); - if (r128 != null) { - arguments - ..add('-metadata') - ..add('R128_${tag}_GAIN=$r128'); - } - } - arguments ..add(tempOutput) ..add('-y'); @@ -1157,6 +1135,11 @@ class FFmpegService { _log.d('Writing ${scope.toLowerCase()} ReplayGain tags via FFmpeg'); final result = await _executeWithArguments(arguments); + if (!result.success) { + _log.e( + '$scope ReplayGain write failed (code ${result.returnCode}): ${result.output}', + ); + } if (result.success) { if (returnTempPath) { try { diff --git a/lib/services/music_player_service.dart b/lib/services/music_player_service.dart index 368daf00..2c974767 100644 --- a/lib/services/music_player_service.dart +++ b/lib/services/music_player_service.dart @@ -37,6 +37,12 @@ void setPlaybackNormalizationEnabled(bool enabled) { _activeMusicPlayerHandler?.reapplyNormalization(); } +/// Refreshes gain tags after a successful file update, including SAF copies. +void refreshPlaybackNormalization(String source) { + final handler = _activeMusicPlayerHandler; + if (handler != null) unawaited(handler._refreshNormalizationSource(source)); +} + List buildShuffleCandidatePool({ required int mediaCount, required int currentIndex, @@ -556,6 +562,24 @@ class MusicPlayerHandler extends BaseAudioHandler onReadError: (error) => _log.w('Failed to read gain tags for normalization: $error'), ); + int _normalizationGeneration = 0; + + Future _refreshNormalizationSource(String source) async { + _normalizationGeneration++; + _normalizationCache.invalidate(source); + // A copy started before the edit can still contain the old comments. + await _pendingSourceResolutions[source]; + final oldPath = _resolvedPathCache.remove(source); + _resolvedPathSizes.remove(source); + _resolvedPathOrder.remove(source); + if (oldPath != null) await _discardResolvedPath(oldPath); + if (_disposed) return; + if (_index >= 0 && + _index < _media.length && + _media[_index].source == source) { + reapplyNormalization(); + } + } Future _normalizationVolumeFor( String path, { @@ -574,6 +598,7 @@ class MusicPlayerHandler extends BaseAudioHandler void reapplyNormalization() { final index = _index; final generation = _playRequestGeneration; + final normalizationGeneration = ++_normalizationGeneration; if (index < 0 || index >= _media.length) return; unawaited(() async { final media = _media[index]; @@ -599,7 +624,11 @@ class MusicPlayerHandler extends BaseAudioHandler if (playbackLease != null) { await PlatformBridge.closeContentUriPlaybackLease(playbackLease.token); } - if (_index != index || generation != _playRequestGeneration) return; + if (_index != index || + generation != _playRequestGeneration || + normalizationGeneration != _normalizationGeneration) { + return; + } try { await _player.setVolume(volume); } catch (e) { diff --git a/lib/services/playback_normalization.dart b/lib/services/playback_normalization.dart index 6001da3e..174c63ab 100644 --- a/lib/services/playback_normalization.dart +++ b/lib/services/playback_normalization.dart @@ -7,10 +7,16 @@ class PlaybackNormalizationCache { readMetadata; final void Function(Object)? onReadError; final Map _volumes = {}; + int _generation = 0; static final _gainNumber = RegExp(r'-?\d+(\.\d+)?'); PlaybackNormalizationCache({required this.readMetadata, this.onReadError}); + void invalidate(String source) { + _volumes.remove(source); + _generation++; + } + Future volumeFor( String path, { String? cacheKey, @@ -19,6 +25,7 @@ class PlaybackNormalizationCache { final key = cacheKey ?? path; final cached = _volumes[key]; if (cached != null) return cached; + final generation = _generation; try { final metadata = await readMetadata(path, displayName: displayName); if (metadata['error'] != null) { @@ -31,8 +38,10 @@ class PlaybackNormalizationCache { 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; + if (generation == _generation) { + if (_volumes.length >= 128) _volumes.remove(_volumes.keys.first); + _volumes[key] = volume; + } return volume; } catch (error) { onReadError?.call(error); diff --git a/lib/services/replaygain_service.dart b/lib/services/replaygain_service.dart index 67024221..fa5dd4cf 100644 --- a/lib/services/replaygain_service.dart +++ b/lib/services/replaygain_service.dart @@ -1,16 +1,16 @@ import 'dart:io'; +import 'package:flutter/foundation.dart'; import 'package:spotiflac_android/services/ffmpeg_service.dart'; +import 'package:spotiflac_android/services/music_player_service.dart'; import 'package:spotiflac_android/services/platform_bridge.dart'; import 'package:spotiflac_android/utils/file_access.dart'; import 'package:spotiflac_android/utils/logger.dart'; /// Standalone ReplayGain (re)scanning for existing audio files. /// -/// Computes EBU R128 loudness via FFmpeg and writes REPLAYGAIN_TRACK_* tags -/// back into the file in place: -/// - FLAC / M4A / MP4 / APE / WV / MPC -> native tag writer (PlatformBridge) -/// - MP3 / Opus / OGG / others -> FFmpeg copy-with-metadata +/// Computes EBU R128 loudness via FFmpeg and writes gain tags using the native +/// metadata editors where supported. Opus uses R128_* rather than legacy tags. /// /// Handles SAF content:// URIs transparently by working on a temporary copy /// and writing it back to the original document. @@ -31,6 +31,9 @@ class ReplayGainService { '.aiff', '.aif', '.aifc', + '.mp3', + '.opus', + '.ogg', }; static bool _isNativeWritableFormat(String path) { @@ -42,7 +45,120 @@ class ReplayGainService { /// /// Returns `true` when tags were successfully written, `false` otherwise /// (scan failed, write failed, or SAF write-back failed). - static Future applyToFile(String filePath) async { + static Future applyToFile( + String filePath, { + @visibleForTesting Future Function(String)? scan, + }) async => await scanAndApplyToFile(filePath, scan: scan) != null; + + /// Returns the scan for album aggregation only after a verified save. + static Future scanAndApplyToFile( + String filePath, { + @visibleForTesting Future Function(String)? scan, + }) async { + ReplayGainResult? scanned; + final written = await _updateFile(filePath, (workingPath) async { + final rg = await (scan ?? FFmpegService.scanReplayGain)(workingPath); + if (rg == null) { + _log.w('ReplayGain scan returned no result for $workingPath'); + return false; + } + scanned = rg; + return _writeLocalTags( + workingPath, + rg.trackGain, + rg.trackPeak, + album: false, + ); + }); + return written ? scanned : null; + } + + static Future writeTrackTags( + String filePath, + String gain, + String peak, + ) => _updateFile( + filePath, + (path) => _writeLocalTags(path, gain, peak, album: false), + ); + + static Future writeAlbumTags( + String filePath, + String gain, + String peak, + ) => _updateFile( + filePath, + (path) => _writeLocalTags(path, gain, peak, album: true), + ); + + static Future _writeLocalTags( + String path, + String gain, + String peak, { + required bool album, + }) async { + final scope = album ? 'album' : 'track'; + var written = false; + if (_isNativeWritableFormat(path)) { + final result = await PlatformBridge.editFileMetadata(path, { + 'replaygain_${scope}_gain': gain, + 'replaygain_${scope}_peak': peak, + }); + written = + result['success'] == true && + result['error'] == null && + result['method'] is String && + (result['method'] == 'native' || + (result['method'] as String).startsWith('native_')); + if (!written) { + _log.w('Native $scope ReplayGain write did not complete: $result'); + } + } + + if (!written) { + // Remuxing all streams rejects attached Opus artwork; mapping only audio + // would discard it. Keep the original if its native editor cannot handle + // the file, rather than silently losing artwork or reporting a no-op. + final lower = path.toLowerCase(); + if (lower.endsWith('.opus') || lower.endsWith('.ogg')) return false; + written = album + ? await FFmpegService.writeAlbumReplayGainTags(path, gain, peak) + : await FFmpegService.writeTrackReplayGainTags(path, gain, peak); + } + if (!written) return false; + + final metadata = await PlatformBridge.readFileMetadata(path); + final expectedGain = _gainDb(gain); + final actualGain = _gainDb(metadata['replaygain_${scope}_gain']); + final expectedPeak = double.tryParse(peak); + final actualPeak = double.tryParse( + metadata['replaygain_${scope}_peak']?.toString() ?? '', + ); + final isOpus = metadata['audio_codec'] == 'opus'; + final verified = + metadata['error'] == null && + expectedGain != null && + actualGain != null && + (actualGain - expectedGain).abs() <= 0.01 && + (isOpus || + (expectedPeak != null && + actualPeak != null && + (actualPeak - expectedPeak).abs() <= 0.000001)); + if (!verified) { + _log.w('$scope ReplayGain verification failed after writing $path'); + return false; + } + return true; + } + + static double? _gainDb(Object? value) => double.tryParse( + (value?.toString() ?? '').replaceFirst(RegExp(r'\s*dB\s*$'), '').trim(), + ); + + static Future _updateFile( + String filePath, + Future Function(String) update, + ) async { if (filePath.isEmpty) return false; final isSaf = isContentUri(filePath); @@ -59,40 +175,18 @@ class ReplayGainService { workingPath = safTempPath; } - final rg = await FFmpegService.scanReplayGain(workingPath); - if (rg == null) { - _log.w('ReplayGain scan returned no result for $workingPath'); - return false; - } - - bool written; - if (_isNativeWritableFormat(workingPath)) { - final result = await PlatformBridge.editFileMetadata(workingPath, { - 'replaygain_track_gain': rg.trackGain, - 'replaygain_track_peak': rg.trackPeak, - }); - written = result['error'] == null; - if (!written) { - _log.w('Native ReplayGain write failed: ${result['error']}'); - } - } else { - written = await FFmpegService.writeTrackReplayGainTags( - workingPath, - rg.trackGain, - rg.trackPeak, - ); - } - - if (!written) return false; + if (!await update(workingPath)) return false; if (isSaf) { final ok = await PlatformBridge.writeTempToSaf(workingPath, filePath); if (!ok) { _log.w('Failed to write ReplayGain temp file back to SAF document'); + return false; } - return ok; } + refreshPlaybackNormalization(filePath); + _log.i('ReplayGain tags written and verified: $filePath'); return true; } catch (e) { _log.e('Failed to apply ReplayGain', e); diff --git a/test/playback_normalization_test.dart b/test/playback_normalization_test.dart index 20877884..bcf27c90 100644 --- a/test/playback_normalization_test.dart +++ b/test/playback_normalization_test.dart @@ -1,7 +1,42 @@ +import 'dart:async'; + import 'package:flutter_test/flutter_test.dart'; import 'package:spotiflac_android/services/playback_normalization.dart'; void main() { + test('updated gain replaces the cached volume for the same URI', () async { + var gain = '-4.00 dB'; + final cache = PlaybackNormalizationCache( + readMetadata: (_, {displayName}) async => {'replaygain_track_gain': gain}, + ); + const source = 'content://music/song'; + expect(await cache.volumeFor(source), closeTo(0.630957, 0.000001)); + gain = '-12.20 dB'; + cache.invalidate(source); + expect(await cache.volumeFor(source), closeTo(0.245471, 0.000001)); + }); + + test( + 'a read started before an edit cannot restore stale cache data', + () async { + final staleRead = Completer>(); + var reads = 0; + final cache = PlaybackNormalizationCache( + readMetadata: (_, {displayName}) async { + if (++reads == 1) return staleRead.future; + return {'replaygain_track_gain': '-12.20 dB'}; + }, + ); + final pending = cache.volumeFor('song'); + cache.invalidate('song'); + await cache.volumeFor('song'); + staleRead.complete({'replaygain_track_gain': '-4.00 dB'}); + await pending; + expect(await cache.volumeFor('song'), closeTo(0.245471, 0.000001)); + expect(reads, 2); + }, + ); + test( 'descriptor normalization uses hint and caches by original URI', () async { diff --git a/test/replaygain_service_test.dart b/test/replaygain_service_test.dart new file mode 100644 index 00000000..78a855c4 --- /dev/null +++ b/test/replaygain_service_test.dart @@ -0,0 +1,197 @@ +import 'dart:convert'; +import 'dart:io'; + +import 'package:flutter/services.dart'; +import 'package:flutter_test/flutter_test.dart'; +import 'package:spotiflac_android/services/ffmpeg_service.dart'; +import 'package:spotiflac_android/services/replaygain_service.dart'; + +void main() { + TestWidgetsFlutterBinding.ensureInitialized(); + const channel = MethodChannel('com.zarz.spotiflac/backend'); + final messenger = + TestDefaultBinaryMessengerBinding.instance.defaultBinaryMessenger; + late Directory directory; + late String tempPath; + late List calls; + late Map metadata; + late Map editedFields; + var method = 'native_ogg'; + var applyEdits = true; + var saveSaf = true; + + setUp(() async { + directory = await Directory.systemTemp.createTemp('replaygain-test-'); + tempPath = '${directory.path}/saf_song.opus'; + calls = []; + editedFields = {}; + metadata = { + 'audio_codec': 'opus', + 'replaygain_track_gain': '-4.00 dB', + 'replaygain_album_gain': '-3.00 dB', + }; + method = 'native_ogg'; + applyEdits = true; + saveSaf = true; + messenger.setMockMethodCallHandler(channel, (call) async { + calls.add(call.method); + final args = Map.from(call.arguments as Map); + switch (call.method) { + case 'safCopyToTemp': + await File(tempPath).writeAsString('original audio'); + return tempPath; + case 'editFileMetadata': + editedFields = Map.from( + jsonDecode(args['metadata_json'] as String) as Map, + ); + if (applyEdits) { + metadata.addAll(editedFields); + if (metadata['audio_codec'] == 'opus') { + metadata.removeWhere((key, _) => key.endsWith('_peak')); + } + } + return jsonEncode({'success': true, 'method': method}); + case 'readFileMetadata': + return jsonEncode(metadata); + case 'writeTempToSaf': + expect(args['temp_path'], tempPath); + expect(await File(tempPath).exists(), isTrue); + return jsonEncode({'success': saveSaf}); + default: + fail('Unexpected method: ${call.method}'); + } + }); + }); + + tearDown(() async { + messenger.setMockMethodCallHandler(channel, null); + await directory.delete(recursive: true); + }); + + test( + 'manual SAF scan writes and verifies Opus before copying back', + () async { + final result = await ReplayGainService.applyToFile( + 'content://music/document/42', + scan: (path) async { + expect(path, tempPath); + expect(await File(path).exists(), isTrue); + calls.add('scan'); + return ReplayGainResult( + trackGain: '-12.20 dB', + trackPeak: '1.258925', + integratedLufs: -5.8, + truePeakLinear: 1.258925, + ); + }, + ); + expect(result, isTrue); + expect(calls, [ + 'safCopyToTemp', + 'scan', + 'editFileMetadata', + 'readFileMetadata', + 'writeTempToSaf', + ]); + expect(editedFields['replaygain_track_gain'], '-12.20 dB'); + expect(await File(tempPath).exists(), isFalse); + }, + ); + + test( + 'album gain on an extensionless SAF URI uses the native writer', + () async { + expect( + await ReplayGainService.writeAlbumTags( + 'content://music/document/42', + '-10.00 dB', + '1.300000', + ), + isTrue, + ); + expect(editedFields, { + 'replaygain_album_gain': '-10.00 dB', + 'replaygain_album_peak': '1.300000', + }); + expect(metadata['replaygain_track_gain'], '-4.00 dB'); + expect(calls.last, 'writeTempToSaf'); + expect(await File(tempPath).exists(), isFalse); + }, + ); + + test('native fallback instruction is not a successful Opus write', () async { + method = 'ffmpeg'; + applyEdits = false; + expect( + await ReplayGainService.writeTrackTags( + 'content://music/document/42', + '-12.20 dB', + '1.258925', + ), + isFalse, + ); + expect(calls, ['safCopyToTemp', 'editFileMetadata']); + expect(await File(tempPath).exists(), isFalse); + }); + + test('unchanged gain fails verification and never overwrites SAF', () async { + applyEdits = false; + expect( + await ReplayGainService.writeTrackTags( + 'content://music/document/42', + '-12.20 dB', + '1.258925', + ), + isFalse, + ); + expect(calls, ['safCopyToTemp', 'editFileMetadata', 'readFileMetadata']); + expect(await File(tempPath).exists(), isFalse); + }); + + test( + 'SAF save failure is reported and the temporary copy is removed', + () async { + saveSaf = false; + expect( + await ReplayGainService.writeTrackTags( + 'content://music/document/42', + '-12.20 dB', + '1.258925', + ), + isFalse, + ); + expect(calls.last, 'writeTempToSaf'); + expect(await File(tempPath).exists(), isFalse); + }, + ); + + test( + 'scan failure leaves SAF untouched and removes its temporary copy', + () async { + expect( + await ReplayGainService.applyToFile( + 'content://music/document/42', + scan: (_) async => null, + ), + isFalse, + ); + expect(calls, ['safCopyToTemp']); + expect(await File(tempPath).exists(), isFalse); + }, + ); + + test('FLAC native method and peak verification remain supported', () async { + method = 'native'; + metadata['audio_codec'] = 'flac'; + expect( + await ReplayGainService.writeTrackTags( + '${directory.path}/song.flac', + '-12.20 dB', + '1.258925', + ), + isTrue, + ); + expect(metadata['replaygain_track_peak'], '1.258925'); + expect(calls, ['editFileMetadata', 'readFileMetadata']); + }); +}