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

Update README.md with better example #23

Open
wants to merge 2 commits into
base: develop
Choose a base branch
from

Conversation

hollingsworthd
Copy link
Contributor

This has less glitches/pops in audio playback on web and desktop

@reki2000
Copy link
Owner

@hollingsworthd
Thanks for your suggestion. Let me clarify your idea in two points:

  • What does 'better' means in this example? Less glitch/noise?
  • Checking kIsWeb doesn't align with the goal of this package (multi-platform), especially in the example code. Is it possible to remove this check?

@hollingsworthd
Copy link
Contributor Author

Yes this produces less clicks/pop. I don't know a good way around this. Maybe I can put the web sample.length at 44100 and bufLen at 8500 and just leave a comment that these should maybe be adjusted per dev judgment. I would also comment that depending on the sound card on desktop there might be no options to play sound at non-standard sample rates (44.1k, 46.0k, etc)

I did try to modify the C and JS code to get the push() method to return the number of samples written thinking that may allow more options for implementors to handle exhaust/full states. But after several hours of work I failed. I might try again one day.

@hollingsworthd
Copy link
Contributor Author

Regardless I think the code looks cleaner than the current example code, IMO, but it will produce different results. The current conditional of t > 0 bothers me a bit because it's just to fill the buffer more.

@hollingsworthd
Copy link
Contributor Author

It may be appropriate to also just revert to your original example code.

Sorry btw for spamming you 3x with comments. :) And thank you very much for open sourcing this project. It's amazing and I hope more people find it. Across the internet many people ask how to synthesize audio and on all platforms for Flutter.

@reki2000
Copy link
Owner

reki2000 commented Jun 1, 2024

@hollingsworthd
Thakns for details. I believe this sample code should avoid platform dependent logic, such as the kIsWeb based parameter switch. Can we refactor this to use a single parameter for both web and other platforms?

@hollingsworthd
Copy link
Contributor Author

hollingsworthd commented Jul 13, 2024

I think I figured it out. If you use a stopwatch to measure the time between calls to push audio samples (to get the perfect buffer size) then it cuts out most pops. The one second periodic timer is only approximate but when used together with a stopwatch it's effective. I'll need to create a clean example of this.

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.

2 participants