-
Notifications
You must be signed in to change notification settings - Fork 390
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
Resolve int16 wt interpretation documentation and comparability #7694
Comments
Yeah this is a remnant from before the format supported full height 16 The full fix here is
Sort of one of those "quick hours" so let me tag it into 1.3.3 so we remember |
@baconpaul IMO this basically means keep the "peak" normalization mode (not as an option but to use it when encoding) because it will make sure all waveforms are hitting that 2^15. Right? EDIT: Actually... no. Normalization should still be an option - for some WTs it makes more sense to have all waveforms peak normed, for some it doesn't. What int-as-15 flag should make sure is just that things don't go over 2^15, UNLESS peak normalizing option is set, which would then drive through all frames and peak norm them. |
Also update the .wt format spec (especially that maximum frame length is 4096 now). |
I think here the spec could be a bit more explicit:
The goal is to have the same sound for everyone. |
Lemme look at what we do in the surge engine and then we can compare notes. Will add it to my list for this week |
FYI, Bitwig was previously loading the 16 bits value into a short, then scaling accordingly to the flag then clipping to -1..1. |
For the mitigation of the wavetable I propose this: int16_t MAX15 = 1 << 14;
int16_t IS16_THR = MAX15 + MAX15 / 4;
int16_t sample = ...;
if ((sample <= -IS16_THR) || (IS16_THR <= sample))
setIs16Bits(true); |
Another option for the spec is to say: no mitigation, if the flags says 15 bits, then it is 15 bits and hard clip. |
Note for myself: 36445 (related issue number in Bitwig's bug tracker) |
Here's what surge does today surge/src/common/dsp/Wavetable.cpp Line 248 in 4fce6dd
If we get full 16 bits flag we pre-process the int with
Except for the We never check if the i15 inbound has bit 16 set. But the >>1 will make sure i16 inbound doesnt Reading this now, a more accurate approach would be to have an I propose the following then
and that seems to be it right? |
I'd keep the --peak flag (rename it to --norm) that would simply peak normalize all frames, optionally. |
Right normalize them to max is +/-2^14 in int15 leave 16 unchanged? Or do you want to re-peak the 16 also so the highest value is 2^15 |
Repeak in both cases to the max value (0 dBFS or -6 dBFS) |
Gotcha. So -i15 no repeak might clip I think we want also a -reduce16to15 which just does a <<1 and leaves it alone. But that's an easy mode to add as well. |
My advice for |
Oh that does solve all the problems right duh me, good idea Alex |
I agree with this |
Ugh this turns out to be a bit uglier to fix for a variety of bad choices in surge. Those choices are
so this one won't be a 'this afternoon' thing. I think what I will need to do is preserve the original wavetable data to stream it into the patch. A bit tricky. |
Wow turns out surge at head does the same today if you load a 16-as-16 wavetable. I guess no-one has been importing the bitwig factory wavetables into surge! This definitely needs a fix. Ugh. Probably the way to do it is to preserve the data and then patch the window oscillator |
wt files with full-range 16 bit ints (so 16-is-16 or flag 0x8 + 0x4 = 0xC) was broken in a couple of ways, but most importnatly, didn't survive a save/restore roundtrip. This fixes it in three ways 1. When converting 16-as-16 to float scale accordingly with a different function and leave the ints untouched 2. Modify the window oscillator so if it gets a 16-as-16 it reduces the height of the ints to be consistent with a 15 with an extra shift, making I15 WT vs Window and I16 WT vs Window consistent 3. Make sure it streams properly. No change required but tested. Addresses surge-synthesizer#7694
wt files with full-range 16 bit ints (so 16-is-16 or flag 0x8 + 0x4 = 0xC) was broken in a couple of ways, but most importnatly, didn't survive a save/restore roundtrip. This fixes it in three ways 1. When converting 16-as-16 to float scale accordingly with a different function and leave the ints untouched 2. Modify the window oscillator so if it gets a 16-as-16 it reduces the height of the ints to be consistent with a 15 with an extra shift, making I15 WT vs Window and I16 WT vs Window consistent 3. Make sure it streams properly. No change required but tested. Addresses surge-synthesizer#7694 Update wt-tool to allow int16 and int15 options both
OK I got it all dealt with, including making the options to wt-tool more rational and testing them fully. Will merge shortly. |
That was fast! |
wt files with full-range 16 bit ints (so 16-is-16 or flag 0x8 + 0x4 = 0xC) was broken in a couple of ways, but most importnatly, didn't survive a save/restore roundtrip. This fixes it in three ways 1. When converting 16-as-16 to float scale accordingly with a different function and leave the ints untouched 2. Modify the window oscillator so if it gets a 16-as-16 it reduces the height of the ints to be consistent with a 15 with an extra shift, making I15 WT vs Window and I16 WT vs Window consistent 3. Make sure it streams properly. No change required but tested. Addresses #7694 Update wt-tool to allow int16 and int15 options both
Yeah, as of now I feel that it is the best option. Let's take a moment to think about it? I'll also adjust Bitwig's code and try it. |
Doesn't this change the sound of existing patches? |
Actually it preserves it. In bitwig we would have played with up to +6dB already and maybe surge too? |
I'm fine with that spec, then! |
Yeah the nice thing about what I wrote is it is actually what surge does after my push I’m off to the dentist now but let me add that sentence to the spec and then maybe cherry pick it and one or two other things into a surge and surge rack point release yeah! |
If you don't mind waiting a bit before racking a point release, I'll need a bit of time but I'd like to check this with Claes as well. (we're very busy atm). |
By the way the spec doesn't mention if the cycle size has to be a power of 2. |
It had to be power of 2 in surge for a variety of reasons and think that’s the case in serum and vital also. May be a good thing to put in spec I’ll reopen this and change the title - and no rush on a point release but would be nice for surge and rack to have working access to their bitwig wt content properly |
You can install all bitwig packs, then right click on a wavetable -> reveal file and you'll find them. The folder structure is quite simple, so go down a few directories and you can find all We have wavetables spread over a few packs I think. |
I wouldn't mind if BWS offers a pack that is all WTs only (merging WTs from disparate packs). :) |
Yeah this is how I found one to test and found the bug I fixed yesterday I’m debating modifying surge storage wavetable scans to look for the bitwig install locations and add them to the menu …. I know the location in Mac but don’t suppose you can share where the packs install win lin could you? |
I guess it should read the BWS prefs file to get the user overriden path, too, then. |
I think it is a bad idea. But you're free to try. You could try the standard locations, if they exists show them to the user. Then let the user add additional wavetables locations in Surge's settings. |
I agree alex. If the user wants additional things they can use soft links to our search paths today. I was just thinking of the "I haven't screwed with my settings lets just make it work magically" |
I've implemented the discussed change in bitwig (allow the +6dB) it'll be in the next beta release. |
OK great. Did you happen to see the windows wavetable path? |
|
It seems that the specification file for the .wt file format hasn't been entirely updated yet, it is missing:
|
Updating it! |
Ahh I was going to do this along with the bitwig paths once you shared the bitwig resource path on windows with me Alex! Do you happen to have that? Or I can ask a bitwig windows user if it’s not easy to see from the code |
|
Ahh and see it is here now. Cool let’s leave this open until I modify storage |
OK to avoid confusion I opened that up as a separate FR in #7720 and will now close this as done. Good stuff! |
…esizer#7703) wt files with full-range 16 bit ints (so 16-is-16 or flag 0x8 + 0x4 = 0xC) was broken in a couple of ways, but most importnatly, didn't survive a save/restore roundtrip. This fixes it in three ways 1. When converting 16-as-16 to float scale accordingly with a different function and leave the ints untouched 2. Modify the window oscillator so if it gets a 16-as-16 it reduces the height of the ints to be consistent with a 15 with an extra shift, making I15 WT vs Window and I16 WT vs Window consistent 3. Make sure it streams properly. No change required but tested. Addresses surge-synthesizer#7694 Update wt-tool to allow int16 and int15 options both
Hi,
According to the specification, if you want to encode 16 bits samples, you need the
flags = 0x0C
while it currently setsflags = 0x04
which is for 15 bits samples.Reference: https://github.com/surge-synthesizer/surge/blob/main/resources/data/wavetables/WT%20fileformat.txt#L11
When looking at https://github.com/surge-synthesizer/surge/blob/main/scripts/wt-tool/wt-tool.py#L43
The bug seems to be here:
surge/scripts/wt-tool/wt-tool.py
Line 43 in 94c8c5b
Cheers,
Alex
The text was updated successfully, but these errors were encountered: