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.

Sign up for GitHub

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

Jump to bottom

Mapping of fx unit output to dry buffers in mix mode. #668

Merged
merged 7 commits into from
Sep 2, 2020
Merged

Mapping of fx unit output to dry buffers in mix mode. #668

merged 7 commits into from
Sep 2, 2020

Conversation

jjceresa
Copy link
Collaborator

Actually all fx unit output (in mix mode) are mapped to the first buffer.
This is not appropriate for synth.audio-groups > 1

This PR allows the mapping of fx output based on fx unit index and synth.audio-groups value.
This allows to get the fx output mixed to the respective buffer on which a MIDI channel is mapped.
For example: with synth.audio-groups = 3 and synth.effect-groups = 3:

  • MIDI chan 0 (dry + fx0) is mapped to buf 0
  • MIDI chan 1 (dry + fx1) is mapped to buf 1
  • MIDI chan 2 (dry + fx2) is mapped to buf 2

@derselbst derselbst mentioned this pull request Aug 31, 2020
Merged
derselbst
derselbst reviewed Sep 2, 2020
View reviewed changes
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.

Overall this looks good to me, one code comment below. I also like the chart, but I would like to ask you for some minor changes:

  • Could you add a legend (or similar) that explains those /s and /m symbols in your flow chart?
  • synth.fx-groups should better be called synth.effects-groups
  • Sometimes you use bounding boxes with a dotted line and sometimes with a solid line; I can imaging that e.g. the dotted around the ladspa box this mean "optional"; however, the dotted box around the "in1" ... "ink" boxes is not quite clear to me
  • In the LADSPA box, the one "fx plugin" box is hard to read due to overlap
  • "fx buf 1" and "fx buf k" arrows currently end in void; would it be worth to add a box "fluid_synth_process()" there?

src/rvoice/fluid_rvoice_mixer.c Show resolved Hide resolved
@mawe42
Copy link
Member

mawe42 commented Sep 2, 2020 •
edited
Loading

Sorry, haven't got enough time to comment in detail here at the moment. Just a quick suggestion: if we add useful charts like the one added in this PR, maybe we can also add the source file or at least an editable version (SVG?) as well? Otherwise those files can only be updated by completely recreating them (or by asking the original contributer to provide an update).

@derselbst
Copy link
Member

maybe we can also add the source file or at least an editable version

Yep, good point. I guess it's .doc :)

@jjceresa
Copy link
Collaborator Author

jjceresa commented Sep 2, 2020

Could you add a legend (or similar) that explains those /s and /m symbols in your flow chart?

This chart is just a quick draft and incomplete. It should be part of a textual document that explains how the mixer behave with true sentences.

Why do you think that "dry buffer mapping, should be done more flexible in the future"?

I just intended to show that the 3 mapping selectors (MIDI chan-to-out, MIDI chan-to-fx, and fx-to-out ), are closely related and oriented MIDI channel. In others words when the musician think for a MIDI instrument mapping configuration, he decide of this 3 mapping at MIDI level:

  • MIDI chan x to dry buf i (actually i = x % audio-groups)
  • MIDI chan x to fx unit j (actually j = x % effects-groups)
  • fx j to dry buf k (actually k = j % audio-groups)
    Actually these mapping are rigid an "should be done more flexible in the future" with the help of API.

Just a quick suggestion: if we add useful charts like the one added in this PR, maybe we can also add the source file or at least an editable version (SVG?) as well?

Yes actually, I use powerpoint to do quick synoptic and functional chart. I will update this one and provide the source.

@jjceresa
Copy link
Collaborator Author

jjceresa commented Sep 2, 2020

Yep, good point. I guess it's .doc :)

When making a document that need text + figure, synoptic... I usually use Word for text, but I dont use the build-in image editor in incorporated in Word. I prefer using powerpoint for fast simple figure editing which is pain less for me. Then I copy -paste the figure in the right place in the .doc. Once the figure is in the.doc, it is still editable by Word 's image editor.
the .doc.
If you are interested, I can provide a .doc with the image above to try this ?.
If you have "open office" you have also powerpoint.

@derselbst
Copy link
Member

