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

dasp::Frame: Generic impl for all [impl Sample;const N: usize] #180

Merged
merged 1 commit into from
Nov 11, 2023

Conversation

rawler
Copy link
Contributor

@rawler rawler commented Nov 15, 2022

Replace the hard-coded macro implementing Frame for [impl Sample;0-32], with a generic impl based on const-generics.

I discovered the need for this, when trying to simplify some macro- -implementations in the ebur128 crate with const-generics.

We cannot remove the need for NumChannels, since (see reference 1) "Associated Const Equality" is not yet stable. We can however adapt the NumChannels strategy from N1..N32 to a NChannels<N>-generic type.

Some opportunities like removing unsafe from zip_map, remain out of reach due to pending stabilization of the standard library.

1: rust-lang/rust#92827

@rawler
Copy link
Contributor Author

rawler commented Nov 15, 2022

I chose to leave out the custom impl:s of scale_amp and add_amp. I believe the trait-provided generic impl:s should be optimized by the compiler into something equivalent. These can easily be added back, if desired.

@rawler
Copy link
Contributor Author

rawler commented Nov 15, 2022

Would solve issue #179.

@rawler rawler changed the title dasp_frame:Generic impl for all [impl Sample;const N: usize] dasp::Frame: Generic impl for all [impl Sample;const N: usize] Nov 15, 2022
Replace the hard-coded macro implementing `Frame` for `[impl Sample;0-32]`,
with a generic impl based on const-generics.

I discovered the need for this, when trying to simplify some macro-
-implementations in the `ebur128` crate with const-generics.

We cannot remove the need for `NumChannels`, since (see reference 1)
"Associated Const Equality" is not yet stable. We can however adapt the
`NumChannels` strategy from `N1..N32` to a `NChannels<N>`-generic type.

Some opportunities like removing `unsafe` from `zip_map`, remain out of
reach due to pending stabilization of the standard library.

1: rust-lang/rust#92827
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 fine to me!

@thomascastleman
Copy link

I'd be interested in seeing this functionality get added - is there a particular reason this never got merged?

@est31 est31 merged commit 4f573d6 into RustAudio:master Nov 11, 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.

3 participants