LMMS / lmms

Cross-platform music production software
https://lmms.io
GNU General Public License v2.0
8.16k stars 1.01k forks source link

Left channel is way higher than right channel #7400

Closed AW1534 closed 3 months ago

AW1534 commented 3 months ago

System Information

Windows 11, Steelseries DAC

LMMS Version(s)

1.3.0-alpha.1.665+g627209ad1

Most Recent Working Version

No response

Bug Summary

For some reason, the left channel is normal for a while, until my kicks and 808s come in? then all of a sudden the left channel skyrockets and starts crackling. If i change the mixer volume it stays unaffected.

patience - LMMS 1.3.0-alpha.1.665+g627209ad1 - [Song-Editor] 2024-07-27 23-14-39.zip

I am dualbooting windows and linux. this worked fine on linux (albeit my build was a week or two old) but as soon as I brought it to windows, it started having issues. Also, there were no errors when loading my project indicating it could be a missing plugin or anything of the sort.

This one is particularly bad, i guess because theres reverb on it

Like that remix_ - LMMS 1.3.0-alpha.1.665+g627209ad1 - [Song-Editor] 2024-07-27 23-32-25.zip

Expected Behaviour

the left channel should be around the same

Steps To Reproduce

Happens on any project i've tried

Logs

I'm sorry I don't know how to get the logs without running my own build. I can include them if you provide me with instructions

Screenshots / Minimum Reproducible Project

No response

Please search the issue tracker for existing bug reports before submitting your own.

michaelgregorius commented 3 months ago

@AW1534, what kind of effects do you have on the channels? Can you try to disable them selectively and individually until the problem goes away? That way we might find out if it is a plugin that behaves differently on Windows and Linux for some reason.

AW1534 commented 3 months ago

@michaelgregorius this happens even on channels with no effects, but funnily enough it didn't happen on my chant which also has no effects. Video demonstrating this: patience_ - LMMS 1.3.0-alpha.1.665+g627209ad1 - [Song-Editor] 2024-07-28 13-38-56.zip

AW1534 commented 3 months ago

I found that the volume on my chant is very low which i thought may be why its not affected by the same issue, so i set the channnel to 100% and it still has the expected behaviour

AW1534 commented 3 months ago

I have also confirmed it is not an issue with my audio files as i have loaded them into fl studio and they are fine

michaelgregorius commented 3 months ago

Does it only happen with sample tracks or also with instrument tracks? Commit https://github.com/LMMS/lmms/commit/2f5f12aaae8453863f62305cb54e4cfe779b661c made some changes with regards to the resampling code. I wonder if that might have introduced the problem, especially if it does not show on an older build.

LostRobotMusic commented 3 months ago

This appears to be the same bug as #7395, and another person just reported the same issue on Discord.

michaelgregorius commented 3 months ago

Yes, these look like very much the same issue.

Would be interesting to know if these issues can somehow be reproduced under Linux.

Also which version of libsamplerate is used for the Windows builds? In case it is caused by the aforementioned commit I wonder if it caused by different version of libsamplerate being used for Windows and for Linux builds.

Rossmaxx commented 3 months ago

Just in case because the library versions on windows came up, is this specific to MinGW builds or does MSVC builds suffer the same?

Rossmaxx commented 3 months ago

Fyi, MinGW builds use outdated versions of almost all libraries and vcpkg uses the latest, if anyone doesn't already know

michaelgregorius commented 3 months ago

Repeating a comment from #7395 here:

Using the info from the last known good version and the used bad one the commit candidates are (from high probability to low one):

The last two entries should have a very close to zero probability though.

sakertooth commented 3 months ago

Also which version of libsamplerate is used for the Windows builds? In case it is caused by the aforementioned commit I wonder if it caused by different version of libsamplerate being used for Windows and for Linux builds.

The CI seems to be using the libsamplerate version 0.1.9, while I am currently on the latest (0.2.2). They did mention that there were some minor bug fixes among other things, so this might be why.

If anyone that has a Windows machine can test to see if swapping out the old libsamplerate with the latest one will work, that'll be appreciated.

AW1534 commented 3 months ago

@sakertooth how do i do that

sakertooth commented 3 months ago

@sakertooth how do i do that

You could try compiling a new version of libsamplerate here and replacing it inside your installation directory for LMMS. I still have doubts if it will work though, but I'm not sure.

michaelgregorius commented 3 months ago

I propose to revert the changes made in commit https://github.com/LMMS/lmms/commit/2f5f12aaae8453863f62305cb54e4cfe779b661c for now and then to reimplement them with more thorough testing on Windows.

