Use rodio for the jackaudio backend

[Lcchy]

Thanks @willstott101 for updating rodio to 0.13 and making this possible!

This works as is with jack at any sample rate (and solves #316, and #343 ?)

The choice to put this in a separate feature called rodiojack-backend is deliberate. I tried to simply add the jack cpal devices to the alsa ones all together in rodio-backend yet as alsa and jack are exclusive, this always prints an error (either jack or alsa) to the terminal when checking on available devices. This can be tested here.

ps: You have to build without using the --no-default-features, please let me know if you see how to fix this (it seems to deactivate rodio for rodiojack)
FYI: I still think that having all rodio devices in one backend is better, I will try to see in the next few days if I can catch the error messages somehow

[willstott101]

I'm glad you got it working.

Honestly I'd be inclined to replace the existing Jack back-end completely. Ideally we could keep the existing jackaudio-backend feature, along with the runtime option --backend jackaudio. Except we'd make them use jack via rodio instead.

I'd update the non-trait functions in rodio.rs to take the host as a parameter. Then only the Open trait would need to change between the two. This could be re-implemented on a new struct, so that the Open trait has a single implementation per-struct. A new trait on each which provides access to rodio_sink could then make it trivial to implement Sink on both.

IDK of a neater solution right now w.r.t. struct layout. But the more we trend towards more backends being managed by CPAL the better.

[Lcchy]

Thanks for the help @willstott101, I integrated your suggestions and seems to work as supposed to!
I deleted the rodiojack-backend and the jackaudio-backend now uses the jack via rodio.

I just couldn't find a way to implement the Sink trait once for both RodioSink structs as you suggested so I duplicated the method but this should have no influence on usage.

[sashahilton00]

I think moving to jack through rodio makes sense, less for us to deal with. Did you fix the no-default-features issue?

[Lcchy]

Hi @sashahilton00,
yes the jackaudio (over rodio) backend now behaves like all other backends regarding the no-default-features.

I merged the upstream into my fork in order to resolve a conflict but this now triggers a new error in the checks. When investigating I found that the Rust 1.40 test fails at compilation because of the object-0.23.0 dependency that uses a slicing syntax that was unstable in that old Rust version (here is the merge that resolved this in Rust 1.42: #67712).
This could be fixed by using an older version of object (or a newer Rust version) in the test.

This being unrelated to the PR and all other tests passing, I think this is good to go as is.

[sashahilton00]

Looks like we'll need to bump the MSRV for this PR. Also, the git commit history will need to be cleaned up.

[Johannesd3]

The 1.40 issue is the same as in #585

[sashahilton00]

Don't worry about the 1.40.0 test failing. I just tried building this on macOS 10.15.6, and am seeing errors when compiling with the jackaudio backend, detailed below:

cargo build --no-default-features --features "jackaudio-backend"
   Compiling librespot-playback v0.1.3 (/Users/sashahilton/Documents/librespot/playback)
error[E0599]: no variant or associated item named `Jack` found for enum `audio_backend::rodio::cpal::HostId` in the current scope
   --> playback/src/audio_backend/rodio.rs:126:49
    |
126 |                 .find(|id| *id == cpal::HostId::Jack)
    |                                                 ^^^^ variant or associated item not found in `audio_backend::rodio::cpal::HostId`

error: aborting due to previous error

For more information about this error, try `rustc --explain E0599`.
error: could not compile `librespot-playback`.

To learn more, run the command again with --verbose.

[Lcchy]

Ah, I didn't think about the different platforms. Cpal (and hence rodio) has only implemented a jack host for linux-like platforms..
This can bee seen here where the missing item that causes your error is only declared for linux.

I think this could maybe be added for macOS, I'm going to set up a VM to try this out.
This raises a more general question of which kind of platforms need to be supported by the jackbackend and librespot. What about windows?

Ps: I think that libjack-jackd2-dev needs to be installed in order to use this audiojack backend, should I add this dependency to the audio backend README in the PR?

[sashahilton00]

This raises a more general question of which kind of platforms need to be supported by the jackbackend and librespot. What about windows?

Ideally I'd like to see Windows and Mac support for Jack. The jack bindings crate has support for it, so it shouldn't be much trouble to make this possible, though it will depend on upstream cpal/rodio. Given that upstream could take a while, I'd be happy to include this as a linux only feature currently, but the existing jack audio backend that supports mac and windows should remain, and a note clarifying the difference between the rodio and jack backends should be made in the readme/wiki, explaining that the old jack backend will eventually be going away once rodio supports mac/windows. That way one can pursue upstream support without blocking this feature, which I know has been desired for quite a while for certain use cases.

Ps: I think that libjack-jackd2-dev needs to be installed in order to use this audiojack backend, should I add this dependency to the audio backend README in the PR?

Yes, if it is required then it should be added in the wiki.

[sashahilton00]

I'll be making a new release in the next day or so, if we want this to be included then the rodiojack feature will need to require Linux as the target os in its current format. Then support for other systems can be added once there is support upstream

[Lcchy]

Ok sounds good to me :)
I have added rodiojack as a separate backend and left the jackaudio as it was originally.
I also edited the travis.yml to update the rust verion to 1.42 and add a test for the new backend.
Ah and I'll edit the wiki entry regarding the new backend (and the available platforms) as soon as this is merged.

[sashahilton00]

@Lcchy merged. If you could update the wiki that would be great. Also, with regards to mac/windows support upstream, is that something you're willing to pursue?

[Lcchy]

Yes, that's something I planned on doing. I am going to open a PR on cpal and open one here once it gets merged, if that is all right with you.

[Lcchy]

Hey @sashahilton00, just a heads up as I have been busy with work and could not get to making the upstream compatible, sorry about that.

[sashahilton00]

No problem. Are you still intending on pursuing at a later date or done until further notice?

[Lcchy]

One day maybe but it's definitely more done until further notice.