Problems when using two instance of a module

[martignasse]

Copy of the original question :

hi,
need help again...

I create a module that fill a buffer with a waveform stored in an external file and read it as a loop (oscillator).

it works nice, but when I add a second instance of the same module (even if not connected) it affect the output of the first module (it sounds like a frequency modulation). WHY???

Normally all the variables and pointers are internals to every instance of the module(private)...or not?


Thanks for your help

[martignasse]

Hi Gizzeta,

Sorry for the lag, i saw you'r question but have to take the train to go to the electrolyse days (was so geat ).

There is no static or global variable exposed in the SDK actually, so it's not coming from here.

We have to confirm with Senso if there is some at usine level (not exposed at SDK level), but even if there is some, it shouldn't make problem with a user buffer declared at the SDK level.

I tested with two instances of the C++ DrawPad user module and there is no problems with the point buffer.

Anyway, it's difficult to answer or help without some code for this kind of problem. It would be interesting to see how you declare, init and modify you'r buffer.

[senso]

to load several instances of the same module you have to check if you have global variables declarations.
in this case the variables are shared between all the instances.
so try to create them into the TModule object declaration.

[Gizzeta]

Hallo,

I declared the variables in the TModule class declaration:

Code: Select all

class TCycle : public TUserModule
{

public:
// Declare all parameters events pointers needed, example :

	TEVT* pFreqIn;
	TEVT* pAudioOut;
	TEVT* ARR;

	FILE* pFile;
	float* buffer;
	int* lSize;
};

and i created it in the create

Code: Select all

void Create(void* &pModule) {
	pModule = new TCycle();	
	
	TCycle* pCycle = ((TCycle*)pModule);

	size_t result;
	pCycle->lSize = new int;

	pCycle->pFile = fopen ( "E:\AUDIO\USINEfiles\Cycle\Sine2048.bin" , "rb" );
	assert(pCycle->pFile != NULL);
		
	//obtain file size:
	fseek (pCycle->pFile , 0 , SEEK_END);
	*pCycle->lSize = ftell (pCycle->pFile);
	*pCycle->lSize /= sizeof(float);
	rewind (pCycle->pFile);
	
	// allocate memory to contain the whole file:
	pCycle->buffer = new float[sizeof(float)**pCycle->lSize];
		
	// copy the file into the buffer:
	result = fread (pCycle->buffer,sizeof(float),*pCycle->lSize,pCycle->pFile); 

	//close
	fclose (pCycle->pFile);
}

What's the mistake?

thanks

[senso]

all of them?
are you sure there is no global hidden variables somewhere?

[Gizzeta]

yes...sure

[martignasse]

hi Gizzeta,

the buffer pointer variable seems ok,

the use of fread too,

the only thing not very clean is that :

Code: Select all

int* lSize;

You not need a pointer to int in this case, just an int is enough :

Code: Select all

int lSize;

It's lot more safe and make you'r code more readable, here the create modified tu use an int instead of a pointer to int :

Code: Select all

void Create(void* &pModule) {
    pModule = new TCycle();    
    
    TCycle* pCycle = ((TCycle*)pModule);

    size_t result;
    
   // no more needed
   // pCycle->lSize = new int;

    pCycle->pFile = fopen ( "E:\AUDIO\USINEfiles\Cycle\Sine2048.bin" , "rb" );
    assert(pCycle->pFile != NULL);
        
    //obtain file size:
    fseek (pCycle->pFile , 0 , SEEK_END);
    // no more * indirection
    pCycle->lSize = ftell (pCycle->pFile);
    pCycle->lSize /= sizeof(float);

    rewind (pCycle->pFile);
    
    // allocate memory to contain the whole file:
   // especially this line was maybe missinterpreted before, it's better like that
    pCycle->buffer = new float[sizeof(float) * pCycle->lSize];
        
    // copy the file into the buffer:
    result = fread (pCycle->buffer, sizeof(float), pCycle->lSize, pCycle->pFile); 

    //close
    fclose (pCycle->pFile);
}

this line is especially suspect and not very readable with these two *

Code: Select all

    pCycle->buffer = new float[sizeof(float)**pCycle->lSize];

you can already change that and see if it help.
but other than that, it's seems ok.

[Gizzeta]

