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

Support OpenBSD #191

Closed
wants to merge 4 commits into from
Closed

Support OpenBSD #191

wants to merge 4 commits into from

Conversation

klemensn
Copy link
Contributor

@klemensn klemensn commented Oct 6, 2021

Track merge requests in dependencies/crates and update documentation accordingly.

All patches required to run psst on OpenBSD/amd64 have been submitted upstream:

Dependencies have been carefully patched to merely integrate those patches
required for OpenBSD support, i.e. no new features or unrelated changes are
pulled in.

This makes cargo build produce a working ./target/debug/psst-gui.

This is already enough to create an OpenBSD port (from my fork) and provide
working binary packages to users.

Input on how to best incorporate all this would be welcome, hence this draft.

@klemensn klemensn marked this pull request as draft October 6, 2021 07:41
@klemensn klemensn changed the title Mention OpenBSD support Support OpenBSD Oct 8, 2021
@jpochyla
Copy link
Owner

jpochyla commented Oct 14, 2021

Hi @klemensn, thanks a lot for the PR and for being interested! I'm sorry for being a bit late here.

Lately I've been trying to switch away from miniaudio-rs crate, and there's a fresh #197 that explores this. Do you think we could wait until it gets merged? I reckon we should build on OpenBSD without any patches with it.

@klemensn
Copy link
Contributor Author

Hi @klemensn, thanks a lot for the PR and for being interested! I'm sorry for being a bit late here.

No worries!

Lately I've been trying to switch away from miniaudio-rs crate, and there's a fresh #197 that explores this. Do you think we could wait until it gets merged? I reckon we should build on OpenBSD without any patches with it.

miniaudio-rs was the only architecture specific crate I encountered, so it being removed would probably ease supporting other architectures (supported by OpenBSD), e.g. macppc, arm64, powerpc64, sparc64, etc.

#197 with fb7d591 cherry-picked on top builds but does not work:

$ ./target/debug/psst-gui
[2021-10-14T10:34:59Z INFO  psst_gui::data::config] loading config: "/home/kn/.config/Psst/config.json"
thread 'main' panicked at 'called `Result::unwrap()` on an `Err` value: AudioOutputError(DeviceNotAvailable)', psst-gui/src/controller/playback.rs:57:42
note: run with `RUST_BACKTRACE=1` environment variable to display a backtrace

Somewhat expected, (without having looked at their code at all) cpl and/pr symphonia need support for OpenBSD's native sndio -- they might also work with our portaudio or pulseaudio ports/packages, but less layers is obviously preferrable.

I'll look into this.

Merge as you seem fit; I reckon (using) new upstream releases for piet and druid will be required anyway regardless of OpenBSD support.

@klemensn
Copy link
Contributor Author

On that note: another hurdle for packaging psst on OpenBSD is its use of git crates, i.e. dependencies sourced from somewhere else but the registry.

This is a general issue in our infrastructure for cargo based software as builds happen without network access and thus the framework fetches crates ahead of build time -- in this regard, git crates behave differently. I have already added support for all this precisely so that psst can be packaged, but I guess git crates will continue to be a maintainence burden for OpenBSD porters.

In an ideal word, all of the porting work gets upstreamed, released and psst can solely depend on "official" crates rather than temporary forks with patches/hacks... but let's face it: software doesn't work this way! ;-)

@klemensn
Copy link
Contributor Author

$ ./target/debug/psst-gui
[2021-10-14T10:34:59Z INFO  psst_gui::data::config] loading config: "/home/kn/.config/Psst/config.json"
thread 'main' panicked at 'called `Result::unwrap()` on an `Err` value: AudioOutputError(DeviceNotAvailable)', psst-gui/src/controller/playback.rs:57:42
note: run with `RUST_BACKTRACE=1` environment variable to display a backtrace

Somewhat expected, (without having looked at their code at all) cpl and/pr symphonia need support for OpenBSD's native sndio -- they might also work with our portaudio or pulseaudio ports/packages, but less layers is obviously preferrable.

Lucky us: some folks (incl. two OpenBSD developers) have been tackling sndio support already RustAudio/cpal#493

@klemensn
Copy link
Contributor Author

klemensn commented Oct 14, 2021

