Pass the new player to nextTrack to allow mixed playlist playback

[kevgrig]

Changes When a playlist is stopped (e.g. track finishes), if the player for the next playlist item is different, then this new player needs to be used as otherwise the current player has been nulled out.

Issues Fixes #5486

Summary by CodeRabbit

• Bug Fixes
  • Improved playback transition handling to ensure smoother switching between media items.
  • Enhanced event notifications when playback stops and a new item is queued.

[sonarqubecloud[bot]]

Quality Gate passed

Issues
0 New issues
0 Accepted issues

Measures
0 Security Hotspots
No data about Coverage
0.0% Duplication on New Code

See analysis details on SonarCloud

[jellyfin-bot]

Cloudflare Pages deployment

Latest commit	8f03986
Status	✅ Deployed!
Preview URL	https://af3d0705.jellyfin-web.pages.dev
Type	🔀 Preview

View build logs View bot logs

[dmitrylyzo]

It crashes here (the player has been killed on the stop event): https://github.com/jellyfin/jellyfin-web/blob/edc70fece4adb4b833ef2e16f40c8c33fad9f5e3/src/components/playback/playbackmanager.js#L2989-L2990

IMO, passing a new player breaks the logic - getting the previous source from the new player.

We probably need to change this check: https://github.com/jellyfin/jellyfin-web/blob/edc70fece4adb4b833ef2e16f40c8c33fad9f5e3/src/components/playback/playbackmanager.js#L3361 to !newPlayer.

This: https://github.com/jellyfin/jellyfin-web/blob/edc70fece4adb4b833ef2e16f40c8c33fad9f5e3/src/components/playback/playbackmanager.js#L3369 to newPlayer.

And kill the player after the event is triggered: https://github.com/jellyfin/jellyfin-web/blob/edc70fece4adb4b833ef2e16f40c8c33fad9f5e3/src/components/playback/playbackmanager.js#L3409-L3414

So the previous player will be alive until it is no longer needed (for the next item).

In fact, our playback code is a mess and requires a lot of refactoring. At first glance, we should also kill the player on error, but this is probably handled by onInterceptorRejection.

[kevgrig]

Makes sense, thanks. I'll try it out when I have a chance.

[kevgrig]

to !newPlayer.

Done in latest commit and confirmed things still work for me.

And kill the player after the event is triggered:

I didn't follow this because isn't this already executing in onPlaybackStopped?

[dmitrylyzo]

Done in latest commit and confirmed things still work for me.

https://github.com/jellyfin/jellyfin-web/blob/1df1262d0ea7620d58627204a17e7ada3a175641/src/components/playback/playbackmanager.js#L3384

-> else if (newPlayer) { because we may have a nextItem but no available player (newPlayer is null).

I didn't follow this because isn't this already executing in onPlaybackStopped?

After replacing newPlayer !== player with !newPlayer, the old player will be alive when we switch to another player. So after the stop event is triggered, we need to destroy the old player, but only when it has been changed. Perhaps we can do this after calling nextTrack similar to the original: if (newPlayer !== player). I haven't tested this case - just guessing.