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

sysaudio: rewrite in zig #641

Merged
merged 6 commits into from
Dec 19, 2022
Merged

sysaudio: rewrite in zig #641

merged 6 commits into from
Dec 19, 2022

Conversation

alichraghi
Copy link
Contributor

@alichraghi alichraghi commented Dec 16, 2022

  • By selecting this checkbox, I agree to license my contributions to this project under the license(s) described in the LICENSE file, and I have the right to do so or have received permission to do so by an employer or client I am producing work for whom has this right.

Closes #625

blockers:

@alichraghi alichraghi force-pushed the main branch 2 times, most recently from 6405617 to 4fa8272 Compare December 17, 2022 13:31
@alichraghi
Copy link
Contributor Author

ready for review. CI failure is because system-sdk must be updated and after that piano example i guess

@kdchambers
Copy link
Contributor

kdchambers commented Dec 17, 2022

I'm getting a similar error as the Linux CI, only for Jack. Obviously I don't have an appropriate audio back-end installed (Pipewire), but perhaps this could be caught at comptime and return a nicer error. I guess this should be fixed once the audio sdk is updated?

As for Context depending on itself, that seems real unfortunately. Maybe just a limitation in Zig that needs to be worked around for now. Similar to ziglang/zig#131

const c = @cImport(@cInclude("jack/jack.h"));
          ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
referenced by:
    Player: libs/sysaudio/src/jack.zig:175:14
    Player: libs/sysaudio/src/jack.zig:169:20
    remaining reference traces hidden; use '-freference-trace' to see all reference traces

/home/keith/github/wrkdir/mach_ali/zig-cache/o/56e0d4bbaaf593154d693621f05d82e3/cimport.h:1:10: error: 'jack/jack.h' file not found
#include <jack/jack.h>
         ^
libs/sysaudio/src/jack.zig:7:21: error: struct 'jack.Context' depends on itself
pub const Context = struct {
                    ^~~~~~
libs/sysaudio/src/jack.zig:7:21: error: struct 'jack.Context' depends on itself
pub const Context = struct {
                    ^~~~~~
libs/sysaudio/src/jack.zig:7:21: error: struct 'jack.Context' depends on itself
pub const Context = struct {
                    ^~~~~~
libs/sysaudio/src/main.zig:104:20: error: struct 'main.Player' depends on itself
pub const Player = struct {
                   ^~~~~~
libs/sysaudio/src/jack.zig:7:21: error: struct 'jack.Context' depends on itself
pub const Context = struct {
                    ^~~~~~
libs/sysaudio/src/jack.zig:7:21: error: struct 'jack.Context' depends on itself
pub const Context = struct {
                    ^~~~~~
libs/sysaudio/src/backends.zig:28:15: error: union 'backends.BackendPlayer__union_4717' depends on itself
    .linux => union(enum) {
              ^~~~~
libs/sysaudio/src/jack.zig:7:21: error: struct 'jack.Context' depends on itself
pub const Context = struct {
                    ^~~~~~
libs/sysaudio/src/backends.zig:28:15: error: union 'backends.BackendPlayer__union_4717' depends on itself
    .linux => union(enum) {
              ^~~~~
libs/sysaudio/src/jack.zig:7:21: error: struct 'jack.Context' depends on itself
pub const Context = struct {
                    ^~~~~~
libs/sysaudio/src/backends.zig:28:15: error: union 'backends.BackendPlayer__union_4717' depends on itself
    .linux => union(enum) {
              ^~~~~
libs/sysaudio/src/backends.zig:28:15: error: union 'backends.BackendPlayer__union_4717' depends on itself
    .linux => union(enum) {
              ^~~~~
libs/sysaudio/src/backends.zig:28:15: error: union 'backends.BackendPlayer__union_4717' depends on itself
    .linux => union(enum) {

I also got the following error when cloning recursively:

fatal: No url found for submodule path 'libs/sysaudio/upstream' in .gitmodule

Which I fixed by removing the path git rm --cached libs/sysaudio/upstream

Seeing as this works for all other targets, if it isn't convenient to test on Linux we could merge this (Or put into a new branch) and I'll fix up these minor issues.

@alichraghi
Copy link
Contributor Author

alichraghi commented Dec 17, 2022

@kdchambers for now you need to checkout into my SDK PRs i mentioned above.
clone them somewhere and set MACH_SDK_PATH=<the_sdks_dir>
also fixed the git error. thanks
btw i just saw u got the actual reason lol

@alichraghi alichraghi force-pushed the main branch 7 times, most recently from 408e6e5 to e3752be Compare December 18, 2022 18:32
pub fn setVolume(self: *Player, vol: f32) !void {
_ = self;
_ = vol;
@panic("incompatible backend");
Copy link
Member

Choose a reason for hiding this comment

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

Is panic desirable behavior here? Seems like this should be a documented no-op instead maybe.

@emidoots
Copy link
Member

Great work on this. I see this as the future so will merge. I think we regress on a few things:

  1. macOS is not currently supported
  2. Linux requires e.g. pulseaudio and jack to both be installed, since we dynamically link against them. In the future we should make Linux backends use dlopen so that if not installed we can provide a nicer error / just disable the backend which doesn't exist.
  3. WebAudio support is removed for now (but is being worked on.)

We'll need to address these in the near future, but for now this is a great step forward.

@emidoots emidoots merged commit ccc938e into hexops:main Dec 19, 2022
@emidoots emidoots mentioned this pull request Dec 27, 2022
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.

sysaudio: rewrite (drop libsoundio dependency)
3 participants