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

Switch to CPAL, Symphonia, and parking_lot #197

Merged
merged 14 commits into from
Nov 16, 2021
Merged

Switch to CPAL, Symphonia, and parking_lot #197

merged 14 commits into from
Nov 16, 2021

Conversation

jpochyla
Copy link
Owner

@jpochyla jpochyla commented Oct 14, 2021

This is an experimental effort in replacing miniaudio and minivorbis crates with cpal for the raw audio output and symphonia for decoding. This has a few benefits:

  1. We don't have to maintain miniaudio-sys bindings for different platforms.
  2. Building is simpler.
  3. Symphonia is of excellent quality, has low CPU usage and supports multiple codecs (relevant for podcast support and playing local tracks).
  4. The upstream feels a bit closer, we can fork, fix bugs, etc.

...and also a few drawbacks:

  1. cpal doesn't do any resampling. Some Windows setups seem to require apps to resample the audio themselves, but so far, the Spotify OGG tracks seem to all be sampled at 41000hz. I suppose that maybe we can assume all audio setups would support this and wait? @H-M-H has integrated dasp with cpal for resampling support: Alternative audio backends #167
  2. We lose native PulseAudio support. I have a feeling we could have multiple audio backends in Psst and use PA directly.

While refactoring this, I've also replaced std mutexes with parking_lot, introduced a ringbuffer between the audio thread and the decoding, and split decoding of different audio files into different threads, to avoid deadlocks in case the I/O gets stuck indefinitely.

Missing:

  • Global volume
  • Normalization
  • More sample formats (now only f32 is supported)

@klemensn
Copy link
Contributor

Builds fine on OpenBSD as is.

Requires pinning cpal to RustAudio/cpal#493 to start psst-gui, otherwise audio devices cannot be found.
(Requires sndio-sys's build dependency on bindgen to be bumped to 0.56.0, otherwise its old 0.53.0 requirement conflicts.)

Requires https://github.com/jpochyla/psst/pull/197/files#diff-bba7fe42fa43c91b64f31715a4f831828de23276c69965b0c5bce3cf59db8ae7R13 to be set to 48000, otherwise psst-gui starts but crashes at playback:

thread '<unnamed>' panicked at 'Playback failed: AudioOutputError(BackendSpecific { err: BackendSpecificError { description: "no configuration for sample rate 41000" } })', psst-gui/src/controller/playback.rs:73:27
note: run with `RUST_BACKTRACE=1` environment variable to display a backtrace
thread '<unnamed>' panicked at 'Audio output died: "SendError(..)"', psst-core/src/audio_output.rs:202:39
[2021-10-14T12:06:21Z INFO  psst_gui::data::config] saved config: "/home/kn/.config/Psst/config.json"
[2021-10-14T12:06:21Z INFO  psst_core::access_token] access token expired, requesting
thread 'main' panicked at 'called `Result::unwrap()` on an `Err` value: "SendError(..)"', psst-gui/src/controller/playback.rs:215:51

Eventually, music plays on OpenBSD/amd64 without crashes, albeit noticably slower.
So a few nits have to ironed out, but nothing that seems like a huge blocker for OpenBSD.

@jpochyla
Copy link
Owner Author

Sigh, so sndio also can't resample the signal on its own. OK, let's put a resampler in there! I'll try dasp and rubato.

@pablodz
Copy link

pablodz commented Oct 17, 2021

Keep doing ,nice work 100/100

@jpochyla
Copy link
Owner Author

jpochyla commented Oct 22, 2021

@klemensn I've added an experimental resampling support through (pure Rust conversion of) libsamplerate. This branch is still not ready (volume and normalization still don't work and we should have a plan for what to do on device change), but we're getting there!

@klemensn
Copy link
Contributor

@klemensn I've added an experimental resampling support through (pure Rust conversion of) libsamplerate. This branch is still not ready (volume and normalization still don't work and we should have a plan for what to do on device change), but we're getting there!

FWIW, it builds and plays on OpenBSD/amd64 without further patching but the playback speed is still too slow.

@jacksongoode
Copy link
Collaborator

@klemensn I've added an experimental resampling support through (pure Rust conversion of) libsamplerate. This branch is still not ready (volume and normalization still don't work and we should have a plan for what to do on device change), but we're getting there!

FWIW, it builds and plays on OpenBSD/amd64 without further patching but the playback speed is still too slow.

