Fix ReplayGain metaflac backend altering input files - #6916
Conversation
|
Thank you for the PR! The changelog has not been updated, so here is a friendly reminder to check if you need to add an entry. |
There was a problem hiding this comment.
Pull request overview
This PR fix ReplayGain metaflac backend so it no longer write ReplayGain tags into input/source files during import. It switch to reading gain data via metaflac --scan-replay-gain output, then write tags only when beets later writes library file.
Changes:
- Switch
MetaflacBackendfrom--add-replay-gain+ tag readback to--scan-replay-gain+ stdout parsing. - Update docs to note FLAC tools version requirement for
metaflacbackend. - Update tests to cover new
--scan-replay-gainoutput parsing.
Reviewed changes
Copilot reviewed 3 out of 3 changed files in this pull request and generated 4 comments.
| File | Description |
|---|---|
| beetsplug/replaygain.py | Use --scan-replay-gain, parse its output, and compute album/track Gain without mutating files. |
| docs/plugins/replaygain.rst | Document minimum FLAC tools version for metaflac backend. |
| test/plugins/test_replaygain.py | Update unit test to validate new metaflac output parsing behavior. |
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
| track_gains = [] | ||
| for item in filter(self.format_supported, task.items): | ||
| self._add_replay_gain([item]) | ||
| track_gains.append( | ||
| self._read_gain(item, "TRACK", task.target_level) | ||
| ) | ||
| result = self._read_gain([item], task.target_level) | ||
|
|
||
| track_gain_value = result[item][1] |
There was a problem hiding this comment.
I'll fix this in my next commit.
| """Run ``metaflac --scan-replay-gain`` on the given files""" | ||
| paths = [str(item.filepath) for item in items] | ||
| call([self.command, "--add-replay-gain", *paths], self._log) | ||
| output = call( | ||
| [self.command, "--scan-replay-gain", *paths], self._log | ||
| ).stdout.decode("utf-8", "ignore") |
There was a problem hiding this comment.
I'll fix this in my next commit.
| output = ( | ||
| b"REPLAYGAIN_TRACK_GAIN=-11.55 dB\nREPLAYGAIN_TRACK_PEAK=0.99998772\n" | ||
| "01.flac: -1.234567 1.234567 1.987654 -1.987654\n" | ||
| "02.flac: -1.234567 1.234567 -1.987654 1.987654\n" | ||
| ) |
There was a problem hiding this comment.
I'll fix this in my next commit.
| for item in items: | ||
| path = str(item.filepath) | ||
|
|
||
| album_gain, album_peak, track_gain, track_peak = gain_by_path[path] | ||
|
|
There was a problem hiding this comment.
I'll fix this in my next commit.
Codecov Report❌ Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## master #6916 +/- ##
==========================================
- Coverage 75.69% 75.68% -0.01%
==========================================
Files 163 163
Lines 21412 21428 +16
Branches 3384 3387 +3
==========================================
+ Hits 16208 16218 +10
- Misses 4405 4410 +5
- Partials 799 800 +1
🚀 New features to boost your workflow:
|
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 4 out of 4 changed files in this pull request and generated no new comments.
Suppressed comments (2)
test/plugins/test_replaygain.py:414
- grug see test use album_peak 1.234567, but code say peak part of full scale (FS is 1.0). peak > 1.0 not real and make reader confuse. grug want peak <= 1.0 in sample output.
"01.flac: -1.234567 1.234567 1.987654 0.123456\n"
"02.flac: -1.234567 1.234567 -1.987654 0.987654\n"
docs/changelog.rst:54
- grug see changelog say 🐛
6916but PR say fix #6915. wrong bug number make release note point wrong place. grug want use 6915.
- :doc:`plugins/replaygain`: Fix ReplayGain metaflac backend altering input
files. :bug:`6916`
|
I have fixed all elements raised by GitHub Copilot. I did try to stay consistent and to keep it simple. Furthermore, I left the conversation opened by GitHub Copilot as-is so you can close them when reviewing the changes I made from its input. Let me know if you need other changes. |
Description
Fixes #6915.
I was able to try out this PR and it works as expected. The ReplayGain tags are now only written to the copy of the file in the library, and not to the original file that was imported from.
To Do
Remaining questions
As this is my first contribution to beets and that I do not consider myself very experienced with Python, I would like to ask for feedback on the following:
Should I add a check if the- GitHub Copilot seems to have answered my question.--scan-replay-gainoption is available and throw aFatalReplayGainErrorexception if not, or is it sufficient to mention it in the documentation?track_gainslist? I find the current implementation more readable, but I am not sure if it is the best practice.Thank you for your time and for reviewing my PR.
AI disclaimer
I have used AI to help me write this PR.