That way the nightly builds would be working again for our users without potentially blowing their ears off. :wink:

sakertooth commented 3 months ago

It's a shame honestly. Simple improvements turn into huge problems, then it has to be reverted. I wish the problem was with the changes themselves, but I cannot find a single problem with them. I don't know how the changes can be reimplemented, but I do know that I'm pretty sure I want to ditch libsamplerate entirely. If the problem is with upstream, I don't think we will get a fix any time soon.

It was discovered that it is only Linear mode that has a problem in the AFP, not Sinc, which complements the out of bound read for libsamplerate's linear resampling mode that doesn't seem like a fix will happen on time, and if it does, who knows when we will get that because of the outdated CI. Then again, it will be a miracle if the problem is with the changes themselves somehow.

messmerd commented 3 months ago

7316 updates libsamplerate to the latest release (0.2.2) for our Linux builds.

Rossmaxx commented 3 months ago

@sakertooth since you said the problem IS upstream, and you want to ditch libsamplerate eventually, why not start the efforts with this PR and roll out a custom linear interpolation function (or use whatever we already have)?

sakertooth commented 3 months ago

@sakertooth since you said the problem IS upstream, and you want to ditch libsamplerate eventually, why not start the efforts with this PR and roll out a custom linear interpolation function (or use whatever we already have)?

I said:

If the problem is with upstream, I don't think we will get a fix any time soon.

Emphasis on if. I don't know if it is because of libsamplerate or not, but I have reasons to believe it might be. I'm holding onto the chance that the changes themselves are the problem somehow, since that would mean an immediate fix can happen.

Rossmaxx commented 3 months ago

We can try a replacement for linear implementation tho anyway

sakertooth commented 3 months ago

And on wanting to ditch libsamplerate, yes, and it's not necessarily because of this issue alone (once again, I'm not completely sure libsamplerate is at fault here, but I find it likely).

libsamplerate is overkill for sample rate conversion IMO. What could be done with a one liner (i.e, linear interpolation, linearInterpolate) now requires this library that does so much more than what is really necessary for us.

I don't really understand why it works the way it does, but the fact that it requires us to keep track of state that has to be dynamically allocated is already confusing. In contrast, doing linear interpolation using linearInterpolate requires absolutely no extra state that has to be dynamically allocated and managed, which provides a simpler implementation.

It lacks Hermite interpolation, which we have, but use this library exclusively for resampling, so it's not an option.

The AFP's use of libsamplerate requires allocating a separate buffer, putting the correct input data in that buffer, and then resampling that. This is horribly inefficient for our purposes, since we should be avoiding the allocation altogether for real-time safety, and although I mentioned that switching to their callback API would help with this, looking at their source, it looks like they use the other API behind the scenes anyways. In contrast, we can do the necessary resampling and interpolation in place using our own interpolation functions. We don't have to do any allocation at all, we can just use the buffer we were given to fill with the correct input data only.

Then there is the problem of depending on upstream, which can be a bad thing if the library isn't battle tested or robust enough for production, but possibly a good thing otherwise if it has value. This is more of a general problem that can come with depending on libraries, not necessarily about libsamplerate in particular.

The only reason we have libsamplerate still is because we don't have a replacement for their Sinc interpolation yet. That's it. We need that to be backwards compatible.

Rossmaxx commented 3 months ago

So using libsamplerate for only sinc implementation might be fine ig. It might be extra work on your end but i believe it will have it's benefits. Regarding state management, why not do that too in place, instead of depending on libsamplerate?

sakertooth commented 3 months ago

@AW1534 What happens if you play the sample but don't let the sample finish playing?

YesImAsh commented 3 months ago

I'd just set it to sinc

sakertooth commented 3 months ago

@YesImAsh, since you reported the same issue on here, its worth asking you the same question. Does the left channel blow up if you just play the sample and don't let it finish playing?

YesImAsh commented 3 months ago

The left channel blows up, yes

AW1534 commented 3 months ago

@AW1534 What happens if you play the sample but don't let the sample finish playing?

@sakertooth The left channel stays high for a second and then goes back down patience - LMMS 1.3.0-alpha.1.665+g627209ad1 - [Song-Editor] 2024-07-30 16-32-41.zip

AW1534 commented 3 months ago

@sakertooth how do i do that

You could try compiling a new version of libsamplerate here and replacing it inside your installation directory for LMMS. I still have doubts if it will work though, but I'm not sure.

@sakertooth swapped out libsamplerate using a prebuild of 0.2.2 available on the github releases page but nothing has changed

sakertooth commented 3 months ago