Hi, thanks for your help,
I found the problem.
I have others variables declared in the process function. I thought that theese were not global...(isn't it?)
So, I declared all the variables (with pointers) in the "TModule" and "create" functions and then i use them in the "process".
Now it seems to work fine.
But it's strange: i tried to do a simpler object with just one variable declared in the "process" without pointer (just for testing).
If i create 2 instances of this object the variable is not shared, while in my more complex "cycle" object, 2 instances share the variables with the same name...why?

however thanks

[martignasse]

yep, strange

but in fact, without shown code, those bugs are very difficult to found. There is surely something in your 'more complex' object that is to obvious to be seen by you (lots of internal automatism when coding), but easily by another one, i mean, someone who not wrote the code !

The most important is that you solved your problem, so

[Gizzeta]

that's my code (not so complex...), if you can help me to understend my mistake :

Code: Select all

class TCycle : public TUserModule
{

public:

	TEVT* pFreqIn;
	TEVT* pAudioOut;

	FILE* pFile;
	float* buffer, *index, *coefFreq, *freqMinus;
	int* lSize;
};


void Process (void* pModule) {

	//pointer to the module
	TCycle* pCycle = ((TCycle*)pModule);
	static float samplesOverSR = (float)*(pCycle->lSize) / (float)pCycle->pMasterInfo->pTimeInfo->sampleRate;
	int i;

	pCycle->pAudioOut->len = pCycle->pMasterInfo->BlocSize;

	// data type CONNECTED
	if(pCycle->pFreqIn->len == 1){
		if(pCycle->pFreqIn->value != *(pCycle->freqMinus)){
			*(pCycle->freqMinus) = pCycle->pFreqIn->value;
			*(pCycle->coefFreq) = pCycle->pFreqIn->value * samplesOverSR;
		}
		for&#40;i = 0; i < pCycle->pMasterInfo->BlocSize; i++&#41;&#123;
			pCycle->pAudioOut->DATA&#91;i&#93; = *&#40;pCycle->buffer + &#40;int&#41;*&#40;pCycle->index&#41;&#41;;
			*&#40;pCycle->index&#41; += *&#40;pCycle->coefFreq&#41;;
			if&#40;*&#40;pCycle->index&#41; >= *&#40;pCycle->lSize&#41;&#41; *&#40;pCycle->index&#41; -= *&#40;pCycle->lSize&#41;;
		&#125;
	&#125;

	// audio type CONNECTED
	else if&#40;pCycle->pFreqIn->len == pCycle->pMasterInfo->BlocSize&#41;&#123;
		for&#40;i = 0; i < pCycle->pMasterInfo->BlocSize; i++&#41;&#123;
			if&#40;pCycle->pFreqIn->DATA&#91;i&#93; != *&#40;pCycle->freqMinus&#41;&#41;&#123;
				*&#40;pCycle->freqMinus&#41; = pCycle->pFreqIn->DATA&#91;i&#93;;
				*&#40;pCycle->coefFreq&#41; = pCycle->pFreqIn->DATA&#91;i&#93; * samplesOverSR;
			&#125;
			pCycle->pAudioOut->DATA&#91;i&#93; = *&#40;pCycle->buffer + &#40;int&#41;&#40;*&#40;pCycle->index&#41;&#41;&#41;;
			*&#40;pCycle->index&#41; += *&#40;pCycle->coefFreq&#41;;
			if&#40;*&#40;pCycle->index&#41; >= *&#40;pCycle->lSize&#41;&#41; *&#40;pCycle->index&#41; -= *&#40;pCycle->lSize&#41;;
		&#125;
	&#125;	
			
	// NO CABLE CONNECTED
	else&#123;
		for&#40;i = 0; i < pCycle->pMasterInfo->BlocSize; i++&#41; pCycle->pAudioOut->DATA&#91;i&#93; = 0.f;
	&#125;;

	pCycle->pFreqIn->len = 0;
&#125;


 void Create&#40;void* &pModule&#41; &#123;
    pModule = new TCycle&#40;&#41;;    
    
    TCycle* pCycle = &#40;&#40;TCycle*&#41;pModule&#41;;

    size_t result;
    pCycle->lSize = new int;

    pCycle->pFile = fopen &#40; "E&#58;\AUDIO\USINEfiles\Cycle\Sine2048.bin" , "rb" &#41;;
    assert&#40;pCycle->pFile != NULL&#41;;
        
    //obtain file size&#58;
    fseek &#40;pCycle->pFile , 0 , SEEK_END&#41;;
    *&#40;pCycle->lSize&#41; = ftell &#40;pCycle->pFile&#41;;
    *&#40;pCycle->lSize&#41; /= sizeof&#40;float&#41;;
    rewind &#40;pCycle->pFile&#41;;
    
    // allocate memory to contain the whole file&#58;
    pCycle->buffer = new float&#91;sizeof&#40;float&#41;*&#40;*&#40;pCycle->lSize&#41;&#41;&#93;;

    // copy the file into the buffer&#58;
    result = fread &#40;pCycle->buffer,sizeof&#40;float&#41;,*pCycle->lSize,pCycle->pFile&#41;; 

	//variables declaration and initialization &#40;used only in "process" function&#41;
	pCycle->index = new float;
	*&#40;pCycle->index&#41; = 0.f; 
	pCycle->coefFreq = new float;
	pCycle->freqMinus = new float;
	*&#40;pCycle->freqMinus&#41; = 0.f; 
	
	//close
    fclose &#40;pCycle->pFile&#41;;
&#125;


void Destroy&#40;void* pModule&#41; &#123;
	TCycle* pCycle = &#40;&#40;TCycle*&#41;pModule&#41;;
	delete pCycle->lSize;
	delete pCycle->buffer;
	delete pCycle->index;
	delete pCycle->coefFreq;
	delete pCycle->freqMinus;
	delete pModule;
&#125;

In the not working version the variables "index", "coefFreq" and "freqMinus" were declared as normal variables(not pointers) in the "process" function. They were shared by differents instances of the module.
Normally if I declare a variable in the process function, it's not global in Usine, isn't it?

[Gizzeta]

...forgot:
lSize is a pointer because I need to access to it in the process function. It's the only way, isn't it?

[martignasse]

...forgot:

no problem, but remenber to ask someone else to read your code it save days, no joke !

lSize is a pointer because I need to access to it in the process function. It's the only way, isn't it?

no, there is no need to make complex things like that.
the process function provide one parameter, a pointer to void who is a pointer to your user module, you have just to cast it conveiniently.

Code: Select all

TMyModule* pMyModule = ((TMyModule*)pModule);

after that, you have access 'into' your module (read or write), and if you have a int called lSize in your module (just a int, not a pointer to) your can access his value like that

Code: Select all

nt local_int_value_of_lSize = pMyModule->lSize;

to modify the value of the lSize variable of your module, it's just the opposite

Code: Select all

pMyModule->lSize = 3;

the thing to understand is that the pointer to your module provide you access 'into' the module, you dont need to add a pointer step for each variable to acces his value.

generally, pointer are conveinient for composite variable (like structure, or class) which is not the case of an int.

hope it enlight you

[Gizzeta]

Yes, very precious for me, thanks!

no problem, but remenber to ask someone else to read your code it save days, no joke !

...maybe you haven't seen my previous post...I have put the whole code...

[martignasse]

sorry, i didn't saw the post from 20.55. :rolleyes:

[Gizzeta]

No matter

[martignasse]

ok, after read your code, i have some other questions/clarifications/advices

1°)

Code: Select all

static float samplesOverSR = (float)*(pCycle->lSize) / (float)pCycle->pMasterInfo->pTimeInfo->sampleRate;

the fact you declare the 'samplesOverSR' variable static make it initialised only one time, at the first execution of the process function, and because declaration and affectation is one the same line, 'samplesOverSR' is never updated with the '(float)*(pCycle->lSize) / (float)pCycle->pMasterInfo->pTimeInfo->sampleRate' result.

More than that, as 'samplesOverSR' is declared static, only one instance of it is shared by all instances of your modules.

Is it the intended behavior ?

2°)
What kind of datatype is your 'pFreqIn' param ?
It seems you use it sometime for data, sometime for audio buffer (strange and potentially dangerous) but i can't see what kind of param is used, array maybe ?

3°)
in general, your code is hard to read and understand, i can't even know what you exactly do just by reading it

