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

AUDIO: Optimise default fluidsynth configuration paramenters #1858

Closed
wants to merge 1 commit into from

Conversation

ccawley2011
Copy link
Member

Originally from libretro/scummvm#108

Regarding the FluidSynth changes, I got the new values from here: https://forums.scummvm.org/viewtopic.php?t=11632

To quote the forum post:

The FluidSynth reverb, while not as good as the old Live!/Audigy hardware reverbs (which were quite good), can still sound decent with the right settings. I have no idea how to change this in ScummVM, but I have found the following FluidSynth settings to sound much better:

Reverb:

Room: 0.61
Damp: 0.23
Width: 0.76
Level: 0.57

Chorus:

N: 3
Level: 1.2
Speed: 0.3
Depth: 8

This gives a much more spacious sound, as opposed to the "stuffy room" that seems to be FluidSynth's default.

I basically tested these settings with every (!) game, and it was indeed a huge improvement.

In addition, the reverbWidth setting in ScummVM master is handled internally as an int. This is a bug, which means the value can never be adjusted correctly - it should be a double, like all the other parameters.

Co-authored-by: jdgleaver <jdgleaver@users.noreply.github.com>
@criezy
Copy link
Member

criezy commented Sep 23, 2019

I am not sure about using custom defaults (different from the fluidsynth default) as I am guessing what is best will be subjective. But I am not really opposed to it either.

However shouldn't our GUI respect the documented value range for each setting? You change the max for the reverb width to 1, but the fluidsynth documentation indicates that the valid range is [0, 100], with a default at 0.5 (http://www.fluidsynth.org/api/fluidsettings.xml#synth.reverb.width). The chorus level is another one with a discrepancy (documented maximum of 10, but we use 2). We are also still using the old chorus depth range (the upper bound was increased from 21 to 256 one year ago: FluidSynth/fluidsynth@fd7db02).

kcgen added a commit to dosbox-staging/dosbox-staging that referenced this pull request Oct 1, 2020
These settings were tuned by ScummVM forum user mrbumpy409
started in 2012 and where made the project's default settings
in 2019.

References:
 - https://forums.scummvm.org/viewtopic.php?t=11632
 - scummvm/scummvm#1858

Co-authored-by: mrbumpy409 <mrbumpy409@forums.scummvm.org>
Co-authored-by: Cameron Cawley <ccawley2011@gmail.com>
Co-authored-by: kcgen <1557255+kcgen@users.noreply.github.com>
kcgen added a commit to dosbox-staging/dosbox-staging that referenced this pull request Oct 1, 2020
These settings were tuned by ScummVM forum user mrbumpy409
started in 2012 and where made the project's default settings
in 2019.

References:
 - https://forums.scummvm.org/viewtopic.php?t=11632
 - scummvm/scummvm#1858

Co-authored-by: mrbumpy409 <mrbumpy409@forums.scummvm.org>
Co-authored-by: Cameron Cawley <ccawley2011@gmail.com>
Co-authored-by: kcgen <1557255+kcgen@users.noreply.github.com>
kcgen added a commit to dosbox-staging/dosbox-staging that referenced this pull request Oct 3, 2020
These settings were tuned by ScummVM forum user mrbumpy409
started in 2012 and where made the project's default settings
in 2019.

References:
 - https://forums.scummvm.org/viewtopic.php?t=11632
 - scummvm/scummvm#1858

Co-authored-by: mrbumpy409 <mrbumpy409@forums.scummvm.org>
Co-authored-by: Cameron Cawley <ccawley2011@gmail.com>
Co-authored-by: kcgen <1557255+kcgen@users.noreply.github.com>
kcgen added a commit to dosbox-staging/dosbox-staging that referenced this pull request Oct 5, 2020
These settings were tuned by ScummVM forum user mrbumpy409
started in 2012 and where made the project's default settings
in 2019.

References:
 - https://forums.scummvm.org/viewtopic.php?t=11632
 - scummvm/scummvm#1858

Co-authored-by: mrbumpy409 <mrbumpy409@forums.scummvm.org>
Co-authored-by: Cameron Cawley <ccawley2011@gmail.com>
Co-authored-by: kcgen <1557255+kcgen@users.noreply.github.com>
kcgen added a commit to dosbox-staging/dosbox-staging that referenced this pull request Oct 5, 2020
These settings were tuned by ScummVM forum user mrbumpy409
started in 2012 and where made the project's default settings
in 2019.

