Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Respect sustain pedal for playback #2

Closed
wants to merge 1 commit into from
Closed

Conversation

e7mac
Copy link

@e7mac e7mac commented Jan 12, 2021

Adding functionality similar to the apply_sustain_control_changes function. This enables the player to respect the sustain pedal for playback, while retaining the correct offTimes visually

@Bewelge
Copy link
Owner

Bewelge commented Jan 17, 2021

Hey there,
thank you for your contribution. This is the first time someone has contributed to a private project of mine, very exciting :-)

However I haven't decided on a license for the project yet, so I'm hesitant to accept any contributions without clearing that up first. I've heard that setting a license with multiple contributors can become complicated.

Now to the changes:
You are reacting to the control-events only on a "per-track" basis. It's my understanding that if a sustain event appears on any track, it will affect all other tracks. Since that's the same logic as the setTempo control events, I simply added the parsing of sustains into the same method in MidiLoader.js

I also didn't want to render notes "as long" (or high) as they are being sustained, because I want to keep the rendered notes as how they would appear in musical notation. Instead of overwriting the .noteOff property, I now save all the sustain-events and set a .sustainOnTime and .sustainOffTime on each note in Song.js.

That way it's easier to control both the audio and rendering aspect of the sustains. I've just pushed an update in which I've added a simple sustain effect in the Player class. It's also now rendering the sustained notes "underneath" the actual notes with decreased opacity. You can change the opacity in the settings.

Let me know what you think or if there's anything else.

@e7mac
Copy link
Author

e7mac commented Jan 17, 2021

No worries! You've made it way better anyway with the visual improvements so this PR is moot!

Do you hear the "clicking" sound when notes are turned off? Even in Mozart's Rondo alla Turka that is happening. Maybe the exponential ramp needs to go to a lower value before turning off possibly?

@e7mac e7mac closed this Jan 17, 2021
@Bewelge Bewelge mentioned this pull request Jan 18, 2021
@e7mac e7mac deleted the sustain branch January 31, 2021 10:16
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants