Skip to content

Add wt import from oscdata to wavetable script state#8194

Open
nuoun wants to merge 15 commits intosurge-synthesizer:mainfrom
nuoun:wts-importwtdata
Open

Add wt import from oscdata to wavetable script state#8194
nuoun wants to merge 15 commits intosurge-synthesizer:mainfrom
nuoun:wts-importwtdata

Conversation

@nuoun
Copy link
Copy Markdown
Contributor

@nuoun nuoun commented Oct 18, 2025

Draft - do not merge yet.

So this adds the option to import wavetable data from each oscillator to the WTS Lua state table with an opt in so existing tables can be processed, edited, used with the PFFFT wrapper, etc.

If wt.importwt = true is true in init() it populates wt.osc1, wt.osc2, etc in the state table passed to generate(). The table layout is wt.osc#[frame index][sample index].

Thoughts welcome!

@baconpaul
Copy link
Copy Markdown
Collaborator

So the code looks fine but let me add the workflow question I added on discord last night.

Let's say you take a wavetable and do a simple transform like amplitude -> amplitude * 0.5. Whatever.

If you run the wtscript to generate 4 times you will end up getting like .00625 the amplitude since the 'prior' isnt' the unmodified table its the currently loaded table.

Am i missing something? Seems we need to keep a cache of 'original table' around somewhere for this feature?

@nuoun
Copy link
Copy Markdown
Contributor Author

nuoun commented Oct 18, 2025

Yes it is destructive and unlike processing in an actual audio/wt editor there is no undo state to go back to either, I considered the use scenario where a wavetable in one of the other oscillators gets used as the source here which would persist but that is perhaps not very intuitive.

With a cache the question becomes what is the most intuitive way to keep a cache of the original WT around and how to manage that, would it need to be saved with the patch?

@baconpaul
Copy link
Copy Markdown
Collaborator

I think its pretty important that seemingly idempotent scripts produce consistent results, yeah?

But yes this would stream twice. Ugh. Answer not obvious. Let's ponder a bit....

We could persist the original with the wtscript. but fragile and stateful.

@nuoun
Copy link
Copy Markdown
Contributor Author

nuoun commented Oct 19, 2025

So I like the possibilities this opens up combined with the simplicity of this approach but after giving it some more thought I think what is objectively bad here is that we're saving the script in .wtscript format or as metadata in .wav/.wt but this does not have any of the oscillator input data so how it's used here is not reproducible or as you put it with a word I had to look up it's not idempotent.

So back to the drawing table and I think a different approach altogether is instead of using Surge's oscillators as sources it could use two different import slots that can load a wavetable. Those slots could be set from the file/hamburger menu and get saved with the .wtscript, patch or metadata for .wt/,wav (the reason for it being two slots is being able to copy from two tables or do things like convolutions).

With that approach the issue becomes how to save the imported wavetable data in all 3 different formats: Surge patches, .wav/.wt metadata and the .wtscript XML, a lot more complicated than I had hoped but also a lot less hacky.

@baconpaul
Copy link
Copy Markdown
Collaborator

Oh sorry idempotent is a word from language nerd and services nerd world I should have been clearer

@Andreya-Autumn
Copy link
Copy Markdown
Collaborator

I read the code and comments now. Agree using the wavetable oscillator contents directly is not a good idea.

My thinking is the script editor could have a data cache with a persistent name, which is empty by default but can load data from a wavetable. The basic level is as we discussed, user loads a table, the frame count and samplerate gets set to that.

@Andreya-Autumn
Copy link
Copy Markdown
Collaborator

That seems to be where you're headed also. That data could be read-only, which would make many things simpler. Indeed there's no reason we need to limit to just one of these, but whether there's one or more I think a fixed set with good names is good UX for the script writer.

A possible next level from there is to have two representations of each loaded table. One is the constant read-only data from the wavetable itself, which the script can't interact with directly, the other is the Lua table that the user sees, and is computed with 2d interpolation from the wavetable size to the number of frames and samples the script UI is currently set to. That would be very convenient, but a bit trickier of course.

