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

Next track automatically starts despite being paused #15

Open
probablykasper opened this issue Dec 26, 2020 · 16 comments
Open

Next track automatically starts despite being paused #15

probablykasper opened this issue Dec 26, 2020 · 16 comments

Comments

@probablykasper
Copy link

In my code below, I'm adding a few tracks, and then I have player.togglePlayPause() connected to a button's click event. When I call click the button to start playback, then click it again shortly afterwards to pause, the next track will start playing about 3 minutes later (the length of the song).

This issue happens in 2.2.3. I tested 1.0.0 as well, and it's an issue there as well.

// import Gapless from 'gapless.js'
import Gapless from '../../gapless2.js'
let sliderBeingDragged = false
let currentTime = 0
let duration = 0
let sliderValue = 0
const sliderSteps = 400
const player = new Gapless.Queue({
  numberOfTracksToPreload: 2,
  onProgress: (track) => {
    if (track) {
      currentTime = track.currentTime
      duration = track.duration
      if (!sliderBeingDragged && duration > 0) {
        sliderValue = currentTime/duration*sliderSteps
      }
    }
  },
})
const tracks = [
  '/Users/kasper/Downloads/01 Neo-Seoul (Part 1).m4a',
  '/Users/kasper/Downloads/02 Neo-Tokyo (Part 2).m4a',
  '/Users/kasper/Downloads/03 Aeon Metropolis (Part 3).m4a',
  '/Users/kasper/Downloads/04 Goodnight Sequence (part 4).m4a',
]
for (const track of tracks) {
  player.addTrack({ trackUrl: 'file://'+track })
}
function playPause() { // this function is tied to a button onclick
  player.togglePlayPause()
}

Below is the console log from when this happens. Once the next track starts playing by itself, the first log message is for the onEnded event.
Screen Shot 2020-12-26 at 1 03 16 AM

I've noticed that running track.loadBuffer() causes the track to start playing, so perhaps that's related.

Woud love to see what's causing this, and whether there's an easy fix for it. Thanks!

@probablykasper
Copy link
Author

Tried logging player.currentTrack.completeState every second, and as you can see, the currentTime is increasing while isPaused is true:
Screen Shot 2020-12-26 at 1 14 51 AM

@switz
Copy link
Member

switz commented Dec 26, 2020

I would wager it's triggered from this line: https://github.com/RelistenNet/gapless.js/blob/master/index.js#L270

But I haven't been able to reproduce it. Is it marked as the activeTrack?

@probablykasper
Copy link
Author

probablykasper commented Dec 26, 2020

@switz Yes, player.currentTrack.isActiveTrack is true.

Made a reproduction repo here: https://github.com/probablykasper/gapless.js-sudden-playback-bug

Noticed that it doesn't actually play the whole second track. It plays the first 5 seconds (length of first track), then it skips and plays the whole third track.

@switz
Copy link
Member

switz commented Dec 26, 2020

I think that repo is private

@probablykasper
Copy link
Author

@switz Ah my bad, made it public

@switz
Copy link
Member

switz commented Dec 26, 2020

Looks like @jgeurts has been quietly commiting and publishing to npm via his own repo... care to comment Jim? If you would like to send PRs back upstream, I'm happy to merge them, but I'm a bit fearful of someone silently pushing npm updates to a repo that's under my name – not that you've done anything malicious. If you'd just prefer to fork it entirely that's totally fine too.

@jgeurts
Copy link
Contributor

jgeurts commented Dec 26, 2020

Hey @switz!

Apologies for pushing the npms without notice - I tried to tag them with next, to hopefully lessen any impact and avoid the new versions being the default install. To give an overview of what I've been working on:

I converted this repo to typescript and have been working on getting things worked out with a side project of mine (similar to relisten). The typescript conversion was mostly to get the mental model straight in my head, but also has the side effect of providing some type definitions. The resulting typescript based API is pretty close to the code in this repo. Off the top of my head, I removed the tracks property from the Queue constructor and renamed a couple internal fields. That said, I'm not 100% confident that I didn't mess up anything when converting. My player UI mostly works, but definitely has at least one bug related to song transitions that I was hoping to iron out over the next week or so. And after reading this thread, the bug I've encountered may be related :)