References:
 - https://forums.scummvm.org/viewtopic.php?t=11632
 - scummvm/scummvm#1858

Co-authored-by: mrbumpy409 <mrbumpy409@forums.scummvm.org>
Co-authored-by: Cameron Cawley <ccawley2011@gmail.com>
Co-authored-by: kcgen <1557255+kcgen@users.noreply.github.com>
kcgen added a commit to dosbox-staging/dosbox-staging that referenced this pull request Oct 9, 2020
These settings were tuned by ScummVM forum user mrbumpy409
started in 2012 and where made the project's default settings
in 2019.

References:
 - https://forums.scummvm.org/viewtopic.php?t=11632
 - scummvm/scummvm#1858

Co-authored-by: mrbumpy409 <mrbumpy409@forums.scummvm.org>
Co-authored-by: Cameron Cawley <ccawley2011@gmail.com>
Co-authored-by: kcgen <1557255+kcgen@users.noreply.github.com>
kcgen added a commit to dosbox-staging/dosbox-staging that referenced this pull request Oct 12, 2020
These settings were tuned by ScummVM forum user mrbumpy409
started in 2012 and where made the project's default settings
in 2019.

References:
 - https://forums.scummvm.org/viewtopic.php?t=11632
 - scummvm/scummvm#1858

Co-authored-by: mrbumpy409 <mrbumpy409@forums.scummvm.org>
Co-authored-by: Cameron Cawley <ccawley2011@gmail.com>
Co-authored-by: kcgen <1557255+kcgen@users.noreply.github.com>
kcgen added a commit to dosbox-staging/dosbox-staging that referenced this pull request Oct 19, 2020
These settings were tuned by ScummVM forum user mrbumpy409
started in 2012 and where made the project's default settings
in 2019.

References:
 - https://forums.scummvm.org/viewtopic.php?t=11632
 - scummvm/scummvm#1858

Co-authored-by: mrbumpy409 <mrbumpy409@forums.scummvm.org>
Co-authored-by: Cameron Cawley <ccawley2011@gmail.com>
Co-authored-by: kcgen <1557255+kcgen@users.noreply.github.com>
kcgen added a commit to dosbox-staging/dosbox-staging that referenced this pull request Oct 19, 2020
These settings were tuned by ScummVM forum user mrbumpy409
started in 2012 and where made the project's default settings
in 2019.

References:
 - https://forums.scummvm.org/viewtopic.php?t=11632
 - scummvm/scummvm#1858

Co-authored-by: mrbumpy409 <mrbumpy409@forums.scummvm.org>
Co-authored-by: Cameron Cawley <ccawley2011@gmail.com>
Co-authored-by: kcgen <1557255+kcgen@users.noreply.github.com>
kcgen added a commit to dosbox-staging/dosbox-staging that referenced this pull request Oct 19, 2020
These settings were tuned by ScummVM forum user mrbumpy409
started in 2012 and where made the project's default settings
in 2019.

References:
 - https://forums.scummvm.org/viewtopic.php?t=11632
 - scummvm/scummvm#1858

Co-authored-by: mrbumpy409 <mrbumpy409@forums.scummvm.org>
Co-authored-by: Cameron Cawley <ccawley2011@gmail.com>
Co-authored-by: kcgen <1557255+kcgen@users.noreply.github.com>
kcgen added a commit to dosbox-staging/dosbox-staging that referenced this pull request Oct 20, 2020
These settings were tuned by ScummVM forum user mrbumpy409
started in 2012 and where made the project's default settings
in 2019.

References:
 - https://forums.scummvm.org/viewtopic.php?t=11632
 - scummvm/scummvm#1858

Co-authored-by: mrbumpy409 <mrbumpy409@forums.scummvm.org>
Co-authored-by: Cameron Cawley <ccawley2011@gmail.com>
Co-authored-by: kcgen <1557255+kcgen@users.noreply.github.com>
kcgen added a commit to dosbox-staging/dosbox-staging that referenced this pull request Oct 20, 2020
These settings were tuned by ScummVM forum user mrbumpy409
started in 2012 and where made the project's default settings
in 2019.

References:
 - https://forums.scummvm.org/viewtopic.php?t=11632
 - scummvm/scummvm#1858

Co-authored-by: mrbumpy409 <mrbumpy409@forums.scummvm.org>
Co-authored-by: Cameron Cawley <ccawley2011@gmail.com>
Co-authored-by: kcgen <1557255+kcgen@users.noreply.github.com>
kcgen added a commit to dosbox-staging/dosbox-staging that referenced this pull request Oct 22, 2020
These settings were tuned by ScummVM forum user mrbumpy409
started in 2012 and where made the project's default settings
in 2019.