@nuoun nuoun closed this Mar 20, 2026
@nuoun nuoun deleted the wts-importwtdata branch March 20, 2026 20:35
@nuoun nuoun restored the wts-importwtdata branch March 20, 2026 20:41
@nuoun nuoun reopened this Mar 20, 2026
@nuoun nuoun force-pushed the wts-importwtdata branch 2 times, most recently from 63011db to 5f8a2eb Compare March 25, 2026 22:51
@nuoun
Copy link
Copy Markdown
Contributor Author

nuoun commented Mar 29, 2026

Sorry for the long delay!

I went back to this, and instead of using the wavetable data from each oscillator directly it now can load tables into a struct inside dawExtraState which, currently, has three different slots to hold wavetables.

UI-wise, all functionality sits in a custom submenu inside the WTSE hamburger menu, the struct and feature is currently called snapshots. Wavetables can be copied from the active scene's oscillators or loaded in using an OS file dialog:

image

On the Lua side, the snapshot slots are set inside a 3D table: wt.snapshot[slot][frame][sample]. Only the outer table is created when all slots are empty, and gaps in the slot table are allowed.

The .wtscript format allows for saving of the snapshot struct. The XML structure looks like this:

<?xml version="1.0" encoding="UTF-8" standalone="yes" ?><wtscript>
	<script lua="[base64 encoded data]" frames="10" samples="5" />
	<snapshots>
		<snapshot slot="0" n_tables="100" size="1024" data="[base64 encoded data]" />
		<snapshot slot="1" n_tables="16" size="2048" data="[base64 encoded data]" />
		<snapshot slot="2" n_tables="16" size="2048" data="[base64 encoded data]" />
	</snapshots>
</wtscript>

The block and each individual element are optional so if no snapshots are loaded, the file format is unchanged from before.

So what this does not do, right now, is saving the snapshots to patches or to .wav / .wt as metadata, or anywhere else that persists outside the current session. This is really a design choice that needs to be made and the main concern here I think is that saving a max size snapshot table (3 wavetables with 256 frames, 4096 samples each base64 encoded) could at most add roughly 16MB to patches and file exports which does seem very expensive for this feature.

On the other hand, omitting saving to all three formats does make it less logical to work with, and one future thing to consider is if someday we add an actual full fledged UI to edit wavetables it could in theory use the same struct.

So I hope this explains things well and feedback is most welcome!

@mkruselj
Copy link
Copy Markdown
Collaborator

IMO those snapshots don't need to be saved to WAV or WT formats, I don't see the reason to. You're importing WAV or WT to the snapshot, not the other way around - I'm not sure I see what problem would storing snapshots to WAV/WT solve. So it's fine if we don't have that.

It is, on the other hand, obligatory that snapshots are saved along with .wtscript since they are instrumental in having that .wtscript fully portable.

@nuoun
Copy link
Copy Markdown
Contributor Author

nuoun commented Mar 30, 2026

I added two unittests, maybe @baconpaul and @Andreya-Autumn have something to add otherwise I think this is ready for review (unless we want to rename this to something other than snapshots).

@baconpaul
Copy link
Copy Markdown
Collaborator

Oh i'm sorry I'm just reading this now.

I'm not sure why you did a few things

First, the WaveTableSnapshot structure seems basically identical to the Wavetable structure. Why is there a new structure?

Second, storing the wavetable as base64 encoded floats in the patch is really really expensive. Could we not have done the same thing we did with convolution and add more binary storage at the end for snapshots?

Finally I'm not sure why the snapshot is in the daw extra state at all. Wouldn't you need it to make the patch work if you share the patch outside of a daw context? The WTSE will do the wrong thing without the snapshot loaded.

So I think we need to think a bit more about serialization and workflow here.

@Andreya-Autumn
Copy link
Copy Markdown
Collaborator

Yeah I was just reading this too and I agree storing in DES is a bit strange.

The patch would still play correctly once the wavetable is generated. But everything else about the script saves with the patch, so users may expect to be able to open the script, make a small change, and re-generate. As Paul correctly points out that operation is non-portable as it stands now. And I agree doing what Surgo did for the convolution seems like the right call.