All of this said, it's up to you for how you'd like to proceed with things. My plan will be to continue with the typescript version and the next major version may be a larger restructure effort to accommodate things like shuffle, different types of track repeats, etc. If you want me to do that under a different project name, I can definitely do that (and remove myself from the npm project, etc). Alternatively, if you're interested in the typescript code, I'm more than happy to open up a PR with what I have.

@switz
Copy link
Member

switz commented Dec 26, 2020

No worries! I'm happy to hear that people have been finding value in this library and that there's interest in keeping it going.

I mostly write typescript myself these days, so typing everything and modernizing it is a welcome sight! I'd love it if you sent a PR and we can try and iron out any bugs together.

And feel free to extend it with shuffling and other features – I say the more features the merrier. I've been meaning to sink a few hours into getting this library improved, with perhaps even better gapless support. @alecgorge pointed me to some newer browser features that may make this even better: http://melatonin64.github.io/gapless-mse-audio/ and https://developers.google.com/web/updates/2015/06/Media-Source-Extensions-for-Audio?hl=en

Thanks for taking the time to respond and the effort you put into modernizing the code. Cheers!

@probablykasper
Copy link
Author

@jgeurts I definitely appreciate the typings! Noticed 2.0 is a CommonJS module, so if it should be importable in the browser that might be worth looking into.

On a side-note, I think it would be helpful to see the possible ways of importing gapless.js in usage examples, and also adding all the methods to the documentation.

Thanks to both of you for making this! :)

@jgeurts
Copy link
Contributor

jgeurts commented Dec 27, 2020

I feel like I tried using ES2020 for the typescript module compiler option, but then had issues with react pulling it in, so I reverted back to commonjs. I don't remember the issues off the top of my head, but I'd be interested in giving it another shot

@probablykasper
Copy link
Author

Have any of you managed to get anywhere with the pausing issue?

I'm pretty clueless about WebAudio, but my guess is that using this.bufferSourceNode.playbackRate.value to pause isn't working.

As far as I can tell, using AudioContext.suspend and AudioContext.resume seems like it should work.
MDN has a nice demo (src)

@jgeurts
Copy link
Contributor

jgeurts commented Dec 28, 2020

I didn't have a chance today. Hoping to get into it a bit more tomorrow, though. I will keep you both posted as I work through things!

@jgeurts
Copy link
Contributor

jgeurts commented Dec 30, 2020

I hit some stumbling blocks trying to get the code working... so, I rewrote the player based on my goals from #15 (comment). I'm using a double buffer technique, using two audio elements. I'm hoping that gets me pretty close to gapless, but I haven't tested things yet. I came across a stackoverflow post about media source extensions that scared me off a bit.

Anyway, I'd be curious to hear your thoughts: https://github.com/jgeurts/gapless.js/blob/v3-double-buffer/index.ts

I'll try to hook it up to my site later tonight or maybe tomorrow. Will keep you both posted on results...

Word of caution: it's a complete rewrite and may not fit anymore with your goals ¯\_(ツ)_/¯

@probablykasper
Copy link
Author

probablykasper commented Dec 30, 2020

@jgeurts Yeah, caniuse says MSE indeed lacks support on iOS. On the other hand, MSE is actually supported on in some cases where the AudioContext API isn't (like IE11 for Windows 8+). So, using MSE with AudioContext would mean going from 94% support to about 98%.

@probablykasper
Copy link
Author

Just noticed that for some tracks, I can hear a pop sound when it switches to web audio right after starting to play a track. Not sure if that's something that can be fixed, but I'm guessing MSE would help with that.

In this screen recording, you can actually see the duration and currentTime change and become more specific, which might be the reason:

Screen.Recording.2020-12-30.at.4.38.04.AM.mp4

probablykasper added a commit to probablykasper/ferrum that referenced this issue Dec 30, 2020
Using <audio> instead of gapless.js because of some bugs it has: RelistenNet/gapless.js#15

The player can now see the tracklist, which means the next/previous buttons work, and after a track is finished, the next one starts
@jgeurts
Copy link
Contributor

jgeurts commented Dec 31, 2020

I made a couple tweaks to the double buffer code and things seem promising. To simulate gapless playback, if there is less than 0.3s left to the current track, the player will kick off the next track while allowing the current track to finish. I have it working with my player UI and I'm not noticing pops between tracks.

The 0.3s is a "magic number" that maybe could be configurable or calculated via MSE/AudioContext.

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