LMMS / lmms

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

High Quality Pitch Scaler + Hard Limiter results in artefact #4090

Open musikBear opened 6 years ago

musikBear commented 6 years ago

D.Ipsum reports this on forum:

1) open LMMS 2) add a Hard Limiter on the master 3) add 2 Vibed in the Song Editor 4) put some notes in the Piano-Roll so that both Vibed play at the same time 5) add a High Quality Pitch Scaler in the FX tab of both Vibed

When two instances of High Quality Pitch Scaler play at the same time, it generates a horrible noise (it's the same with TripleOsc ... I did not try on other synths)

6) Disable one of the two High Quality Pitch Scaler

When both Vibed play at the same time (so with only one High Quality Pitch Scaler), everything is fine.

Here with 3oc: issue_HardLimiterPitchScaler.txt

DeRobyJ commented 6 years ago

Tested

Actually, when two Higher Quality Pitch Scaler are active, they will produce strage sounds even if played alone.

So this looks like Higher Quality Pitch Scaler relies on some fixed memory locations that both instances try to use.

its source should be this https://github.com/swh/ladspa/blob/master/pitch_scale_1194.xml

I guess it's not really our bug...

PhysSong commented 5 years ago

SWH plugin's pitch_scale function uses static buffers and FFT plans. It means sharing the library instance between plugin instances means they'll share those static variables. However, LMMS processes effects using multiple worker threads. Due to sharing static variables, it's thread unsafe.

tresf commented 5 years ago

@PhysSong I have some questions:

  1. Is this our bug?
    • If this IS our bug is it worth fixing?
      • If it IS worth fixing, what needs to be done to the code to prevent unsafe thread access the swh High Quality Pitch Scaler plugin?
      • If it is NOT worth fixing, can we close as wontfix with a proper explanation?
    • If this is NOT our bug can we file a bug upstream?
PhysSong commented 2 years ago

Is this our bug?

SWH's pitch_scale implementation doesn't take care of multiple instances and thread safety, so I'd say no.

tresf commented 2 years ago

Is this our bug?

SWH's pitch_scale implementation doesn't take care of multiple instances and thread safety, so I'd say no.

Thanks. Is it easy and possible to patch the static buffers upstream?

PhysSong commented 2 years ago

It is possible, but I'm not sure whether it's easy. I'm pretty sure if won't be difficult, though.