From c104a5d8a3ab1580c96625a2e08aef04b98bb247 Mon Sep 17 00:00:00 2001 From: zarzet Date: Thu, 9 Apr 2026 16:53:08 +0700 Subject: [PATCH] fix: align metadata sanitization and lyrics editing --- .../kotlin/com/zarz/spotiflac/MainActivity.kt | 19 +- go_backend/exports.go | 1 + go_backend/filename.go | 33 +- go_backend/filename_test.go | 15 + lib/providers/download_queue_provider.dart | 32 +- lib/screens/track_metadata_screen.dart | 163 ++++++-- lib/services/ffmpeg_service.dart | 362 +++++++++++------- 7 files changed, 427 insertions(+), 198 deletions(-) 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 c089b729..43460b4e 100644 --- a/android/app/src/main/kotlin/com/zarz/spotiflac/MainActivity.kt +++ b/android/app/src/main/kotlin/com/zarz/spotiflac/MainActivity.kt @@ -308,7 +308,24 @@ class MainActivity: FlutterFragmentActivity() { } private fun sanitizeFilename(name: String): String { - return name.replace(Regex("[\\\\/:*?\"<>|]"), "_").trim() + var sanitized = name + .replace("/", " ") + .replace(Regex("[\\\\:*?\"<>|]"), " ") + .filter { ch -> + val code = ch.code + !((code < 0x20 && ch != '\t' && ch != '\n' && ch != '\r') || + code == 0x7F || + (Character.isISOControl(ch) && ch != '\t' && ch != '\n' && ch != '\r')) + } + .trim() + .trim('.', ' ') + + sanitized = sanitized + .replace(Regex("\\s+"), " ") + .replace(Regex("_+"), "_") + .trim('_', ' ') + + return if (sanitized.isBlank()) "Unknown" else sanitized } private fun sanitizeRelativeDir(relativeDir: String): String { diff --git a/go_backend/exports.go b/go_backend/exports.go index c68fb397..7a04eeb7 100644 --- a/go_backend/exports.go +++ b/go_backend/exports.go @@ -1361,6 +1361,7 @@ func EditFileMetadata(filePath, metadataJSON string) (string, error) { DiscNumber: discNum, TotalDiscs: totalDiscs, ISRC: fields["isrc"], + Lyrics: fields["lyrics"], Genre: fields["genre"], Label: fields["label"], Copyright: fields["copyright"], diff --git a/go_backend/filename.go b/go_backend/filename.go index 2d3bc497..a94b328a 100644 --- a/go_backend/filename.go +++ b/go_backend/filename.go @@ -6,6 +6,8 @@ import ( "strconv" "strings" "time" + "unicode" + "unicode/utf8" ) var ( @@ -17,19 +19,42 @@ var ( ) func sanitizeFilename(filename string) string { - sanitized := invalidChars.ReplaceAllString(filename, "_") + sanitized := strings.ReplaceAll(filename, "/", " ") + sanitized = invalidChars.ReplaceAllString(sanitized, " ") + var builder strings.Builder + for _, r := range sanitized { + if r < 0x20 && r != 0x09 && r != 0x0A && r != 0x0D { + continue + } + if r == 0x7F { + continue + } + if unicode.IsControl(r) && r != 0x09 && r != 0x0A && r != 0x0D { + continue + } + builder.WriteRune(r) + } + + sanitized = builder.String() sanitized = strings.TrimSpace(sanitized) - sanitized = strings.Trim(sanitized, ".") - + sanitized = strings.Trim(sanitized, ". ") + sanitized = strings.Join(strings.Fields(sanitized), " ") sanitized = multiUnderscore.ReplaceAllString(sanitized, "_") + sanitized = strings.Trim(sanitized, "_ ") + + if !utf8.ValidString(sanitized) { + sanitized = strings.ToValidUTF8(sanitized, "_") + } if len(sanitized) > 200 { sanitized = sanitized[:200] + sanitized = strings.TrimSpace(strings.Trim(sanitized, ". ")) + sanitized = strings.Trim(sanitized, "_ ") } if sanitized == "" { - sanitized = "untitled" + return "Unknown" } return sanitized diff --git a/go_backend/filename_test.go b/go_backend/filename_test.go index 0b7940af..b3647cc1 100644 --- a/go_backend/filename_test.go +++ b/go_backend/filename_test.go @@ -83,3 +83,18 @@ func TestBuildFilenameFromTemplate_DateStrftimeFormattingWithYearOnly(t *testing t.Fatalf("expected %q, got %q", expected, formatted) } } + +func TestSanitizeFilenameMatchesDesktopSpacingBehavior(t *testing.T) { + got := sanitizeFilename(` "Text In Quotes"?%* / Demo `) + want := "Text In Quotes % Demo" + if got != want { + t.Fatalf("expected %q, got %q", want, got) + } +} + +func TestSanitizeFilenameFallsBackToUnknownWhenEmpty(t *testing.T) { + got := sanitizeFilename(`<>:"/\|?*`) + if got != "Unknown" { + t.Fatalf("expected %q, got %q", "Unknown", got) + } +} diff --git a/lib/providers/download_queue_provider.dart b/lib/providers/download_queue_provider.dart index 73825951..70bcc315 100644 --- a/lib/providers/download_queue_provider.dart +++ b/lib/providers/download_queue_provider.dart @@ -26,7 +26,10 @@ final _log = AppLogger('DownloadQueue'); final _historyLog = AppLogger('DownloadHistory'); final _invalidFolderChars = RegExp(r'[<>:"/\\|?*]'); -final _trailingDotsRegex = RegExp(r'\.+$'); +final _trimDotsAndSpacesRegex = RegExp(r'^[. ]+|[. ]+$'); +final _trimUnderscoresAndSpacesRegex = RegExp(r'^[_ ]+|[_ ]+$'); +final _multiWhitespaceRegex = RegExp(r'\s+'); +final _multiUnderscoreRegex = RegExp(r'_+'); /// log10 helper using dart:math's natural log. double _log10(num x) => log(x) / ln10; @@ -2165,10 +2168,29 @@ class DownloadQueueNotifier extends Notifier { } String _sanitizeFolderName(String name) { - return name - .replaceAll(_invalidFolderChars, '_') - .replaceAll(_trailingDotsRegex, '') - .trim(); + final buffer = StringBuffer(); + for (final rune in name.runes) { + if (rune < 0x20 || rune == 0x7f) { + continue; + } + final char = String.fromCharCode(rune); + if (_invalidFolderChars.hasMatch(char)) { + buffer.write(' '); + continue; + } + buffer.write(char); + } + + var sanitized = buffer.toString().trim(); + sanitized = sanitized.replaceAll(_trimDotsAndSpacesRegex, ''); + sanitized = sanitized.replaceAll(_multiWhitespaceRegex, ' '); + sanitized = sanitized.replaceAll(_multiUnderscoreRegex, '_'); + sanitized = sanitized.replaceAll(_trimUnderscoresAndSpacesRegex, ''); + + if (sanitized.isEmpty) { + return 'Unknown'; + } + return sanitized; } static final _featuredArtistPattern = RegExp( diff --git a/lib/screens/track_metadata_screen.dart b/lib/screens/track_metadata_screen.dart index a009e9c3..5d94b6ed 100644 --- a/lib/screens/track_metadata_screen.dart +++ b/lib/screens/track_metadata_screen.dart @@ -4270,8 +4270,12 @@ class _TrackMetadataScreenState extends ConsumerState { 'copyright': val('copyright', copyright), 'composer': val('composer', composer), 'comment': fileMetadata?['comment']?.toString() ?? '', + 'lyrics': fileMetadata?['lyrics']?.toString() ?? '', }; + final initialDurationSeconds = + _readPositiveInt(fileMetadata?['duration']) ?? duration ?? 0; + if (!context.mounted) return; final saved = await showModalBottomSheet( @@ -4287,6 +4291,9 @@ class _TrackMetadataScreenState extends ConsumerState { initialValues: initialValues, filePath: cleanFilePath, sourceTrackId: _spotifyId, + durationMs: initialDurationSeconds > 0 + ? initialDurationSeconds * 1000 + : 0, artistTagMode: ref.read(settingsProvider).artistTagMode, ), ); @@ -4297,7 +4304,24 @@ class _TrackMetadataScreenState extends ConsumerState { ); try { final refreshed = await PlatformBridge.readFileMetadata(cleanFilePath); - setState(() => _editedMetadata = refreshed); + final refreshedLyrics = refreshed['lyrics']?.toString().trim() ?? ''; + setState(() { + _editedMetadata = refreshed; + _lyricsError = null; + _isInstrumental = false; + _embeddedLyricsChecked = true; + if (refreshedLyrics.isNotEmpty) { + _lyrics = _cleanLrcForDisplay(refreshedLyrics); + _rawLyrics = refreshedLyrics; + _lyricsSource = 'Embedded'; + _lyricsEmbedded = true; + } else { + _lyrics = null; + _rawLyrics = null; + _lyricsSource = null; + _lyricsEmbedded = false; + } + }); } catch (_) { setState(() {}); } @@ -4514,6 +4538,7 @@ class _EditMetadataSheet extends StatefulWidget { final Map initialValues; final String filePath; final String? sourceTrackId; + final int durationMs; final String artistTagMode; const _EditMetadataSheet({ @@ -4521,6 +4546,7 @@ class _EditMetadataSheet extends StatefulWidget { required this.initialValues, required this.filePath, this.sourceTrackId, + required this.durationMs, required this.artistTagMode, }); @@ -4560,6 +4586,7 @@ class _EditMetadataSheetState extends State<_EditMetadataSheet> { 'total_discs': 'total_discs', 'genre': 'genre', 'isrc': 'isrc', + 'lyrics': 'lyrics', 'label': 'label', 'copyright': 'copyright', 'composer': 'composer', @@ -4577,6 +4604,7 @@ class _EditMetadataSheetState extends State<_EditMetadataSheet> { late final TextEditingController _discTotalCtrl; late final TextEditingController _genreCtrl; late final TextEditingController _isrcCtrl; + late final TextEditingController _lyricsCtrl; late final TextEditingController _labelCtrl; late final TextEditingController _copyrightCtrl; late final TextEditingController _composerCtrl; @@ -4772,6 +4800,8 @@ class _EditMetadataSheetState extends State<_EditMetadataSheet> { return l10n.editMetadataFieldGenre; case 'isrc': return l10n.editMetadataFieldIsrc; + case 'lyrics': + return l10n.trackLyrics; case 'label': return l10n.editMetadataFieldLabel; case 'copyright': @@ -4809,6 +4839,8 @@ class _EditMetadataSheetState extends State<_EditMetadataSheet> { return _genreCtrl; case 'isrc': return _isrcCtrl; + case 'lyrics': + return _lyricsCtrl; case 'label': return _labelCtrl; case 'copyright': @@ -5107,19 +5139,23 @@ class _EditMetadataSheetState extends State<_EditMetadataSheet> { final artist = _artistCtrl.text.trim(); final album = _albumCtrl.text.trim(); final currentIsrc = _isrcCtrl.text.trim().toUpperCase(); + final shouldFetchLyrics = _autoFillFields.contains('lyrics'); + final needsTrackLookup = _autoFillFields.any((key) => key != 'lyrics'); Map? best; String? deezerId; - try { - final resolved = await _resolveAutoFillTrackFromIdentifiers( - currentIsrc, - ); - if (resolved != null) { - best = resolved.track; - deezerId = resolved.deezerId; + if (needsTrackLookup) { + try { + final resolved = await _resolveAutoFillTrackFromIdentifiers( + currentIsrc, + ); + if (resolved != null) { + best = resolved.track; + deezerId = resolved.deezerId; + } + } catch (e) { + _log.w('Identifier-first autofill lookup failed: $e'); } - } catch (e) { - _log.w('Identifier-first autofill lookup failed: $e'); } final queryParts = []; @@ -5127,7 +5163,7 @@ class _EditMetadataSheetState extends State<_EditMetadataSheet> { if (artist.isNotEmpty) queryParts.add(artist); if (queryParts.isEmpty && album.isNotEmpty) queryParts.add(album); - if (best == null && queryParts.isEmpty) { + if (needsTrackLookup && best == null && queryParts.isEmpty) { if (mounted) { ScaffoldMessenger.of(context).showSnackBar( SnackBar(content: Text(context.l10n.editMetadataAutoFillNoResults)), @@ -5140,7 +5176,7 @@ class _EditMetadataSheetState extends State<_EditMetadataSheet> { final normalizedArtist = _normalizeMetadataText(artist); final normalizedAlbum = _normalizeMetadataText(album); - if (best == null) { + if (needsTrackLookup && best == null) { final query = queryParts.join(' '); final results = await PlatformBridge.searchTracksWithMetadataProviders( query, @@ -5175,39 +5211,47 @@ class _EditMetadataSheetState extends State<_EditMetadataSheet> { } final selectedBest = best; - if (selectedBest == null) { + if (needsTrackLookup && selectedBest == null) { throw StateError('No metadata match resolved for auto-fill'); } - final enriched = { - 'title': (selectedBest['name'] ?? '').toString(), - 'artist': (selectedBest['artists'] ?? selectedBest['artist'] ?? '') - .toString(), - 'album': (selectedBest['album_name'] ?? selectedBest['album'] ?? '') - .toString(), - 'album_artist': (selectedBest['album_artist'] ?? '').toString(), - 'date': (selectedBest['release_date'] ?? '').toString(), - 'track_number': (selectedBest['track_number'] ?? '').toString(), - 'total_tracks': (selectedBest['total_tracks'] ?? '').toString(), - 'disc_number': (selectedBest['disc_number'] ?? '').toString(), - 'total_discs': (selectedBest['total_discs'] ?? '').toString(), - 'isrc': (selectedBest['isrc'] ?? '').toString(), - 'composer': (selectedBest['composer'] ?? '').toString(), - }; - _mergeOnlineTrackData(enriched, selectedBest); + final enriched = {}; + if (selectedBest != null) { + enriched.addAll({ + 'title': (selectedBest['name'] ?? '').toString(), + 'artist': (selectedBest['artists'] ?? selectedBest['artist'] ?? '') + .toString(), + 'album': (selectedBest['album_name'] ?? selectedBest['album'] ?? '') + .toString(), + 'album_artist': (selectedBest['album_artist'] ?? '').toString(), + 'date': (selectedBest['release_date'] ?? '').toString(), + 'track_number': (selectedBest['track_number'] ?? '').toString(), + 'total_tracks': (selectedBest['total_tracks'] ?? '').toString(), + 'disc_number': (selectedBest['disc_number'] ?? '').toString(), + 'total_discs': (selectedBest['total_discs'] ?? '').toString(), + 'isrc': (selectedBest['isrc'] ?? '').toString(), + 'composer': (selectedBest['composer'] ?? '').toString(), + }); + _mergeOnlineTrackData(enriched, selectedBest); + } + final enrichedIsrc = (enriched['isrc'] ?? '').trim(); final needsIsrc = - _autoFillFields.contains('isrc') && enriched['isrc']!.isEmpty; + _autoFillFields.contains('isrc') && enrichedIsrc.isEmpty; final needsExtended = _autoFillFields.contains('genre') || _autoFillFields.contains('label') || _autoFillFields.contains('copyright') || _autoFillFields.contains('composer'); - final rawSpotifyId = _extractRawSpotifyTrackId(selectedBest); + final rawSpotifyId = selectedBest == null + ? _extractRawSpotifyTrackIdFromValue(widget.sourceTrackId) + : _extractRawSpotifyTrackId(selectedBest); - deezerId ??= _extractRawDeezerTrackId(selectedBest); - final candidateIsrc = enriched['isrc']!.trim().toUpperCase(); + deezerId ??= selectedBest == null + ? null + : _extractRawDeezerTrackId(selectedBest); + final candidateIsrc = enrichedIsrc.toUpperCase(); final deezerLookupIsrc = _looksLikeIsrc(currentIsrc) ? currentIsrc : (_looksLikeIsrc(candidateIsrc) ? candidateIsrc : ''); @@ -5243,7 +5287,9 @@ class _EditMetadataSheetState extends State<_EditMetadataSheet> { if (!mounted) return; // Fetch ISRC from Deezer track metadata if still missing - if (needsIsrc && enriched['isrc']!.isEmpty && deezerId != null) { + if (needsIsrc && + (enriched['isrc'] ?? '').trim().isEmpty && + deezerId != null) { try { final deezerMeta = await PlatformBridge.getDeezerMetadata( 'track', @@ -5275,6 +5321,37 @@ class _EditMetadataSheetState extends State<_EditMetadataSheet> { } } + if (shouldFetchLyrics) { + final lyricsTitle = + ((selectedBest?['name'] ?? selectedBest?['title'] ?? title) + .toString()) + .trim(); + final lyricsArtist = + ((selectedBest?['artists'] ?? selectedBest?['artist'] ?? artist) + .toString()) + .trim(); + + if (lyricsTitle.isNotEmpty && lyricsArtist.isNotEmpty) { + try { + final lyricsResult = await PlatformBridge.getLyricsLRCWithSource( + rawSpotifyId ?? '', + lyricsTitle, + lyricsArtist, + durationMs: widget.durationMs, + ); + final lyricsText = lyricsResult['lyrics']?.toString().trim() ?? ''; + final instrumental = + (lyricsResult['instrumental'] as bool? ?? false) || + lyricsText == '[instrumental:true]'; + if (!instrumental && lyricsText.isNotEmpty) { + enriched['lyrics'] = lyricsText; + } + } catch (e) { + _log.w('Lyrics autofill failed: $e'); + } + } + } + if (!mounted) return; var filledCount = 0; @@ -5293,7 +5370,7 @@ class _EditMetadataSheetState extends State<_EditMetadataSheet> { } } - if (_autoFillFields.contains('cover')) { + if (_autoFillFields.contains('cover') && selectedBest != null) { final coverUrl = (selectedBest['cover_url'] ?? selectedBest['images'] ?? '') .toString(); @@ -5369,6 +5446,7 @@ class _EditMetadataSheetState extends State<_EditMetadataSheet> { _discTotalCtrl = TextEditingController(text: v['total_discs'] ?? ''); _genreCtrl = TextEditingController(text: v['genre'] ?? ''); _isrcCtrl = TextEditingController(text: v['isrc'] ?? ''); + _lyricsCtrl = TextEditingController(text: v['lyrics'] ?? ''); _labelCtrl = TextEditingController(text: v['label'] ?? ''); _copyrightCtrl = TextEditingController(text: v['copyright'] ?? ''); _composerCtrl = TextEditingController(text: v['composer'] ?? ''); @@ -5391,6 +5469,7 @@ class _EditMetadataSheetState extends State<_EditMetadataSheet> { _discTotalCtrl.dispose(); _genreCtrl.dispose(); _isrcCtrl.dispose(); + _lyricsCtrl.dispose(); _labelCtrl.dispose(); _copyrightCtrl.dispose(); _composerCtrl.dispose(); @@ -5413,6 +5492,7 @@ class _EditMetadataSheetState extends State<_EditMetadataSheet> { 'disc_total': _discTotalCtrl.text, 'genre': _genreCtrl.text, 'isrc': _isrcCtrl.text, + 'lyrics': _lyricsCtrl.text, 'label': _labelCtrl.text, 'copyright': _copyrightCtrl.text, 'composer': _composerCtrl.text, @@ -5477,6 +5557,8 @@ class _EditMetadataSheetState extends State<_EditMetadataSheet> { : '', 'GENRE': metadata['genre'] ?? '', 'ISRC': metadata['isrc'] ?? '', + 'LYRICS': metadata['lyrics'] ?? '', + 'UNSYNCEDLYRICS': metadata['lyrics'] ?? '', 'ORGANIZATION': metadata['label'] ?? '', 'COPYRIGHT': metadata['copyright'] ?? '', 'COMPOSER': metadata['composer'] ?? '', @@ -5486,11 +5568,6 @@ class _EditMetadataSheetState extends State<_EditMetadataSheet> { final existingMetadata = await PlatformBridge.readFileMetadata( ffmpegTarget, ); - final existingLyrics = existingMetadata['lyrics']?.toString().trim(); - if (existingLyrics != null && existingLyrics.isNotEmpty) { - vorbisMap['LYRICS'] = existingLyrics; - vorbisMap['UNSYNCEDLYRICS'] = existingLyrics; - } // Preserve ReplayGain tags if present — these are computed once // during download and should survive manual metadata edits. final rgFields = { @@ -5717,6 +5794,12 @@ class _EditMetadataSheetState extends State<_EditMetadataSheet> { ), _field('Genre', _genreCtrl), _field('ISRC', _isrcCtrl), + _field( + context.l10n.trackLyrics, + _lyricsCtrl, + maxLines: 8, + keyboard: TextInputType.multiline, + ), Padding( padding: const EdgeInsets.only(top: 8, bottom: 4), child: InkWell( diff --git a/lib/services/ffmpeg_service.dart b/lib/services/ffmpeg_service.dart index 0d08b26e..e34c5174 100644 --- a/lib/services/ffmpeg_service.dart +++ b/lib/services/ffmpeg_service.dart @@ -1139,20 +1139,28 @@ class FFmpegService { : '.tmp'; final tempDir = await getTemporaryDirectory(); final tempOutput = _nextTempEmbedPath(tempDir.path, ext); - - final sanitizedGain = albumGain.replaceAll('"', '\\"'); - final sanitizedPeak = albumPeak.replaceAll('"', '\\"'); - - // -map_metadata 0 preserves all existing metadata from the input. - // -metadata flags add/overwrite only the specified keys. - final command = - '-v error -hide_banner -i "$filePath" -map 0 -c copy -map_metadata 0 ' - '-metadata REPLAYGAIN_ALBUM_GAIN="$sanitizedGain" ' - '-metadata REPLAYGAIN_ALBUM_PEAK="$sanitizedPeak" ' - '"$tempOutput" -y'; + final arguments = [ + '-v', + 'error', + '-hide_banner', + '-i', + filePath, + '-map', + '0', + '-c', + 'copy', + '-map_metadata', + '0', + '-metadata', + 'REPLAYGAIN_ALBUM_GAIN=$albumGain', + '-metadata', + 'REPLAYGAIN_ALBUM_PEAK=$albumPeak', + tempOutput, + '-y', + ]; _log.d('Writing album ReplayGain tags via FFmpeg'); - final result = await _execute(command); + final result = await _executeWithArguments(arguments); if (result.success) { try { @@ -1194,41 +1202,50 @@ class FFmpegService { }) async { final tempDir = await getTemporaryDirectory(); final tempOutput = _nextTempEmbedPath(tempDir.path, '.flac'); - - final StringBuffer cmdBuffer = StringBuffer(); - cmdBuffer.write('-v error -hide_banner '); - cmdBuffer.write('-i "$flacPath" '); + final arguments = ['-v', 'error', '-hide_banner', '-i', flacPath]; if (coverPath != null) { - cmdBuffer.write('-i "$coverPath" '); + arguments + ..add('-i') + ..add(coverPath); } - cmdBuffer.write('-map 0:a '); + arguments + ..add('-map') + ..add('0:a'); if (coverPath != null) { - cmdBuffer.write('-map 1:0 '); - cmdBuffer.write('-c:v copy '); - cmdBuffer.write('-disposition:v attached_pic '); - cmdBuffer.write('-metadata:s:v title="Album cover" '); - cmdBuffer.write('-metadata:s:v comment="Cover (front)" '); + arguments + ..add('-map') + ..add('1:0') + ..add('-c:v') + ..add('copy') + ..add('-disposition:v') + ..add('attached_pic') + ..add('-metadata:s:v') + ..add('title=Album cover') + ..add('-metadata:s:v') + ..add('comment=Cover (front)'); } - cmdBuffer.write('-c:a copy '); + arguments + ..add('-c:a') + ..add('copy'); if (metadata != null) { - _appendVorbisMetadataToCommandBuffer( - cmdBuffer, + _appendVorbisMetadataToArguments( + arguments, metadata, artistTagMode: artistTagMode, ); } - cmdBuffer.write('"$tempOutput" -y'); + arguments + ..add(tempOutput) + ..add('-y'); - final command = cmdBuffer.toString(); - _log.d('Executing FFmpeg command: ${_previewCommandForLog(command)}'); - - final result = await _execute(command); + _log.d('Executing FFmpeg FLAC embed command'); + final result = await _executeWithArguments(arguments); if (result.success) { try { @@ -1274,46 +1291,50 @@ class FFmpegService { }) async { final tempDir = await getTemporaryDirectory(); final tempOutput = _nextTempEmbedPath(tempDir.path, '.mp3'); - - final StringBuffer cmdBuffer = StringBuffer(); - cmdBuffer.write('-v error -hide_banner '); - cmdBuffer.write('-i "$mp3Path" '); + final arguments = ['-v', 'error', '-hide_banner', '-i', mp3Path]; if (coverPath != null) { - cmdBuffer.write('-i "$coverPath" '); + arguments + ..add('-i') + ..add(coverPath); } - cmdBuffer.write('-map 0:a '); - cmdBuffer.write( - preserveMetadata ? '-map_metadata 0 ' : '-map_metadata -1 ', - ); + arguments + ..add('-map') + ..add('0:a') + ..add('-map_metadata') + ..add(preserveMetadata ? '0' : '-1'); if (coverPath != null) { - cmdBuffer.write('-map 1:0 '); - cmdBuffer.write('-c:v:0 copy '); - cmdBuffer.write('-id3v2_version 3 '); - cmdBuffer.write('-metadata:s:v title="Album cover" '); - cmdBuffer.write('-metadata:s:v comment="Cover (front)" '); + arguments + ..add('-map') + ..add('1:0') + ..add('-c:v:0') + ..add('copy') + ..add('-id3v2_version') + ..add('3') + ..add('-metadata:s:v') + ..add('title=Album cover') + ..add('-metadata:s:v') + ..add('comment=Cover (front)'); } - cmdBuffer.write('-c:a copy '); + arguments + ..add('-c:a') + ..add('copy'); if (metadata != null) { - final id3Metadata = _convertToId3Tags(metadata); - id3Metadata.forEach((key, value) { - final sanitizedValue = value.replaceAll('"', '\\"'); - cmdBuffer.write('-metadata $key="$sanitizedValue" '); - }); + _appendMappedMetadataToArguments(arguments, _convertToId3Tags(metadata)); } - cmdBuffer.write('-id3v2_version 3 "$tempOutput" -y'); + arguments + ..add('-id3v2_version') + ..add('3') + ..add(tempOutput) + ..add('-y'); - final command = cmdBuffer.toString(); - _log.d( - 'Executing FFmpeg MP3 embed command: ${_previewCommandForLog(command)}', - ); - - final result = await _execute(command); + _log.d('Executing FFmpeg MP3 embed command'); + final result = await _executeWithArguments(arguments); if (result.success) { try { @@ -1456,10 +1477,7 @@ class FFmpegService { }) async { final tempDir = await getTemporaryDirectory(); final tempOutput = _nextTempEmbedPath(tempDir.path, '.m4a'); - - final cmdBuffer = StringBuffer(); - cmdBuffer.write('-v error -hide_banner '); - cmdBuffer.write('-i "$m4aPath" '); + final arguments = ['-v', 'error', '-hide_banner', '-i', m4aPath]; final normalizedCoverPath = coverPath?.trim(); final hasCover = @@ -1467,48 +1485,61 @@ class FFmpegService { normalizedCoverPath.isNotEmpty && await File(normalizedCoverPath).exists(); if (hasCover) { - cmdBuffer.write('-i "$normalizedCoverPath" '); + arguments + ..add('-i') + ..add(normalizedCoverPath); } final preserveExistingStreams = preserveMetadata && !hasCover; if (preserveExistingStreams) { // When no replacement cover is provided, preserve all input streams so // the existing attached artwork is not dropped during the metadata rewrite. - cmdBuffer.write('-map 0 -c copy '); + arguments + ..add('-map') + ..add('0') + ..add('-c') + ..add('copy'); } else { - cmdBuffer.write('-map 0:a -c:a copy '); + arguments + ..add('-map') + ..add('0:a') + ..add('-c:a') + ..add('copy'); } - cmdBuffer.write( - preserveMetadata ? '-map_metadata 0 ' : '-map_metadata -1 ', - ); + arguments + ..add('-map_metadata') + ..add(preserveMetadata ? '0' : '-1'); // For M4A cover replacements, mark the image as an attached picture so the // mp4 muxer writes a proper covr atom instead of a generic MJPEG video track. // Force the mp4 muxer because the default ipod muxer (auto-selected for .m4a) // does not register a codec tag for mjpeg on FFmpeg 8.0+. if (hasCover) { - cmdBuffer.write('-map 1:v -c:v copy -disposition:v:0 attached_pic '); - cmdBuffer.write('-metadata:s:v title="Album cover" '); - cmdBuffer.write('-metadata:s:v comment="Cover (front)" '); - cmdBuffer.write('-f mp4 '); + arguments + ..add('-map') + ..add('1:v') + ..add('-c:v') + ..add('copy') + ..add('-disposition:v:0') + ..add('attached_pic') + ..add('-metadata:s:v') + ..add('title=Album cover') + ..add('-metadata:s:v') + ..add('comment=Cover (front)') + ..add('-f') + ..add('mp4'); } if (metadata != null) { - final m4aMetadata = _convertToM4aTags(metadata); - for (final entry in m4aMetadata.entries) { - final sanitizedValue = entry.value.replaceAll('"', '\\"'); - cmdBuffer.write('-metadata ${entry.key}="$sanitizedValue" '); - } + _appendMappedMetadataToArguments(arguments, _convertToM4aTags(metadata)); } - cmdBuffer.write('"$tempOutput" -y'); + arguments + ..add(tempOutput) + ..add('-y'); - final command = cmdBuffer.toString(); - _log.d( - 'Executing FFmpeg M4A embed command: ${_previewCommandForLog(command)}', - ); - - final result = await _execute(command); + _log.d('Executing FFmpeg M4A embed command'); + final result = await _executeWithArguments(arguments); if (result.success) { try { @@ -1767,40 +1798,50 @@ class FFmpegService { bool deleteOriginal = true, }) async { final outputPath = _buildOutputPath(inputPath, '.m4a'); - - final cmdBuffer = StringBuffer(); - cmdBuffer.write('-v error -hide_banner '); - cmdBuffer.write('-i "$inputPath" '); + final arguments = ['-v', 'error', '-hide_banner', '-i', inputPath]; final hasCover = coverPath != null && coverPath.trim().isNotEmpty && await File(coverPath).exists(); if (hasCover) { - cmdBuffer.write('-i "$coverPath" '); + arguments + ..add('-i') + ..add(coverPath); } - cmdBuffer.write('-map 0:a '); + arguments + ..add('-map') + ..add('0:a'); if (hasCover) { - cmdBuffer.write('-map 1:v -c:v copy -disposition:v:0 attached_pic '); - cmdBuffer.write('-metadata:s:v title="Album cover" '); - cmdBuffer.write('-metadata:s:v comment="Cover (front)" '); + arguments + ..add('-map') + ..add('1:v') + ..add('-c:v') + ..add('copy') + ..add('-disposition:v:0') + ..add('attached_pic') + ..add('-metadata:s:v') + ..add('title=Album cover') + ..add('-metadata:s:v') + ..add('comment=Cover (front)'); } - cmdBuffer.write('-c:a alac '); - cmdBuffer.write('-map_metadata -1 '); + arguments + ..add('-c:a') + ..add('alac') + ..add('-map_metadata') + ..add('-1'); - final m4aTags = _convertToM4aTags(metadata); - for (final entry in m4aTags.entries) { - final sanitized = entry.value.replaceAll('"', '\\"'); - cmdBuffer.write('-metadata ${entry.key}="$sanitized" '); - } + _appendMappedMetadataToArguments(arguments, _convertToM4aTags(metadata)); - cmdBuffer.write('"$outputPath" -y'); + arguments + ..add(outputPath) + ..add('-y'); _log.i( 'Converting ${inputPath.split(Platform.pathSeparator).last} to ALAC', ); - final result = await _execute(cmdBuffer.toString()); + final result = await _executeWithArguments(arguments); if (!result.success) { _log.e('ALAC conversion failed: ${result.output}'); @@ -1830,40 +1871,56 @@ class FFmpegService { bool deleteOriginal = true, }) async { final outputPath = _buildOutputPath(inputPath, '.flac'); - - final cmdBuffer = StringBuffer(); - cmdBuffer.write('-v error -hide_banner '); - cmdBuffer.write('-i "$inputPath" '); + final arguments = ['-v', 'error', '-hide_banner', '-i', inputPath]; final hasCover = coverPath != null && coverPath.trim().isNotEmpty && await File(coverPath).exists(); if (hasCover) { - cmdBuffer.write('-i "$coverPath" '); + arguments + ..add('-i') + ..add(coverPath); } - cmdBuffer.write('-map 0:a '); + arguments + ..add('-map') + ..add('0:a'); if (hasCover) { - cmdBuffer.write('-map 1:v -c:v copy -disposition:v:0 attached_pic '); - cmdBuffer.write('-metadata:s:v title="Album cover" '); - cmdBuffer.write('-metadata:s:v comment="Cover (front)" '); + arguments + ..add('-map') + ..add('1:v') + ..add('-c:v') + ..add('copy') + ..add('-disposition:v:0') + ..add('attached_pic') + ..add('-metadata:s:v') + ..add('title=Album cover') + ..add('-metadata:s:v') + ..add('comment=Cover (front)'); } - cmdBuffer.write('-c:a flac -compression_level 8 '); - cmdBuffer.write('-map_metadata 0 '); + arguments + ..add('-c:a') + ..add('flac') + ..add('-compression_level') + ..add('8') + ..add('-map_metadata') + ..add('0'); - _appendVorbisMetadataToCommandBuffer( - cmdBuffer, + _appendVorbisMetadataToArguments( + arguments, metadata, artistTagMode: artistTagMode, ); - cmdBuffer.write('"$outputPath" -y'); + arguments + ..add(outputPath) + ..add('-y'); _log.i( 'Converting ${inputPath.split(Platform.pathSeparator).last} to FLAC', ); - final result = await _execute(cmdBuffer.toString()); + final result = await _executeWithArguments(arguments); if (!result.success) { _log.e('FLAC conversion failed: ${result.output}'); @@ -1969,20 +2026,6 @@ class FFmpegService { return vorbis; } - static void _appendVorbisMetadataToCommandBuffer( - StringBuffer cmdBuffer, - Map metadata, { - String artistTagMode = artistTagModeJoined, - }) { - for (final entry in _buildVorbisMetadataEntries( - metadata, - artistTagMode: artistTagMode, - )) { - final sanitized = entry.value.replaceAll('"', '\\"'); - cmdBuffer.write('-metadata ${entry.key}="$sanitized" '); - } - } - static void _appendVorbisMetadataToArguments( List arguments, Map metadata, { @@ -1998,6 +2041,17 @@ class FFmpegService { } } + static void _appendMappedMetadataToArguments( + List arguments, + Map metadata, + ) { + for (final entry in metadata.entries) { + arguments + ..add('-metadata') + ..add('${entry.key}=${entry.value}'); + } + } + static List> _buildVorbisMetadataEntries( Map metadata, { String artistTagMode = artistTagModeJoined, @@ -2255,23 +2309,36 @@ class FFmpegService { final trackNumStr = track.number.toString().padLeft(2, '0'); final outputFileName = '$trackNumStr - $sanitizedTitle.$outputExt'; final outputPath = '$outputDir${Platform.pathSeparator}$outputFileName'; - - final StringBuffer cmdBuffer = StringBuffer(); - cmdBuffer.write('-v error -hide_banner '); - cmdBuffer.write('-i "$audioPath" '); + final arguments = [ + '-v', + 'error', + '-hide_banner', + '-i', + audioPath, + ]; final startTime = _formatSecondsForFFmpeg(track.startSec); - cmdBuffer.write('-ss $startTime '); + arguments + ..add('-ss') + ..add(startTime); if (track.endSec > 0) { final endTime = _formatSecondsForFFmpeg(track.endSec); - cmdBuffer.write('-to $endTime '); + arguments + ..add('-to') + ..add(endTime); } if (outputExt == 'flac') { - cmdBuffer.write('-c:a flac -compression_level 8 '); + arguments + ..add('-c:a') + ..add('flac') + ..add('-compression_level') + ..add('8'); } else { - cmdBuffer.write('-c:a copy '); + arguments + ..add('-c:a') + ..add('copy'); } final artist = track.artist.isNotEmpty @@ -2280,11 +2347,11 @@ class FFmpegService { final album = albumMetadata['album'] ?? ''; final genre = albumMetadata['genre'] ?? ''; final date = albumMetadata['date'] ?? ''; + final cueMetadata = {}; void addMeta(String key, String value) { if (value.isNotEmpty) { - final sanitized = value.replaceAll('"', '\\"'); - cmdBuffer.write('-metadata $key="$sanitized" '); + cueMetadata[key] = value; } } @@ -2298,14 +2365,13 @@ class FFmpegService { if (track.isrc.isNotEmpty) addMeta('ISRC', track.isrc); if (track.composer.isNotEmpty) addMeta('COMPOSER', track.composer); - cmdBuffer.write('"$outputPath" -y'); + _appendMappedMetadataToArguments(arguments, cueMetadata); + arguments + ..add(outputPath) + ..add('-y'); - final command = cmdBuffer.toString(); - _log.d( - 'CUE split track ${track.number}: ${_previewCommandForLog(command)}', - ); - - final result = await _execute(command); + _log.d('CUE split track ${track.number}'); + final result = await _executeWithArguments(arguments); if (!result.success) { _log.e('CUE split failed for track ${track.number}: ${result.output}'); continue;