metabrainz / picard-plugins

Picard plugins: use 1.0 branch for Picard < 2.0 (python 2/Qt4) and 2.0 branch for Picard >= 2.0 (python 3/Qt5)
https://picard.musicbrainz.org/plugins/
145 stars 95 forks source link

[Proposal] New Backend for ReplayGain Plugin #335

Closed complexlogic closed 2 years ago

complexlogic commented 2 years ago

I am the developer of rsgain, a cross-platform Windows/Mac/Linux ReplayGain tagging utility. I propose to make the Picard ReplayGain plugin use rsgain as the sole backend instead of the existing list of backends. This would have several benefits:

If this proposal is acceptable, I will work on a PR to update the plugin. Please let me know what you think.

rdswift commented 2 years ago

I don't currently use ReplayGain, although it's on my list of things to investigate some day. Having said that, if you can simplify the plugin for the users, I don't see any reason not to give it a try. If it's a significant rewrite (as I expect it might be), you could also just create a new plugin that would render the current plugin obsolete and it could (eventually) be retired from the list of available plugins.

Sophist-UK commented 2 years ago

You may need to add some code to your utility - because (as you probably already know) you can equalise replay gain either by re-encoding or by adding regain tags.

See also #89 (which I never had time to get to) and also #121.

complexlogic commented 2 years ago

you can equalise replay gain either by re-encoding or by adding regain tags

I don't plan to ever support re-encoding in my utility, but I wouldn't think that Picard would want that functionality easier since it's a metadata manager.

I saw your comment in #89 :

As and when I have time, my plan is to plagiarise (with permission) the regainer code and use @phw 's ffmpeg image in order to calculate the replay gain values and put them into the correct Picard variables so that they are only saved when you click Save in Picard.

If possible (and I haven't looked at whether this is possible or not) I will try to include the additional tags calculated by loudgain as defined here.

My program includes an output mode that writes /t separated data to stdout which could be easily parsed by Picard and stored internally. If that is the route you want to go I can support that too.

Sophist-UK commented 2 years ago

I don't have time ATM for any of the open source projects I want to contribute to - and this job is not top of the priority list either.

So, I thought I would give you pointers to the other thinking that has previously happened, and then if you go ahead with this you can decide which you think is the best technical solution and make it happen.

phw commented 2 years ago

@complexlogic A reworked replaygain plugin would be very welcomed indeed :+1: In the past there had been talk about providing plugins that either use loudgain or regainer as the backend for such a plugin, but nobody ever found the time or will to actually do it.

But basically everyone agrees that the existing replaygain plugin is bad and needs to be replaced. I wrote the original, and I'm the first one to be happy to have this fully reworked. I have not used rsgain myself, but it looks like a good choice for this as well.

A few general thoughts:

I don't plan to ever support re-encoding in my utility, but I wouldn't think that Picard would want that functionality easier since it's a metadata manager.

I agree with that. I'd rather have Picard focus on metadata instead of adding actual audio processing features.

Sophist-UK commented 2 years ago

Ideally I think that it would be better for the code to call a thread aware python C-library (which gives up the GIL) to process the files and return the replaygain values (rather than call an external process). But, it probably makes more sense to do whate ver is best from a pragmatic get-it-done perspective i.e. if there is an external tool which can be used to write the replaygain values to stdout, then you can run the executable and process the output using standard Python calls.

phw commented 2 years ago

I honestly don't see a single advantage in running the audio decoding in-process. Picard used to do this with the old fingerprinting before AcoustID, and it only caused trouble. If an external process is called I only see advantages:

Sophist-UK commented 2 years ago

@phw Phillip - I defer to your far greater knowledge and experience.

phw commented 2 years ago

I might very well be biased by the experience with old fingerprinting, because back than in addition to the crashes it caused in Picard it also was a PITA to build ffmpeg on Windows, and it frequently resulted in non-operational binaries. Anyway, bug reports about audio files crashing Picard are pretty much a thing of the past, while back then it was something that frequently happened.

complexlogic commented 2 years ago

@Sophist-UK @phw Thanks to both of you for providing the technical background. I believe I can solve most of the issues with the current ReplayGain plugin, notably the desire to have Picard tag the files instead of the external tool. I plan to work on this myself and eventually submit a PR, and we can discuss specific implementation details there.