Playback speed slow meaning the sample rate is incorrect?

@jacksongoode
Copy link
Collaborator

jacksongoode commented Nov 1, 2021

This branch definitely starts playback much, much faster on boot. No issues on my side with latest commit (Fedora 35, Pipewire 3.39) though I'll randomly get a psst_core::audio_file] failed to download: ... status code 502 which I believe has nothing to do with this. However, it looks like CPU usage is 20-30% on my machine? Does cargo run significantly differ from the binary?

@paolobarbolini
Copy link
Contributor

paolobarbolini commented Nov 6, 2021

However, it looks like CPU usage is 20-30% on my machine? Does cargo run significantly differ from the binary?

Did you cargo build or cargo run it with --release? If not you're using a debug build, which is much slower in Rust

@jpochyla
Copy link
Owner Author

Uff, I'm sorry this is taking so long, folks. I believe that as of the latest commit, the design is sound:

  • No decoding or blocking (except waking up other threads) on the audio thread.
  • Reasonably long audio buffer, decoded in a dedicated thread.
  • Low latency pause/resume/seek (seek has still some issues around the reported position).
  • No resampling yet, I'll re-add it ASAP to the output side of things.
  • No real gapless playback yet. Shouldn't be THAT hard.

@jpochyla
Copy link
Owner Author

OK, so I think we should bite the bullet and merge this. One major thing that is missing is additional sample format support, opening the stream will fail on configs without f32 support, but let's do this in master.

@klemensn
Copy link
Contributor

All these commits still build on OpenBSD (with dependency tweaks).

Playback speed, however, is still very slow, e.g. dark voices.
One comment seemed to suggest debug builds degrading performance quite a bit,
but given that I have always been building debug builds from the start and
psst did play audio just fine before the sound backend revamp, this is not
my issue.

There is another regression: Pause does not work, so once playing it can only
be stopped by quitting psst; below are the logs after building the latest
commit.

$ ./target/debug/psst-gui   
[2021-11-12T13:31:34Z INFO  psst_gui::data::config] loading config: "/home/kn/.config/Psst/config.json"
[2021-11-12T13:31:34Z INFO  psst_core::audio::output] using audio device: "sndio default device"
[2021-11-12T13:31:34Z INFO  psst_core::cache] using cache: "/home/kn/.cache/Psst"
[2021-11-12T13:31:34Z INFO  psst_core::audio::output] opening output stream: StreamConfig { channels: 1, sample_rate: SampleRate(48000), buffer_size: Default }
[2021-11-12T13:31:34Z INFO  psst_gui::data::config] saved config: "/home/kn/.config/Psst/config.json"
[2021-11-12T13:31:34Z INFO  psst_core::session::access_token] access token expired, requesting
[2021-11-12T13:31:36Z INFO  psst_core::session::access_token] access token expired, requesting
[2021-11-12T13:31:36Z INFO  symphonia_format_ogg::demuxer] starting new physical stream
[2021-11-12T13:31:36Z INFO  symphonia_format_ogg::demuxer] selected vorbis mapper for stream with serial=0x0
[2021-11-12T13:31:36Z INFO  psst_core::player::file] blocked at 2457397
[2021-11-12T13:31:36Z INFO  psst_core::player::file] blocked at 2472477
[2021-11-12T13:31:36Z INFO  psst_core::player::file] blocked at 2488929
[2021-11-12T13:31:36Z INFO  psst_core::player::file] blocked at 2504010
[2021-11-12T13:31:36Z INFO  psst_core::player::file] blocked at 2520462
[2021-11-12T13:31:36Z INFO  psst_core::player] starting playback
[2021-11-12T13:31:36Z INFO  psst_gui::controller::playback] playing
[2021-11-12T13:31:39Z INFO  psst_core::player] pausing playback
[2021-11-12T13:31:39Z ERROR psst_core::audio::output] failed to stop stream: A backend-specific error has occurred: pausing is not implemented

Do note that the play/pause icon does toggle in the UI, so I dare say that this
is another bug, i.e. the icon must not toggle unless pause/play was in fact
successful.

@jpochyla
Copy link
Owner Author

@klemensn Would you please try the playback speed in the release mode? Play/pause is reliant on the CPAL support for pausing/resuming the stream, so it seems the problem is in the OpenBSD backend side, but we should do client-side pausing anyway (to get fadeouts).

