Skip to content

Update librespot to fix #1356 / #1358 #1360

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.

Sign up for GitHub

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

Closed
raku-cat wants to merge 4 commits into from
Closed

Update librespot to fix #1356 / #1358 #1360

Show file tree
Hide file tree
Changes from 3 commits
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
8c40ac7
Update to librespot dev branch, minimal changes to allow for compliation
raku-cat Aug 12, 2025
5ddafbc
Update main_loop.rs
raku-cat Aug 12, 2025
0ea1a9c
Should probably pin these to just the commit for the fix
raku-cat Aug 12, 2025
33e590e
Integrate patches from PR and attempt at fix for linting errors
raku-cat Aug 20, 2025
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
279 changes: 163 additions & 116 deletions Cargo.lock
View file
Open in desktop

Large diffs are not rendered by default.

16 changes: 8 additions & 8 deletions Cargo.toml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -26,14 +26,14 @@ sha-1 = "0.10"
tokio = {version = "1.26.0", features = ["signal", "rt-multi-thread", "process", "io-std"] }
tokio-stream = "0.1.7"
url = "2.2.2"
librespot-audio = { version = "0.6", default-features = false }
librespot-playback = { version = "0.6", default-features = false }
librespot-core = "0.6"
librespot-discovery = "0.6"
librespot-connect = "0.6"
librespot-metadata = "0.6"
librespot-protocol = "0.6"
librespot-oauth = "0.6"
librespot-audio = { git = 'https://github.com/librespot-org/librespot.git', rev = 'ba3d501', default-features = false }
librespot-playback = { git = 'https://github.com/librespot-org/librespot.git', rev = 'ba3d501', default-features = false }
librespot-core = { git = 'https://github.com/librespot-org/librespot.git', rev = 'ba3d501' }
librespot-discovery = { git = 'https://github.com/librespot-org/librespot.git', rev = 'ba3d501' }
librespot-connect = { git = 'https://github.com/librespot-org/librespot.git', rev = 'ba3d501' }
librespot-metadata = { git = 'https://github.com/librespot-org/librespot.git', rev = 'ba3d501' }
librespot-protocol = { git = 'https://github.com/librespot-org/librespot.git', rev = 'ba3d501' }
librespot-oauth = { git = 'https://github.com/librespot-org/librespot.git', rev = 'ba3d501' }
toml = "0.8.19"
color-eyre = "0.6"
directories = "6.0.0"
Expand Down
6 changes: 3 additions & 3 deletions src/alsa_mixer.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -36,12 +36,12 @@ impl AlsaMixer {
}

impl Mixer for AlsaMixer {
fn open(_: MixerConfig) -> AlsaMixer {
AlsaMixer {
fn open(_: MixerConfig) -> Result<Self, librespot_core::Error> {
Ok(AlsaMixer {
device: "default".to_string(),
mixer: "Master".to_string(),
linear_scaling: false,
}
})
}

fn volume(&self) -> u16 {
Expand Down
2 changes: 0 additions & 2 deletions src/config.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -69,7 +69,6 @@ pub enum DeviceType {
UnknownSpotify,
CarThing,
Observer,
HomeThing,
}

impl From<DeviceType> for LSDeviceType {
Expand All @@ -93,7 +92,6 @@ impl From<DeviceType> for LSDeviceType {
DeviceType::UnknownSpotify => LSDeviceType::UnknownSpotify,
DeviceType::CarThing => LSDeviceType::CarThing,
DeviceType::Observer => LSDeviceType::Observer,
DeviceType::HomeThing => LSDeviceType::HomeThing,
}
}
}
Expand Down
7 changes: 4 additions & 3 deletions src/main_loop.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -14,7 +14,7 @@ use futures::{
stream::Peekable,
Future, FutureExt, StreamExt,
};
use librespot_connect::{config::ConnectConfig, spirc::Spirc};
use librespot_connect::{ConnectConfig, Spirc};
use librespot_core::{
authentication::Credentials, cache::Cache, config::DeviceType, session::Session, Error,
SessionConfig,
Expand Down Expand Up @@ -125,8 +125,9 @@ impl MainLoop {
name: self.device_name.clone(),
device_type: self.device_type,
is_group: false,
initial_volume: self.initial_volume,
has_volume_ctrl: self.has_volume_ctrl,
initial_volume: self.initial_volume.unwrap_or(50),
disable_volume: !self.has_volume_ctrl,
volume_steps: 64,
},
session.clone(),
creds.clone(),
Expand Down
4 changes: 2 additions & 2 deletions src/no_mixer.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -3,8 +3,8 @@ use librespot_playback::mixer::{Mixer, MixerConfig};
pub struct NoMixer;

impl Mixer for NoMixer {
fn open(_: MixerConfig) -> NoMixer {
NoMixer {}
fn open(_: MixerConfig) -> Result<NoMixer, librespot_core::Error> {
Ok(NoMixer {})
}

fn volume(&self) -> u16 {
Expand Down
11 changes: 4 additions & 7 deletions src/process.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -179,14 +179,10 @@ pub(crate) fn spawn_program_on_event(
env.insert("PLAYER_EVENT", "shuffle_changed".to_string());
env.insert("SHUFFLE", shuffle.to_string());
}
PlayerEvent::RepeatChanged { repeat } => {
PlayerEvent::RepeatChanged { context, track } => {
env.insert("PLAYER_EVENT", "repeat_changed".to_string());
let val = match repeat {
true => "all",
false => "none",
}
.to_string();
env.insert("REPEAT", val);
env.insert("REPEAT", format!("{:?}", context));
env.insert("REPEAT_TRACK", format!("{:?}", track));
Copy link
Member

@eladyn eladyn Aug 17, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

To be in line with the previous behaviour, maybe something like

Suggested change
env.insert("REPEAT", format!("{:?}", context));
env.insert("REPEAT_TRACK", format!("{:?}", track));
let val = match (context, track) {
(_, true) => "track",
(true, false) => "all",
(false, false) => "none",
}
.to_string();
env.insert("REPEAT", val);

could work? (not tested)

}
PlayerEvent::AutoPlayChanged { auto_play } => {
env.insert("PLAYER_EVENT", "autoplay_changed".to_string());
Expand All @@ -196,6 +192,7 @@ pub(crate) fn spawn_program_on_event(
env.insert("PLAYER_EVENT", "filterexplicit_changed".to_string());
env.insert("FILTEREXPLICIT", filter.to_string());
}
PlayerEvent::PositionChanged { .. } => todo!(),
Copy link
Member

@eladyn eladyn Aug 17, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

todo!() will crash the application, when this line is reached, so it should generally not be in production code.
In our case, since we never activate this signal, we shouldn't receive this event at all and could either do unreachable!("this event is not enabled") (which would also crash the application, but communicates the purpose more clearly) or just add it as a second case to the Seeked branch like so:

PlayerEvent::Seeked {
            play_request_id,
            track_id,
            position_ms,
} | PlayerEvent::PositionChanged {
            play_request_id,
            track_id,
            position_ms,
}

Both would be fine to me, but the first one sounds like the cleaner solution.

}
spawn_program(shell, cmd, env)
}
Expand Down
2 changes: 1 addition & 1 deletion src/setup.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -44,7 +44,7 @@ pub(crate) fn initial_state(
}
_ => {
info!("Using software volume controller.");
Arc::new(mixer::softmixer::SoftMixer::open(MixerConfig::default()))
Arc::new(mixer::softmixer::SoftMixer::open(MixerConfig::default())?)
}
}
};
Expand Down
Loading