So while sound code is being revamped, we could still merge druid and piet updates as that's basically done.
In order to have psst build from registry dependencies, we'd need to have

  • piet ship another release
  • druid merge your jpochyla/druid@a9522f5 fix
  • druid ship another release
  • bump dependencies

Starting with 2. I can prod this.

@klemensn
Copy link
Contributor Author

piet released 0.5.0 containing my commit.
druid master contains my fix and an internal update to piet 0.5.0.

I tried bumping both dependencies in psst but my rust/cargo foo is not strong enough;
it keeps building piet-0.5.0-pre1 which obviously fails.

Can someone help me out?

@jpochyla
Copy link
Owner

Hi @klemensn, I've updated the Druid dep in master!

@klemensn
Copy link
Contributor Author

Thanks a bunch. Would you mind telling me how to do that?
I've modified the Cargo.{lock,toml} files and/or run cargo update -p piet -p piet-common ... but somehow it always went back to the -pre1 version.

@jpochyla
Copy link
Owner

Oh, sorry, it's locked in Cargo.toml to my psst branch of Druid, I've rebased it on top of master.

@klemensn
Copy link
Contributor Author

klemensn commented Nov 17, 2021

Once Sinono3/souvlaki#19 is merged, another overridden dependency can go.

  • crank souvlaki after OpenBSD build fix -> done in master

@klemensn
Copy link
Contributor Author

klemensn commented Nov 17, 2021

Playback works with cpal but has remaining issues

README.md Outdated Show resolved Hide resolved
@klemensn
Copy link
Contributor Author

klemensn commented Nov 18, 2021

Alright, cpal seems to be the last blocker (for now!).
I'll see what I can do over there to get the sndio backend working properly (and help land it in master).

@klemensn
Copy link
Contributor Author

Rebasing this branch onto latest master breaks playback:

$ RUST_BACKTRACE=full ./target/release/psst-gui
[2021-11-30T00:10:07Z INFO  psst_gui::data::config] loading config: "/home/kn/.config/Psst/config.json"
[2021-11-30T00:10:07Z INFO  psst_core::audio::output] using audio device: "sndio default device"
[2021-11-30T00:10:07Z INFO  psst_core::cache] using cache: "/home/kn/.cache/Psst"
[2021-11-30T00:10:07Z INFO  psst_core::audio::output] opening output stream: StreamConfig { channels: 1, sample_rate: SampleRate(48000), buffer_size: Default }
[2021-11-30T00:10:07Z INFO  psst_gui::data::config] saved config: "/home/kn/.config/Psst/config.json"
[2021-11-30T00:10:07Z INFO  psst_core::session::access_token] access token expired, requesting
[2021-11-30T00:10:09Z INFO  symphonia_format_ogg::demuxer] starting new physical stream
[2021-11-30T00:10:09Z INFO  symphonia_format_ogg::demuxer] selected vorbis mapper for stream with serial=0x0
[2021-11-30T00:10:09Z INFO  psst_core::player] starting playback
[2021-11-30T00:10:09Z INFO  psst_gui::controller::playback] playing
thread '<unnamed>' panicked at 'index out of bounds: the len is 1 but the index is 1', psst-core/src/audio/source.rs:65:13
stack backtrace:
   0:      0x33b342e3bbc - <unknown>
   1:      0x33b3431ca6b - <unknown>
   2:      0x33b342d32e3 - <unknown>
   3:      0x33b342d686b - <unknown>
   4:      0x33b342d63f7 - <unknown>
   5:      0x33b342d6f56 - <unknown>
   6:      0x33b342e43a5 - <unknown>
   7:      0x33b342e3d07 - <unknown>
   8:      0x33b342d6a26 - <unknown>
   9:      0x33b34311150 - <unknown>
  10:      0x33b34311117 - <unknown>
  11:      0x33b340700ce - <unknown>
  12:      0x33b34066cc8 - <unknown>
  13:      0x33b3406e41a - <unknown>
  14:      0x33b341e6be0 - <unknown>
  15:      0x33b340a63da - <unknown>
  16:      0x33b3403339a - <unknown>
  17:      0x33b342de179 - <unknown>
  18:      0x33d46e136a1 - <unknown>
  19:      0x33e0cc4808a - <unknown>

