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

Generate samples at a multiple of channels #249

Merged
merged 2 commits into from Nov 16, 2019
Merged

Conversation

Cocalus
Copy link
Contributor

@Cocalus Cocalus commented Sep 13, 2019

I ran into an issue where pausing and unpausing would swap the channels of a stereo source. It was because the number of samples the pause source generated while paused wasn't a multiple of 2. This has two fixes for that issue.

One Pausable now always generates Zero's in a multiple of the number of channels.
Two Periodic only changes things at a multiple of the number of channels. This might help other similar issues.

I also fixed Delay since it has the same potential issue.

I suspect channel ordering issues can also occur if current_frame_len isn't a multiple of the channels, when called before iterating. Also if the iterator returns a None that's not aligned with current_frame_len. Those could cause the issue with Repeat, Stop and various ways of chaining Sources.

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.

Overall ok, except for two nits. Sorry for the late review.

/// Sets whether the filter applies.
///
/// If set to true, the inner sound stops playing and no samples are processed from it.
#[inline]
pub fn set_paused(&mut self, paused: bool) {
self.paused = paused;
// Minimize calls to channels by only calling on state change
Copy link
Member

Choose a reason for hiding this comment

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

This comment is wrong. You always call the function when paused is true, even if it was true before. In general, set_paused shouldn't be called often so minimizing calls shouldn't be needed. I'd say you should just remove the comment.

Pausable {
input: source,
paused: paused,
paused_channels: paused_channels,
Copy link
Member

Choose a reason for hiding this comment

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

Please use init shorthand here.

@est31 est31 mentioned this pull request Nov 16, 2019
@Cocalus
Copy link
Contributor Author

Cocalus commented Nov 16, 2019

I think I fixed those issues. Long term using something like the sample crates Frame trait would avoid these issues at the type level, but that's a big change to the API.

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.

Thanks!

@est31 est31 merged commit d8723a5 into RustAudio:master Nov 16, 2019
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.

None yet

2 participants