Actively playing notes do not stretch properly when the tempo is changed

ryuneo9 commented 5 years ago

I am using version 1.2.0 (Stable Versions) (windows 10 64bit)

The sound is broken when change it using automation tempo / bpm.

(ex) The first note that starts when change from 60 to 54 is broken.

The instrument uses piano vsti.

Maybe it's a bug?

I attached the lmms file and the mp3 file.

bug.zip

add. )

Kontakt instrument vsti is broken too

But sound font instruments are not broken. Lmms built-in instrument (TripleOscillator) is not broken. It seems to be related to vsti.

And it is not broken when tempo is less than 15(Change value)

The rc8 version is not broken. But not perfect.

musikBear commented 5 years ago

Maybe it's a bug?

ya, kind of, but if you analyse the event, you would realize that if a tempo change happens discrete inside a note, then an artefact is almost certain The 'solution' is to use a non-discrete shape! -But over very short time. You will not hear that, but it give LMMS the necessary time to respond

SecondFlight commented 5 years ago

This may be specific to the VST you're using. Regardless of whether the issue is originating in LMMS or in the VST, I suspect it's happening because the note off signal from the first set of notes happens after the note on from the second set. Either way, I don't think replacing the sudden jump with a linear ramp, as @musikBear suggests, will fix the issue.

Can you reproduce the same issue with TripleOscillator (or another built-in synth)? I'd test it myself, but I'm at work.

ryuneo9 commented 5 years ago

Kontakt instrument vsti is broken too

But sound font instruments are not broken. Lmms built-in instrument (TripleOscillator) is not broken. It seems to be related to vsti.

And it is not broken when tempo is less than 15(Change value)

The rc8 version is not broken. But not perfect.

SecondFlight commented 5 years ago

Thanks for looking further into this. #3640 seems related. I can confirm that this is due to an artificial overlap due to the tempo change occurring either in the middle of a playing note or on the boundary between two notes. LMMS doesn't appear to handle tempo changes on notes that are currently playing.

DomClark commented 5 years ago

LMMS does support tempo changes for notes that are currently playing, as evidenced by testing with TripleOscillator. Before 1.2, this was broken for tempo changes just as a note starts (see #4928), which is probably the issue observed with rc8. The issue here is with the note overlap.

Here are the MIDI messages sent to the plugin in the provided project (buffer size of 256 at 44100Hz):

Offset in buffer  Byte 1  Byte 2  Byte 3
             000      90      3c      3f
             000      90      40      3f
             000      90      43      3f
----
             068      90      3c      3f
             068      90      40      3f
             068      90      43      3f
             076      80      3c      00
             076      80      40      00
             076      80      43      00
----
             172      80      3c      00
             172      80      40      00
             172      80      43      00
             172      90      3c      3f
             172      90      40      3f
             172      90      43      3f
----
             020      80      40      00
             020      80      3c      00
             020      80      43      00
             020      90      3c      3f
             020      90      40      3f
             020      90      43      3f
----
             112      80      3c      00
             112      80      40      00
             112      80      43      00

The issue is with the second group of messages, where the note-on messages are sent at frame 68 and the note-off messages are sent at frame 76. For each buffer, automation is processed before any notes. The tempo change is at frame 68, from 60bpm to 54bpm, but LMMS doesn't support sample-exact automation for tempo, so this is applied to all currently playing notes. The effect is that the notes which are supposed to end at frame 68 now end at frame (60/54)*68=76, and the note-off events for these cancel the notes that begin correctly at frame 68.

DomClark commented 4 years ago

Ways this could be addressed:

Sample-exact tempo automation. Ideal long-term solution, and we would probably like this anyway. Not suitable for targeting stable-1.2.
Apply tempo change before new notes are processed. Would mean instrument and automation tracks would be processed by different amounts, which could cause problems. Would also require fiddling with Song::processNextBuffer, which I find confusing enough as it is. I don't like this option.
Ignore tempo changes for notes ending during the current buffer. Easy to implement and probably a safe change. If this turns out to work, I will open a PR for it against stable-1.2 unless anyone has any other ideas or anything else to say.

JohannesLorenz commented 4 years ago

This sounds like a core issue, and it should affect all MIDI plugins. Why, however, can't I reproduce this for built-in ZynAddSubFX?

JohannesLorenz commented 4 years ago

I'll look for a fix now.

JohannesLorenz commented 4 years ago

If we "ignore tempo changes for notes ending during the current buffer" (3rd proposal from @DomClark ), doesn't this only fix a special case? Imagine in the above example, the notes were shorter, e.g. 1/8, but the tempo was changed to less than the half, wouldn't the notes still overlap?

So maybe we should ignore tempo change for any running notes?

musikBear commented 4 years ago

So maybe we should ignore tempo change for any running notes?

Agree! rethink this, as if it was analogue: An analogue band would look at this situation in a complete different context - To be able to 'change tempo' while a note from any performer is playing in a former tempo, that mean that the other performers ques would come in a different timing. They would go from simple to compound-time In a DAW you would change the number of notes played / beat. And that is actually how it should bee! Changing BPM is not really musically correct. Going from simple-time to compound-time is, but that is not a DAW-technical trick, that is a composer action. The composition need to b changed, not the BPM setting. Does anyone know how other DAWs handles this?

DomClark commented 4 years ago

This sounds like a core issue, and it should affect all MIDI plugins. Why, however, can't I reproduce this for built-in ZynAddSubFX?

It's because of 5edaea4daa4214406ac4dd80675a4907ec86a53b, which sorts MIDI events into chronological order before delivering them to VST plugins. Even though the event timings are still incorrect for Zyn, it processes the events in the order given and thus the issue doesn't arise.

If we "ignore tempo changes for notes ending during the current buffer" (3rd proposal from @DomClark ), doesn't this only fix a special case? Imagine in the above example, the notes were shorter, e.g. 1/8, but the tempo was changed to less than the half, wouldn't the notes still overlap?

LMMS handles notes that overlap by more than a buffer (specifically when the new notes start in a different buffer to that where the old ones end) correctly; a note-off followed by a note-on is sent for the new notes, and the end of the old notes is ignored. This is done simply by maintaining a counter for each pitch, incremented when a note starts and decremented when a note ends. Increment from 0->1 sends note-on, any other increment sends note-off note-on, and decrement from 1->0 sends note-off.

This is broken for overlaps within a single buffer since 461faccaa0d9399e137e471d5bfa0f2b43434d9b, because that commit causes MIDI messages for new notes to be processed after old notes, which means the counter doesn't detect the overlap. (We get 1->0 and send the incorrect note-off, then 0->1 and send note-on. Previously we would have 1->2 and send correct note-off note-on, then 2->1 and do nothing.) Reverting that commit is an option, but that would reintroduce #3919 and #4826.

So maybe we should ignore tempo change for any running notes?

That's a major change in behaviour, arguably with a wider impact than this issue and harder to work around.

JohannesLorenz commented 4 years ago

I don't understand what you are saying. Anyways, as

No one wants to fix this
This is not crashing/losing data/damaging speakers etc.
A simple workaround is to shorten the notes a bit

this issue is not critical. I'll un-milestone this from 1.2.

LMMS / lmms

Actively playing notes do not stretch properly when the tempo is changed #5122