moinejf/abc2svg

play: onend() does not fire if play is aborted via stop()

Closed this issue · 5 comments

bwl21 commented

I have implemented stop such that the play button becomes active after the music really stopped. When user hits the stop button the events already being queued still fire.

But now after all I do not get the onend() fired after the last event being queued was fired. onend() fires if the music comes to its end (without being stopped manually).

It was working with 1.15.8

I did not change anything about onend() since 1.15.8. There should be a bug of yours.

bwl21 commented

The problem was introduced in commit 1b6564a

As you fire endplay() immediately when user hits the stop button, you cannot notice that onend() does not fire if play is aborted. If you you can try this with edit-1.xhtml if you comment out line 556 in edit.js

	if (playing) {
		abcplay.stop();
		//endplay()   <- let onend() switch the button text
		return
	}

As the player does not stop immediately (it plays the already queued events), I am running the following approach:

  • user hits the play/stop - button: -> Button changes to "stopping"; and abcplay.stop(); is called
  • when abcplay has played all the queued events, it fires onend, then Button changes back to "play"

I think the root cause is that you clear timeout in line toaudio.js line 580. therefore play_next is not invoked anymore after calling stop(), and therefore has no chance to fire onend(),

Solution could be in line 462 to assign the result of
timout = setTimeout(play_next, (t + stime - ac.currentTime)

not to timout

bwl21 commented

sorry, ba892a3 does not fully solve the problem. I tried in editor-1.js. When I click on "stop", the button changes to "play" immediately, even if the player still fires the note_on events.

I guess we should not cancel all the timeouts such that at least one more call to play_next happens. play_next emits onend() if there is nothing more to play.

I implemented my proposal and it works. In contrast to 1.15.8, the sound now stops immediately, but the "follow" events still happen, so one can see the walking highlights in the notes. After these events are all done, then "Stop" becomes "Play".

You are right. The fix was too easy...
Thanks.

bwl21 commented

Now it works. Thanks. I think it is an improvement, that stop() imeditately causes silence, even if there are some "follow" events to fire.