Try to let things simple and clear. For this, here some general rules :

- one meaning by variable, it preserve you from unwanted side effect and make readability better, for example, 'pCycle->pFreqIn->len' is initially used to store the size of the DATA array of 'pCycle->pFreqIn', but you use it also to detect if you make data or audio treatment.

- avoid duplication of same code, for example, you surely can avoid the double time use of this block (used in your 'if' AND 'else' statement)

Code: Select all

            pCycle->pAudioOut->DATA[i] = *(pCycle->buffer + (int)*(pCycle->index));
            *(pCycle->index) += *(pCycle->coefFreq);
            if(*(pCycle->index) >= *(pCycle->lSize)) *(pCycle->index) -= *(pCycle->lSize);

it's easier to maintain and fr bug tracking.

- reduce the use of class/structure member call for reading info in 'for' loop, make variable for it before the loop, for example

Code: Select all

int iAudioOutLen = pCycle->pAudioOut->len = pCycle->pMasterInfo->BlocSize;
int i ;
...
for &#40;i = 0; i < iAudioOutLen &#41; &#123;
   ...
&#125;

it make code more readable and optimized, if your BlockSize is 512, you save 512 class/structure member calls.

Hope it help

[Gizzeta]

Hi Martignasse,

The module is a simple oscillator. It fill a buffer with a file where there's a one-cycle waveform.
Then it calculates wich sample he has to put out, according with the actual frequency parameter.