@klemensn
Copy link
Contributor

Here is how I have always built psst:

$ export LIBCLANG_PATH=/usr/local/lib
$ export RUSTFLAGS='-C codegen-units=1 -C debuginfo=0 -C opt-level=0'
$ cargo build

I have to disable optimizations, otherwise rustc(1) (read: LLVM) runs OOM
while building the gtk crate; the errors vary, but the cause is always
the same.
(rustc's RSS grows short to 3G before OpenBSD kills the process.)

Now I did

$ export LIBCLANG_PATH=/usr/local/lib
$ export RUSTFLAGS='-C codegen-units=1 -C debuginfo=0 -C opt-level=0'
$ cargo build --release

and ./target/release/psst-gui exhibits the exact same behaviour.

FWIW, here are the changes I need on-top due to OpenBSD:
https://gist.github.com/9b60ef7a2679f6cbc9fdaf61f6d069f3

@jpochyla
Copy link
Owner Author

@klemensn While it's definitely possible there's a bug, I'd fully expect the playback to be slow when compiled without optimisations. The previous media pipeline was mostly in C (minivorbis + miniaudio), which is less sensitive to compiling in debug mode.

@paolobarbolini
Copy link
Contributor

paolobarbolini commented Nov 12, 2021

Here is how I have always built psst:

$ export LIBCLANG_PATH=/usr/local/lib
$ export RUSTFLAGS='-C codegen-units=1 -C debuginfo=0 -C opt-level=0'
$ cargo build

I have to disable optimizations, otherwise rustc(1) (read: LLVM) runs OOM while building the gtk crate; the errors vary, but the cause is always the same. (rustc's RSS grows short to 3G before OpenBSD kills the process.)

Now I did

$ export LIBCLANG_PATH=/usr/local/lib
$ export RUSTFLAGS='-C codegen-units=1 -C debuginfo=0 -C opt-level=0'
$ cargo build --release

and ./target/release/psst-gui exhibits the exact same behaviour.

FWIW, here are the changes I need on-top due to OpenBSD: https://gist.github.com/9b60ef7a2679f6cbc9fdaf61f6d069f3

AFAIK opt-level=0 means absolutely no optimizinations, inlining ecc. I'm not sure I completely understand what's causing OOM but you could try to pass it -j SOME_NUMBER to reduce the number of crates it compiles in parallel.

@klemensn
Copy link
Contributor

$ export LIBCLANG_PATH=/usr/local/lib
$ unset RUSTFLAGS
$ cargo build --release -j1

This builds the gtk crate but psst has the same problem, still; I can't hear a difference between release and debug.

@klemensn
Copy link
Contributor

klemensn commented Nov 12, 2021

e037c42 Fixes pause/play, i.e. psst-gui in fact pauses and resumes tracks, but the backend error is still printed whenever I pause:

[2021-11-12T20:06:53Z ERROR psst_core::audio::output] failed to stop stream: A backend-specific error has occurred: pausing is not implemented

Edit: while having a track paused to write this comment, another (new) error appeared and playback is completely silent now. Not even restarting psst-gui fixes it:

[2021-11-12T20:08:06Z ERROR psst_core::audio::output] audio output error: A backend-specific error has occurred: no bytes read; EOF?
[2021-11-12T20:08:15Z INFO  psst_core::player] resuming playback
[2021-11-12T20:08:19Z INFO  psst_core::player::file] blocked at 1263390
[2021-11-12T20:08:19Z INFO  psst_core::player::file] blocked at 633735
[2021-11-12T20:08:19Z INFO  psst_core::player::file] blocked at 636474
[2021-11-12T20:08:22Z INFO  psst_core::player] pausing playback

So I close the app and reopen it, but the no bytes read; EOF? immediately appears and no sound is ever played.
I have not wiped any cache or so.
This is with the latest commit.

@klemensn
Copy link
Contributor

Please don't treat these OpenBSD related issues as a blocker.

I'm just a user at this point and support for OpenBSD is still in its early days (other parts need fixing/updating regardless of the sound backend).

So if all this works on Linux/MacOS/Windows and is a clear improvement, the rest (read: OpenBSD/sndio support) can still be done afterwards, imho.

@jpochyla jpochyla merged commit 96059d9 into master Nov 16, 2021
@jpochyla jpochyla deleted the cpal-symphonia branch November 16, 2021 14:50
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.

5 participants