Hopefully #7408 fixes this. It would make sense if it does, since at the last chunk of frames to play, the buffer may not be filled in entirely. If we don't zero out that part, its just undefined behavior.

I covered this edge case prior to that PR, but then I just removed it? I don't know.

I don't really understand how this library achieves sample rate conversion under the hood if it isn't just simple interpolation, so maybe all of what it does is necessary and we should keep using it? I don't know. I shouldn't make it seem like I was pointing fingers, but sometimes when you look at your code it takes a minute to see where you messed up. Until then, you're in disbelief.

sakertooth commented 3 months ago

So #7408 didn't fix the issue, so maybe the problem is actually with upstream. I think a revert of #7361 is the only viable solution for now.

messmerd commented 3 months ago

@sakertooth I was able to reproduce this on Linux via Wine. Both channels would max out in volume in the Mixer's peak indicator when playing a sample in AFP, and the left channel was the last to return to zero after the sample stopped playing. However, I did not notice any issues in how it sounded during my brief test - it was primarily an issue seen in the Mixer's peak indicator.

sakertooth commented 3 months ago

I was able to reproduce this on Linux via Wine. Both channels would max out in volume in the Mixer's peak indicator when playing a sample in AFP, and the left channel was the last to return to zero after the sample stopped playing. However, I did not notice any issues in how it sounded during my brief test - it was primarily an issue seen in the Mixer's peak indicator.

Yeah, I find this issue to be very strange. What version of Wine were you using? I'm using 9.14 and couldn't reproduce.

messmerd commented 3 months ago

Wine 6.0.3 on Linux Mint 21.2 (Ubuntu 22.04-based)

messmerd commented 3 months ago

I tried replacing libsamplerate-0.dll in the LMMS installation with the v0.2.2 DLL which I downloaded, and the issue remained.

AW1534 commented 3 months ago

I tried replacing libsamplerate-0.dll in the LMMS installation with the v0.2.2 DLL which I downloaded, and the issue remained.

@messmerd it shouldve been fixed by #7410

sakertooth commented 3 months ago

Yeah, I think we should revert and then work on replacing libsamplerate later.

musikBear commented 3 months ago

I'd just set it to sinc

No, that will cost cpu. Even with only a few samples you get spikes on avr. hardware

michaelgregorius commented 3 months ago

@sakertooth, I don't think it makes sense to replace libsamplerate with an own implementation. First of all, check the projects which make use of libsamplerate: https://archlinux.org/packages/extra/x86_64/libsamplerate/

You can for example find Ardour, CSound, Jack2, Muse, QTractor and Rosegarden in that list. Are they all using a bad library without knowing?

Also, sample rate conversion is a one liner if you want really crappy results. Here you can get an impression what's involved with resampling from 44,1 kHz to 48 kHz: https://dsp.stackexchange.com/a/67408

sakertooth commented 3 months ago

@michaelgregorius I realized what I said about libsamplerate wasn't exactly correct. For sample rate conversion in particular, I agree that using libsamplerate is fine. The problem lies in the fact that the AFP uses it to achieve playing the signal slower or faster depending on what note is pressed (i.e., the AFP uses Sample, which uses libsamplerate to achieve this, the same goes for anything else that uses Sample, like Patman), and after talking with @LostRobotMusic, using libsamplerate for this purpose may not be wise.

LostRobotMusic commented 3 months ago

@sakertooth, I don't think it makes sense to replace libsamplerate with an own implementation. First of all, check the projects which make use of libsamplerate: https://archlinux.org/packages/extra/x86_64/libsamplerate/

You can for example find Ardour, CSound, Jack2, Muse, QTractor and Rosegarden in that list. Are they all using a bad library without knowing?

Also, sample rate conversion is a one liner if you want really crappy results. Here you can get an impression what's involved with resampling from 44,1 kHz to 48 kHz: https://dsp.stackexchange.com/a/67408

libsamplerate is a library for converting audio from one sample rate to another. In cases where we want to change the sample rate of an audio signal, libsamplerate is a library that can be used for this.

This is not the case for AudioFileProcessor. Playing different notes in AFP is not a task of sample rate conversion, it is a task of fractional sampling. With its current feature set, we support playing the sample both forwards and backwards, looping at any set points, and changing its pitch to anything we want over time. To add onto this, in the future we'll also want to add support for granular pitch shifting, timestretching, and loop crossfades. All of these features call for fractional sampling of the audio sample at any location, not trying to contort and abuse a sample rate conversion library into achieving similar results. Incorrectly using this library in this way is what is resulting in comically low performance, a total lack of decent quality options (linear is unacceptably low-quality and sinc is unacceptably low-performance, with no in-between), and myriad of bugs and glitches (e.g. the horrible buzzing sound from shifting the pitch too quickly).