1°)

'samplesOverSR' is never updated with the '(float)*(pCycle->lSize) / (float)pCycle->pMasterInfo->pTimeInfo->sampleRate' result.

that's what I want. lSize/SR is always the same, for all the module duration. I calculate it only at creation, to economize CPU.
Maybe can I initialize it in the "InitModule" function?
Can you explain me the exact order in wich the finctions are called by Usine? (I guess: "Create", then "getModuleInfo", then "initModule"....)

More than that, as 'samplesOverSR' is declared static, only one instance of it is shared by all instances of your modules.

I thik this was my problem...
So a variable declared as static is shared by all instances? (I'm sorry, I'm a c++ beginner...)
And if I want a variable, private to the module, that keep the value between more functions calls, I have to declare it in the "create" function? And it can be a normal (not pointer) variable?


2°)

What kind of datatype is your 'pFreqIn' param ?

It's a pDataFader. Because I want that it could be possible to control the module frequency by a data value and by a signal value (for frequency modulation, eg: at every sample the frequency value can variate).
So, if the incoming data is a single value (len == 1), it's a normal data frequency control. In this case I calculate (pFreqIn*samplesOverSR) just once for "process" call.
If the incoming data is an array whose lenght is the same as Block Size, it's a frequency modulation and (pFreqIn*samplesOverSR) has to be calculated once for sample.
Finally, if there's no cable connected, I want the module to out a 0.f signal (silence).


3°)

'pCycle->pFreqIn->len' is initially used to store the size of the DATA array of 'pCycle->pFreqIn', but you use it also to detect if you make data or audio treatment.

..hem...I don't understend...I detect what kind of message (data or signal) is coming in, by the size if the 'pCycle->pFreqIn'.
Empircally, I found that I have to set to 0 the 'pCycle->pFreqIn->len' at the end of each "process" call. Otherwise if a cable is not connected, it keep the last lenght.

- avoid duplication of same code,

Do you think it's better to create a function on purpose?

- reduce the use of class/structure member call for reading info in 'for' loop, make variable for it before the loop, for example

Code: Select all

int iAudioOutLen = pCycle->pAudioOut->len = pCycle->pMasterInfo->BlocSize;
int i ;
...
for &#40;i = 0; i < iAudioOutLen &#41; &#123;
   ...
&#125;

it make code more readable and optimized, if your BlockSize is 512, you save 512 class/structure member calls.

In term of CPU economy what is more expensive: a class/structure member calls or a variable more?



really thanks for your counsels!

[martignasse]

The module is a simple oscillator. It fill a buffer with a file where there's a one-cycle waveform.
Then it calculates wich sample he has to put out, according with the actual frequency parameter.

ok, it's what i supposed

Can you explain me the exact order in wich the finctions are called by Usine? (I guess: "Create", then "getModuleInfo", then "initModule"....)

Good question, i dont know the exact order, but i'm sure of that :

1/ Create
2/ InitModule,
3/ Process <- called each Usine cycle (800/second) while your module is alive
4/ Destroy

eventually:
CallBack is called by usine each time a param is modified in the module parameters panel

for the rest, i 'm not sure, i'll try to make a time sequenced diagram in the wiki,

So a variable declared as static is shared by all instances? (I'm sorry, I'm a c++ beginner...)