You can just provide the powerpoint, that's fine. We can write up a markdown file and just reference to the PDF.

@derselbst
Copy link
Member

Great! I extracted the chart from this PR and committed it separately.

@derselbst derselbst merged commit 57af880 into master Sep 2, 2020
@derselbst derselbst deleted the map-fx-to-out branch September 2, 2020 19:10
CartoonFan pushed a commit to CartoonFan/fluidsynth that referenced this pull request Dec 20, 2020
@pull @derselbst
@ffontaine @jjceresa @dvzrv @komh @JonLatane @mawe42 @Some1NamedNate @chirs241097 @carlo-bramini
[pull] master from FluidSynth:master (#2)
4fe2c70 
* Properly handle overlapping notes when using fluid_event_note() (FluidSynth#637)

* Fix regression introduced in a893994

Mentioned commit broke fluid_synth_start() when using a DLS soundfont.

* Fix an uninitialized memory access

that could possibly trigger an FPE trap for instruments that use the exclusive class generator

* Update API docs

* Bump to 2.1.4

* Update Doxyfile

* Turn incompatible-pointer-types warning into error

* Fix passing arguments from incompatible pointer type

* Fix a NULL deref in jack driver

* Fix a possible race condition during midi autoconnect

* Fix printf format warnings

* Update Android Asset loader to new callback API

* Update Travis CI (FluidSynth#658)

* update to Ubuntu Focal
* use clang10
* avoid unintentional fallbacks to  default `/usr/bin/c++` compiler
* fix related compiler warnings

* fix NULL permitted for out and fx pointer buffer

Closes FluidSynth#659

* CMakeLists.txt: fix build with gcc 4.8 (FluidSynth#661)

-Werror=incompatible-pointer-types is unconditionally used since version
2.1.4 and 137a14e. This will raise a
build failure when checking for threads on gcc 4.8:

/home/buildroot/autobuild/run/instance-3/output-1/host/bin/arm-none-linux-gnueabi-gcc --sysroot=/home/buildroot/autobuild/run/instance-3/output-1/host/arm-buildroot-linux-gnueabi/sysroot -DTESTKEYWORD=inline  -D_LARGEFILE_SOURCE -D_LARGEFILE64_SOURCE -D_FILE_OFFSET_BITS=64 -Os -Wall -W -Wpointer-arith -Wcast-qual -Wstrict-prototypes -Wno-unused-parameter -Wdeclaration-after-statement -Werror=implicit-function-declaration -Werror=incompatible-pointer-types -Wbad-function-cast -Wcast-align   -DNDEBUG -fPIE   -o CMakeFiles/cmTC_98946.dir/CheckIncludeFile.c.o   -c /home/buildroot/autobuild/run/instance-3/output-1/build/fluidsynth-2.1.4/CMakeFiles/CMakeTmp/CheckIncludeFile.c
cc1: error: -Werror=incompatible-pointer-types: no option -Wincompatible-pointer-types

Fixes:
 - http://autobuild.buildroot.org/results/13cbba871db56ef8657a3d13c6ac8e1b4da0d244

Signed-off-by: Fabrice Fontaine <fontaine.fabrice@gmail.com>

* TravisCI: add a build for GCC 4.8 (FluidSynth#662)

* Remove unused member variable

* Limiting audio-channels to audio-groups (FluidSynth#663)

* Use a runtime check to detect version of libinstpatch (FluidSynth#666)

It could be that during runtime an older version of libinstpatch is used than the one fluidsynth was compiled against. In this case, libinstpatch will fail to load DLS fonts, because libinstpatch's initialization semantics don't match those compiled into fluidsynth.

* Add a chart about voice mixing and rendering

* Mapping of fx unit output to dry buffers in mix mode. (FluidSynth#668)

Currently, all fx unit output (in mix mode) are mapped to the `first buffer`.
This is not appropriate for synth.audio-groups > 1

This PR allows the mapping of fx output based on `fx unit index` and `synth.audio-groups` value.
This allows us to get the `fx output `mixed to the respective  `buffer` on which a `MIDI channel` is mapped.
For example: with `synth.audio-groups = 3` and  `synth.effect-groups = 3`:
- MIDI chan 0 (dry + fx0) is mapped to buf 0
- MIDI chan 1 (dry + fx1) is mapped to buf 1
- MIDI chan 2 (dry + fx2) is mapped to buf 2

* Add multi channels support for audio driver. (FluidSynth#667)

This PR addresses FluidSynth#665.

1) Add new functions for multi channels support: `fluid_synth_write_float_channels()`, `fluid_synth_write_s16_channels()`
2) `dsound` and `waveout` driver make use of this support. tested on 2 audio devices: 
    - creative SB Live! (6 channels).
    - Realtek: ALC889A (8 channels).

* Bump to 2.1.5

* Add SonarQube static code analysis (FluidSynth#671)

* Add SonarQube and LGTM badges to README

* Remove fluid_event_any_control_change() from public API (FluidSynth#674)

Originally, I have only marked it deprecated. But since we have an SOVERSION bump next release and because this function was only meant for internal usage, I think it's safe to remove it right now.

* Remove dead code

* Fix an impossible NULL deref

* Fix a NULL dereference

Access to field 'zone' results in a dereference of a null pointer (loaded from variable 'prev_preset'), if `size` is negative. Problem is: Parameter `size` is `chunk.size` and should be unsigned.

* Fix another NULL dereference

Access to field 'zone' results in a dereference of a null pointer (loaded from variable 'pr'), if size is negative. However, size should be unsigned.

* Remove a FIXME

I don't see any problem calling fluid_channel_init() from within synth context

* Remove a FIXME

I don't see any 'allocation' of preset. And ALL public synth functions have a mutex lock which might potentially block when called from synth context, but only then if the client app pessimizes this situation by extensively calling the synth from outside the synth context.

* Remove a FIXME

I have no clue what it refers to or what it's meant by that.

* Add comment into empty block

* Remove a FIXME

Not aware of any problems caused by the old glib thread API. It will be removed sooner or later anyway.

* Remove a FIXME

* Set the systemd unit target to default.target

fluidsynth.service.in:
The [Install] section [1] in systemd unit declares in which target the
service will be started.
The `multi-user.target` [2] - managed by the systemd _system_ service
manager - is used in the `fluidsynth.service`.
However, as it is a _user_ unit it needs to be pulled in by the
`default.target` [3] instead, which is the main target for the user
session (as started by `user@.service` [4]).

[1] https://www.freedesktop.org/software/systemd/man/systemd.unit.html#%5BInstall%5D%20Section%20Options
[2] https://www.freedesktop.org/software/systemd/man/systemd.special.html#multi-user.target
[3] https://www.freedesktop.org/software/systemd/man/systemd.special.html#default.target1
[4] https://www.freedesktop.org/software/systemd/man/user@.service.html

* Define FLUIDSYNTH_API on OS/2

Previously, CMake on OS/2 exported all the symbols unconditionally. Now
it exports necessary symbols only. As a result, it's necessary to
define FLUIDSYNTH_API correctly.

Addresses FluidSynth#678

* Make winmidi driver multi devices capable. (FluidSynth#677)

* Fix minor bug in windows audio driver (FluidSynth#680)

* Improve error reporting in Waveout and DSound drivers

* Fix Windows build

* Add proper unicode support to Windows error reporting

* Fix build on Windows 9x/ME

Addresses FluidSynth#679

* Promote Controller/Pressure/Bend event functions to 32bits (FluidSynth#670)

* Elaborate on synth.cpu-cores

* Add FluidMixer chart to API docs

* Ensure WaveOut compatibility with Win9X/NT (FluidSynth#687)

* Update and rename README.Android.md to README.md

* Update Android CircleCI build to use latest orb, Android API, Oboe and Cerbero (FluidSynth#690)

This fixes the currently-broken CircleCI build for Android-useable .so files.

Currently the Cerbero build is based off https://github.com/falrm/cerbero until https://gitlab.freedesktop.org/gstreamer/cerbero/-/merge_requests/641 is merged and deployed to the GitHub cerbero mirror.

Here is a successful build with the updated CircleCI workflow: https://app.circleci.com/pipelines/github/falrm/fluidsynth-android/31/workflows/0ad3186a-394c-4736-984b-96496b608053/jobs/32

Fixes FluidSynth#688

* Replace FreeBSD 13.0 with 11.4 (FluidSynth#692)

13.0 hasn't been released yet and the CI build keeps failing for long.

* Remove unused variable

* Fix possible uninitialized use of dry_idx variable

* avoid an unlikely race condition

* Add hint message when compiled without getopt support (FluidSynth#697)

* Add getopt support to CMake summary

* Add public API to pin and unpin presets to the sample cache (FluidSynth#698)

Following the discussion about an API to pin and unpin preset samples in the sample cache here:
https://lists.nongnu.org/archive/html/fluid-dev/2020-10/msg00016.html

Short explanation of the change:

Only the default loader currently supports dynamic sample loading, so I thought it might be a good idea to keep the changes for this feature mostly contained in the default loader as well. I've added two new preset notify flags (FLUID_PRESET_PIN and FLUID_PRESET_UNPIN) that are handled by the preset->notify callback and trigger the loading and possibly unloading of the samples.

* Revert "remove VintageDreamsWaves-v2.sf3"

This reverts commit a36c06c. We've got
explicit permission from Ian Wilson to convert it to SF3.

Addresses FluidSynth#701.

* Updated XSL / styling for fluidsettings.xml

* Cleanup section label markup and rendering

* Use (empty string) for empty default values of str settings

* shell.port is an int setting, not num

* Update periods and period-size with current values from source

* Consistently format all floats

* Better explain currently unused effects-channels

* Update effects-groups description to avoid the word "unit"

* Update ladspa.active description

Use 1 (TRUE) for consistency and mention LADSPA documentation

* As gs is default for midi-bank-select, list it as first option for clarity

* Options seems to be more widely used, so use that instead of Choices

* Remove FLUIDSYNTH_API and FLUID_DEPRECATED macros from documentation

* Remove "References" and "Referenced by" links from doc

They auto generated links are quite long on some functions, making
the documentation harder to read.

* Enable navigation sidebar

* Make larger enums easier to read

* Move doxygen customizations into separate directory

* Restructure devdocs into separate pages

* Change files into groups / modules

* Some additional subgrouping

* Use xsltproc to include settings in API documentation

* Replace all links to fluidsettings.xml with proper \ref's

* Command Shell group for all shell related commands

With subgroups for command handler, shell and server.

* Audio output group

With subgroups Audio Driver and File Renderer

* Logging interface

* MIDI input group

Contains MIDI Driver, MIDI Router, MIDI Player and MIDI Events

* MIDI Seqencer documentation

* Settings documentation

* Miscellaneous group

* SoundFont API

Includes Generators, Modulators, Loader etc

* Add version defines and functions to misc group

* Rename setting reference page name to lowercase, for consistency

* Structure the large synth header into subgroups

Also include version.h and ladspa.h in the Synthesizer group.

* Consistent capitalization of usage guide section names

* Some more brief message abbreviation hints

* Custom doxygen layout to rename modules to API Reference

* Sort groups/modules, briefs and members

* Updated documentation styling

* Remove footer, as it takes away valuable vertical space

* Make sure libxslt is only searched if doxygen is available as well

* Also update the styling of the deprecated list

* Mark settings with callbacks as realtime and output this in the generated docs

* Separate new_* and delete_* functions from the rest

* Sync the static Doxyfile with Doxyfile.cmake

Still missing is the integration of the generated fluidsettings.txt,
as that requires a build script currently not available on the
server generating the public API docs.

* Split doxygen INPUT into separate lines, for easier readability

* Move recent changes into separate file

* Move usage guide pages into separate files in doc/usage

* Move examples into doc/examples directory

* Split HTML_EXTRA_FILEs into separate lines

* Use \image for images and improve quality of FluidMixer image

* Use custom \setting{} alias to link to fluid settings

* Smaller cleanup and reformatting of long lines.

* Add generated fluidsettings.txt for fluidsynth.org API doc build

Probably not the final solution, but works for now.

* Hide nav sync toggle button

* Style improvements for small screens
- hide side nav
- hide search box
- make content full height

* Improve styling of field tables (enum values)

* Document how to revert the styling and layout changes

* Add documentation hints to style guide

* Make top links black on hover, not white

* Add missing group brief descriptions

* Remove debug leftover

* Remove obsolete doxygen config options

* Add intro text to deprecated list

* Use SVG for fluid mixer image

* Workaround for doxygen bug with linebreaks in ALIASES

Using \_linebr is not ideal, as it's an internal command. But that
seems to be the most compatible way to specify line breaks in ALIASES
accross different doxygen versions at the moment.

* GitHub Action to build the API docs from master branch (FluidSynth#704)

Uploads the complete HTML API docs as an artifact called api_docs.zip

* Remove unused command alias and sync Doxyfile.cmake and Doxyfile

* Settings reference style more consistent with rest of reference pages

* Update generated fluidsettings.txt for API doc build on fluidsynth.org

* Fx unit api (FluidSynth#673)

This PR addresses FluidSynth#669 point 2.1.
It proposes set/get API functions to change/read fx unit parameters.
The deprecated shell commands are updated. Now the commands line have 2 parameters:
- first parameter is the fx unit index.
- second parameter is the value to apply to the fx unit.

* Update owner of the SoundFont registered trademark. (FluidSynth#706)

As of the time of this PR, the SoundFont registered trademark is owned by Creative Technology Ltd.
http://tmsearch.uspto.gov/bin/showfield?f=doc&state=4803:rj74xq.2.1
http://assignments.uspto.gov/assignments/q?db=tm&qt=sno&reel=&frame=&sno=74325965

* Handle GS SysEx messages for setting whether a channel is used for rhythm part. (FluidSynth#708)

Some MIDI files that uses the GS standard uses channels other than channel 10 as percussion channel. Currently FluidSynth ignores the messages setting that up, causing notes meant to be played with a drum instrument played with a melodic instrument or vice versa. This patch will partially fix the issue.

Currently the implementation in this patch doesn't cover a specific "quirk" in Roland GS Modules: they seem to remember the last used instrument in the selected mode. This patch simply sets the instrument number to 0 instead.

A test file is attached. If played correctly (with `-o synth.device-id=16`) no out of place drum or piano sounds should be heard.

[wikipedia_MIDI_sample_gstest.mid.gz](https://github.com/FluidSynth/fluidsynth/files/5610727/wikipedia_MIDI_sample_gstest.mid.gz)

* Fix Windows CI

Remove fake pkg-config

* Re-enable unit tests with mingw

and allow them to fail to ensure build artifacts are being published

* Update API doc build to upload to GH pages

* Fix build path in API doc publish step

* Clean existing files in API doc on GH pages

* Fix commit message for deploying API doc

* Also set commit name and email for api doc build commits

* Commit to test API doc build

Will be removed with next commit again.

* Revert "Commit to test API doc build"

This reverts commit fd39f6e.

* Make some strings const (FluidSynth#716)

* Replace g_ascii_strtoll() with FLUID_STRTOL() (FluidSynth#717)

* Elaborate on synth.device-id

* Breaking unit tests for WindowsXP should be fatal

* Update Issue templates to point to GitHub discussion

Co-authored-by: Tom M <tom.mbrt@googlemail.com>
Co-authored-by: jjceresa <jjc_fluid@orange.fr>
Co-authored-by: Fabrice Fontaine <fontaine.fabrice@gmail.com>
Co-authored-by: jjceresa <32781294+jjceresa@users.noreply.github.com>
Co-authored-by: David Runge <dave@sleepmap.de>
Co-authored-by: KO Myung-Hun <komh@chollian.net>
Co-authored-by: Jon Latané <jonlatane@gmail.com>
Co-authored-by: Marcus Weseloh <marcus@weseloh.cc>
Co-authored-by: Nathan Umali <some1namednate@gmail.com>
Co-authored-by: Chris Xiong <chirs241097@gmail.com>
Co-authored-by: Carlo Bramini <30959007+carlo-bramini@users.noreply.github.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@derselbst derselbst derselbst left review comments

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
3 participants
@jjceresa @mawe42 @derselbst