First playlist item is skipped sometimes due to client race

[spodpn]

How to reproduce:

1. Start a youtube video and put 2 other videos in the playlist
2. Open three more tabs so that there are 4 users (can also happen with 2 users but with 4 is more likely)
3. Jump right before the end of the video to trigger the playlist

The first playlist item is loaded but after a very short amount of time it is replaced by the next one. The playlist counter goes quickly from (2) to (0). The first item on the playlist was effectively skipped.

Server log: (I inserted some debug printing)

[RELEASE][4] 0 rooms in batch
startHosting https://www.youtube.com/watch?v=kJQP7kiw5Fk
cmdHost( https://www.youtube.com/watch?v=kJQP7kiw5Fk )
[RELEASE][5] 0 rooms in batch
[RELEASE][6] 0 rooms in batch
playlistNext( https://www.youtube.com/watch?v=kJQP7kiw5Fk )
playlistNext( https://www.youtube.com/watch?v=kJQP7kiw5Fk )
cmdHost( https://www.youtube.com/watch?v=JGwWNGJdvx8 )
playlistNext( https://www.youtube.com/watch?v=JGwWNGJdvx8 )
playlistNext( https://www.youtube.com/watch?v=JGwWNGJdvx8 )
cmdHost( https://www.youtube.com/watch?v=pRpeEdMmmQ0 )
[RELEASE][7] 0 rooms in batch

When the slowest 2 clients reach the end, they trigger again a video change even though the video was already changed. The playlist is advanced twice, the first item was quickly skipped.

Defective client log: (I inserted some debug printing)

syncing self to leader or custom: 275.6778259953327
REC:host App.tsx:393
video stopped App.tsx:437
window.YT?.PlayerState?.ENDED App.tsx:846
onVideoEnded https://www.youtube.com/watch?v=JGwWNGJdvx8 App.tsx:1798
REC:host App.tsx:393
video stopped App.tsx:437
play yt

After the video was changed (REC:host) and should be now stopped, the defective client still gets an ENDED event on his player which triggers another playlistNext.

I wonder if we could reliably prevent this ENDED event from firing after the video was already changed by another client.

[howardchung]

We can maybe try to avoid this by having each client send the url of the next video along with the "finished" event. If it doesn't match, then ignore the message

[howardchung]

The current implementation attempts to wait until half the clients have submitted a "playlistNext" event:

  private playlistNext = (socket: Socket | null, data?: string) => {
    if (data && data.length > 20000) {
      return;
    }
    if (socket && data === this.video) {
      this.nextVotes[socket.id] = data;
    }
    const votes = this.roster.filter((user) => this.nextVotes[user.id]).length;
    if (!socket || votes >= Math.floor(this.roster.length / 2)) {
      const next = this.playlist.shift();
      this.io.of(this.roomId).emit('playlist', this.playlist);
      if (next) {
        this.cmdHost(null, next.url);
      }
    }
  };