Yep, static variables are created in the static memory, shared by all functions (don't miss the SDK interface is C, not C++)

And if I want a variable, private to the module, that keep the value between more functions calls, I have to declare it in the "create" function? And it can be a normal (not pointer) variable?

the simple way is to declare it in your module definition (the include file), and initialise it in the InitModule function if you need some infos stored in pMasterInfo
in the include file :

Code: Select all

class MuModule : public TUserModule
{
...
private:
	int			iMyVariable;

...


};

and in the cpp file :

Code: Select all

...
//-----------------------------------------------------------------------------
// initialisation
void InitModule (void* pModule, TMasterInfo* pMasterInfo, TModuleInfo* pModuleInfo) {

	// conveinient pointer
        MyModule* pMyModule = ((MyModule*)pModule);

	// remember, it's up to us to initialise the pMasterInfo poimnter herited from TUserModule
	pMyModule->pMasterInfo = pMasterInfo;

	// initialisation of other variable
	pMyModule->iMyVariable = pMasterInfo->pTimeInfo->sampleRate; // or whatever, you got the idea


}
...

It's a pDataFader. Because I want that it could be possible to control the module frequency by a data value and by a signal value (for frequency modulation, eg: at every sample the frequency value can variate).
So, if the incoming data is a single value (len == 1), it's a normal data frequency control. In this case I calculate (pFreqIn*samplesOverSR) just once for "process" call.
If the incoming data is an array whose lenght is the same as Block Size, it's a frequency modulation and (pFreqIn*samplesOverSR) has to be calculated once for sample.
Finally, if there's no cable connected, I want the module to out a 0.f signal (silence).

well, it's here your design is not very clear, i think, you have two signification for the same param, it's not clean.
What prevent you to split that in two param ? like a 'base freq' one who is a value of the wanted reference frequence and a 'mod' one who is a signal of the modulation wanted around the 'base freq' value.

I detect what kind of message (data or signal) is coming in, by the size if the 'pCycle->pFreqIn'.

but Usine provide different params for differents kind of signal, like said above. seems like your design push you to make extra work here.

Do you think it's better to create a function on purpose?

not automaticly, for your case, it's once again the design who push you to make like that. In your Process function, there is some mix between the detection of the param (data or signal) and the audio treatment, it make the code tricky

[Gizzeta]

I profit by your patience...

3/ Process <- called each Usine cycle (800/second) while your module is alive

"process" is called syncronicall with the block size?
ie. if the block size is 128, audio process is called every 128 samples?
Or, if the sample rate is 44100 and "process" is called 800/sec, the audio array is around 55 samples?(but, normally, it can't be less than the block size)

(don't miss the SDK interface is C, not C++)

??? sorry, I don't understand: this SDK is C++, right? What do you mean with SDK interface?

What prevent you to split that in two param ? like a 'base freq' one who is a value of the wanted reference frequence and a 'mod' one who is a signal of the modulation wanted around the 'base freq' value.

that's true....

thanks again

[martignasse]

oups, missed the response,

"process" is called syncronicall with the block size?
ie. if the block size is 128, audio process is called every 128 samples?
Or, if the sample rate is 44100 and "process" is called 800/sec, the audio array is around 55 samples?(but, normally, it can't be less than the block size)

seems you've got confirmation from senso in the other thread that the process function is called every BLOC samples. I prefer that the info come from senso, as he is the one who know all that stuff, i'm like you, discovering the beast (not senso...Usine )

sorry, I don't understand: this SDK is C++, right? What do you mean with SDK interface?

yep, the SDK itself is in C++, but the brunch of functions to communicate with usine (at the end of the UserDefinitions.h) use C signature. Anyway, not very relevant, should be pretty tired when i wrote that, sorry

I'm impatient to see your modules

[senso]

the process is called every BLOC samples, (see setup/best bloc size).
more precisely Usine call process each time we need to calculate BLOC number of samples.
but don't forget that there is a time jitter, due to the sound card latency and the multi thread core. For example Usine can process 2 blocs very quickly and then wait few ms before treat another set of 2 blocs...

[martignasse]

For knowledge durability reasons, you have to know that important informations evoked in this conversation are now wikified.

wiki::sdk::usine architecture

Of course, you have rights and duties to consult, modify and make alive these informations

[Gizzeta]

Thanks, it's quite clear now!