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

Effects #84

Merged
merged 23 commits into from
Mar 7, 2020
Merged

Effects #84

merged 23 commits into from
Mar 7, 2020

Conversation

jpcima
Copy link
Collaborator

@jpcima jpcima commented Feb 27, 2020

Add effects and their routing. Work in progress

  • Effect factory

Designed such that it can receive user-defined effects.
An effect is designated by its "type" opcode.

  • Routing with main bus and up to 256 effect buses
  • Mixing per-region
  • Basic "lofi" effect to start which is relatively CW-compliant

TODO

  • review the mixing behavior in case of use of main/fx buses together, or one of these only

There are still some things and dirty and it's not tried.
Regardless, I submit the code for review if you would like to have a peek.

Remark: I would like if we can introduce HIIR downsampler separately of this PR

@jpcima jpcima force-pushed the effects branch 10 times, most recently from 29b6c35 to f5f0315 Compare March 2, 2020 22:06
@jpcima jpcima marked this pull request as ready for review March 2, 2020 22:13
@jpcima jpcima requested a review from paulfd March 2, 2020 22:13
@jpcima
Copy link
Collaborator Author

jpcima commented Mar 4, 2020

Below some sfz for testing

Bitcrusher on main bus

<region>
lokey=0
hikey=127
sample=*sine

<effect>
type=lofi
bitred=90
decim=10

Bitcrusher on bus 1, main bus turned off

<region>
lokey=0
hikey=127
sample=*sine
effect1=100

<effect>
directtomain=0
fx1tomain=100
type=lofi
bus=fx1
bitred=90
decim=10

Bitcrusher on bus 1, main and fx1 mixed 50/50

<region>
lokey=0
hikey=127
sample=*sine
effect1=100

<effect>
directtomain=50
fx1tomain=50
type=lofi
bus=fx1
bitred=90
decim=10


float phase = fPhase;
float lastValue = fLastValue;
hiir::Downsampler2xFpu<12>& downsampler2x = fDownsampler2x;
Copy link
Member

Choose a reason for hiding this comment

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

The other ref

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

not very useful, it can go, or I don't mind to change to auto&

}

float lastValue = fLastValue;
hiir::Downsampler2xFpu<12>& downsampler2x = fDownsampler2x;
Copy link
Member

Choose a reason for hiding this comment

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

Is this ref (and the one below) useful somehow?

void Lofi::process(const float* const inputs[2], float* const outputs[2], unsigned nframes)
{
for (unsigned c = 0; c < EffectChannels; ++c) {
_bitred[c].setDepth(_bitred_depth);
Copy link
Member

Choose a reason for hiding this comment

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

For now this is unmodulated but it should be at some point right?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

It must have modulation.
Since the focus is the framework of effects, it's limited in implementation.
Let's add this when the modulation matrix is well handled.

@@ -751,6 +751,20 @@ bool sfz::Region::parseOpcode(const Opcode& opcode)
setCCPairFromOpcode(opcode, amplitudeEG.ccSustain, Default::egOnCCPercentRange);
break;

case hash("effect"): // effect&
Copy link
Member

Choose a reason for hiding this comment

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

When we merge the other one I'll rebase this

break;

case hash("fxtomain"): // fx&tomain
if (auto numberOpt = opcode.firstParameter()) {
Copy link
Member

Choose a reason for hiding this comment

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

There too, it'll be cleaner with the new parser

@paulfd
Copy link
Member

paulfd commented Mar 5, 2020

All good for me, can merge :) Thanks alot!

@jpcima
Copy link
Collaborator Author

jpcima commented Mar 6, 2020

On blockSize and sampleRate : I don't really agree with the renaming setSampleRate

The entry point init is really intended for an initialization role.

  • It should setup anything which is rate dependent, and clear the state.
  • It should not modify the values of parameters

I would suggest to pass both of sample rate and buffer size to this init method.

For information, I wish this process to be equivalent with a design of future C API which can be used for registering external effects into sfizz.

(in such a way, we can register MDA's effect package externally like Sforzando, or do anything custom)

@paulfd
Copy link
Member

paulfd commented Mar 6, 2020

OK, I'm good with this, I'll update.

@paulfd paulfd merged commit 53a4cf0 into sfztools:develop Mar 7, 2020
This was referenced Mar 7, 2020
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