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

Add 44kHz WT and option for max voices #20

Merged
merged 6 commits into from
Sep 22, 2024
Merged

Conversation

rsp4jack
Copy link

The 44kHz WT is found at https://github.com/pedrolcl/sonivox/blob/0c7beb23c2da2f2b8d3aa7c7bf126b50c53e46f9/arm-wt-22k/jetcreator_lib_src/darwin-x86/wt_44khz.c.

I think it weird that the 44k one has the same samples as the 22k one, only articulations and regions are different. How it works? 😕

Tests are broken, and I do not know how to work on it.

@rsp4jack rsp4jack changed the base branch from master to devel September 17, 2024 02:57
Copy link
Owner

@pedrolcl pedrolcl left a comment

Choose a reason for hiding this comment

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

The 44kHz WT is found at https://github.com/pedrolcl/sonivox/blob/0c7beb23c2da2f2b8d3aa7c7bf126b50c53e46f9/arm-wt-22k/jetcreator_lib_src/darwin-x86/wt_44khz.c.

And should remain there. There is no need to make a copy.

I think it weird that the 44k one has the same samples as the 22k one, only articulations and regions are different. How it works? 😕

wt_22khz.c has both 8 bits and 16 bits samples conditioned by the symbol _8_BIT_SAMPLES, whilewt_44khz.c has only 8 bit samples.

Tests are broken, and I do not know how to work on it.

The test suite uses the sample rate as an argument. It only needed a small change to fix the tests.

Anyway, this branch has lower quality results than the default, which uses 16 bits samples. The sample rate has less impact on the overall quality. Instead of using a different embedded soundfont, a better solution is to use an external DLS.

CMakeLists.txt Outdated Show resolved Hide resolved
@pedrolcl
Copy link
Owner

You asked about how it works. The name wt_44khz.c is somewhat misleading. You interpreted that 44kHz was the sample rate of the included samples, but the each one of the 8 bit samples of this soundfont has different sample rate, and the only place where the number 44100 is used in this file is here, at line number 14695. This value is compared with the output sample rate when loading the soundfont here.

Here is an alternate PR using the other soundfont wt_22khz.c, but also with audio output sample rate of 44.1 kHz: #21

@rsp4jack
Copy link
Author

rsp4jack commented Sep 22, 2024

Got it. Thanks for reviewing, #21 is nice, and I think it is okay to close this PR (#20).

@rsp4jack rsp4jack closed this Sep 22, 2024
@rsp4jack rsp4jack mentioned this pull request Sep 22, 2024
@rsp4jack rsp4jack reopened this Sep 22, 2024
@rsp4jack
Copy link
Author

rsp4jack commented Sep 22, 2024

2aed483 (#20) removes arm-wt-22k/lib_src/wt_44khz.c, which is different from arm-wt-22k/jetcreator_lib_src/darwin-x86/wt_44khz.c. arm-wt-22k/lib_src/wt_44khz.c got 16 bits sample support. (I should mention it earlier 😕)

@pedrolcl
Copy link
Owner

2aed483 (#20) removes arm-wt-22k/lib_src/wt_44khz.c, which is different from arm-wt-22k/jetcreator_lib_src/darwin-x86/wt_44khz.c. arm-wt-22k/lib_src/wt_44khz.c got 16 bits sample support. (I should mention it earlier 😕)

Then, you should re-add your file, commit and push it.

@pedrolcl
Copy link
Owner

This works with both sample rates 22050 and 44100. I still don't like having, in the same repository, two large source files that are mostly identical. What about unifying them into a single one? We could rename the unified source as: "wt_200k_g.c", which was the original file name according to one top comment.

By the way, you can find the original soundfont "wt_200k_G.sf2" in this archive.

@rsp4jack
Copy link
Author

I just split the WTs and merged the same parts together. It should look better now.

@pedrolcl
Copy link
Owner

Thanks for your work!

@pedrolcl pedrolcl merged commit 8c1646d into pedrolcl:devel Sep 22, 2024
7 checks passed
@rsp4jack rsp4jack deleted the pr branch September 22, 2024 14:07
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