Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Replaygain 2.0 #520

Merged
merged 33 commits into from
Apr 13, 2016
Merged

Replaygain 2.0 #520

merged 33 commits into from
Apr 13, 2016

Conversation

daschuer
Copy link
Member

This is a evaluation branch of ReplaGain 2.0

Warning: using this branch replaces your current ReplayGain value by a new calculated ReplayGain 2.0 value in any case.

I have used a light weight EBU R 128 implementation based on http://kokkinizita.linuxaudio.org/linuxaudio/ebumeter-doc/quickguide.html
it is much faster than our current ReplayGain analysis:

Debug [Main]: Stat("AnalyserEbur128::process()","count=2736,sum=1.75087e+08ns,average=63993.7ns,min=45760ns,max=340437ns,variance=1.11145e+08ns^2,stddev=10542.5ns")
Debug [Main]: Stat("AnalyserGain::process()","count=2736,sum=7.63906e+08ns,average=279205ns,min=263269ns,max=1.14068e+06ns,variance=1.06941e+09ns^2,stddev=32701.9ns")

Currently RG1 and RG2 are processed and printed by qDebug()
RG2 is applied.

Whats next:
I am in favor to adopt it to Mixxx.

We may either switch silently to ReplayGain 2.0 or do all the work to switch between RG1 and RG2 this means that RG1 is used for all ReplayGain values calculated by Mixxx 1.11. and RG2 for new tracks.
Mixxx prefers to use RG from the file. But we cannot find out which version is it.

We may also store in a DB coloumn which RG version was used.
We may also offer an option to clear all RG values to analyze it with the new RG2 algorithm.

What do you think?

http://wiki.hydrogenaud.io/index.php?title=ReplayGain_1.0_specification
http://wiki.hydrogenaud.io/index.php?title=ReplayGain_2.0_specification

@daschuer
Copy link
Member Author

On a second thought this approach sound good to me:

Since we do not know the ReplaGain version from the file meta data, there is not much sense to track the ReplaGain version in the Mixxx library.
So it should be enough to allow to select the ReplayGain version 1.0 or 2.0 and add a Button for clearing all ReplayGain values from the Mixxx library.

Any thoughts?

@daschuer
Copy link
Member Author

License:
I have ask Fons Adriaensen if it is possible to distribute his code under the Mixxx license with the apple app store exception. It is not.

@Pegasus-RPG
Copy link
Member

Hold on there, cowboy. According to the RG 2.0 spec document, the spec is still under construction, which means it may change, so we definitely do not want to merge this unless and until the specification is finalized and the code is updated to match.
It's of course fine to have it for testing in the meantime though.

@daschuer
Copy link
Member Author

The RG 2.0 spec is currently not an issue for Mixxx, since it covers how to store loudness informations in the file metadata, which Mixxx does not do yet.

The interesting part of RG 2.0 is the proposed algorithm. It is the ITU BS.1770-3[3] standard, already published August 2012. And this is the base for the LUFS unit published as EBU R 128.
Since the boost sider has now a LUFS scale it is almost natural to switch to a standard loudness calculation algorithm. Which aims to have an constant average LUFS value and is improved compared to the RG 1.0 algorithm.

A nice addition will be an option to switch the vu-metres to a Loudness meter.

@uklotzde
Copy link
Contributor

I'm starting to test this stuff. It would be great if we could improve the replay gain algorithm.

I only found some minor issues in the code so far, but first I want to compare the results.

@daschuer
Copy link
Member Author

daschuer commented Nov 7, 2015

Branch is now rebased on the replaygain column changes in master.

@uklotzde
Copy link
Contributor

@daschuer Could you please align (or rebase) this branch on master? I would recommend rebasing for a clean start, since we haven't written any code review comments yet.

It would be nice to have the EBU R 128 variant avialable in 2.1. What about the licensing issues?

@uklotzde
Copy link
Contributor

How about using this implementation?

https://github.com/jiixyj/libebur128

@daschuer
Copy link
Member Author

Rebased!

libebur128 is included in Ubuntu from Vivid
https://launchpad.net/ubuntu/+source/libebur128
The Vivid package works on Trusty as well.
I will try to switch over.

@daschuer
Copy link
Member Author

This was a bad deal, the libebur128 (MIT) version takes 8 x more CPU, compared to the ebu_r128 (GPL) version.

Debug [Main]: Stat("AnalyserEbur128::process()","count=1778,sum=8.83455e+08ns,average=496881ns,min=316358ns,max=997272ns,variance=1.10587e+10ns^2,stddev=105160ns")
Debug [Main]: Stat("AnalyzerGain::process()"   ,"count=1778,sum=1.00864e+09ns,average=567289ns,min=406704ns,max=1.89389e+07ns,variance=2.26583e+12ns^2,stddev=1.50527e+06ns")

@uklotzde
Copy link
Contributor

libebur128 supports different modes with varying performance
characteristics. Which one did you use?

On 02/21/2016 01:35 AM, Daniel Schürmann wrote:

This was a bad deal, the libebur128 version takes 8 x more CPU, compared
to the ebu_r128 version.

|Debug [Main]:
Stat("AnalyserEbur128::process()","count=1778,sum=8.83455e+08ns,average=496881ns,min=316358ns,max=997272ns,variance=1.10587e+10ns^2,stddev=105160ns")
Debug [Main]: Stat("AnalyzerGain::process()"
,"count=1778,sum=1.00864e+09ns,average=567289ns,min=406704ns,max=1.89389e+07ns,variance=2.26583e+12ns^2,stddev=1.50527e+06ns")
|


Reply to this email directly or view it on GitHub
#520 (comment).

