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

Loaded file is skipping the first chord. #231

Open
fatdzo opened this issue Mar 3, 2018 · 4 comments
Open

Loaded file is skipping the first chord. #231

fatdzo opened this issue Mar 3, 2018 · 4 comments

Comments

@fatdzo
Copy link

fatdzo commented Mar 3, 2018

Greetings,
first of all thank you for developing this library, it is awesome and I like it a lot. One of the problems I ran into is that the MIDI file loaded with the loadFile() function and played with start() is not the playing the first chord, it is playing the first note but not the chord that is backing it. I am using MidiJS in combination with AngularJS.

It is a bit hard for me to explain in words so I have put the software I have been working online. It is still work in progress but enough to explain the issue.
Go to http://melodybrains.azurewebsites.net/ and hit the generate button. Sometimes you have to hit the Generate button twice, that is a bug on my side, don't worry about it too much.
Once the melogy is generated wou will be presented with a some buttons:

  1. Play button is playing the loaded MIDI file. Every time I use that button it is not playing the first chord. I don't do any crazy struff in code I just load the file and start it onsuccess.
    $scope.player.loadFile($scope.midiLocation, function () { $scope.player.start(); });
  2. the Play2 button is transforming the notes into noteOn() and noteOff() events. This one playes all the notes correctly.
  3. the Download button downloads the generated MIDI file. When I load it into my DAW everything looks and sounds ok, and all notes are played.
@gklyne
Copy link

gklyne commented Oct 5, 2018

We've run into a similar problem with some work we're doing.

I've also encountered some problems playing the first notes of a sequence using the note/chord level interface. I've created some test code, which is adapted from the Basic.html example file, that exhibits this problem - see https://github.com/oerc-music/nin-remixer-public/tree/master/spike/midijs/tests. Using Firefox or Safari on MacOS 10.11.6, and maybe other systems.

There are two test files:

Test 1 (midijstest1.html). Loads the MIDI plugin on window load, and immediately starts playing some notes. There is a configurable delay to start (about line 57) - if this is set to long enough, say 3-4 seconds, then the notes play fine. But for smaller values, some get dropped or mis-timed.

Test 2 (midijstest2.html). Similar to test 1, except that playing the test sequence doesn't start until a "Play" button is clicked. Similar results to above, except that on second and subsequent clicks of the play button, all is fine.

UPDATE: if I wait long enough before starting the first playback then that is OK too. See next comment.

@gklyne
Copy link

gklyne commented Oct 17, 2018

After a little more digging, I think I see the problem.

File plugin.webaudio.js has the following code at about line 72:

		/// convert relative delay to absolute delay
		if (delay < ctx.currentTime) {
			delay += ctx.currentTime;
		}

Similar logic appears about lines 131 and 188.

The audioContext.currentTime value that is being tested starts from zero when it is initially created (e.g. when a page is loaded). (This accords with the web audio spec per https://www.w3.org/TR/webaudio/#dom-baseaudiocontext-currenttime) Until the currentTime reaches a value that is greater than the length of music being cued to play, some of the note delays are incorrectly treated as absolute delays, which results in them being played at the wrong time. I claim this is a bug in the relative/absolute logic used. Not being able to find a clear specification of how the midijs interface is expected to behave, it's not obvious what would be a correct fix. My inclination would be to treat all delays as relative and drop the if (delay < ctx.currentTime) test.

A workaround with the current code is to submit only absolute delay values by adding MIDI.getContext().currentTime to all submitted delay values.

@gklyne
Copy link

gklyne commented Oct 17, 2018

Reflecting...

A problem with my first idea for a fix is that it could introduce time-skews if the notes are added over a period of time.

The workaround suggested above remains valid as long as the all the values added are greater than MIDI.getContext().currentTime when they are added, but if currentTime marches on and overtakes the note delay values things could go wrong again. Better, maybe, would be to:

  1. call MIDI.getContext().suspend()
  2. obtain a value MIDI.getContext().currentTime to be added to the delay value for all note events generated
  3. add note events to audio context (MIDI.noteOn(), etc.) (with the saved value of currentTime added to the note delays).
  4. call MIDI.getContext().resume()

(This hasn't been tested yet.)

@jpnp
Copy link

jpnp commented Oct 24, 2018

See also #165 and the associated PR #227

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

No branches or pull requests

3 participants