In addition (though none of this actually needs to be said due to the previous paragraph), 2/3 options libsamplerate provides are 1-liners, with Linear being covered entirely by linearInterpolate in interpolation.h and None being a no-op. It's a tad embarrassing that so much code and CPU power is being wasted on a whole process of piping AFP through an irrelevant library to perform something that could literally be done with one line of code. The (extremely CPU-heavy) Sinc option is the only mode providing any value here (though even that shouldn't be used for the reasons I mentioned in the previous paragraph).


None of the stuff below here is important to that initial point, it's just useful additional information.

It's important to point out that out of these three quality options, all of them are bad. Linear is a good high-performance default but has extremely poor quality. None uses a nearly identical amount of CPU and is several orders of magnitude worse in terms of quality, useful only when intentionally desiring a degraded sound (e.g. for chiptune or whatever). Sinc has nearly ideal quality, but completely tanks the CPU (playing one note at C9 fills up nearly my entire CPU meter), and results in a sinc-filter nearly-perfect frequency cutoff for the higher frequencies which is usually undesirable for percussion and other sounds that still need high-frequency content. A reasonable middle ground is not provided by this library (because it isn't even supposed to be used for this in the first place, but I already covered that).

Quoting "Polynomial Interpolators for High-Quality Resampling of Oversampled Audio" (commonly known as the "pink elephant paper"), at https://yehar.com/blog/wp-content/uploads/2009/08/deip.pdf: Also, it shall only be commented that using polynomial interpolators with unoversampled input is a choice that can only be made when the quality is not that important but speed is essential, the most useful interpolators in that case being linear and 4-point Hermite, and Watte tri-linear, which is somewhere between those two in both quality and computational complexity.

Here's a visual comparison when playing an audio file at a lower pitch: image

As can be seen, 4-point Hermite interpolation provides extremely high quality (Linear's artifacts being quadruple the amplitude), while also having very high performance. It needs to sample from 4 points, compared to Linear's 2 and Sinc's multiple hundreds. I've always used it as the default in my own personal programs and the results have been consistently excellent. It's also used in my Granular Pitch Shifter plugin which was just recently merged into LMMS. (Linear may still be a viable default over Hermite in AFP specifically because the large majority of AFP instances are mainly used to play samples at or near their original pitch, in which case the interpolation makes no audible difference and the higher-performance option should be preferred as a result.)


None, Linear, and 4-point Hermite should be implemented by LMMS itself, there's absolutely no excuse whatsoever for libsamplerate (or even a library that's actually relevant to this task) to be involved in any way for those. Ideally libsamplerate will eventually be able to be pulled out of AFP entirely once a backwards-compatible-enough Sinc interpolation implementation is added into LMMS, though that will be a difficult task since LMMS tragically interleaves its audio, preventing much-needed SIMD instructions from being used for that task.

michaelgregorius commented 3 months ago

@LostRobotMusic, thanks for that overview!

It would be interesting to know what kind of interpolation the samplers in other DAWs use, e.g. the one in Bitwig. Can you please provide me with the sample that you have used and at which pitch you have played it relative to the base pitch? Judging by the screenshots it might be something like three octaves lower? I would then check the spectrum that Bitwig's sampler gives for that sample (at default values).

You have already mentioned interpolation.h which includes several interpolation methods including Hermite interpolation and Lagrange interpolation. So it seems that AFP would simply need to be switched to use these. Perhaps it could even be made an option in AFP. Alternatively AFP could check if the base note is being played and use no interpolation in this case and in all other cases use something like Hermite interpolation.

I agree that it is unfortunate that LMMS uses interleaved audio. It's also sad that none of the operations have been abstracted away and that there is likely tons of code that indexes explicitly into the buffers and does interpolations and other operations explicitly.

With pull request https://github.com/LMMS/lmms/pull/7156 I have merged some first steps to change this. The sample frame is now a class (albeit still interleaved) which supports several operations. The next step could be to introduce a class like Buffer which would represent a block of samples with a known size. The core framework and the plugins should then operate on buffers instead of raw arrays of sample frames. The buffer class or some helper class could then for example provide methods which return a fractional sample using certain interpolations.

Put differently, it's a bit sad that LMMS does not really have dedicated DSP classes and the code is scattered instead of for example being collected in a DSP directory.

By the way, what does "None" do? Is it nearest-neighbor?

sakertooth commented 3 months ago