References:
 - https://forums.scummvm.org/viewtopic.php?t=11632
 - scummvm/scummvm#1858

Co-authored-by: mrbumpy409 <mrbumpy409@forums.scummvm.org>
Co-authored-by: Cameron Cawley <ccawley2011@gmail.com>
Co-authored-by: kcgen <1557255+kcgen@users.noreply.github.com>
kcgen added a commit to dosbox-staging/dosbox-staging that referenced this pull request Oct 24, 2020
These settings were tuned by ScummVM forum user mrbumpy409
started in 2012 and where made the project's default settings
in 2019.

References:
 - https://forums.scummvm.org/viewtopic.php?t=11632
 - scummvm/scummvm#1858

Co-authored-by: mrbumpy409 <mrbumpy409@forums.scummvm.org>
Co-authored-by: Cameron Cawley <ccawley2011@gmail.com>
Co-authored-by: kcgen <1557255+kcgen@users.noreply.github.com>
kcgen added a commit to dosbox-staging/dosbox-staging that referenced this pull request Oct 25, 2020
These settings were tuned by ScummVM forum user mrbumpy409
started in 2012 and where made the project's default settings
in 2019.

References:
 - https://forums.scummvm.org/viewtopic.php?t=11632
 - scummvm/scummvm#1858

Co-authored-by: mrbumpy409 <mrbumpy409@forums.scummvm.org>
Co-authored-by: Cameron Cawley <ccawley2011@gmail.com>
Co-authored-by: kcgen <1557255+kcgen@users.noreply.github.com>
@neuromancer
Copy link
Contributor

@coderabbitai review

Copy link
Contributor

coderabbitai bot commented Nov 10, 2023

Walkthrough

Walkthrough

The changes primarily focus on enhancing the audio effects in the application by modifying the default values of various parameters related to reverb and chorus effects. The type of the reverbWidth variable has also been changed from int to double for more precise control.

Changes

File Path Change Summary
.../fluidsynth.cpp The type of the reverbWidth variable has been changed from int to double in the MidiDriver_FluidSynth::open function.
.../commandLine.cpp Default values for parameters fluidsynth_chorus_level, fluidsynth_reverb_roomsize, fluidsynth_reverb_damping, fluidsynth_reverb_width, and fluidsynth_reverb_level have been adjusted in the registerDefaults function.
.../fluidsynth-dialog.cpp Default values for sliders and labels related to reverb and chorus effects in the FluidSynthSettingsDialog class have been modified.

Tips

Chat with CodeRabbit Bot (@coderabbitai)

  • If you reply to a review comment from CodeRabbit, the bot will automatically respond.
  • To engage with CodeRabbit bot directly around the specific lines of code in the PR, mention @coderabbitai in your review comment
  • Note: Review comments are made on code diffs or files, not on the PR overview.
  • Add @coderabbitai ignore anywhere in the PR description to prevent this PR from being reviewed.

CodeRabbit Commands (invoked as PR comments)

  • @coderabbitai pause to pause the reviews on a PR.
  • @coderabbitai resume to resume the paused reviews.
  • @coderabbitai review to trigger a review. This is useful when automatic reviews are disabled for the repository.
  • @coderabbitai help to get help.
  • @coderabbitai resolve to resolve all the CodeRabbit review comments.

Note: For conversation with the bot, please use the review comments on code diffs or files.

CodeRabbit Configration File (.coderabbit.yaml)

  • You can programmatically configure CodeRabbit by adding a .coderabbit.yaml file to the root of your repository.
  • The JSON schema for the configuration file is available here.
  • If your editor has YAML language server enabled, you can add the path at the top of this file to enable auto-completion and validation: # yaml-language-server: $schema=https://coderabbit.ai/integrations/coderabbit-overrides.v2.json

@bluegr
Copy link
Member

bluegr commented Sep 2, 2024

These settings have been updated in 2023 (two years after this PR) with commit f1abbff - hence the conflicts in all the files that this PR touches.

The updated settings are set to value ranges and defaults used by the current FluidSynth version 2.3.4.

Since basically every setting has been updated, there's no point in keeping this PR around. If we do find that the suggested settings improve the sound output, a new PR can be opened

@bluegr bluegr closed this Sep 2, 2024
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