From 9d530cea9e334117f3aeb4f0a9cd788b65af99e6 Mon Sep 17 00:00:00 2001 From: zarzet <42882290+zarzet@users.noreply.github.com> Date: Fri, 25 Sep 2026 17:06:12 +0700 Subject: [PATCH] feat(metadata): remove ReplayGain from individual and selected tracks --- lib/l10n/arb/app_en.arb | 31 ++ lib/l10n/arb/app_id.arb | 7 + lib/screens/downloaded_album_screen.dart | 30 +- lib/screens/local_album_screen.dart | 35 +- lib/screens/queue_tab_batch_actions.dart | 6 +- lib/screens/queue_tab_item_widgets.dart | 26 +- lib/screens/track_metadata_convert.dart | 49 ++- lib/screens/track_metadata_screen_menu.dart | 8 +- lib/services/batch_track_actions.dart | 37 ++- lib/services/replaygain_service.dart | 35 +- rust_backend/crates/core/src/tags/write.rs | 15 + .../crates/core/src/tags/write/ape.rs | 14 +- .../crates/core/src/tags/write/mp4.rs | 14 +- .../crates/core/src/tags/write/riff.rs | 49 ++- .../crates/core/tests/replaygain_removal.rs | 298 ++++++++++++++++++ .../crates/extensions/src/backend/tags.rs | 27 +- test/album_replaygain_action_test.dart | 204 ++++++------ test/replaygain_service_test.dart | 74 +++++ test/track_metadata_replaygain_test.dart | 47 +++ 19 files changed, 835 insertions(+), 171 deletions(-) create mode 100644 rust_backend/crates/core/tests/replaygain_removal.rs diff --git a/lib/l10n/arb/app_en.arb b/lib/l10n/arb/app_en.arb index afbdc261..c5115468 100644 --- a/lib/l10n/arb/app_en.arb +++ b/lib/l10n/arb/app_en.arb @@ -259,6 +259,37 @@ "@trackReplayGainFailed": { "description": "Snackbar message when ReplayGain scan/write fails" }, + "trackRemoveReplayGain": "Remove ReplayGain", + "trackRemoveReplayGainSuccess": "ReplayGain tags removed", + "trackRemoveReplayGainFailed": "Failed to remove ReplayGain tags", + "replayGainRemoving": "Removing ReplayGain...", + "replayGainRemoveConfirmMessage": "Remove track and album ReplayGain tags from {count} track(s)? Audio, artwork, and other metadata will be preserved.", + "@replayGainRemoveConfirmMessage": { + "placeholders": { + "count": { + "type": "int" + } + } + }, + "selectionRemoveReplayGainCount": "Remove ReplayGain ({count})", + "@selectionRemoveReplayGainCount": { + "placeholders": { + "count": { + "type": "int" + } + } + }, + "replayGainRemoveBatchSuccess": "ReplayGain removed from {success} of {total} tracks", + "@replayGainRemoveBatchSuccess": { + "placeholders": { + "success": { + "type": "int" + }, + "total": { + "type": "int" + } + } + }, "selectionReplayGainCount": "ReplayGain ({count})", "@selectionReplayGainCount": { "description": "Batch selection action button label for ReplayGain", diff --git a/lib/l10n/arb/app_id.arb b/lib/l10n/arb/app_id.arb index f00e54aa..1adcb62a 100644 --- a/lib/l10n/arb/app_id.arb +++ b/lib/l10n/arb/app_id.arb @@ -6051,6 +6051,13 @@ "extensionsNoDownloadProvider": "Tidak ada ekstensi dengan provider unduhan", "libraryFilterSortGenreAsc": "Genre (A-Z)", "trackReplayGainFailed": "Gagal menambahkan tag ReplayGain", + "trackRemoveReplayGain": "Hapus ReplayGain", + "trackRemoveReplayGainSuccess": "Tag ReplayGain dihapus", + "trackRemoveReplayGainFailed": "Gagal menghapus tag ReplayGain", + "replayGainRemoving": "Menghapus ReplayGain...", + "replayGainRemoveConfirmMessage": "Hapus tag ReplayGain lagu dan album dari {count} lagu? Audio, sampul, dan metadata lainnya tetap tersimpan.", + "selectionRemoveReplayGainCount": "Hapus ReplayGain ({count})", + "replayGainRemoveBatchSuccess": "ReplayGain dihapus dari {success} dari {total} lagu", "collectionPlaylistRemoveCover": "Remove cover image", "storeEmptyNoResults": "Tidak ada ekstensi ditemukan", "logIssueTrackNotFoundDescription": "Some tracks could not be found on download services", diff --git a/lib/screens/downloaded_album_screen.dart b/lib/screens/downloaded_album_screen.dart index 0face0dc..824fe556 100644 --- a/lib/screens/downloaded_album_screen.dart +++ b/lib/screens/downloaded_album_screen.dart @@ -763,18 +763,24 @@ class _DownloadedAlbumScreenState extends ConsumerState : null, colorScheme: colorScheme, ), - SelectionActionButton( - icon: Icons.graphic_eq, - label: context.l10n.selectionReplayGainCount(selectedCount), - onPressed: selectedCount > 0 - ? () => runBatchReplayGain( - this.context, - _selectedUnifiedItems(tracks), - onExitSelectionMode: exitSelectionMode, - ) - : null, - colorScheme: colorScheme, - ), + for (final remove in [false, true]) + SelectionActionButton( + icon: remove ? Icons.remove_circle_outline : Icons.graphic_eq, + label: remove + ? context.l10n.selectionRemoveReplayGainCount( + selectedCount, + ) + : context.l10n.selectionReplayGainCount(selectedCount), + onPressed: selectedCount > 0 + ? () => runBatchReplayGain( + this.context, + _selectedUnifiedItems(tracks), + onExitSelectionMode: exitSelectionMode, + remove: remove, + ) + : null, + colorScheme: colorScheme, + ), ]; return Wrap( diff --git a/lib/screens/local_album_screen.dart b/lib/screens/local_album_screen.dart index 1f2b2c2b..35d4634d 100644 --- a/lib/screens/local_album_screen.dart +++ b/lib/screens/local_album_screen.dart @@ -572,20 +572,27 @@ class _LocalAlbumScreenState extends ConsumerState ), ); - actions.add( - SelectionActionButton( - icon: Icons.graphic_eq, - label: context.l10n.selectionReplayGainCount(selectedCount), - onPressed: selectedCount > 0 - ? () => runBatchReplayGain( - this.context, - _selectedUnifiedItems(tracks), - onExitSelectionMode: exitSelectionMode, - ) - : null, - colorScheme: colorScheme, - ), - ); + for (final remove in [false, true]) { + actions.add( + SelectionActionButton( + icon: remove ? Icons.remove_circle_outline : Icons.graphic_eq, + label: remove + ? context.l10n.selectionRemoveReplayGainCount( + selectedCount, + ) + : context.l10n.selectionReplayGainCount(selectedCount), + onPressed: selectedCount > 0 + ? () => runBatchReplayGain( + this.context, + _selectedUnifiedItems(tracks), + onExitSelectionMode: exitSelectionMode, + remove: remove, + ) + : null, + colorScheme: colorScheme, + ), + ); + } return Wrap( spacing: spacing, diff --git a/lib/screens/queue_tab_batch_actions.dart b/lib/screens/queue_tab_batch_actions.dart index fd8494dc..b6b1926d 100644 --- a/lib/screens/queue_tab_batch_actions.dart +++ b/lib/screens/queue_tab_batch_actions.dart @@ -119,11 +119,15 @@ extension _QueueTabBatchActions on _QueueTabState { ); } - Future _runBatchReplayGain(List allItems) { + Future _runBatchReplayGain( + List allItems, { + bool remove = false, + }) { return runBatchReplayGain( context, _selectedItemsFromAll(allItems), onExitSelectionMode: _exitSelectionMode, + remove: remove, onConfirmOpen: () { _suppressSelectionOverlay = true; _hideSelectionOverlay(); diff --git a/lib/screens/queue_tab_item_widgets.dart b/lib/screens/queue_tab_item_widgets.dart index 450f044e..8e123ed8 100644 --- a/lib/screens/queue_tab_item_widgets.dart +++ b/lib/screens/queue_tab_item_widgets.dart @@ -75,16 +75,22 @@ extension _QueueTabItemWidgets on _QueueTabState { ), ); - actions.add( - SelectionActionButton( - icon: Icons.graphic_eq, - label: context.l10n.selectionReplayGainCount(selectedCount), - onPressed: selectedCount > 0 - ? () => _runBatchReplayGain(unifiedItems) - : null, - colorScheme: colorScheme, - ), - ); + for (final remove in [false, true]) { + actions.add( + SelectionActionButton( + icon: remove ? Icons.remove_circle_outline : Icons.graphic_eq, + label: remove + ? context.l10n.selectionRemoveReplayGainCount( + selectedCount, + ) + : context.l10n.selectionReplayGainCount(selectedCount), + onPressed: selectedCount > 0 + ? () => _runBatchReplayGain(unifiedItems, remove: remove) + : null, + colorScheme: colorScheme, + ), + ); + } return Wrap( spacing: spacing, diff --git a/lib/screens/track_metadata_convert.dart b/lib/screens/track_metadata_convert.dart index 534d96ff..bb1ca8b0 100644 --- a/lib/screens/track_metadata_convert.dart +++ b/lib/screens/track_metadata_convert.dart @@ -191,7 +191,32 @@ extension _TrackMetadataConvertAndCueSplit on _TrackMetadataScreenState { return normalized; } - Future _rescanReplayGain() async { + Future _removeReplayGain() async { + if (!_fileExists) return; + final sourcePath = cleanFilePath; + final confirmed = await showAppDialog( + context: context, + builder: (ctx) => AppAlertDialog( + title: Text(ctx.l10n.trackRemoveReplayGain), + content: Text(ctx.l10n.replayGainRemoveConfirmMessage(1)), + actions: [ + AppDialogAction( + onPressed: () => Navigator.pop(ctx, false), + child: Text(ctx.l10n.dialogCancel), + ), + AppDialogAction( + filled: true, + onPressed: () => Navigator.pop(ctx, true), + child: Text(ctx.l10n.trackRemoveReplayGain), + ), + ], + ), + ); + if (confirmed != true || !mounted || sourcePath != cleanFilePath) return; + await _updateReplayGain(remove: true); + } + + Future _updateReplayGain({bool remove = false}) async { if (!_fileExists) return; final sourcePath = cleanFilePath; final generation = _metadataLoadGeneration; @@ -199,15 +224,21 @@ extension _TrackMetadataConvertAndCueSplit on _TrackMetadataScreenState { messenger.clearSnackBars(); messenger.showSnackBar( SnackBar( - content: Text(context.l10n.trackReplayGainScanning), + content: Text( + remove + ? context.l10n.replayGainRemoving + : context.l10n.trackReplayGainScanning, + ), duration: const Duration(seconds: 30), ), ); bool ok = false; try { - ok = await ReplayGainService.applyToFile(sourcePath); + ok = remove + ? await ReplayGainService.removeFromFile(sourcePath) + : await ReplayGainService.applyToFile(sourcePath); } catch (e) { - _log.w('ReplayGain rescan failed: $e'); + _log.w('ReplayGain update failed: $e'); } if (!mounted) return; if (ok && @@ -221,9 +252,13 @@ extension _TrackMetadataConvertAndCueSplit on _TrackMetadataScreenState { messenger.showSnackBar( SnackBar( content: Text( - ok - ? context.l10n.trackReplayGainSuccess - : context.l10n.trackReplayGainFailed, + remove + ? (ok + ? context.l10n.trackRemoveReplayGainSuccess + : context.l10n.trackRemoveReplayGainFailed) + : (ok + ? context.l10n.trackReplayGainSuccess + : context.l10n.trackReplayGainFailed), ), ), ); diff --git a/lib/screens/track_metadata_screen_menu.dart b/lib/screens/track_metadata_screen_menu.dart index 14abb66e..3ea3960d 100644 --- a/lib/screens/track_metadata_screen_menu.dart +++ b/lib/screens/track_metadata_screen_menu.dart @@ -72,7 +72,13 @@ extension _TrackMetadataMenu on _TrackMetadataScreenState { _MetadataOption( icon: Icons.graphic_eq, label: l10n.trackReplayGain, - onTap: () => _rescanReplayGain(), + onTap: () => _updateReplayGain(), + ), + if (_fileExists && !_isCueFile) + _MetadataOption( + icon: Icons.remove_circle_outline, + label: l10n.trackRemoveReplayGain, + onTap: () => _removeReplayGain(), ), if (_fileExists && _isCueFile) _MetadataOption( diff --git a/lib/services/batch_track_actions.dart b/lib/services/batch_track_actions.dart index 8b626d26..04ae3292 100644 --- a/lib/services/batch_track_actions.dart +++ b/lib/services/batch_track_actions.dart @@ -536,7 +536,7 @@ Future _performBatchConversion( } } -/// Batch-scans loudness and writes ReplayGain tags to [selectedItems]. +/// Adds or removes ReplayGain tags for [selectedItems]. /// /// [onConfirmOpen] / [onConfirmClosed] let the caller hide and restore any /// selection UI around the confirmation dialog; [onConfirmClosed] receives @@ -545,6 +545,7 @@ Future runBatchReplayGain( BuildContext context, List selectedItems, { required VoidCallback onExitSelectionMode, + bool remove = false, VoidCallback? onConfirmOpen, Future Function(bool confirmed)? onConfirmClosed, }) async { @@ -555,9 +556,15 @@ Future runBatchReplayGain( final confirmed = await showAppDialog( context: context, builder: (ctx) => AppAlertDialog( - title: Text(ctx.l10n.replayGainBatchConfirmTitle), + title: Text( + remove + ? ctx.l10n.trackRemoveReplayGain + : ctx.l10n.replayGainBatchConfirmTitle, + ), content: Text( - ctx.l10n.replayGainBatchConfirmMessage(selectedItems.length), + remove + ? ctx.l10n.replayGainRemoveConfirmMessage(selectedItems.length) + : ctx.l10n.replayGainBatchConfirmMessage(selectedItems.length), ), actions: [ AppDialogAction( @@ -568,7 +575,11 @@ Future runBatchReplayGain( filled: true, isDefault: true, onPressed: () => Navigator.pop(ctx, true), - child: Text(ctx.l10n.replayGainBatchConfirmTitle), + child: Text( + remove + ? ctx.l10n.trackRemoveReplayGain + : ctx.l10n.replayGainBatchConfirmTitle, + ), ), ], ), @@ -585,7 +596,9 @@ Future runBatchReplayGain( BatchProgressDialog.show( context: context, - title: context.l10n.replayGainBatchAnalyzing, + title: remove + ? context.l10n.replayGainRemoving + : context.l10n.replayGainBatchAnalyzing, total: total, icon: Icons.graphic_eq, onCancel: () { @@ -599,10 +612,12 @@ Future runBatchReplayGain( final item = selectedItems[i]; BatchProgressDialog.update(current: i + 1, detail: item.trackName); try { - final ok = await ReplayGainService.applyToFile( - item.filePath, - onUnsupportedDecoder: () => unsupportedDecoder = true, - ); + final ok = remove + ? await ReplayGainService.removeFromFile(item.filePath) + : await ReplayGainService.applyToFile( + item.filePath, + onUnsupportedDecoder: () => unsupportedDecoder = true, + ); if (ok) successCount++; } catch (_) {} } @@ -618,7 +633,9 @@ Future runBatchReplayGain( SnackBar( content: Text( [ - context.l10n.replayGainBatchSuccess(successCount, total), + remove + ? context.l10n.replayGainRemoveBatchSuccess(successCount, total) + : context.l10n.replayGainBatchSuccess(successCount, total), if (unsupportedDecoder) context.l10n.replayGainUnsupportedDecoder, ].join('\n'), ), diff --git a/lib/services/replaygain_service.dart b/lib/services/replaygain_service.dart index 9ed89c9c..0b72b66e 100644 --- a/lib/services/replaygain_service.dart +++ b/lib/services/replaygain_service.dart @@ -7,7 +7,7 @@ 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. +/// ReplayGain scanning and tag removal for existing audio files. /// /// Computes EBU R128 loudness via FFmpeg and writes gain tags using the native /// metadata editors where supported. Opus uses R128_* rather than legacy tags. @@ -104,6 +104,33 @@ class ReplayGainService { (path) => _writeLocalTags(path, gain, peak, album: true), ); + /// Removes track/album tags, including Opus R128 and M4A Sound Check tags. + /// Native editors copy the audio payload unchanged and publish atomically. + static Future removeFromFile(String filePath) => + _updateFile(filePath, (path) async { + if (!_isNativeWritableFormat(path)) return false; + const fields = { + 'replaygain_track_gain': '', + 'replaygain_track_peak': '', + 'replaygain_album_gain': '', + 'replaygain_album_peak': '', + }; + final result = await PlatformBridge.editFileMetadata(path, fields); + final method = result['method']; + if (result['success'] != true || + result['error'] != null || + method is! String || + !(method == 'native' || method.startsWith('native_'))) { + return false; + } + final metadata = await PlatformBridge.readFileMetadata(path); + return metadata['error'] == null && + metadata['audio_codec'] != null && + fields.keys.every( + (key) => (metadata[key]?.toString() ?? '').trim().isEmpty, + ); + }); + static Future _writeLocalTags( String path, String gain, @@ -182,7 +209,7 @@ class ReplayGainService { if (isSaf) { safTempPath = await PlatformBridge.copyContentUriToTemp(filePath); if (safTempPath == null || safTempPath.isEmpty) { - _log.w('Failed to copy SAF file to temp for ReplayGain scan'); + _log.w('Failed to copy SAF file to temp for ReplayGain update'); return false; } workingPath = safTempPath; @@ -199,10 +226,10 @@ class ReplayGainService { } refreshPlaybackNormalization(filePath); - _log.i('ReplayGain tags written and verified: $filePath'); + _log.i('ReplayGain tags updated and verified: $filePath'); return true; } catch (e) { - _log.e('Failed to apply ReplayGain', e); + _log.e('Failed to update ReplayGain', e); return false; } finally { if (safTempPath != null) { diff --git a/rust_backend/crates/core/src/tags/write.rs b/rust_backend/crates/core/src/tags/write.rs index c1ecee78..f3b45126 100644 --- a/rust_backend/crates/core/src/tags/write.rs +++ b/rust_backend/crates/core/src/tags/write.rs @@ -19,6 +19,21 @@ use std::sync::LazyLock; type Fields = BTreeMap; const MAX_TAG_BYTES: usize = 64 * 1024 * 1024; +fn clears_replay_gain(fields: &Fields) -> bool { + [ + "replaygain_track_gain", + "replaygain_track_peak", + "replaygain_album_gain", + "replaygain_album_peak", + ] + .iter() + .all(|key| { + fields + .get(*key) + .is_some_and(|value| value.trim().is_empty()) + }) +} + struct Section { start: u64, end: u64, diff --git a/rust_backend/crates/core/src/tags/write/ape.rs b/rust_backend/crates/core/src/tags/write/ape.rs index 44fd1db1..4ffa714a 100644 --- a/rust_backend/crates/core/src/tags/write/ape.rs +++ b/rust_backend/crates/core/src/tags/write/ape.rs @@ -16,7 +16,7 @@ pub(super) fn edit( cover: Option<&[u8]>, check: &dyn Fn() -> Result<(), String>, ) -> Result { - let end = seek(source, SeekFrom::End(0))?; + let mut end = seek(source, SeekFrom::End(0))?; let mut start = end; let mut existing = None; for offset in [end.checked_sub(32), end.checked_sub(161).map(|_| end - 160)] @@ -32,6 +32,11 @@ pub(super) fn edit( start = (offset + 32) .checked_sub(size) .ok_or("invalid APE tag size")?; + if fields.len() == 4 && super::clears_replay_gain(fields) { + // A legacy ID3v1 tag can follow the APE footer. Removing gain + // must leave that unrelated metadata in place too. + end = offset + 32; + } } if existing.is_none() && let Ok(begin) = footer.items_start(offset) @@ -140,6 +145,13 @@ pub(super) fn edit( items.retain(|item| !remove.contains(&uppercase(&String::from_utf8_lossy(&item.key)))); items.extend(added); if items.is_empty() { + if super::clears_replay_gain(fields) { + return Ok(Section { + start, + end, + data: Vec::new(), + }); + } return Err("empty APE tag".into()); } let mut body = Vec::new(); diff --git a/rust_backend/crates/core/src/tags/write/mp4.rs b/rust_backend/crates/core/src/tags/write/mp4.rs index eab3781b..8b48dee2 100644 --- a/rust_backend/crates/core/src/tags/write/mp4.rs +++ b/rust_backend/crates/core/src/tags/write/mp4.rs @@ -82,8 +82,8 @@ pub(super) fn edit( } let replay_gain = replay_gain(fields); if !replay_gain.is_empty() { - // Go replaces the entire ReplayGain group when any supplied value is - // nonempty. A request containing only empty values leaves it alone. + // Replace the entire group, or explicitly clear it when all four + // ReplayGain fields are supplied empty (including derived Sound Check). remove_names.extend( [ "REPLAYGAIN_TRACK_GAIN", @@ -95,7 +95,9 @@ pub(super) fn edit( .map(str::to_owned), ); for (name, value) in replay_gain { - appended.extend(freeform(&name, &value)); + if !value.is_empty() { + appended.extend(freeform(&name, &value)); + } } } let edit_track = fields.contains_key("track_number") || fields.contains_key("track_total"); @@ -499,8 +501,12 @@ fn replay_gain(fields: &Fields) -> Fields { .get(name) .map(|v| (name.to_owned(), v.trim().to_owned())) }) - .filter(|(_, value)| !value.is_empty()) .collect(); + // Preserve partial empty updates as no-ops; clearing the whole group is an + // explicit request so ordinary metadata edits cannot discard normalization. + if !super::clears_replay_gain(fields) { + result.retain(|_, value| !value.is_empty()); + } static NUMBER: LazyLock = LazyLock::new(|| Regex::new(r"[+-]?[0-9]+(?:\.[0-9]+)?").unwrap()); let gain = result diff --git a/rust_backend/crates/core/src/tags/write/riff.rs b/rust_backend/crates/core/src/tags/write/riff.rs index 35cbb5b3..e52f62db 100644 --- a/rust_backend/crates/core/src/tags/write/riff.rs +++ b/rust_backend/crates/core/src/tags/write/riff.rs @@ -1,6 +1,6 @@ use super::{Fields, Section, bytes, id3, metadata_fields, seek}; use crate::tags::{file::ObservedReader, read_audio_tags}; -use std::io::{Read, Seek, SeekFrom}; +use std::io::{Cursor, Read, Seek, SeekFrom}; pub(super) fn edit( source: &mut (impl Read + Seek), @@ -30,6 +30,7 @@ pub(super) fn edit( let mut start = 12_u64; let mut body_size = 4_u64; let mut embedded_cover = None; + let remove_gain_only = fields.len() == 4 && super::clears_replay_gain(fields); for _ in 0..65536 { check()?; if start + 8 > length { @@ -44,6 +45,38 @@ pub(super) fn edit( }; let end = start + 8 + u64::from(size) + u64::from(size & 1); if header[..4].eq_ignore_ascii_case(b"id3 ") { + if remove_gain_only { + // Edit the existing ID3 frames so unknown tags and all artwork + // survive removal, instead of rebuilding from parsed metadata. + if size as usize > super::MAX_TAG_BYTES || end > length { + return Err("invalid RIFF ID3 chunk size".into()); + } + let (tag, _) = id3::header( + &mut Cursor::new(bytes(source, size as usize)?), + fields, + None, + check, + )?; + let tag_size = tag.len() as u32; + let mut chunk = header[..4].to_vec(); + chunk.extend(if aiff { + tag_size.to_be_bytes() + } else { + tag_size.to_le_bytes() + }); + chunk.extend(tag); + if tag_size & 1 == 1 { + chunk.push(0); + } + body_size += chunk.len() as u64; + sections.push(Section { + start, + end, + data: chunk, + }); + start = end; + continue; + } if cover.is_none() && size > 0 && size <= 16 * 1024 * 1024 @@ -68,6 +101,20 @@ pub(super) fn edit( if start + 8 <= length { return Err("RIFF chunk count exceeds 65536".into()); } + if remove_gain_only { + let size = u32::try_from(body_size).map_err(|_| "RIFF container exceeds 32-bit size")?; + sections.push(Section { + start: 4, + end: 8, + data: if aiff { + size.to_be_bytes() + } else { + size.to_le_bytes() + } + .to_vec(), + }); + return Ok(sections); + } let cover_path = fields.get("cover_path").map(|p| p.trim()).unwrap_or(""); if !cover_path.is_empty() && cover.is_none() { return Err("read cover art: file not found".into()); diff --git a/rust_backend/crates/core/tests/replaygain_removal.rs b/rust_backend/crates/core/tests/replaygain_removal.rs new file mode 100644 index 00000000..b3ae1d18 --- /dev/null +++ b/rust_backend/crates/core/tests/replaygain_removal.rs @@ -0,0 +1,298 @@ +use spotiflac_core::tags::{ + extract_cover, read_audio_tags, rewrite_audio_tags, rewrite_m4a_freeform, +}; +use std::collections::BTreeMap; +use std::io::Cursor; + +const PAYLOAD: &[u8] = b"\xff\xf8\x12\x34unchanged audio payload"; +const COVER: &[u8] = b"\xff\xd8\xff\xd9"; +const GAIN_FIELDS: [&str; 4] = [ + "replaygain_track_gain", + "replaygain_track_peak", + "replaygain_album_gain", + "replaygain_album_peak", +]; + +fn atom(kind: &[u8; 4], body: &[u8]) -> Vec { + [ + ((body.len() + 8) as u32).to_be_bytes().as_slice(), + kind, + body, + ] + .concat() +} + +fn ogg_page(packet: &[u8], sequence: u32, flags: u8) -> Vec { + let mut header = vec![0; 27]; + header[..4].copy_from_slice(b"OggS"); + header[5] = flags; + header[14..18].copy_from_slice(&1_u32.to_le_bytes()); + header[18..22].copy_from_slice(&sequence.to_le_bytes()); + header[26] = 1; + header.push(packet.len() as u8); + header.extend(packet); + header +} + +fn source(format: &str) -> Vec { + match format { + "flac" => [b"fLaC\x80\0\0\x22".as_slice(), &[0; 34], PAYLOAD].concat(), + "mp3" | "ape" => PAYLOAD.to_vec(), + "m4a" => [ + atom(b"ftyp", b"M4A \0\0\0\0"), + atom(b"moov", &[]), + atom(b"mdat", PAYLOAD), + ] + .concat(), + "opus" => [ + ogg_page(b"OpusHead\x01\x02\0\0\x80\xbb\0\0\0\0\0", 0, 2), + ogg_page(b"OpusTags\0\0\0\0\0\0\0\0", 1, 0), + ogg_page(PAYLOAD, 2, 4), + ] + .concat(), + "wav" | "aiff" => { + let aiff = format == "aiff"; + let mut body = if aiff { b"AIFFSSND" } else { b"WAVEdata" }.to_vec(); + let length = PAYLOAD.len() as u32; + body.extend(if aiff { + length.to_be_bytes() + } else { + length.to_le_bytes() + }); + body.extend(PAYLOAD); + if length % 2 == 1 { + body.push(0); + } + let length = body.len() as u32; + [ + if aiff { b"FORM" } else { b"RIFF" }.as_slice(), + &if aiff { + length.to_be_bytes() + } else { + length.to_le_bytes() + }, + &body, + ] + .concat() + } + _ => unreachable!(), + } +} + +#[test] +fn removal_preserves_audio_artwork_and_other_metadata_in_native_containers() { + for format in ["flac", "mp3", "m4a", "opus", "ape", "wav", "aiff"] { + let fields = BTreeMap::from([ + ("title".into(), "Preserved title".into()), + ("artist".into(), "Preserved artist".into()), + ("lyrics".into(), "Preserved lyrics".into()), + ("cover_path".into(), "cover.jpg".into()), + (GAIN_FIELDS[0].into(), "-6.00 dB".into()), + (GAIN_FIELDS[1].into(), "0.950000".into()), + (GAIN_FIELDS[2].into(), "-4.00 dB".into()), + (GAIN_FIELDS[3].into(), "0.980000".into()), + ]); + let empty = GAIN_FIELDS.map(|key| (key.into(), String::new())).into(); + let mut tagged = Vec::new(); + rewrite_audio_tags( + &mut Cursor::new(source(format)), + &mut tagged, + format, + &fields, + Some(COVER), + &|| Ok(()), + ) + .unwrap(); + let before = read_audio_tags(&mut Cursor::new(&tagged), format, &|| Ok(())).unwrap(); + assert!(!before.replay_gain_track_gain.is_empty(), "{format}"); + assert!(!before.replay_gain_album_gain.is_empty(), "{format}"); + + for freeform in [false, true] { + if freeform && format != "m4a" { + continue; + } + let mut removed = Vec::new(); + if freeform { + assert!( + rewrite_m4a_freeform( + &mut Cursor::new(&tagged), + &mut removed, + &empty, + true, + &|| Ok(()), + ) + .unwrap() + ); + } else { + rewrite_audio_tags( + &mut Cursor::new(&tagged), + &mut removed, + format, + &empty, + None, + &|| Ok(()), + ) + .unwrap(); + } + let after = read_audio_tags(&mut Cursor::new(&removed), format, &|| Ok(())).unwrap(); + assert!(after.replay_gain_track_gain.is_empty(), "{format}"); + assert!(after.replay_gain_track_peak.is_empty(), "{format}"); + assert!(after.replay_gain_album_gain.is_empty(), "{format}"); + assert!(after.replay_gain_album_peak.is_empty(), "{format}"); + let mut expected = before.clone(); + expected.replay_gain_track_gain.clear(); + expected.replay_gain_track_peak.clear(); + expected.replay_gain_album_gain.clear(); + expected.replay_gain_album_peak.clear(); + assert_eq!(after, expected, "{format}"); + if matches!(format, "flac" | "mp3" | "m4a" | "opus") { + assert_eq!( + extract_cover(&mut Cursor::new(&removed), format, &|| Ok(())) + .unwrap() + .data, + COVER, + "{format}" + ); + } else { + assert!( + removed.windows(COVER.len()).any(|part| part == COVER), + "{format}" + ); + } + assert_eq!( + removed + .windows(PAYLOAD.len()) + .filter(|part| *part == PAYLOAD) + .count(), + 1, + "{format}" + ); + for tag in [ + "REPLAYGAIN_", + "R128_TRACK_GAIN", + "R128_ALBUM_GAIN", + "ITUNNORM", + ] { + assert!( + !String::from_utf8_lossy(&removed) + .to_uppercase() + .contains(tag), + "{format}: {tag}" + ); + } + } + } +} + +#[test] +fn partial_empty_m4a_updates_do_not_remove_existing_gain() { + let fields = BTreeMap::from([(GAIN_FIELDS[0].into(), "-6.00 dB".into())]); + let mut tagged = Vec::new(); + rewrite_audio_tags( + &mut Cursor::new(source("m4a")), + &mut tagged, + "m4a", + &fields, + None, + &|| Ok(()), + ) + .unwrap(); + let mut output = Vec::new(); + let empty = BTreeMap::from([(GAIN_FIELDS[0].into(), String::new())]); + assert!( + !rewrite_m4a_freeform( + &mut Cursor::new(&tagged), + &mut output, + &empty, + true, + &|| Ok(()) + ) + .unwrap() + ); + assert!(output.is_empty()); +} + +#[test] +fn removal_can_clear_the_last_ape_tags() { + let fields = BTreeMap::from([(GAIN_FIELDS[0].into(), "-6.00 dB".into())]); + let mut tagged = Vec::new(); + rewrite_audio_tags( + &mut Cursor::new(PAYLOAD), + &mut tagged, + "ape", + &fields, + None, + &|| Ok(()), + ) + .unwrap(); + let empty = GAIN_FIELDS.map(|key| (key.into(), String::new())).into(); + let mut id3v1 = b"TAGPreserved legacy title".to_vec(); + id3v1.resize(128, 0); + for suffix in [b"".as_slice(), id3v1.as_slice()] { + let input = [tagged.as_slice(), suffix].concat(); + let mut removed = Vec::new(); + rewrite_audio_tags( + &mut Cursor::new(&input), + &mut removed, + "ape", + &empty, + None, + &|| Ok(()), + ) + .unwrap(); + assert_eq!(removed, [PAYLOAD, suffix].concat()); + } +} + +#[test] +fn riff_removal_preserves_unrecognized_id3_frames() { + // A private ID3 frame that is deliberately absent from AudioMetadata. + let private = b"private-owner\0preserved bytes"; + let mut frame = b"PRIV\0\0\0".to_vec(); + frame.push(private.len() as u8); + frame.extend([0, 0]); + frame.extend(private); + let mut tag = b"ID3\x04\0\0\0\0\0".to_vec(); + tag.push(frame.len() as u8); + tag.extend(frame); + for format in ["wav", "aiff"] { + let aiff = format == "aiff"; + let mut input = source(format); + input.extend(b"ID3 "); + let size = tag.len() as u32; + input.extend(if aiff { + size.to_be_bytes() + } else { + size.to_le_bytes() + }); + input.extend(&tag); + if size & 1 == 1 { + input.push(0); + } + let size = (input.len() - 8) as u32; + input[4..8].copy_from_slice(&if aiff { + size.to_be_bytes() + } else { + size.to_le_bytes() + }); + let empty = GAIN_FIELDS.map(|key| (key.into(), String::new())).into(); + let mut removed = Vec::new(); + rewrite_audio_tags( + &mut Cursor::new(&input), + &mut removed, + format, + &empty, + None, + &|| Ok(()), + ) + .unwrap(); + assert!( + removed.windows(private.len()).any(|part| part == private), + "{format}" + ); + assert!( + removed.windows(PAYLOAD.len()).any(|part| part == PAYLOAD), + "{format}" + ); + } +} diff --git a/rust_backend/crates/extensions/src/backend/tags.rs b/rust_backend/crates/extensions/src/backend/tags.rs index 3ff7c198..4a261c30 100644 --- a/rust_backend/crates/extensions/src/backend/tags.rs +++ b/rust_backend/crates/extensions/src/backend/tags.rs @@ -116,20 +116,17 @@ impl Backend { }); check()?; let mut replay_gain = false; - let only_replay_gain = fields - .iter() - .filter(|(_, value)| !value.trim().is_empty()) - .all(|(key, _)| { - let allowed = matches!( - key.trim().to_lowercase().as_str(), - "replaygain_track_gain" - | "replaygain_track_peak" - | "replaygain_album_gain" - | "replaygain_album_peak" - ); - replay_gain |= allowed; - allowed - }); + let only_replay_gain = fields.iter().all(|(key, _)| { + let allowed = matches!( + key.trim().to_lowercase().as_str(), + "replaygain_track_gain" + | "replaygain_track_peak" + | "replaygain_album_gain" + | "replaygain_album_peak" + ); + replay_gain |= allowed; + allowed + }); let success = |method: &str| json!({"success":true,"method":method}); if only_replay_gain && replay_gain && (m4a || mp4) { self.edit_m4a_freeform(path, &fields, true, &check) @@ -251,7 +248,7 @@ impl Backend { "replaygain_album_peak", ] .iter() - .any(|key| fields.get(*key).is_some_and(|v| !v.trim().is_empty())) + .any(|key| fields.contains_key(*key)) } else { fields.contains_key("isrc") || fields.contains_key("label") }; diff --git a/test/album_replaygain_action_test.dart b/test/album_replaygain_action_test.dart index 8311edec..58e017c4 100644 --- a/test/album_replaygain_action_test.dart +++ b/test/album_replaygain_action_test.dart @@ -21,102 +21,124 @@ void main() { for (final mornye in [false, true]) { for (final downloaded in [false, true]) { - testWidgets( - 'ReplayGain survives hiding the album selection (Mornye: $mornye, downloaded: $downloaded)', - (tester) async { - SharedPreferences.setMockInitialValues({}); - var attempts = 0; - messenger.setMockMethodCallHandler(channel, (call) async { - if (call.method == 'safCopyToTemp') attempts++; - // Simulate an inaccessible file. Reaching this call and showing - // the result proves confirmation actually starts the operation. - return null; - }); - addTearDown(() => messenger.setMockMethodCallHandler(channel, null)); - const path = 'content://library/document/track.flac'; - final item = DownloadHistoryItem( - id: 'track', - trackName: 'Track', - artistName: 'Artist', - albumName: 'Album', - filePath: path, - service: 'provider-a', - downloadedAt: DateTime(2026), - ); - await tester.pumpWidget( - ProviderScope( - overrides: [ - downloadedAlbumTracksProvider( - const DownloadedAlbumTracksRequest( - albumName: 'Album', - artistName: 'Artist', + for (final remove in [false, true]) { + testWidgets( + 'ReplayGain survives hiding the album selection (Mornye: $mornye, downloaded: $downloaded, remove: $remove)', + (tester) async { + SharedPreferences.setMockInitialValues({}); + var attempts = 0; + messenger.setMockMethodCallHandler(channel, (call) async { + if (call.method == 'safCopyToTemp') attempts++; + // Simulate an inaccessible file. Reaching this call and showing + // the result proves confirmation actually starts the operation. + return null; + }); + addTearDown( + () => messenger.setMockMethodCallHandler(channel, null), + ); + const path = 'content://library/document/track.flac'; + final item = DownloadHistoryItem( + id: 'track', + trackName: 'Track', + artistName: 'Artist', + albumName: 'Album', + filePath: path, + service: 'provider-a', + downloadedAt: DateTime(2026), + ); + await tester.pumpWidget( + ProviderScope( + overrides: [ + downloadedAlbumTracksProvider( + const DownloadedAlbumTracksRequest( + albumName: 'Album', + artistName: 'Artist', + ), + ).overrideWith((ref) async => [item]), + ], + child: MaterialApp( + theme: mornye + ? MornyeTheme.build(Brightness.light) + : AppTheme.light(), + localizationsDelegates: + AppLocalizations.localizationsDelegates, + supportedLocales: AppLocalizations.supportedLocales, + home: SelectionOverlayHost( + child: downloaded + ? const DownloadedAlbumScreen( + albumName: 'Album', + artistName: 'Artist', + ) + : LocalAlbumScreen( + albumName: 'Album', + artistName: 'Artist', + tracks: [ + LocalLibraryItem( + id: 'track', + trackName: 'Track', + artistName: 'Artist', + albumName: 'Album', + filePath: path, + scannedAt: DateTime(2026), + ), + ], + ), ), - ).overrideWith((ref) async => [item]), - ], - child: MaterialApp( - theme: mornye - ? MornyeTheme.build(Brightness.light) - : AppTheme.light(), - localizationsDelegates: AppLocalizations.localizationsDelegates, - supportedLocales: AppLocalizations.supportedLocales, - home: SelectionOverlayHost( - child: downloaded - ? const DownloadedAlbumScreen( - albumName: 'Album', - artistName: 'Artist', - ) - : LocalAlbumScreen( - albumName: 'Album', - artistName: 'Artist', - tracks: [ - LocalLibraryItem( - id: 'track', - trackName: 'Track', - artistName: 'Artist', - albumName: 'Album', - filePath: path, - scannedAt: DateTime(2026), - ), - ], - ), ), ), - ), - ); - await tester.pumpAndSettle(); - await tester.scrollUntilVisible(find.text('Track'), 200); - await tester.longPress(find.text('Track')); - await tester.pumpAndSettle(); - final l10n = AppLocalizations.of( - tester.element(find.byType(SelectionBottomBar)), - ); - Future openConfirmation() async { - await tester.tap(find.text(l10n.selectionReplayGainCount(1))); + ); await tester.pumpAndSettle(); + await tester.scrollUntilVisible(find.text('Track'), 200); + await tester.longPress(find.text('Track')); + await tester.pumpAndSettle(); + final l10n = AppLocalizations.of( + tester.element(find.byType(SelectionBottomBar)), + ); + Future openConfirmation() async { + final action = find.text( + remove + ? l10n.selectionRemoveReplayGainCount(1) + : l10n.selectionReplayGainCount(1), + ); + await tester.ensureVisible(action); + await tester.tap(action); + await tester.pumpAndSettle(); + expect(find.byType(SelectionBottomBar), findsNothing); + expect(find.byType(AppAlertDialog), findsOneWidget); + } + + await openConfirmation(); + await tester.tap(find.text(l10n.dialogCancel)); + await tester.pumpAndSettle(); + expect(attempts, 0); + expect(find.byType(SelectionBottomBar), findsOneWidget); + + await openConfirmation(); + await tester.tap( + find.descendant( + of: find.byType(AppDialogAction), + matching: find.text( + remove + ? l10n.trackRemoveReplayGain + : l10n.replayGainBatchConfirmTitle, + ), + ), + ); + await tester.pumpAndSettle(); + expect(attempts, 1); + expect( + find.text( + remove + ? l10n.replayGainRemoveBatchSuccess(0, 1) + : l10n.replayGainBatchSuccess(0, 1), + ), + findsOneWidget, + ); expect(find.byType(SelectionBottomBar), findsNothing); - expect(find.byType(AppAlertDialog), findsOneWidget); - } - - await openConfirmation(); - await tester.tap(find.text(l10n.dialogCancel)); - await tester.pumpAndSettle(); - expect(attempts, 0); - expect(find.byType(SelectionBottomBar), findsOneWidget); - - await openConfirmation(); - await tester.tap( - find.descendant( - of: find.byType(AppDialogAction), - matching: find.text(l10n.replayGainBatchConfirmTitle), - ), - ); - await tester.pumpAndSettle(); - expect(attempts, 1); - expect(find.text(l10n.replayGainBatchSuccess(0, 1)), findsOneWidget); - expect(find.byType(SelectionBottomBar), findsNothing); - expect(tester.takeException(), isNull); - }, - ); + expect(tester.takeException(), isNull); + }, + ); + } } } } diff --git a/test/replaygain_service_test.dart b/test/replaygain_service_test.dart index 78a855c4..db50a9fe 100644 --- a/test/replaygain_service_test.dart +++ b/test/replaygain_service_test.dart @@ -194,4 +194,78 @@ void main() { expect(metadata['replaygain_track_peak'], '1.258925'); expect(calls, ['editFileMetadata', 'readFileMetadata']); }); + + test('removal clears both scopes and verifies before saving SAF', () async { + metadata['title'] = 'Preserved title'; + expect( + await ReplayGainService.removeFromFile('content://music/document/42'), + isTrue, + ); + expect(editedFields, { + 'replaygain_track_gain': '', + 'replaygain_track_peak': '', + 'replaygain_album_gain': '', + 'replaygain_album_peak': '', + }); + expect(metadata['title'], 'Preserved title'); + expect(calls, [ + 'safCopyToTemp', + 'editFileMetadata', + 'readFileMetadata', + 'writeTempToSaf', + ]); + expect(await File(tempPath).exists(), isFalse); + }); + + test('removal refuses to save when the gain tags remain', () async { + applyEdits = false; + expect( + await ReplayGainService.removeFromFile('content://music/document/42'), + isFalse, + ); + expect(calls, ['safCopyToTemp', 'editFileMetadata', 'readFileMetadata']); + expect(await File(tempPath).exists(), isFalse); + }); + + test('removal does not treat a fallback instruction as success', () async { + method = 'ffmpeg'; + expect( + await ReplayGainService.removeFromFile('content://music/document/42'), + isFalse, + ); + expect(calls, ['safCopyToTemp', 'editFileMetadata']); + expect(await File(tempPath).exists(), isFalse); + }); + + test( + 'removal reports SAF save failure and cleans its temporary copy', + () async { + saveSaf = false; + expect( + await ReplayGainService.removeFromFile('content://music/document/42'), + isFalse, + ); + expect(calls.last, 'writeTempToSaf'); + expect(await File(tempPath).exists(), isFalse); + }, + ); + + test('removal rejects an empty metadata response', () async { + metadata = {}; + expect( + await ReplayGainService.removeFromFile('content://music/document/42'), + isFalse, + ); + expect(calls, ['safCopyToTemp', 'editFileMetadata', 'readFileMetadata']); + }); + + test('removal is idempotent for an untagged local file', () async { + method = 'native'; + metadata = {'audio_codec': 'flac'}; + expect( + await ReplayGainService.removeFromFile('${directory.path}/song.flac'), + isTrue, + ); + expect(calls, ['editFileMetadata', 'readFileMetadata']); + }); } diff --git a/test/track_metadata_replaygain_test.dart b/test/track_metadata_replaygain_test.dart index 4a1019d4..f454ea9b 100644 --- a/test/track_metadata_replaygain_test.dart +++ b/test/track_metadata_replaygain_test.dart @@ -8,6 +8,7 @@ import 'package:shared_preferences/shared_preferences.dart'; import 'package:spotiflac_android/l10n/l10n.dart'; import 'package:spotiflac_android/providers/download_history_provider.dart'; import 'package:spotiflac_android/screens/track_metadata_screen.dart'; +import 'package:spotiflac_android/widgets/app_alert_dialog.dart'; void main() { TestWidgetsFlutterBinding.ensureInitialized(); @@ -28,10 +29,26 @@ void main() { 'replaygain_album_peak': '1.234567', } : {'replaygain_track_gain': ' ', 'replaygain_album_peak': null}; + var edits = 0; messenger.setMockMethodCallHandler(channel, (call) async { + if (call.method == 'editFileMetadata') { + edits++; + final args = call.arguments as Map; + final fields = Map.from( + jsonDecode(args['metadata_json'] as String) as Map, + ); + metadata.addAll(fields); + return jsonEncode({'success': true, 'method': 'native'}); + } return switch (call.method) { 'safStat' => jsonEncode({'exists': true, 'size': 100}), 'readAudioMetadata' => jsonEncode(metadata), + 'readFileMetadata' => jsonEncode({ + ...metadata, + 'audio_codec': 'flac', + }), + 'safCopyToTemp' => 'temporary-song.flac', + 'writeTempToSaf' => jsonEncode({'success': true}), 'getLyricsLRCWithSource' => jsonEncode({'lyrics': '', 'source': ''}), 'getSafFileModTimes' => '{}', _ => null, @@ -74,6 +91,36 @@ void main() { ]) { expect(find.text(text), hasTags ? findsOneWidget : findsNothing); } + Future openRemoval() async { + await tester.tap(find.byIcon(Icons.more_vert)); + await tester.pumpAndSettle(); + await tester.ensureVisible(find.text('Remove ReplayGain')); + await tester.tap(find.text('Remove ReplayGain')); + await tester.pumpAndSettle(); + expect(find.byType(AppAlertDialog), findsOneWidget); + } + + await openRemoval(); + await tester.tap(find.text('Cancel')); + await tester.pumpAndSettle(); + expect(edits, 0); + if (hasTags) expect(find.text('-6.20 dB'), findsOneWidget); + + await openRemoval(); + await tester.tap( + find.descendant( + of: find.byType(AppDialogAction), + matching: find.text('Remove ReplayGain'), + ), + ); + await tester.runAsync(() async { + await Future.delayed(const Duration(milliseconds: 100)); + }); + await tester.pumpAndSettle(); + expect(edits, 1); + expect(find.text('ReplayGain tags removed'), findsOneWidget); + expect(find.text('ReplayGain Track Gain'), findsNothing); + expect(find.text('ReplayGain Album Gain'), findsNothing); expect(tester.takeException(), isNull); }); }