paplayer: restore option for replaygain reduction as clipping protection

[DaveTBlake]

There have been a number of user reports of replaygain not working since #10924 that fixed the issue of replay gain not being applied if peak value was missing, and also removed the avoid clipping setting. See http://forum.kodi.tv/showthread.php?tid=296751&pid=2517978#pid2517978

The regression is that using stream amplification rather than setting replaygain whenever peak is not 1.0 does not result in the required change in volume (users say there is no audible change)

This is fixed by #11868 and this PR depend up that one being merged.

On reading http://wiki.hydrogenaud.io/index.php?title=ReplayGain_1.0_specification I believe that the "reduced gain" approach to clipping protection, as previously implemented, should be restored along with the AvoidClipping setting that gave the users this option.

This reverts those aspects of that PR, but keeps the fix of the original user issue.

[FernetMenta]

The regression is that using stream amplification rather than setting replaygain whenever peak is not 1.0 does not result in the required change in volume (users say there is no audible change)

Did you verify this? Sounds strange bcause both methods do the same thing. The only difference is that stream amplification has clipping protection.

[DaveTBlake]

Yes, users claimed that unless peak was 1.0 they heard no change, and I found the same in my own testing.

I was unable to see what setting the stream replay gain did compared to setting amplification to the replay gain value, but si->m_stream->SetAmplification(gain) does not have the same audible result as si->m_stream->SetReplayGain(gain)

I modified the code explicitly to confirm, and used gain levels that would not need clipping

[DaveTBlake]

A test build is available on the mirrors e8114f0

[FernetMenta]

but si->m_stream->SetAmplification(gain) does not have the same audible result as si->m_stream->SetReplayGain(gain)

if this is the case, there is an issue that needs to be fixed. can you share a sample you have used for testing?

[DaveTBlake]

if this is the case, there is an issue that needs to be fixed. can you share a sample you have used for testing?

I just added replaygain and peak tags to my own music files.

[DaveTBlake]

I would add too, that it seems right to me to offer an option to avoid clipping on replaygained files by reducing gain rather than "audio limiting" which is I assume how stream amplification protects against clipping.

For testing apply a very audible -20Db reduction, and compare SetAmplification(gain) to SetReplayGain(gain). Since this is a reduction in loudness peak value should have no impact (unless for the track is it was >10)

[FernetMenta]

this is imo the better fix: FernetMenta@efbb55a

bringing back clipping protection by a setting has imo no value for the user. the limiter of AE is the more sophisticated approach.

the actual problem here was that I forgot to consider rgain values < 1.0

[FernetMenta]

avoid clipping on replaygained files by reducing gain rather than "audio limiting"

AE's clipping protection reduces gain in areas where clipping would occur and not for the entire file. Reducing rgain for the entire file is imo wrong. Why should s.o. set rgain and peak to values that we change based on some setting?

[DaveTBlake]

That does not seem to work with rgain < 1.0 and peak > 1.0 either. Are you sure setting rgain is same as setting amplification to rgain?

"Audio limiting"
Does the clipping protection AE now has sound different to using a gain reduction approach? It must do. Users have generated rgain and peak values using certain algorthims and want Kodi sound levels to vary in the same way as these values are used by other players. No doubt the more sophisticated approach AE has is "better", but sadly that does not mean that all users want that.

[FernetMenta]

If users have calculated rgain to match their needs, they certainly don't want it to be changed by this silly avoidClipping setting. Do you agree?

[DaveTBlake]

I don't use this at all so my opinion is moot, but I found the HydrogenAudio article about rgain quite persuasive. It proposes gain reduction as a valid clipping alternative to audio limiting. Some users seem to agree see http://forum.kodi.tv/showthread.php?tid=296751&pid=2550466#pid2550466

Would it be so bad to have the option to avoid clipping by reducing gain across the file (if that is the sound effect you want) rather than have AE cleverly reduce gain just where clipping would happen. This has different sounds.

[FernetMenta]

AE's clipping protection is a protection, means it only does something if sound would clip otherwise. The only reason why it is not always engaged is that it requires additional processing power. So clients of AE let AE know in advance if protection is required.

If we re-add the avoidClipping setting AE's clipping pretection still has to be engaged if clipping may occure. That is in case of peak * gain > 1.0 because user has not enabled the setting.

[DaveTBlake]

So on a low power system, would not clipping protection of at least those replaygained files via rgain reduction therefore be useful?

But also I can see that is AE's clipping protection is enaged, then when "replaygain reduction" is not enabled the file should still take the set amplification route to prevent clipping.

[DaveTBlake]

Would this not be a valid solution in paplayer
if (peak * gain <= 1.0)
si->m_stream->SetReplayGain(gain);
else if (replayGainSettings.bAvoidClipping)
{
// prevent clipping by gain reduction across all file
si->m_stream->SetReplayGain(1.0f / fabs(peak));
}
else
// protect clipping using audio limiting
si->m_stream->SetAmplification(gain);

[DaveTBlake]

For an immediate fix I have submitted the small change you suggested as #11868 (and backported).

This PR then becomes a discussion of if we want to have replaygain reduction as a clipping protection option.

Unfortunately the changes on my repo DaveTBlake@7e03f8e are not showing here, not sure why?

[FernetMenta]

based on the fix re-adding the option would look like this (slightly modified this change here): FernetMenta@24991c0

Maybe change the name of the setting to reflect what it actually does. Actually it does not avoid clipping (this is done by AE), it normalises volume by reducing rgain.

[DaveTBlake]

Agree on setting name.

How about paplayer change like this master...DaveTBlake:ReplayGainFixLeia#diff-93bb7e1d3fde102162d12dc19141ffb4R499

(just can't get this PR to follow my repo anymore - Ah ha, a case sensitivity thing!)

[FernetMenta]

How about paplayer change like this

yes, having the setting handled by paplayer is ok too.

[MartijnKaijser]

jenkins build this please
Was this ready for merge?