I also wonder if we can do something about frame count mismatches, but that's a problem we could deal with later. Other than those things I think this is much improved over the original proposal.

@nuoun
Copy link
Copy Markdown
Contributor Author

nuoun commented Mar 31, 2026

Thanks for the comments, responding to them one by one:

First, the WaveTableSnapshot structure seems basically identical to the Wavetable structure. Why is there a new structure?

Mostly because it's structure is an array for the snapshot slots, has an extra bool plus a lot of things in the Wavetable struct that are not needed for this, seemed different enough at first but I agree there's no reason not to use the Wavetable struct in another, snapshots dedicated struct.

Second, storing the wavetable as base64 encoded floats in the patch is really really expensive. Could we not have done the same thing we did with convolution and add more binary storage at the end for snapshots?

It's currently not storing in the patch at all (patches save the scripts, but only use wavetable data set in oscdata directly) , the base64 is just for saving and loading to the .wtscript format but I agree base64 does seem overkill since these are just floats, the floats bytecode still needs to be encoded for XML so I'll have a look at how Convolution does it.

Finally I'm not sure why the snapshot is in the daw extra state at all. Wouldn't you need it to make the patch work if you share the patch outside of a daw context?

That seemed like the logical place since we store a lot of Lua editors stuff there already, I'm not sure what a better home would be for it to be honest, it's not DSP so probably not OscillatorStorage. Suggestions welcome.

The WTSE will do the wrong thing without the snapshot loaded.
so users may expect to be able to open the script, make a small change, and re-generate.

The initial idea is that this is where .wtscript comes in, we can dedicate that format and that format alone to saving snapshots without adding a lot to the size of patches only for this feature, seems worth it imo, since otherwise it's also a footgun, a user will have no idea why a patch is ~16MB larger because deeply buried in a feature of a feature it's storing data that only a few users might find useful. So, again suggestions welcome.

I also wonder if we can do something about frame count mismatches, but that's a problem we could deal with later. Other than those things I think this is much improved over the original proposal.

Having no size constrains for frames and samples (other than it needing to be valid wavetables) means the scripts can do things like copy parts of to a new wavetable with different frame counts, or up- or downsampling to a new table. Limiting this would probably also limit functionality I think.

@Andreya-Autumn
Copy link
Copy Markdown
Collaborator

I see your point about patch size. Here's the tradeoff we're weighing though:

User A somehow notices that a patch their friend sent them is way bigger than it should be. "That's weird..." they think.
User B opens a patch with a scripted wavetable and plays it. They go in the script editor and for some reason press generate. The sound is totally different than before. "That's really weird..." they think.

I think the weirdness experienced by user B is way more important than that experienced by user A. It is already a fact that some Surge patches are way bigger than others based on what features are used. User probably don't even notice or care about that 99.99% of the time.

@Andreya-Autumn
Copy link
Copy Markdown
Collaborator

And yeah you may be right about the frame size and sample count mismatches. It's not a blocking concern here anyway. I'm more thinking maybe we can eventually add convenience functions to help interpolate or wrap etc etc. But that's for later!

@mkruselj
Copy link
Copy Markdown
Collaborator

mkruselj commented Apr 1, 2026

I mean the ultimate solution is of course referencing wtscript files rather than embedding them...

@nuoun
Copy link
Copy Markdown
Contributor Author

nuoun commented Apr 2, 2026

If @mkruselj agrees I will happily make it that the snapshot data saves in all three* locations. But right now I'm not sure if we all agree on that.

(*: The third location being metadata in .wt/,wav, a novel concept of having wav data as metadata there but I think it will work.)

And yes, worth pointing out maybe that in the above scenario, with the current implementation, the patch would work fine, which is probably all that 99% of users need, but the included script would error when trying to regenerate the WT at which point user B could ask for the .wtscript (of course the point stands, user C who downloads the patch and has no way of contacting user A will still be out of luck),

I mean the ultimate solution is of course referencing wtscript files rather than embedding them...

Yes maybe, wonder what that would look like in practice, something like generating a hash of the script, and load that if it can be found in either the factory or user folders? Or make it so that the .wtscripts needs to be in the same folder as the patch? Could make sharing patches that include the wavetable scripts a bit harder though.

