Skip to content
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.

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

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

Merged
merged 4 commits into from
Feb 20, 2021

Conversation

Johannesd3
Copy link
Contributor

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.


list_formats(&default_device);

println!("Other Available Audio Devices:");
Copy link
Contributor

Choose a reason for hiding this comment

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

Was println! before, but would this not make more sense to be info!? Like in general have no println! in the lib because it can not be filtered?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

This is just executed in the case someone specifies "?" as device name and explicitly wishes to see the available devices.

@ashthespy ashthespy merged commit afacaea into librespot-org:tokio_migration Feb 20, 2021
@Johannesd3 Johannesd3 deleted the tokio_migration branch February 20, 2021 10:33
Johannesd3 added a commit to Johannesd3/librespot that referenced this pull request Apr 9, 2021
Firstly, fe37186 added the restriction that `Sink`s must be `Send`.
It turned out later that this restrictions was unnecessary, and
since some `Sink`s aren't `Send` yet, this restriction is lifted again.

librespot-org#601 refactored the `RodioSink` in order to make
it `Send`. These changes are partly reverted in favour of the initial
simpler design.

There were also some compile errors in the gstreamer backend which are
hereby solved.
Johannesd3 added a commit to Johannesd3/librespot that referenced this pull request Apr 9, 2021
fe37186 added the restriction that `Sink`s must be `Send`. It turned
out later that this restrictions was unnecessary, and since some
`Sink`s aren't `Send` yet, this restriction is lifted again.

librespot-org#601 refactored the `RodioSink` in order to make
it `Send`. These changes are partly reverted in favour of the initial
simpler design.

Furthermore, there were some compile errors in the gstreamer backend
which are hereby fixed.
Johannesd3 added a commit to Johannesd3/librespot that referenced this pull request Apr 9, 2021
fe37186 added the restriction that `Sink`s must be `Send`. It turned
out later that this restrictions was unnecessary, and since some
`Sink`s aren't `Send` yet, this restriction is lifted again.

librespot-org#601 refactored the `RodioSink` in order to make
it `Send`. These changes are partly reverted in favour of the initial
simpler design.

Furthermore, there were some compile errors in the gstreamer backend
which are hereby fixed.
Johannesd3 added a commit to Johannesd3/librespot that referenced this pull request Apr 9, 2021
fe37186 added the restriction that `Sink`s must be `Send`. It turned
out later that this restrictions was unnecessary, and since some
`Sink`s aren't `Send` yet, this restriction is lifted again.

librespot-org#601 refactored the `RodioSink` in order to make
it `Send`. These changes are partly reverted in favour of the initial
simpler design.

Furthermore, there were some compile errors in the gstreamer backend
which are hereby fixed.
Johannesd3 added a commit to Johannesd3/librespot that referenced this pull request Apr 9, 2021
fe37186 added the restriction that `Sink`s must be `Send`. It turned
out later that this restrictions was unnecessary, and since some
`Sink`s aren't `Send` yet, this restriction is lifted again.

librespot-org#601 refactored the `RodioSink` in order to make
it `Send`. These changes are partly reverted in favour of the initial
simpler design.

Furthermore, there were some compile errors in the gstreamer backend
which are hereby fixed.
Johannesd3 added a commit to Johannesd3/librespot that referenced this pull request Apr 9, 2021
fe37186 added the restriction that `Sink`s must be `Send`. It turned
out later that this restrictions was unnecessary, and since some
`Sink`s aren't `Send` yet, this restriction is lifted again.

librespot-org#601 refactored the `RodioSink` in order to make
it `Send`. These changes are partly reverted in favour of the initial
simpler design.

Furthermore, there were some compile errors in the gstreamer backend
which are hereby fixed.
Johannesd3 added a commit to Johannesd3/librespot that referenced this pull request Apr 10, 2021
fe37186 added the restriction that `Sink`s must be `Send`. It turned
out later that this restrictions was unnecessary, and since some
`Sink`s aren't `Send` yet, this restriction is lifted again.

librespot-org#601 refactored the `RodioSink` in order to make
it `Send`. These changes are partly reverted in favour of the initial
simpler design.

Furthermore, there were some compile errors in the gstreamer backend
which are hereby fixed.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants