[PR #300] [CLOSED] Rewrite and simplify Player #868

Closed
opened 2026-02-27 20:00:15 +03:00 by kerem · 0 comments
Owner

📋 Pull Request Information

Original PR: https://github.com/librespot-org/librespot/pull/300
Author: @udoprog
Created: 3/11/2019
Status: Closed

Base: devHead: master


📝 Commits (2)

📊 Changes

4 files changed (+243 additions, -270 deletions)

View changed files

📝 playback/src/audio_backend/portaudio.rs (+30 -17)
📝 playback/src/config.rs (+2 -0)
📝 playback/src/player.rs (+210 -253)
📝 src/main.rs (+1 -0)

📄 Description

Hey,

This is a mostly complete rewrite of how logic is handled in the Player.

  • There are no dead code paths causing the player thread to exit sporadically. The only way that a player thread currently can shut down is either for the commands channel to disconnect or for it to panic.
  • The various player states have been simplified and documented. In particular the code has been rewritten to not make use of an Invalid state any longer. The EndOfTrack state has been removed.
  • The loaded track is no longer stored in the player state. Instead it is stored in an loaded_track: Option<LoadedTrack>. This makes it clearer what the lifetime of a loaded_track is and affords us to implement some things decoupled from the state.
  • The player has no interim panics. Instead all errors are propagated as errors. If an error reaches the thread it will panic.
  • The end_of_track oneshot is called in the Drop implementation of LoadedTrack to make sure we don't forget about it. Hopefully this means we can write clients without having to match against Err(Cancelled) in the receiving end.

This was also built on top of a previous patch which incorporated linear volume control:

This adds linear volume control at the same stage as normalization happens and adds player events to control the volume directly.

I'm pretty sure this can be implemented somewhere in the audio backend, as an audio filter, or as part of the mixer. But this avoids iterating over the samples twice if normalization happens anyways by multiplying it with the normalization factor.


🔄 This issue represents a GitHub Pull Request. It cannot be merged through Gitea due to API limitations.

## 📋 Pull Request Information **Original PR:** https://github.com/librespot-org/librespot/pull/300 **Author:** [@udoprog](https://github.com/udoprog) **Created:** 3/11/2019 **Status:** ❌ Closed **Base:** `dev` ← **Head:** `master` --- ### 📝 Commits (2) - [`879c046`](https://github.com/librespot-org/librespot/commit/879c046fb79e4bc823064f0122f5acb392a3a330) Rewrite Player - [`25dafb5`](https://github.com/librespot-org/librespot/commit/25dafb5a5973102058639dc8ac703fac47d6d0de) Skip unavailable songs ### 📊 Changes **4 files changed** (+243 additions, -270 deletions) <details> <summary>View changed files</summary> 📝 `playback/src/audio_backend/portaudio.rs` (+30 -17) 📝 `playback/src/config.rs` (+2 -0) 📝 `playback/src/player.rs` (+210 -253) 📝 `src/main.rs` (+1 -0) </details> ### 📄 Description Hey, This is a mostly complete rewrite of how logic is handled in the Player. * There are no dead code paths causing the player thread to exit sporadically. The only way that a player thread currently can shut down is either for the commands channel to disconnect or for it to panic. * The various player states have been simplified and documented. In particular the code has been rewritten to not make use of an `Invalid` state any longer. The `EndOfTrack` state has been removed. * The loaded track is no longer stored in the player state. Instead it is stored in an `loaded_track: Option<LoadedTrack>`. This makes it clearer what the lifetime of a `loaded_track` is and affords us to implement some things decoupled from the state. * The player has no interim panics. Instead all errors are propagated as errors. If an error reaches the thread it will panic. * The `end_of_track` oneshot is called in the `Drop` implementation of `LoadedTrack` to make sure we don't forget about it. Hopefully this means we can write clients without having to match against `Err(Cancelled)` in the receiving end. This was also built on top of a previous patch which incorporated linear volume control: This adds linear volume control at the same stage as normalization happens and adds player events to control the volume directly. I'm pretty sure this can be implemented somewhere in the audio backend, as an audio filter, or as part of the mixer. But this avoids iterating over the samples twice if normalization happens anyways by multiplying it with the normalization factor. --- <sub>🔄 This issue represents a GitHub Pull Request. It cannot be merged through Gitea due to API limitations.</sub>
kerem 2026-02-27 20:00:15 +03:00
Sign in to join this conversation.
No milestone
No project
No assignees
1 participant
Notifications
Due date
The due date is invalid or out of range. Please use the format "yyyy-mm-dd".

No due date set.

Dependencies

No dependencies set.

Reference
starred/librespot#868
No description provided.