@uklotzde
Copy link
Contributor

libebur128 uses double for calculations while ebu_r128 uses float.

If the resampler is enabled for libebur128 the performance penalty becomes
reasonable:

#ifdef USE_SPEEX_RESAMPLER
static int ebur128_init_resampler(ebur128_state* st) {
int errcode = EBUR128_SUCCESS;

if (st->samplerate < 96000) {
st->d->oversample_factor = 4;
...

On 02/21/2016 01:35 AM, Daniel Schürmann wrote:

This was a bad deal, the libebur128 version takes 8 x more CPU, compared
to the ebu_r128 version.

|Debug [Main]:
Stat("AnalyserEbur128::process()","count=1778,sum=8.83455e+08ns,average=496881ns,min=316358ns,max=997272ns,variance=1.10587e+10ns^2,stddev=105160ns")
Debug [Main]: Stat("AnalyzerGain::process()"
,"count=1778,sum=1.00864e+09ns,average=567289ns,min=406704ns,max=1.89389e+07ns,variance=2.26583e+12ns^2,stddev=1.50527e+06ns")
|


Reply to this email directly or view it on GitHub
#520 (comment).

@uklotzde
Copy link
Contributor

I would recommend to use a widespread, thoroughly tested library instead of any special implementation that we need to maintain in the future. Hopefully the performance penalties can be reduced by an appropriate configuration of the libebur128 analyzer.

@daschuer
Copy link
Member Author

Now I have added both implementations parallel. The Vivid's libebur128 (MIT) seams to suffer some issues:

Debug [AnalyzerQueue 1]: ReplayGain1 result is -8.86 dB for "/home/daniel/Musik/Album_Atlantis.mp3" 
Debug [AnalyzerQueue 1]: ReplayGain2 (GPL) result is -9.69012 dB for "/home/daniel/Musik/Album_Atlantis.mp3" 
Debug [AnalyzerQueue 1]: ReplayGain2 (MIT) result is inf dB for "/home/daniel/Musik/Album_Atlantis.mp3" 

@daschuer
Copy link
Member Author

The same happens with Xenials version 1.1.0-1

@daschuer
Copy link
Member Author

Of cause the -INF error was my fault ;-)
Now it is working. The GPL version is still twice as fast.

Debug [Main]: Stat("AnalyserEbur128Gpl::process()","count=2252,sum=2.11134e+08ns,average=93753.9ns,min=89101ns,max=121240ns,variance=1.8037e+07ns^2,stddev=4247ns")
Debug [Main]: Stat("AnalyserEbur128Mit::process()","count=2252,sum=4.00588e+08ns,average=177881ns,min=116183ns,max=266434ns,variance=3.40518e+08ns^2,stddev=18453.1ns")
Debug [Main]: Stat("AnalyzerGain::process()","count=2252,sum=9.33756e+08ns,average=414634ns,min=405359ns,max=517953ns,variance=2.11559e+08ns^2,stddev=14545.1ns")

The oversampling is only required for the true peak calculation. It is not used for loudness calculation. The float vs double issue should make a big difference on a 64 bit build.

Debug [AnalyzerQueue 1]: ReplayGain1 result is -8.86 dB for "/home/daniel/Musik/Album_Atlantis.mp3" 
Debug [AnalyzerQueue 1]: ReplayGain2 (GPL) result is -9.69012 dB for "/home/daniel/Musik/Album_Atlantis.mp3" 
Debug [AnalyzerQueue 1]: ReplayGain2 (MIT) result is -9.70166 dB for "/home/daniel/Musik/Album_Atlantis.mp3" 

Both implementations do not result in the same values, but they are close, probably due to the halve precision of the GPL version.

Since the Analysis is not too time critical but the values are stored in the file, the exactness matters more then the performance.

If we decide to introduce a live loudness meter, which I like to have, the performance matters more.

@daschuer
Copy link
Member Author

daschuer commented Mar 9, 2016

Notes addressed.
I have followed Uwes idea for a separate settings class,

The isDisabledOrLoadStoredSuccess API change, just changes the name of a current function to
a more explaining and correct name. The meaning of the function has not changed.

Conflicts:
	src/engine/sidechain/engineshoutcast.cpp
@daschuer
Copy link
Member Author

This merges again cleanly.
Ready to merge?

@@ -246,6 +221,43 @@
</widget>
</item>
<item>
<widget class="QGroupBox" name="groupBoxAnalyzis">
<property name="title">
<string>Analyzis</string>
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

typo. Should be "Analysis"

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It should be American English:
http://grammarist.com/spelling/analyse-analyze/

Just to verify: Is it "to analyze" but analysis? Strange.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

"To analyze" is the correct spelling of the verb in American English, but "analysis" is the correct spelling of that noun in all English dialects. Yes, English is strange.

@Be-ing
Copy link
Contributor

Be-ing commented Mar 23, 2016

I also confirm that this works well with the -18 LUFS default target loudness. Before I had to set target loudness to -12 LUFS to have loud tracks not be brought way down.

@daschuer
Copy link
Member Author

Typo fixed. Any objections to merge this?

@uklotzde
Copy link
Contributor

I'm using this branch since quite some time now. Very satisfied with the results. Should be included in 2.1 and is also a nice entry for both the change log and the feature list ;)

@Be-ing
Copy link
Contributor

Be-ing commented Apr 13, 2016

@daschuer doesn't seem like it. I'd like to see this merged.

* Author: daniel
*/

#include "replaygainsettings.h"
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

please use an absolute include path

@daschuer
Copy link
Member Author

Done.

@rryan
Copy link
Member

rryan commented Apr 13, 2016

Thanks for the changes! -- LGTM

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

5 participants