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

migrate Sample to dasp_sample #689

Merged
merged 31 commits into from
Oct 7, 2022
Merged

migrate Sample to dasp_sample #689

merged 31 commits into from
Oct 7, 2022

Conversation

kawogi
Copy link
Contributor

@kawogi kawogi commented Sep 9, 2022

This is an attempt to fix #488
In the course of migrating to dasp_module I also unlocked i8, u8, i32, u32, i64, u64 and f64. I did not extend this to i24, u24, i48, u48 because I felt there are open question regarding the memory layout.

So far I did run the tests for ALSA. I might need some help to adjust the implementation for other platforms.

@dheijl
Copy link
Contributor

dheijl commented Sep 11, 2022

I opened #488 because I needed i24 support, but I ended up rolling my own f32 -> i24 conversion because I didn't need anything else.

Copy link
Member

@est31 est31 left a comment

Choose a reason for hiding this comment

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

LGTM. I think this moves into the direction that @mitchmindtree suggested.

examples/synth_tones.rs Outdated Show resolved Hide resolved
@kawogi
Copy link
Contributor Author

kawogi commented Sep 12, 2022

Any advice on how I could verify that the non-Linux code would compile?

@est31
Copy link
Member

est31 commented Sep 13, 2022

@kawogi you can check the CI of this PR for that.

@est31
Copy link
Member

est31 commented Sep 13, 2022

There seem to be some build failures for some of the platforms, that seem to be caused by this PR.

@kawogi
Copy link
Contributor Author

kawogi commented Sep 13, 2022

Can someone tell me how I can get IDE support for foreign platforms in VScode? At the moment I'm trying to fix the above problems blindly with rust-analyzer being ignorant about everything.

@kawogi
Copy link
Contributor Author

kawogi commented Sep 14, 2022

Hmmm - is that correct that I have to fix edition 2018 deprecation stuff? Did I branch off from the correct branch?
Also: one test keeps failing because of some stack depth error which looks completely unrelated to my changes.

@kawogi
Copy link
Contributor Author

kawogi commented Sep 18, 2022

Friendly reminder: can I get another run?

@est31
Copy link
Member

est31 commented Sep 18, 2022

I've re-pushed the button. Sadly, github does not expose an UI option to trust new contributors, even if you just want to trust one.

@kawogi would it be possible to file a trivial PR to this repo? Ideally something that might still be useful at least a little. Then you would be auto approved by github.

@kawogi
Copy link
Contributor Author

kawogi commented Sep 20, 2022

I just removed the lossy sample type conversion in asio as i32 and f64 are now supported by cpal.

@kawogi
Copy link
Contributor Author

kawogi commented Sep 20, 2022

The failing check is about the recursion depth when compiling stdweb. Updating the version in the Cargo.toml solves this problem as the current version is rather old, but that gives me all kinds of new problems.

Can someone have a look into this so I can check that my changes aren't breaking the test (compiling to wasm-unknown-unknown works fine btw)?

@kawogi
Copy link
Contributor Author

kawogi commented Oct 6, 2022

Is there a way I can help to bring this forward?

@est31 est31 self-requested a review October 6, 2022 22:03
Copy link
Member

@est31 est31 left a comment

Choose a reason for hiding this comment

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

Looks great, thanks for doing this!

@est31 est31 merged commit 0631a45 into RustAudio:master Oct 7, 2022
est31 pushed a commit that referenced this pull request Oct 7, 2022
@est31
Copy link
Member

est31 commented Oct 7, 2022

85d773d

@kawogi kawogi deleted the dasp_sample branch October 17, 2022 08:08
Hoodad pushed a commit to EmbarkStudios/cpal that referenced this pull request Feb 8, 2023
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.

Is there any reason for not using dasp-sample ?
4 participants