Yes, ".cargo" is in .gitignore and this is better set in the developer
shell's environment, but having it here documents the issue and greatly
helps hacking on it.

https://doc.rust-lang.org/nightly/cargo/reference/config.html#env
`[env]` has been stabalised in Rust 1.56.0, so crank the requirement.

This is probably best dealt with in the OpenBSD port's Makefile once
psst is ready for it.
Done with editing Cargo.toml followed by `cargo update -p cpal`,
which, sadly, change a whole bunch of other dependencies.

This makes audio play, but two issues remain:

1. Playback speed is noticably slow.
2. Toggling play -> shows this error, although playback is successfully
   paused:
   ```
   [2021-11-17T16:03:04Z ERROR psst_core::audio::output] failed to stop stream: A backend-specific error has occurred: pausing is not implemented
   ```
"Add support for OpenBSD platform via sndio host"
RustAudio/cpal#493 is based on an older cpal
release;  maybe rebasing on top of HEAD and therefore bringing in the
cpal changes that current psst already uses helps with/fixes any of the
two issues?
This fixes the following whenever I try playin a file:
```
$ RUST_BACKTRACE=full ./target/release/psst-gui
[2021-11-30T00:10:07Z INFO  psst_gui::data::config] loading config: "/home/kn/.config/Psst/config.json"
[2021-11-30T00:10:07Z INFO  psst_core::audio::output] using audio device: "sndio default device"
[2021-11-30T00:10:07Z INFO  psst_core::cache] using cache: "/home/kn/.cache/Psst"
[2021-11-30T00:10:07Z INFO  psst_core::audio::output] opening output stream: StreamConfig { channels: 1, sample_rate: SampleRate(48000), buffer_size: Default }
[2021-11-30T00:10:07Z INFO  psst_gui::data::config] saved config: "/home/kn/.config/Psst/config.json"
[2021-11-30T00:10:07Z INFO  psst_core::session::access_token] access token expired, requesting
[2021-11-30T00:10:09Z INFO  symphonia_format_ogg::demuxer] starting new physical stream
[2021-11-30T00:10:09Z INFO  symphonia_format_ogg::demuxer] selected vorbis mapper for stream with serial=0x0
[2021-11-30T00:10:09Z INFO  psst_core::player] starting playback
[2021-11-30T00:10:09Z INFO  psst_gui::controller::playback] playing
thread '<unnamed>' panicked at 'index out of bounds: the len is 1 but the index is 1', psst-core/src/audio/source.rs:65:13
stack backtrace:
   0:      0x33b342e3bbc - <unknown>
   1:      0x33b3431ca6b - <unknown>
   2:      0x33b342d32e3 - <unknown>
   3:      0x33b342d686b - <unknown>
   4:      0x33b342d63f7 - <unknown>
   5:      0x33b342d6f56 - <unknown>
   6:      0x33b342e43a5 - <unknown>
   7:      0x33b342e3d07 - <unknown>
   8:      0x33b342d6a26 - <unknown>
   9:      0x33b34311150 - <unknown>
  10:      0x33b34311117 - <unknown>
  11:      0x33b340700ce - <unknown>
  12:      0x33b34066cc8 - <unknown>
  13:      0x33b3406e41a - <unknown>
  14:      0x33b341e6be0 - <unknown>
  15:      0x33b340a63da - <unknown>
  16:      0x33b3403339a - <unknown>
  17:      0x33b342de179 - <unknown>
  18:      0x33d46e136a1 - <unknown>
  19:      0x33e0cc4808a - <unknown># Please enter the commit message for your changes. Lines starting
```

With this, `psst` plays audio fine on OpenBSD;  the playback speed is
back to normal!
@klemensn
Copy link
Contributor Author

[x] druid ship another release

So that just happened!

I haven't used psst ever since, basically because hacking and testing it on a still semi-supported OS was very time consuming.

I'll give this another shot to see how things improved.

@klemensn
Copy link
Contributor Author

klemensn commented Jan 4, 2024

Having ported spotifyd and spotify-qt to OpenBSD, which now ship as packages and work great, I won't persue this effort of finishing psst and its dependencies.

@klemensn klemensn closed this Jan 4, 2024
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.

2 participants