@mkruselj
Copy link
Copy Markdown
Collaborator

mkruselj commented Apr 2, 2026

Could simply scan the WT folder for all wtscript that match by name first then hash second, I suppose?

I would not want to mix wtscript and FXP in the same folder.

"Simply", says he...

@Andreya-Autumn
Copy link
Copy Markdown
Collaborator

I think the solution is "just store the stuff with the patch".

It's fine. No one cares about 5-10mb of storage here and there in 2026. Except maybe the rPi/embedded folks, but they are already succesfully dealing with it anyway.

The .wav metadata thing is interesting! But a bit involved, potentially bug-prone, and not really a solution to any problem we currently have I think? So I contend let's not do that for now.

In the unlikely event someone eventually submits 300 fantastic patches which all rely on a 15mb wtscript, we put those .wtscripts in a separate download location like we link the Adventure Kid/EMU HQ/etc wavetables, and flatten the patches to use the normal wavetable outputs.

Then in XT2, we do a bigger restructure to allow wavetables, scripts and IRs all to be referenced instead of stored.

That makes sense to me anyway.

@mkruselj
Copy link
Copy Markdown
Collaborator

mkruselj commented Apr 2, 2026

I think it is very uncool to have a soundbank of 100 patches that could potentially end up over a gigabyte in size. It's a no go from my side.

I'm not even talking about anyone submitting such a soundbank to us. It could be somebody distributing it on their own. I still think that is unacceptable. .wtscripts should be referenced and snapshots should not be stored in a patch, that's my 2c.

@Andreya-Autumn
Copy link
Copy Markdown
Collaborator

Let's zoom out a bit. Only in the worst cases (2-3 script oscillators, each using more than one slot, each slot loading the largest size wavetables we load at all) would the scripts add more weight to the .fxp than the convolution does with my piano impulse loaded.

