[tim-janik/beast] One fluid synth per track (#102)

classic Classic list List threaded Threaded
10 messages Options
Reply | Threaded
Open this post in threaded view
|

[tim-janik/beast] One fluid synth per track (#102)

Gnome - Beast mailing list

This gets rid of all bse module locks in the fluid synth code, by using one fluid synth instance per track.

I observed that if we load the soundfont every time the user presses play, this can be slow (50ms to load fluid r2 * number of soundfont tracks), so this keeps the fluid synth instances even after the project stops playing, and re-uses the instances on play.

What this doesn't implement is changing the soundfont on a track while the project is playing (the gui doesn't let me do this anyway), but this could be implemented by preparing a new fluid_synth instance and exchanging it on the fly using an engine job.

I believe that this is the best/correct way to do it, better than #85


You can view, comment on, or merge this pull request online at:

  https://github.com/tim-janik/beast/pull/102

Commit Summary

  • BSE: SF2: use one fluid_synth_t per track
  • BSE: SF2: clean up old unused code
  • BSE: SF2: remove unused mutex
  • BSE: SF2: use module data free function
  • config-checks.mk: require the latest fluidsynth version, 2.0.5
  • BSE: SF2: use fluid_synth_process() instead of deprecated function
  • BSE: SF2: cleanup: fix leak, remove debugging output
  • BSE: SF2: update documentation block.
  • BSE: SF2: don't reload the soundfont every time the user presses play

File Changes

Patch Links:


You are receiving this because you are subscribed to this thread.
Reply to this email directly, view it on GitHub, or mute the thread.

<script type="application/json" data-scope="inboxmarkup">{"api_version":"1.0","publisher":{"api_key":"05dde50f1d1a384dd78767c55493e4bb","name":"GitHub"},"entity":{"external_key":"github/tim-janik/beast","title":"tim-janik/beast","subtitle":"GitHub repository","main_image_url":"https://github.githubassets.com/images/email/message_cards/header.png","avatar_image_url":"https://github.githubassets.com/images/email/message_cards/avatar.png","action":{"name":"Open in GitHub","url":"https://github.com/tim-janik/beast"}},"updates":{"snippets":[{"icon":"DESCRIPTION","message":"One fluid synth per track (#102)"}],"action":{"name":"View Pull Request","url":"https://github.com/tim-janik/beast/pull/102"}}}</script> <script type="application/ld+json">[ { "@context": "http://schema.org", "@type": "EmailMessage", "potentialAction": { "@type": "ViewAction", "target": "https://github.com/tim-janik/beast/pull/102", "url": "https://github.com/tim-janik/beast/pull/102", "name": "View Pull Request" }, "description": "View this Pull Request on GitHub", "publisher": { "@type": "Organization", "name": "GitHub", "url": "https://github.com" } } ]</script>
_______________________________________________
beast mailing list
[hidden email]
https://mail.gnome.org/mailman/listinfo/beast
Reply | Threaded
Open this post in threaded view
|

Re: [tim-janik/beast] One fluid synth per track (#102)

Gnome - Beast mailing list

@swesterfeld pushed 1 commit.

  • a2451c6 BSE: SF2: cleanup sound sont repo code


You are receiving this because you are subscribed to this thread.
View it on GitHub or mute the thread.

<script type="application/json" data-scope="inboxmarkup">{"api_version":"1.0","publisher":{"api_key":"05dde50f1d1a384dd78767c55493e4bb","name":"GitHub"},"entity":{"external_key":"github/tim-janik/beast","title":"tim-janik/beast","subtitle":"GitHub repository","main_image_url":"https://github.githubassets.com/images/email/message_cards/header.png","avatar_image_url":"https://github.githubassets.com/images/email/message_cards/avatar.png","action":{"name":"Open in GitHub","url":"https://github.com/tim-janik/beast"}},"updates":{"snippets":[{"icon":"PERSON","message":"@swesterfeld pushed 1 commit in #102"}],"action":{"name":"View Pull Request","url":"https://github.com/tim-janik/beast/pull/102/files/39778d736d4644e28d9656da1cd6cc153a148e9a..a2451c669e32aae1e4ba91c9b6a7fc3496259a4e"}}}</script> <script type="application/ld+json">[ { "@context": "http://schema.org", "@type": "EmailMessage", "potentialAction": { "@type": "ViewAction", "target": "https://github.com/tim-janik/beast/pull/102/files/39778d736d4644e28d9656da1cd6cc153a148e9a..a2451c669e32aae1e4ba91c9b6a7fc3496259a4e", "url": "https://github.com/tim-janik/beast/pull/102/files/39778d736d4644e28d9656da1cd6cc153a148e9a..a2451c669e32aae1e4ba91c9b6a7fc3496259a4e", "name": "View Pull Request" }, "description": "View this Pull Request on GitHub", "publisher": { "@type": "Organization", "name": "GitHub", "url": "https://github.com" } } ]</script>
_______________________________________________
beast mailing list
[hidden email]
https://mail.gnome.org/mailman/listinfo/beast
Reply | Threaded
Open this post in threaded view
|

Re: [tim-janik/beast] One fluid synth per track (#102)

Gnome - Beast mailing list
In reply to this post by Gnome - Beast mailing list

As requested, I've updated this branch so that it doesn't require fluidsynth >= 2.0.5.


You are receiving this because you are subscribed to this thread.
Reply to this email directly, view it on GitHub, or mute the thread.

<script type="application/json" data-scope="inboxmarkup">{"api_version":"1.0","publisher":{"api_key":"05dde50f1d1a384dd78767c55493e4bb","name":"GitHub"},"entity":{"external_key":"github/tim-janik/beast","title":"tim-janik/beast","subtitle":"GitHub repository","main_image_url":"https://github.githubassets.com/images/email/message_cards/header.png","avatar_image_url":"https://github.githubassets.com/images/email/message_cards/avatar.png","action":{"name":"Open in GitHub","url":"https://github.com/tim-janik/beast"}},"updates":{"snippets":[{"icon":"PERSON","message":"@swesterfeld in #102: As requested, I've updated this branch so that it doesn't require fluidsynth \u003e= 2.0.5."}],"action":{"name":"View Pull Request","url":"https://github.com/tim-janik/beast/pull/102#issuecomment-487873175"}}}</script> <script type="application/ld+json">[ { "@context": "http://schema.org", "@type": "EmailMessage", "potentialAction": { "@type": "ViewAction", "target": "https://github.com/tim-janik/beast/pull/102#issuecomment-487873175", "url": "https://github.com/tim-janik/beast/pull/102#issuecomment-487873175", "name": "View Pull Request" }, "description": "View this Pull Request on GitHub", "publisher": { "@type": "Organization", "name": "GitHub", "url": "https://github.com" } } ]</script>
_______________________________________________
beast mailing list
[hidden email]
https://mail.gnome.org/mailman/listinfo/beast
Reply | Threaded
Open this post in threaded view
|

Re: [tim-janik/beast] One fluid synth per track (#102)

Gnome - Beast mailing list
In reply to this post by Gnome - Beast mailing list

@tim-janik requested changes on this pull request.

What this doesn't implement is changing the soundfont on a track while the project is playing
(the gui doesn't let me do this anyway), but this could be implemented by preparing a new
fluid_synth instance and exchanging it on the fly using an engine job.

The reason the GUI blocks instrument changes is that the BSE core hasn't had support for this yet. If soundfonts can implement changes during runtime, the associated, we'll of course release that restriction and make it user accessible. I guess that's better left for another PR, right?


In bse/bsesoundfont.cc:

> @@ -12,6 +12,8 @@
 
 #define parse_or_return         bse_storage_scanner_parse_or_return
 
+using std::string;

We need to get rid of global using clauses. The std namespace defines lots of lower case symbols/types that are likely to clash with variable names in our code, so it's best to always use explicit prefixing (like std::string, std::max). For this case specifically, there's also Bse::String, so at least inside the Bse namespacse, String can simply be used. Also, using clauses are not as bad in inner scopes, like do { using Bse; String s; ... } while (...)


In bse/bsesoundfont.cc:

> @@ -186,12 +186,12 @@ bse_sound_font_reload (BseSoundFont *sound_font)
   return bse_sound_font_load_blob (sound_font, sound_font_impl->blob, FALSE);
 }
 
-int
-bse_sound_font_get_id (BseSoundFont *sound_font)
+string

Should be std::string.


You are receiving this because you are subscribed to this thread.
Reply to this email directly, view it on GitHub, or mute the thread.

<script type="application/ld+json">[ { "@context": "http://schema.org", "@type": "EmailMessage", "potentialAction": { "@type": "ViewAction", "target": "https://github.com/tim-janik/beast/pull/102?email_source=notifications\u0026email_token=AIVS7XV4VK2PERZK4WOBNQDPWPQ43A5CNFSM4HHXXGKKYY3PNVWWK3TUL52HS4DFWFIHK3DMKJSXC5LFON2FEZLWNFSXPKTDN5WW2ZLOORPWSZGOBZHAQAI#pullrequestreview-239994881", "url": "https://github.com/tim-janik/beast/pull/102?email_source=notifications\u0026email_token=AIVS7XV4VK2PERZK4WOBNQDPWPQ43A5CNFSM4HHXXGKKYY3PNVWWK3TUL52HS4DFWFIHK3DMKJSXC5LFON2FEZLWNFSXPKTDN5WW2ZLOORPWSZGOBZHAQAI#pullrequestreview-239994881", "name": "View Pull Request" }, "description": "View this Pull Request on GitHub", "publisher": { "@type": "Organization", "name": "GitHub", "url": "https://github.com" } } ]</script>
_______________________________________________
beast mailing list
[hidden email]
https://mail.gnome.org/mailman/listinfo/beast
Reply | Threaded
Open this post in threaded view
|

Re: [tim-janik/beast] One fluid synth per track (#102)

Gnome - Beast mailing list
In reply to this post by Gnome - Beast mailing list

@tim-janik commented on this pull request.


In bse/bsesoundfontrepo.cc:

> @@ -387,10 +277,6 @@ SoundFontRepoImpl::~SoundFontRepoImpl ()
       delete_fluid_settings (fluid_settings);
       fluid_settings = NULL;

It'd be better to merge the above two lines into delete_fluid_settings (&fluid_settings); where delete_fluid_settings automatically assigns NULL to the pointer passed in.


You are receiving this because you are subscribed to this thread.
Reply to this email directly, view it on GitHub, or mute the thread.

<script type="application/ld+json">[ { "@context": "http://schema.org", "@type": "EmailMessage", "potentialAction": { "@type": "ViewAction", "target": "https://github.com/tim-janik/beast/pull/102?email_source=notifications\u0026email_token=AIVS7XT3SBASZFJ5NOD3ZWTPWPTJVA5CNFSM4HHXXGKKYY3PNVWWK3TUL52HS4DFWFIHK3DMKJSXC5LFON2FEZLWNFSXPKTDN5WW2ZLOORPWSZGOBZHEITY#pullrequestreview-240010319", "url": "https://github.com/tim-janik/beast/pull/102?email_source=notifications\u0026email_token=AIVS7XT3SBASZFJ5NOD3ZWTPWPTJVA5CNFSM4HHXXGKKYY3PNVWWK3TUL52HS4DFWFIHK3DMKJSXC5LFON2FEZLWNFSXPKTDN5WW2ZLOORPWSZGOBZHEITY#pullrequestreview-240010319", "name": "View Pull Request" }, "description": "View this Pull Request on GitHub", "publisher": { "@type": "Organization", "name": "GitHub", "url": "https://github.com" } } ]</script>
_______________________________________________
beast mailing list
[hidden email]
https://mail.gnome.org/mailman/listinfo/beast
Reply | Threaded
Open this post in threaded view
|

Re: [tim-janik/beast] One fluid synth per track (#102)

Gnome - Beast mailing list
In reply to this post by Gnome - Beast mailing list

@swesterfeld commented on this pull request.


In bse/bsesoundfontrepo.cc:

> @@ -387,10 +277,6 @@ SoundFontRepoImpl::~SoundFontRepoImpl ()
       delete_fluid_settings (fluid_settings);
       fluid_settings = NULL;

delete_fluid_settings is part of the fluid synth api, so I cannot change what it does


You are receiving this because you are subscribed to this thread.
Reply to this email directly, view it on GitHub, or mute the thread.

<script type="application/ld+json">[ { "@context": "http://schema.org", "@type": "EmailMessage", "potentialAction": { "@type": "ViewAction", "target": "https://github.com/tim-janik/beast/pull/102?email_source=notifications\u0026email_token=AIVS7XUETP3BPHL3XGEGRWLPWQYEJA5CNFSM4HHXXGKKYY3PNVWWK3TUL52HS4DFWFIHK3DMKJSXC5LFON2FEZLWNFSXPKTDN5WW2ZLOORPWSZGOBZISW5A#discussion_r286145922", "url": "https://github.com/tim-janik/beast/pull/102?email_source=notifications\u0026email_token=AIVS7XUETP3BPHL3XGEGRWLPWQYEJA5CNFSM4HHXXGKKYY3PNVWWK3TUL52HS4DFWFIHK3DMKJSXC5LFON2FEZLWNFSXPKTDN5WW2ZLOORPWSZGOBZISW5A#discussion_r286145922", "name": "View Pull Request" }, "description": "View this Pull Request on GitHub", "publisher": { "@type": "Organization", "name": "GitHub", "url": "https://github.com" } } ]</script>
_______________________________________________
beast mailing list
[hidden email]
https://mail.gnome.org/mailman/listinfo/beast
Reply | Threaded
Open this post in threaded view
|

Re: [tim-janik/beast] One fluid synth per track (#102)

Gnome - Beast mailing list
In reply to this post by Gnome - Beast mailing list

@swesterfeld pushed 1 commit.

  • 9a55741 BSE: SF2: avoid global using std::string


You are receiving this because you are subscribed to this thread.
View it on GitHub or mute the thread.

<script type="application/ld+json">[ { "@context": "http://schema.org", "@type": "EmailMessage", "potentialAction": { "@type": "ViewAction", "target": "https://github.com/tim-janik/beast/pull/102/files/85b667db0490195a6a96170082fe821998742f09..9a557418278511ffb21898f1c67fce33b274e0e2?email_source=notifications\u0026email_token=AIVS7XQ6QFM67BR7BV5OAL3PWQ67FA5CNFSM4HHXXGKKYY3PNVWWK3TUL52HS4DFXNIHK3DMKJSXC5LFON2FA5LTNBHG65DJMZUWGYLUNFXW5KTDN5WW2ZLOORPWSZF5KB2WY3BDGI3TENZSGEZDGOKQOVZWQIZTGYZTENJSHA2DANY", "url": "https://github.com/tim-janik/beast/pull/102/files/85b667db0490195a6a96170082fe821998742f09..9a557418278511ffb21898f1c67fce33b274e0e2?email_source=notifications\u0026email_token=AIVS7XQ6QFM67BR7BV5OAL3PWQ67FA5CNFSM4HHXXGKKYY3PNVWWK3TUL52HS4DFXNIHK3DMKJSXC5LFON2FA5LTNBHG65DJMZUWGYLUNFXW5KTDN5WW2ZLOORPWSZF5KB2WY3BDGI3TENZSGEZDGOKQOVZWQIZTGYZTENJSHA2DANY", "name": "View Pull Request" }, "description": "View this Pull Request on GitHub", "publisher": { "@type": "Organization", "name": "GitHub", "url": "https://github.com" } } ]</script>
_______________________________________________
beast mailing list
[hidden email]
https://mail.gnome.org/mailman/listinfo/beast
Reply | Threaded
Open this post in threaded view
|

Re: [tim-janik/beast] One fluid synth per track (#102)

Gnome - Beast mailing list
In reply to this post by Gnome - Beast mailing list

@swesterfeld commented on this pull request.


In bse/bsesoundfont.cc:

> @@ -12,6 +12,8 @@
 
 #define parse_or_return         bse_storage_scanner_parse_or_return
 
+using std::string;

Right, I've fixed this now by using std::string - I also tried using Bse::String - this doesn't have any advantage here, as all code is outside the Bse namespace.


You are receiving this because you are subscribed to this thread.
Reply to this email directly, view it on GitHub, or mute the thread.

<script type="application/ld+json">[ { "@context": "http://schema.org", "@type": "EmailMessage", "potentialAction": { "@type": "ViewAction", "target": "https://github.com/tim-janik/beast/pull/102?email_source=notifications\u0026email_token=AIVS7XQEVVGEK4UZ3DBNA6LPWQ7EHA5CNFSM4HHXXGKKYY3PNVWWK3TUL52HS4DFWFIHK3DMKJSXC5LFON2FEZLWNFSXPKTDN5WW2ZLOORPWSZGOBZI2MEY#discussion_r286170889", "url": "https://github.com/tim-janik/beast/pull/102?email_source=notifications\u0026email_token=AIVS7XQEVVGEK4UZ3DBNA6LPWQ7EHA5CNFSM4HHXXGKKYY3PNVWWK3TUL52HS4DFWFIHK3DMKJSXC5LFON2FEZLWNFSXPKTDN5WW2ZLOORPWSZGOBZI2MEY#discussion_r286170889", "name": "View Pull Request" }, "description": "View this Pull Request on GitHub", "publisher": { "@type": "Organization", "name": "GitHub", "url": "https://github.com" } } ]</script>
_______________________________________________
beast mailing list
[hidden email]
https://mail.gnome.org/mailman/listinfo/beast
Reply | Threaded
Open this post in threaded view
|

Re: [tim-janik/beast] One fluid synth per track (#102)

Gnome - Beast mailing list
In reply to this post by Gnome - Beast mailing list

The reason the GUI blocks instrument changes is that the BSE core hasn't had support for this yet. If soundfonts can implement changes during runtime, the associated, we'll of course release that restriction and make it user accessible. I guess that's better left for another PR, right?

I agree. As I understand it, updating beast-gtk is to be avoided, I suppose this should be done once ebeast supports assigning soundfont & preset with the new ui. The fluid code as it is supports changing the preset of an existing track during play. It doesn't support changing the SF2 filename of a track during play, but this could be implemented by swapping the fluid_synth_t instance in this case. Maybe that should be done after you have the new engine module api.

Anyway I think I've addressed the other issues now, will reassign this to you so you can merge it.


You are receiving this because you are subscribed to this thread.
Reply to this email directly, view it on GitHub, or mute the thread.

<script type="application/ld+json">[ { "@context": "http://schema.org", "@type": "EmailMessage", "potentialAction": { "@type": "ViewAction", "target": "https://github.com/tim-janik/beast/pull/102?email_source=notifications\u0026email_token=AIVS7XXGYPXAIBJQGWCV4ELPWQ7ZBA5CNFSM4HHXXGKKYY3PNVWWK3TUL52HS4DFVREXG43VMVBW63LNMVXHJKTDN5WW2ZLOORPWSZGODV42SDY#issuecomment-494512399", "url": "https://github.com/tim-janik/beast/pull/102?email_source=notifications\u0026email_token=AIVS7XXGYPXAIBJQGWCV4ELPWQ7ZBA5CNFSM4HHXXGKKYY3PNVWWK3TUL52HS4DFVREXG43VMVBW63LNMVXHJKTDN5WW2ZLOORPWSZGODV42SDY#issuecomment-494512399", "name": "View Pull Request" }, "description": "View this Pull Request on GitHub", "publisher": { "@type": "Organization", "name": "GitHub", "url": "https://github.com" } } ]</script>
_______________________________________________
beast mailing list
[hidden email]
https://mail.gnome.org/mailman/listinfo/beast
Reply | Threaded
Open this post in threaded view
|

Re: [tim-janik/beast] One fluid synth per track (#102)

Gnome - Beast mailing list
In reply to this post by Gnome - Beast mailing list

Closed #102 via 5f75d57.


You are receiving this because you are subscribed to this thread.
Reply to this email directly, view it on GitHub, or mute the thread.

<script type="application/ld+json">[ { "@context": "http://schema.org", "@type": "EmailMessage", "potentialAction": { "@type": "ViewAction", "target": "https://github.com/tim-janik/beast/pull/102?email_source=notifications\u0026email_token=AIVS7XQTMFMBNFJC3GNQ333P6FBLXA5CNFSM4HHXXGKKYY3PNVWWK3TUL52HS4DFWZEXG43VMVCXMZLOORHG65DJMZUWGYLUNFXW5KTDN5WW2ZLOORPWSZGOSLQDMQI#event-2464167489", "url": "https://github.com/tim-janik/beast/pull/102?email_source=notifications\u0026email_token=AIVS7XQTMFMBNFJC3GNQ333P6FBLXA5CNFSM4HHXXGKKYY3PNVWWK3TUL52HS4DFWZEXG43VMVCXMZLOORHG65DJMZUWGYLUNFXW5KTDN5WW2ZLOORPWSZGOSLQDMQI#event-2464167489", "name": "View Pull Request" }, "description": "View this Pull Request on GitHub", "publisher": { "@type": "Organization", "name": "GitHub", "url": "https://github.com" } } ]</script>
_______________________________________________
beast mailing list
[hidden email]
https://mail.gnome.org/mailman/listinfo/beast