[PR #601] [MERGED] [Tokio migration] Make RodioSink Send and other improvements #992

New issue

Closed

opened 2026-02-27 20:00:44 +03:00 by kerem · 0 comments

kerem commented

2026-02-27 20:00:44 +03:00

Owner

📋 Pull Request Information

Original PR: https://github.com/librespot-org/librespot/pull/601
Author: @Johannesd3
Created: 2/12/2021
Status: ✅ Merged
Merged: 2/20/2021
Merged by: @ashthespy

Base: tokio_migration ← Head: tokio_migration

📝 Commits (4)

2f05ddf Fix bugs in player
b2f1be4 Make RodioSink Send and improve error handling
689415a Improved error handling in rodio backend
b77f0a1 Fix formatting

📊 Changes

5 files changed (+152 additions, -92 deletions)

View changed files

📝 Cargo.lock (+1 -0)
📝 core/src/connection/mod.rs (+1 -1)
📝 playback/Cargo.toml (+6 -3)
📝 playback/src/audio_backend/rodio.rs (+127 -71)
📝 playback/src/player.rs (+17 -17)

📄 Description

Firstly, this PR fixes two bugs in the playback crate (well, I also introduced them).

Secondly, RodioSink was modified such that it implements Send now. The solution spawns an own thread to create the rodio::OutputStream. When RodioSink is dropped, this thread is notified via a channel and the OutputStream is dropped as well. So RodioSink doesn't carry the OutputStream anymore and is now Send.

I also used this occasion to improve the error handling in the RodioSink using thiserror, but this should only be the beginning.

If it's ok, I will continue with those relatively small PRs for the tokio migration, so it's easier to follow what I'm doing and easier to contribute.

_{🔄 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/601 **Author:** [@Johannesd3](https://github.com/Johannesd3) **Created:** 2/12/2021 **Status:** ✅ Merged **Merged:** 2/20/2021 **Merged by:** [@ashthespy](https://github.com/ashthespy) **Base:** `tokio_migration` ← **Head:** `tokio_migration` --- ### 📝 Commits (4) - [`2f05ddf`](https://github.com/librespot-org/librespot/commit/2f05ddfbc20fb04131a2ad1ce68dbc181fe6b7cf) Fix bugs in player - [`b2f1be4`](https://github.com/librespot-org/librespot/commit/b2f1be4374e091267d0676e92870b5f3eb30fa7c) Make `RodioSink` `Send` and improve error handling - [`689415a`](https://github.com/librespot-org/librespot/commit/689415a6f1580547dae5be592cb21a847114edb1) Improved error handling in rodio backend - [`b77f0a1`](https://github.com/librespot-org/librespot/commit/b77f0a18cef0c893cb48d2b34746287c7c00cdc6) Fix formatting ### 📊 Changes **5 files changed** (+152 additions, -92 deletions) <details> <summary>View changed files</summary> 📝 `Cargo.lock` (+1 -0) 📝 `core/src/connection/mod.rs` (+1 -1) 📝 `playback/Cargo.toml` (+6 -3) 📝 `playback/src/audio_backend/rodio.rs` (+127 -71) 📝 `playback/src/player.rs` (+17 -17) </details> ### 📄 Description Firstly, this PR fixes two bugs in the playback crate (well, I also introduced them). Secondly, `RodioSink` was modified such that it implements `Send` now. The solution spawns an own thread to create the `rodio::OutputStream`. When `RodioSink` is dropped, this thread is notified via a channel and the `OutputStream` is dropped as well. So `RodioSink` doesn't carry the `OutputStream` anymore and is now `Send`. I also used this occasion to improve the error handling in the `RodioSink` using `thiserror`, but this should only be the beginning. If it's ok, I will continue with those relatively small PRs for the tokio migration, so it's easier to follow what I'm doing and easier to contribute. --- <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:44 +03:00

closed this issue
added the
pull-request
label

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#992

No description provided.

Rows
Columns