Seek Behavior Changed in 4.2.0

[sundayz]

Seeking while playing audio now pauses. In the past, it would keep playing.

Wavesurfer.js version(s):

4.3.0

Code needed to reproduce the issue:

4.1.0 behaviour: https://codepen.io/sundays/pen/XWjZpxG

4.3.0 behaviour: https://codepen.io/sundays/pen/KKgQvwG

Use behaviour needed to reproduce the issue:

1. Click codepen for 4.1.0
2. Click play
3. Click on waveform to seek. It keeps playing.
4. Go to the codepen for 4.3.0, repeat the steps. The audio pauses when clicking to seek

The question is which is the correct behavior? Should seeking while playing audio cause it to pause? Or should it keep going? #2045 seems to have caused this.

Workaround to restore old behavior is to listen to the 'seek' event, check isPlaying(), and call play() yourself.

[thijstriemstra]

ping @todoroff

[todoroff]

Yeah, I don't think it's supposed to be pausing either.

From what I can tell, this seems to be a problem related to the play and seekTo implementations for the WebAudio backend and the way they interplay. This is how it expects things to roughly happen, skipping irrelevant details:

1. The user tries to seek
2. If the current state is playing, call pause (had that step before the change)
3. Call seekTo backend method with 1 argument.
4. The seekTo method for WebAudio sets a new startPosition.
5. If the state from Step 2 was playing, hence now is paused, call the backend's play method with no arguments. (had that step before the change, now the stuff below doesn't happen)
6. The play method for WebAudio then calls seekTo with no arguments.
7. seekTo calls getCurrentTime. The backend state is PAUSED, so in this case getCurrentTime returns the startPosition which seekTo previously set in Step 4. seekTo returns the start/end positions to play.
8. play starts the source by offsetting it with the new start position.

This implementation seems somewhat brittle to me, curious what you think.

P.S. Perhaps a temporary solution could be to revert the changes from #2045, but set a condition to skip those lines if the backend is MediaElement.

[sundayz]

Thanks for the in depth analysis. You're right, it actually doesn't seem to happen with MediaElement.

In my opinion, MediaElement shouldn't be extending WebAudio since it doesn't actually need the state machine that backend uses for the play/paused states, since the HTMLMediaElement already handles it. Changing that is more of a longer term goal though. When we have Typescript, perhaps it should look something like this:

interface AudioBackend {
    ...
}

class WebAudio implements AudioBackend {}
class MediaElement implements AudioBackend {}
class MediaElementWebAudio extends WebAudio {} // actually requires the WebAudio stuff

In the shorter term, I think your temporary solution would be fine.

[todoroff]

I think MediaElementWebAudio should be extending MediaElement (as it is now), since it's virtually the same, only feeding it into a webaudio node. Speaking of which, if we revert the changes from from #2045, it seems like those should be ignored by the MediaElementWebAudio backend as well.

[thijstriemstra]

I think MediaElementWebAudio should be extending MediaElement (as it is now), since it's virtually the same, only feeding it into a webaudio node. Speaking of which, if we revert the changes from from #2045, it seems like those should be ignored by the MediaElementWebAudio backend as well.

can all these changes be done without breaking backward compatibility?

[todoroff]

Yep, basically both MediaElement backends have problems when the lines that I removed in #2045 are present, but it turns out WebAudio has problems (due to the specific implementation) when they are not present. So when those lines are brought back, there needs to be a condition to skip them if the backend isn't WebAudio.

[thijstriemstra]

So when those lines are brought back, there needs to be a condition to skip them if the backend isn't WebAudio.

Sounds good. Want to make a pr (since you "screwed up" 😛)?