replaygain: target level refactor #3065
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
|
The latest push merges master (or rather #3056, from which this branch was forked), resolving conflicts. It also fixes the |
|
Is there any guideline on when to merge and when to rebase for this project? I will rework this branch's structure if a rebase is preferred. |
zsinskri
force-pushed
the
r128-targetlevel-config
branch
from
29418b9
to
26b83b0
Compare
zsinskri
added a commit
to zsinskri/beets
that referenced
this pull request
Jun 19, 2019
When using the ffmpeg replaygain backend to create R128_*_GAIN tags, automatically set the targetlevel to -23 LUFS. Note: This will be made configurable by GitHub PullRequest beetbox#3065
zsinskri
added a commit
to zsinskri/beets
that referenced
this pull request
Jul 14, 2019
Add replaygain backend using ffmpeg's ebur128 filter.
The album gain is calculated as the mean of all BS.1770 gating block powers.
Besides differences in gating block offset, this should be equivalent to a
BS.1770 analysis of a proper concatenation of all tracks.
Just calculating the mean of all track gains (as implemented by the bs1770gain
backend) yields incorrect results as that would:
- completely ignore track lengths
- just using length in seconds won't work either (e.g. BS.1770 ignores
passages below a threshold)
- take the mean of track loudness, not power
When using the ffmpeg replaygain backend to create R128_*_GAIN tags, the
targetlevel will be set to -23 LUFS. GitHub PullRequest beetbox#3065 will make this
configurable.
It will also skip peak calculation, as there is no R128_*_PEAK tag.
It is checked if the libavfilter library supports replaygain calculation. Before
version 6.67.100 that did require the `--enable-libebur128` compile-time-option,
after that the ebur128 library is included in libavfilter itself. Thus we
require either a recent enough libavfilter version or the `--enable-libebur128`
option.
zsinskri
added a commit
to zsinskri/beets
that referenced
this pull request
Jul 19, 2019
Add replaygain backend using ffmpeg's ebur128 filter.
The album gain is calculated as the mean of all BS.1770 gating block powers.
Besides differences in gating block offset, this should be equivalent to a
BS.1770 analysis of a proper concatenation of all tracks.
Just calculating the mean of all track gains (as implemented by the bs1770gain
backend) yields incorrect results as that would:
- completely ignore track lengths
- just using length in seconds won't work either (e.g. BS.1770 ignores
passages below a threshold)
- take the mean of track loudness, not power
When using the ffmpeg replaygain backend to create R128_*_GAIN tags, the
targetlevel will be set to -23 LUFS. GitHub PullRequest beetbox#3065 will make this
configurable.
It will also skip peak calculation, as there is no R128_*_PEAK tag.
It is checked if the libavfilter library supports replaygain calculation. Before
version 6.67.100 that did require the `--enable-libebur128` compile-time-option,
after that the ebur128 library is included in libavfilter itself. Thus we
require either a recent enough libavfilter version or the `--enable-libebur128`
option.
This test caused other tests to fail due to missing cleanup.
Configure the replaygain analysis by passing arguments to the Backends. This avoids the difference between ReplayGain and EBU r128 backends; every Backend can now fulfil both tasks. Additionally it eases Backend development as the difference between the two tag formats is now completely handled in the main Plugin, not in the Backends.
Allow to configure the target level for R128_* tags separately from REPLAYGAIN_* tags and skip peak calculation for R128_* tags if possible.
Assert that analysing the same track with different target levels yields different gain adjustments.
Assert that the replaygain plugin does not write REPLAYGAIN_* tags but R128_* tags, when instructed to do so. This test is skipped for the `command` backend as it does not support OPUS.
- document `r128_targetlevel` - explain difference between `targetlevel` and `r128_targetlevel` - deprecate `method` option: use `targetlevel` instead.
Add changelog entries for the introduction of the `r128_targetlevel` configuration option, superseding the `method` option.
zsinskri
force-pushed
the
r128-targetlevel-config
branch
from
d0b9749
to
da602d7
Compare
|
Rebased onto master. |
|
#3056 is merged. This is now ready for review. |
sampsyo
reviewed
Jul 25, 2019
Apply improvements suggested in GitHub PullRequest beetbox#3065: - be idiomatic - 0 is falsy - check enum equality, not identity - mutate list by constructing a new one - improve documentation - fix a typo - do not mention deprecation of a config option
|
Awesome; thank you! This is a heroic effort that required lots of special domain expertise, so I wanted to offer extra appreciation for getting this right. ✨ |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Remove REPLAYGAIN_* vs R128_* tag handling from the Backends, restrict it to the main plugin logic. That is archieved by passing the target loudness level to the backend with every album/track. It also necessitates that all Backends properly implement the target level.
As the different tag formats call for different default target levels this also seperates the configuration of those levels.