The likelihood someone makes, for distribution, 100 patches with that Piano IR, or 100 of those very heaviest wtscript patches, are both non-zero but probably really low (I'm sure we agree). But if one of these is unacceptable the other is also.

The same solution applies to both (and wavetables in general), and I agree it's a good one. But do we need make that much bigger infrastructural change before XT2, or don't we?

A positive answer seems likely to delay XT1.4 another couple months. And how long after that before it's time to start working on XT2 anyway?

I thus judge store with the patch as "good enough for now".

@baconpaul
Copy link
Copy Markdown
Collaborator

I haven’t been following but - Why would we store it anywhere other than the patch?

The patch is self contained and fully renderable and editable. If we don’t meet that constraint don’t merge the feature

@Andreya-Autumn
Copy link
Copy Markdown
Collaborator

I haven’t been following but - Why would we store it anywhere other than the patch?

The patch is self contained and fully renderable and editable. If we don’t meet that constraint don’t merge the feature

I agree. The others' concern is "it can make patches really big". A valid concern indeed but, yeah see above.

@baconpaul
Copy link
Copy Markdown
Collaborator

baconpaul commented Apr 2, 2026

yeah i get that. that's a good reason to not merge the feature perhaps. i don't think it is but if someone wants to make that case i'm ok

but if we add items-by-reference to patch we need

  1. a missing file resolution in the ui
  2. a missing file resolution or error reporting state in the midi program change, osc, python, and rust code paths for when the ui isn't open
  3. a way to know whats missing (so like store filename and hash)

thats not un-doable, but item 2 makes it way harder than short circuit and its a lot of code to write.

the thing which isn't ok is to have patch have reference to external data without doing and testing all that work.

@baconpaul
Copy link
Copy Markdown
Collaborator

I don’t see why adding another wt or two to the wt section of the fxp isn’t just the answer tho?

@Andreya-Autumn
Copy link
Copy Markdown
Collaborator

Yeah I agree. I don't think you need to drop it!

@nuoun
Copy link
Copy Markdown
Contributor Author

nuoun commented Apr 3, 2026

I looked into binary storage a little and if I nerf it down to two slots instead of the current three I think it will at most (so two max size WTs) be a ~8MB patch size increase (and worst case ~6-7MB with zstd, it tends to compress poorly).

For the average use scenario it will be much less of course so I also think it's fine but only if there are no objections, I rather do this by consensus or not at all so please let me know @mkruselj.

@baconpaul
Copy link
Copy Markdown
Collaborator

those slots will only be used if (1) you use wtzscript and (2) you use it to transform an extant wavetable right?

i see absolutely no problem with that.

@nuoun
Copy link
Copy Markdown
Contributor Author

nuoun commented Apr 4, 2026

those slots will only be used if (1) you use wtzscript and (2) you use it to transform an extant wavetable right?

Yes on both, so I think this is the plan if there are no objections:

  1. Use binary storage to save to the data to patch and .wtscript, compress with zstd (hold off saving it to .wav / .wt for now)
  2. Find a better place than DawExtraStorage for the struct to live
  3. Use the existing WT struct in the snapshot struct
  4. Nerf it down to two slots

@mkruselj
Copy link
Copy Markdown
Collaborator

mkruselj commented Apr 4, 2026

Sure I guess I can get behind that.

@baconpaul
Copy link
Copy Markdown
Collaborator

I get it all except for the 2 slots part. Why not allow n slots?

@nuoun
Copy link
Copy Markdown
Contributor Author

nuoun commented Apr 4, 2026

To make things like carrier / modulator modulation possible, or spectral morphing from WT 1 to WT 2, or as simple as being able to procedurally copy two tables to a new table. Without having at least two it's kind of limited.

@Andreya-Autumn
Copy link
Copy Markdown
Collaborator

I think the question was "why not 3".

@Andreya-Autumn
Copy link
Copy Markdown
Collaborator

I think that last proposal sounds good. And would also be fine with keeping the three slots but if it alleviates some of the file size concern I am not attached.

@nuoun
Copy link
Copy Markdown
Contributor Author

nuoun commented Apr 4, 2026

Oh, misread that, yes what Andreya said. Two tables covers most use cases anyway and if more tables really are needed the feature can at least be used to copy new tables one at a time.

@nuoun
Copy link
Copy Markdown
Contributor Author

nuoun commented Apr 5, 2026

To elaborate slightly, there is imo kind of a poor design choice here where a patch just keeps saving the snapshot data no matter what as long as there is data in the snapshots struct (which UI is buried rather deeply). I don't have a good alternative for it since I'm pretty sure we don't want a pop-up on patch save that asks confirmation on also saving snapshots so I feel a little better by limiting the amount of slots available but suggestions welcome.

@nuoun
Copy link
Copy Markdown
Contributor Author

nuoun commented Apr 12, 2026

Ok I think this is in pretty good shape now, per Discord discussion the structure that was favored was to add the WT class struct to OscillatorStorage so each WTSE instance can use it's own snapshots.

I first tried std::array<std::optional<Wavetable>, n_wt_snapshots> wtSnapshots;, but that blew out the Windows testrunner stack so went for std::array<std::unique_ptr<Wavetable>, n_wt_snapshots> wtSnapshots; instead.

Other changes made since this post:

  • With Surgo's help (thanks again!), saving snapshots to patch now uses zstd and arbitrary_block_storage like Convolution
  • Saving snapshots to .wtscript presets now uses zstd instead of base64, an optional binary header is used to set the sizes needed to parse the XML and binary parts like .fxp does
  • Lua snapshots table is now created once per generation cycle then cached
  • Snapshots are included in Copy/Paste Osc/Scene clipboard functions
  • Store WTS snapshots in patch option in Save Patch dialog box:
image

@nuoun nuoun force-pushed the wts-importwtdata branch from 49ce5be to 7adc523 Compare April 12, 2026 18:26
@mkruselj
Copy link
Copy Markdown
Collaborator

One UI suggestion for the save dialog:

Put checkboxes horizontally next to each other then OK/Cancel buttons below them. Should look a lot nicer. Also maybe fully qualify "wavetable script" instead of WTS, for clarity.

Other than that, this looks great to me! This PR possibly has the longest discussion chain out of all PRs weve ever had!

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.

4 participants