@michaelgregorius, None drops the fractional part. For example, if I wanted to interpolate the value 3.2, it would return the sample value 3.

michaelgregorius commented 3 months ago

That sounds even worse than nearest neighbor then. :thinking:

LostRobotMusic commented 3 months ago

Can you please provide me with the sample that you have used and at which pitch you have played it relative to the base pitch?

I can't remember, it was far too long ago. My recommendation is to use pure white noise (exported losslessly) and to play it downward by three octaves.

You have already mentioned interpolation.h which includes several interpolation methods including Hermite interpolation and Lagrange interpolation. So it seems that AFP would simply need to be switched to use these.

Yes. I recommend None, Linear, 4-point Hermite, and in the future, multiple power-of-2 window sizes for Windowed Sinc. I haven't done any tests of 6-point Hermite but it could potentially be another useful intermediate option. I don't think there's any reason to use any of the other options in that file for this purpose.

Alternatively AFP could check if the base note is being played and use no interpolation in this case and in all other cases use something like Hermite interpolation.

This unfortunately isn't possible. Interpolation would be required the moment a fractional sample index is needed under any circumstance, which is the case when there's a sample rate difference between the sample and the DAW, or when the pitch changes even slightly at any point in the note, or if the pitch is even slightly off from 440 Hz (e.g. when microtuning), and this number of situations will grow even larger as more features are added into the synthesizer.

I personally recommend using 4-point Hermite as the default.

michaelgregorius commented 3 months ago

Can you please provide me with the sample that you have used and at which pitch you have played it relative to the base pitch?

I can't remember, it was far too long ago. My recommendation is to use pure white noise (exported losslessly) and to play it downward by three octaves.

Thanks, I will try that! Looking at the spectrum I was already wondering if it is white noise or perhaps a single cycle saw.

Alternatively AFP could check if the base note is being played and use no interpolation in this case and in all other cases use something like Hermite interpolation.

This unfortunately isn't possible. Interpolation would be required the moment a fractional sample index is needed under any circumstance, which is the case when there's a sample rate difference between the sample and the DAW, or when the pitch changes even slightly at any point in the note, or if the pitch is even slightly off from 440 Hz (e.g. when microtuning), and this number of situations will grow even larger as more features are added into the synthesizer.

Yes, it would only work in situations where the sample could be played in consecutive samples as is. It does not make sense if the pitch is modulated in any way because then it's highly unlikely that we even hit the sweet spot where no interpolation is needed.

I personally recommend using 4-point Hermite as the default.

How does AFP currently perform when a full spectrum signal is pitched up? Using the interpolations mentioned above should lead to aliasing in these cases and I'm not sure how the existing libsamplerate implementation performs here.

michaelgregorius commented 3 months ago

Here's the spectrum of a white noise sample that's played three octaves down in Bitwig's sampler:

BitwigSamplerWhiteNoiseThreeOctavesDown

DAW is running at 48 kHz, the root note of the sampler is set to C5 and C2 is being played. The sample is a few seconds of white noise.

Comparing the curve to @LostRobotMusic's screenshots it looks like they are using a more relaxed sinc interpolation.

LostRobotMusic commented 3 months ago

How does AFP currently perform when a full spectrum signal is pitched up? Using the interpolations mentioned above should lead to aliasing in these cases and I'm not sure how the existing libsamplerate implementation performs here.

libsamplerate does nothing to suppress aliasing unless the Sinc mode is being used.

Comparing the curve to @LostRobotMusic's screenshots it looks like they are using a more relaxed sinc interpolation.

Your screenshot only goes down to -78 dBFS, as opposed to -102 dBFS in the images I sent. I recommend checking again with the full vertical range.

I'm curious, how much aliasing do you see with Bitwig's Sampler if you pitch a saw wave upward a few octaves? I should be able to know what aliasing suppression method they're using by looking at it.

michaelgregorius commented 3 months ago

@LostRobotMusic, here's the spectrum with -102 dbFS as the lower boundary:

Screenshot 2024-08-03 190333

And for completeness here with -145 dbFS:

Screenshot 2024-08-03 190450

Playing high note for a saw in the Sampler

The setup is the following. A single cycle wave of a Virus saw was loaded into Bitwig's sampler and the base note set to C-1 (as in "C minus 1"). The different octaves of C were played. Here C5-C8. I also have ones starting at C0 but I don't think they are that interesting.

Fundamental around 2 kHz: RootC-1PlayedC5

Fundamental around 4 kHz: RootC-1PlayedC6

Fundamental around 8 kHz: RootC-1PlayedC7

Fundamental around 16 kHz: RootC-1PlayedC8