AudioKit / AudioKitSynthOne

AudioKit Synth One: Open-Source iOS Synthesizer App
http://audiokitpro.com/synth
MIT License
1.67k stars 217 forks source link

sp_oscmorph2d is the wrong version #156

Closed cmaughan closed 4 years ago

cmaughan commented 4 years ago

sp_oscmorph2d does not have the structure element 'enableBandlimit'. Is the version checked into the tree incorrect? I was curious how the bandlimited oscillators worked, but it seems that the key part is missing.....

pictune commented 4 years ago

Hi, we ran into a similar issue while working on a modified version of SynthOne's code. The issue was an access violation at the following line of S1NoteState::init()

oscmorph1->enableBandlimit = getParam(oscBandlimitEnable);

The program was trying to access a memory address 8 bytes right of a 40 bytes memory area (which corresponded to the sp_oscmorph2d structure).

We found out that there are two conflicting definitions of sp_oscmorph2d. The structure is defined in oscmorph2d.c:

typedef struct {
    SPFLOAT freq, amp, iphs;
    int32_t lphs;
    sp_ftbl **tbl;
    int inc;
    SPFLOAT wtpos;
    int nft;
} sp_oscmorph2d;

And also in AudioKit's soundpipeextension.h:

typedef struct {
    SPFLOAT freq, amp, iphs;
    int32_t lphs;
    sp_ftbl **tbl;
    int inc;
    SPFLOAT wtpos;
    int nft; // number of waveforms
    int nbl; // number of bandlimited tables per waveform
    float *fbl; // array of frequencies per bandlimited waveform
    int enableBandlimit; // if 0 use index 0, if 1 select index based on freq
    int bandlimitIndexOverride; // temporary
} sp_oscmorph2d;

I don't know if this is a bug, but I just though I'd share this in case it helps someone.

Cheers

cmaughan commented 4 years ago

Hi @aure Did this get fixed?

aure commented 4 years ago

@cmaughan pictune's explanation is right. The extension version is the correct one as it has enableBandLimit.

cmaughan commented 4 years ago

Hi @aure - thanks, but my point was really that the bug was closed without actually fixing it; the one in situ is incorrect. So the next person who comes along also gets confused.... Would be nice to fix the code to have the right version.....