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

Chorus enhancement. #548

Merged
merged 19 commits into from
Oct 23, 2019
Merged

Chorus enhancement. #548

merged 19 commits into from
Oct 23, 2019

Conversation

jjceresa
Copy link
Collaborator

This PR is a proposal to add chorus enhancement. It adds 2 configurations macros (NEW_MOD, STEREO_UNIT) just to allow fast tries to compare with actual chorus.
NEW_MOD macro allows a memory size optimization useful for embedded hardware.
STEREO_UNITmacro adds a stereo unit (that works similarly than for reverb).


NEW_MOD: when defined, uses new LFO modulators:

  • these modulators are computed on the fly, instead of using lfo lookup table.
  • The advantages are:
    • Avoiding a lost of 608272 memory bytes when lfo speed is low (0.3Hz).
    • Allows to diminish the lfo speed lower limit to 0.1Hz instead of 0.3Hz.
      A speed of 0.1 is interresting for chorus. Using a lookuptable for 0.1Hz
      would require too much memory (1824816 bytes).
    • Make use of first order all-pass interpolator instead of bandlimited interpolation.
    • Although lfo modulator is computed on the fly, cpu load is lower than
      using lfo lookup table with bandlimited interpolator.

NEW_MOD: when not defined:

  • use memory lfo lookup table.
  • use bandlimited interpolation with sinc lookup table.

STEREO_UNIT: when defined, adds a stereo unit controlled by WIDTH macro.
WIDTH [0..10] value define a stereo separation between left and right.
When 0, the output is monophonic. When > 0 , the output is stereophonic.

Actually WIDTH is fixed to maximum value. But in the future, we could add a
setting (e.g "synth.chorus.width") allowing the user to get a gradually stereo
effect from minimum (monophonic) to maximum stereo effect.

jjceresa added 3 commits July 22, 2019 23:16
NEW_MOD: when defined, uses new LFO modulators:
 - these modulators are computed on the fly, instead of using lfo lookup
   table.
   The advantages are:
      - Avoiding a lost of 608272 memory bytes when lfo speed is low (0.3Hz).
      - Allows to diminish the lfo speed lower limit to 0.1Hz instead of 0.3Hz.
        A speed of 0.1 is interresting for chorus. Using a lookuptable for 0.1Hz
        would require too much memory (1824816 bytes).
      - Make use of first order all-pass interpolator instead of bandlimited interpolation.
      - Although lfo modulator is computed on the fly, cpu load is lower than using
        lfo lookup table with bandlimited interpolator.

NEW_MOD: when not defined:
      - use memory lfo lookup table.
      - use bandlimited interpolation with sinc lookup table.
 STEREO_UNIT: when defined, adds a stereo unit controlled by WIDTH macro.
 WIDTH [0..10] value define a stereo separation between left and right.
 - When 0, the output is monophonic.
 - When > 0 , the output is stereophonic.

 Actually WIDTH is fixed to maximum value. But in the future, we could add
 a setting (e.g "synth.chorus.width") allowing the user to get a gradually
 stereo effect from minimum (monophonic) to maximum stereo effect.
@derselbst
Copy link
Member

I never really got used to the chorus my self. Still, I gave it a try.

The stereo unit definitely makes it sound richer. By far not as rich as reverb, but still audible. I'm not sure if it makes sense to introduce synth.chorus.width to actually lower this stereo sound. At least to me, monophonic chorus sounds very boring. So, the stereo unit would be a really nice addition to chorus and it should become the default.

I didn't had time to do some performance measurements my self. At least to me it sounds interesting as well to lower the chorus limit to 0.1 Hz. And performing those calculations doesn't seem to expensive to me given the amount of memory it saves. But perhaps someone else without a FPU can give an opinion (@carlo-bramini ?) .

@jjceresa
Copy link
Collaborator Author

By far not as rich as reverb, but still audible.

Reverb have at least 8 delays line completely uncorrelated. These conditions are very favorable for producing large stereo effect.

The chorus unit have only one delay line (common to all chorus block). This is why stereo chorus is less richer then stereo reverb. The only way to get uncorrelated signal at the output of each block is to have lfos with different phase. These out-of-phase lfos are a requirement to produce chorus but aren't not sufficient to produce large stereo, but still audible (even with only one chorus block!).


Please, note that with the presence of stereo unit, it is not necessary to choose a number of chorus block (synth.chorus.nr setting) above 2.

derselbst and others added 11 commits October 22, 2019 17:48
Although it may be useful, it suggests that the chorus implementation
consists of two maintained implementations, which is not true. Removing
the macro makes clear that the new chorus implementation solely relies
on on-the-fly calculation of LFO modulators.
For maximum modulation depth et maximum lfo speed
INTERP_SAMPLES_NBR should not be 0.
@jjceresa
Copy link
Collaborator Author

#define MAX_CHORUS 99 /* number maximum of block */

It seems that this value is excessive . May be a value of 4 or 6 could be sufficient ?.

@derselbst
Copy link
Member

Yes, it is excessive, but it also creates interesting effects with the right level and speed setting. Let's keep it is as an upper limit.

Copy link
Member

@derselbst derselbst left a comment

Choose a reason for hiding this comment

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

Looks good to me now. Let me know when you think it's ready to merge.

@jjceresa
Copy link
Collaborator Author

I think now it's ready to merge. Thanks for review this.

@derselbst derselbst merged commit 0288466 into master Oct 23, 2019
@derselbst derselbst deleted the chorus-newmod branch October 23, 2019 14:41
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.

None yet

2 participants