Skip to content

Fix crackling audio when using Sink#320

Merged
est31 merged 2 commits intoRustAudio:masterfrom
HalfVoxel:master
Oct 3, 2020
Merged

Fix crackling audio when using Sink#320
est31 merged 2 commits intoRustAudio:masterfrom
HalfVoxel:master

Conversation

@HalfVoxel
Copy link
Copy Markdown
Contributor

Fixes #318.
Fixes #319.
Fixes #266.

Combined, these two fixes also fix crackling audio on my computer. The reason for the crackling audio was that Done did not implement size_hint so it would always fall back to a frame size of 512. Somewhere down the pipeline this causing crackling every 512 samples I suspect (which might be worth fixing too).

The current_frame_length fix is necessary because in my case I was playing from a sound file and it returns a size hint on the format (very_large_number, Some(very_large_number)) and the current_frame_length method would just ignore that and fall back to a frame length of 512 samples anyway.

This might also fix #225 (I am running on linux, so I cannot test that).

Copy link
Copy Markdown
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.

There should still be an upper bound to the length. It influences buffer sizes for example. If THRESHOLD is too small and that causes issues, one can define another larger constant by which to clamp the returned value, e.g. 4096.

@est31 est31 added this to the 0.12 milestone Oct 2, 2020
@HalfVoxel
Copy link
Copy Markdown
Contributor Author

Is this method really the right place to clamp it though? If a method further down the pipeline needs to break things up into chunks for more efficient buffering it should be free to do that anyway, right?

The frame_len is only supposed to be an upper bound on the allowed buffer if I understand it correctly.

@HalfVoxel
Copy link
Copy Markdown
Contributor Author

In a test I changed it to always return Some(4096) and that does get rid of the crackling as well.

@est31
Copy link
Copy Markdown
Member

est31 commented Oct 3, 2020

Hmm yeah I think I misread src/source/buffered.rs. It's fine.

Copy link
Copy Markdown
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 3bc614f into RustAudio:master Oct 3, 2020
@HalfVoxel
Copy link
Copy Markdown
Contributor Author

Hmmm, actually UniformSourceIterator::bootstrap does use the frame size without any limits. Might want to have some limit there.

@est31
Copy link
Copy Markdown
Member

est31 commented Oct 3, 2020

@HalfVoxel PRs welcome!

@HalfVoxel
Copy link
Copy Markdown
Contributor Author

I would gladly open one, but I'm not sure what the best limit value is? I haven't got enough insight into the library to have a feel for the possible consequences.

@est31
Copy link
Copy Markdown
Member

est31 commented Oct 3, 2020

buffered.rs uses 32768. I guess that value is okay.

@Xaeroxe
Copy link
Copy Markdown
Contributor

Xaeroxe commented Oct 3, 2020

giphy

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

3 participants