Forced clipping issues

[zonkmachine]

As commented here, since #3706 was merged old projects may sound differently. The forced clipping is by design and necessary for the rest of that PR to function but may need some tweaking.

We can bump the clipping level to maybe +/-10 or so.

lmms/src/core/MixHelpers.cpp

Line 87 in fbdd09d

src[f][c] = qBound( -4.0f, src[f][c], 4.0f );

Any other ideas?

@Jousboxx Can you comment on the difference reported on your track, BuzzerBeater in 1.2.0-rc6 or later?

[aerobinsonIV]

There isn't much audible change in Buzzer Beater on 1.2 rc7, but FX4 peaks at +14db, so I'm guessing it isn't audible just because there is also a hard limiter (basically clipping) on the master.

I just read through the thread you linked, and I can see why clipping the FX channels would help remove NaN/inf values. However, I think it should be optional and not on by default. I've seen a whole bunch of projects (including my own) from less experienced producers that heavily rely on the mixers not clipping. Even though it isn't good practice to go over 0db, it's really easy to just turn something up if it sounds too quiet rather than turning everything else down, and that's what a lot of people do. Eventually you end up with red mixers everywhere. To demonstrate this, here's a video of one of my brother's projects after about 2 years of producing: https://youtu.be/jq4A6cWw7o8. He doesn't have any FX channels, but the idea of turning stuff up still applies, and he has other similar projects that do use FX channels. He made several songs like that that sounded quite good with the limiter on, and he's only recently started using proper mixing techniques.

If the 10db clipping was on by default, a lot of noobs would probably think that LMMS "just sounds bad", or they might not even notice and end up with clipping in their final mix.

In my entire time using LMMS I've never had any problems with NaN/infs except in projects that were sent to me explicitly to demonstrate them. This is by no means a common issue! I think the option should just be available so that if someone gets really unlucky and has them, they can still use their project.

[zonkmachine]

However, I think it should be optional and not on by default. I've seen a whole bunch of projects (including my own) from less experienced producers that heavily rely on the mixers not clipping. Even though it isn't good practice to go over 0db

If this was made an option I'd rather see it on by default and then the kids can advice each other on Discord how to switch it off.

@JohannesLorenz It's actually by design. I understand fully if people see this as a bug though and you can label it as you see fit.

I don't have any other suggestion here other than bumping the clipping level a bit.

[JohannesLorenz]

@zonkmachine I labeled it as bug, as it's about clipping, plus it's a compatibility issue. I don't have knowledge about the topic though, feel free to change.

In the end, every issue should be labeled either bug or enhancement (IMO) as it may be necessary to know how many bugs we have in total.

[zonkmachine]

https://github.com/LMMS/lmms/blob/dd6d4a552b3e1c51fc39db7c0cfc19d3f33908ef/src/core/MixHelpers.cpp#L72:L92

What if, instead of setting the actual sample to 0.0f, we go back and clear out the whole buffer?
Maybe we can cut off most of the odd values leading up to inf/nan without having to clip the output?

	for( int f = 0; f < frames; ++f )
	{
		for( int c = 0; c < 2; ++c )
		{
			if( isinff( src[f][c] ) || isnanf( src[f][c] ) )
			{
				src[f][c] = 0.0f;
				found = true;
			}
			else
			{
				src[f][c] = qBound( -4.0f, src[f][c], 4.0f );
			}
		}
	}

to

	for( int f = 0; f < frames; ++f )
	{
		for( int c = 0; c < 2; ++c )
		{
			if( isinff( src[f][c] ) || isnanf( src[f][c] ) )
			{
				for( int f = 0; f < frames; ++f )
				{
					for( int c = 0; c < 2; ++c )
					{
							src[f][c] = 0.0f;
					}
				}
				found = true
				return found;
				break;
			}
		}
	}

[zonkmachine]

What if, instead of setting the actual sample to 0.0f, we go back and clear out the whole buffer?

That actually worked brilliantly!
I'm preparing a PR with this change. If we find a nan/inf, we declared the whole buffer bad and set it to 0.0f. I'm still clipping the signal, for now at least, but set the level at +/-100.f .

Sample output with project envelopeNaN.mmp from #3706. Every line being the output from one project reload/play. Sometims two consecutive buffers would be cleared generating two lines. An audible 'tick' was generated each time.

Bad data, clearing buffer. frame: 1: value -inf
Bad data, clearing buffer. frame: 1: value inf
Bad data, clearing buffer. frame: 1: value inf
Bad data, clearing buffer. frame: 1: value inf
Bad data, clearing buffer. frame: 75: value -inf
Bad data, clearing buffer. frame: 1: value -inf
Bad data, clearing buffer. frame: 1: value inf
Bad data, clearing buffer. frame: 1: value -inf
Bad data, clearing buffer. frame: 165: value inf
Bad data, clearing buffer. frame: 1: value inf

/*! \brief Function for sanitizing a buffer of infs/nans - returns true if those are found */
bool sanitize( sampleFrame * src, int frames )
{
	bool found = false;
	for( int f = 0; f < frames; ++f )
	{
		for( int c = 0; c < 2; ++c )
		{
			if( isinff( src[f][c] ) || isnanf( src[f][c] ) )
			{
				printf("Bad data, clearing buffer. frame: ");
				printf("%d: value %f\n", f, src[f][c]);
				for( int f = 0; f < frames; ++f )
				{
					for( int c = 0; c < 2; ++c )
					{
						src[f][c] = 0.0f;
					}
				}
				found = true;
				return found;
			}
			else
			{
				src[f][c] = qBound( -100.0f, src[f][c], 100.0f );
			}
		}
	}
	return found;
}

[zonkmachine]

Here is a soundclip with noise from a loud transient hitting a reverb with different clipping levels for comparison. In order the clipping levels are: +/-10 (current level), +/-100, +/-1000, and unclipped noise.
vynilnoise.ogg.zip ps. loud...

@Jousboxx There will be a setting to disable this altogether: #4787

[aerobinsonIV]

Good to know. My opinion is that the setting to clip should not be hidden, and it should be off by default. I'm just really worried about inexperienced users putting tons of volume through a mixer and then using an amplifier later to bring it back down, not realizing that the sound had been clipped. NaNs are such an uncommon issue that most people would never even have to know the setting existed, and those that did could just flip the switch.

In other words, I think having the setting on by default would cause more problems than it solves.

[Spekular]

If anything, I think disabling this protection is a feature for more advanced users, rather than the other way around. We already have beginners asking why the sound is distorted as they clip the master channel, so telling people to turn their volume knobs down is nothing new.

…

On Mon, Jan 21, 2019, 22:47 Jousboxx ***@***.*** wrote: Good to know. My opinion is that the setting to clip should not be hidden, and it should be off by default. I'm just really worried about inexperienced users putting tons of volume through a mixer and then using an amplifier later to bring it back down, not realizing that the sound had been clipped. NaNs are such an uncommon issue that most people would never even have to know the setting existed, and those that did could just flip the switch. In other words, I think having the setting on by default would cause more problems than it solves. — You are receiving this because you are subscribed to this thread. Reply to this email directly, view it on GitHub <#4665 (comment)>, or mute the thread <https://github.com/notifications/unsubscribe-auth/AIgVmgJD4eJXiMOQhDDmTjRI2QR-CgOdks5vFjWNgaJpZM4XbO0M> .