Tonejs / Tone.js

A Web Audio framework for making interactive music in the browser.
https://tonejs.github.io
MIT License
13.41k stars 976 forks source link

Unable to loop midi - **I'm probably missing something** #959

Closed rhelsing closed 2 years ago

rhelsing commented 2 years ago

Describe the bug

I am trying to loop a midi file I have loaded and it only plays once:

To Reproduce

function onFileLoad(elementId, event) {
  var fileContent = event.target.result
  const midi = new Midi(fileContent)

  currentMidi = midi

  const synths = [];
  const now = Tone.now();
  currentMidi.tracks.forEach((track) => {
    //create a synth for each track
    const synth = new Tone.PolySynth(Tone.Synth, {
      envelope: {
        attack: 0.02,
        decay: 0.1,
        sustain: 0.3,
        release: 1,
      },
    }).toDestination();
    synths.push(synth);
    synth.sync();

    track.notes.forEach((note) => {
      synth.triggerAttackRelease(
        note.name,
        note.duration,
        note.time + now,
        note.velocity
        );
    });

  });

  Tone.Transport.bpm.value = 120
  Tone.Transport.loop.value = true
  Tone.Transport.start()

}
tambien commented 2 years ago

i think this would probably work if you changed a couple of lines:

Instead of note.time + now, just note.time since you're synced to the Transport timeline therefore scheduling things relative to that time (which starts from 0) instead of the AudioContext clock which is what 'now' is referring to.

track.notes.forEach((note) => {
      synth.triggerAttackRelease(
        note.name,
        note.duration,
        note.time,
        note.velocity
        );
    });

Also you've got a typo with the loop value. Should just be Tone.Transport.loop = true. No need to add .value since it's not a schedule-able parameter like bpm.

rhelsing commented 2 years ago

@tambien appreciate it!

rhelsing commented 2 years ago

@tambien I don't want to take up much of your time, but I'm still having issues:

https://codepen.io/rhelsing/pen/Jjydxqp

When loop is set to true, the notes seem to keep layering upon themselves. Like the loop length is very short? Do I need to set the loop length somehow? Without the loop=true, it plays once in time on midi upload.

tambien commented 2 years ago

yeah you should set the Transport.loopEnd time to the last note + the last note duration. Otherwise the default loopEnd is i think 4 measure loop which is 8 seconds at the default 120bpm

rhelsing commented 2 years ago

@tambien Thank you, it does seemed to be timed correctly now, but there does seem to still be an additional layer that kicks in on each loop pass... and then I eventually hit a max polyphony error and notes drop. Its almost like the notes dont release. Do I need to destruct previous synths/notes/loops or something? Really appreciate your help 🤙

https://codepen.io/rhelsing/pen/Jjydxqp

Screen Shot 2021-10-13 at 4 03 54 PM
rhelsing commented 2 years ago

Almost like the duration of the notes become unbounded after the first loop maybe?

 track.notes.forEach((note) => {
            synth.triggerAttackRelease(
                note.name,
                0.3, //note.duration - unbounded after first loop?
                note.time,
                note.velocity
            );
          lastNote = note
        });

when I provide this static value to duration, it fixes the problem. I wonder if having the loop end exactly on or slightly before a note ends, that note carries on? How might I mitigate this? and force the notes to end as the loop begins again?

@tambien The bug seems to have to do with the release on the envelope + the note end landing after the end of the loop.. then the note remains on. I'd love to try to help solve this in the codebase. Could you point me in the right direction?

tambien commented 2 years ago

That other issue that you bring up seems to be related to #924. Basically since it loops just before loopEnd (i.e. < not ≤) the note end of the last note is never triggered.

Easy solution would be to add a small time value to the loopEnd time, maybe like: loopEnd = lastNote.time + lastNote.duration + 0.01