ReplayGain plugin with mp3gain backend is modifying source files.

[jaquer]

If one enables the replaygain module using the mp3gain backend, the gain is being applied to the source files. Furthermore, the modifications are not reversible because the undo information is not being written to either source or destination files.

To reproduce:

Create this one-line ~/.config/beets/config.yaml:

plugins: replaygain

Per the docs, this will default to automatically analyze for replaygain, using the command backend.

Now, beet import -A a file. I use this one, which has a 222912248f934554ec2eaaf1b191f938607ffbab sha1sum.

The replaygain plugin calls mp3gain on the source with the "-r" parameter, which will apply the gain to the source file. Calling sha1sum on the source file after the import will show it has been modified.

Per the plugin docs, the gain should not be applied unless the apply config is set to yes, but even then, I don't believe that the intention was to modify at the source, was it?

By the way, the apply option was dropped from the plugin at 86ee30d. This is relatively easy to bring back, and I'd be happy to submit a PR fixing that omission, but the real problem is that even bringing that option/conditional check, the gain will still be applied to the source files.

[sampsyo]

Hmm, this does seem like a problem. The intention with dropping that option was to never apply the adjustment to the files themselves; that was determined to be out of scope for the plugin.

Do you have any clues about how we can force mp3gain to disable adjustments, while still reporting the right values?

[jaquer]

What do you think of this approach? Removing cmd = cmd + ['-a' if is_album else '-r'] from line 182 stops applying the gain to the source files.

Furthermore, the way handle_[album|track] were being called from the imported function, with a default False value for write would cause the computed values to be lost on import.

Let me know what you think and I can submit a PR.

[sampsyo]

What about album-level gain calculation? Will that still work without the -a flag?

I also don't quite see what you're saying about the write flag. Is this a different bug, or is it related to this one?

[jaquer]

Correct, album-level calculation works without the -a flag. Both -r and -a are used to apply the gain to the file directly. I see what you're alluding to though, you want to have an option to analyze files individually, without grouping them as an album. That is the -e flag, and the code needs a little more refactoring to use it.

Yes, the problem about the write flag is a separate bug. I'll open another issue to address it.

[sampsyo]

Ah, cool. Sounds good! If album-granularity analysis works either way, I think we can indeed just remove that line. No need for -e, I think, since we still separate out track and album values.

[jaquer]

Got it. Let me fix the commit. I'm still relatively new to PRs and stuff.