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

Plugin framework #3743

Merged
merged 2 commits into from
Apr 16, 2021
Merged

Plugin framework #3743

merged 2 commits into from
Apr 16, 2021

Conversation

Krzmbrzl
Copy link
Member

@Krzmbrzl Krzmbrzl commented Aug 10, 2019 •
edited
Loading

This PR is about implementing a plugin framework (in the conventional sense).

See #3742 for further context.

This PR introduces the following murmur config options:

  • pluginmessagelimit
  • pluginmessageburst

API:

  • Finalize API to include all requirements listed in the linked issue
  • Discuss the naming conventions (use custom or try to use names as close as possible to the TS API)
  • If using TS names: Provide macros that can be used to transform the implementing function names to either follow the TS naming scheme (prefixed by e.g. ts3plugin_) or Mumble naming scheme (?)).
  • Let the API be thoroughly reviewed

Implementation:

  • Write (wrapper-)classes to handle plugins inside Mumble
  • Replace the current Plugins class in Mumble
  • Redirect events from Mumble to the plugin-callbacks
  • Write MumbleAPI for use by plugins
  • Write example plugin
  • Write test-cases
  • Implement move to password-protected channel
  • Implement plugin update
  • Port to cmake
  • Use Poco instead of QuaZip
  • Unload plugin's library if plugin is unloaded/shutdown via settings window
  • Make all locks non re-entrant and narrow locking scopes
  • Expose user's comment to plugin API
  • Expose user and server hash to plugin API
  • Expose "silence below", "speech above", "voice hold" settings to plugin API (Programmatically setting Silence Below, Speech Above, and voice hold #4419)
  • Explain PCM format
  • Include sample rate in the audio callback parameters
  • Rework how Strings can be passed to Mumble from the plugin so that they don't have to be static (as e.g. Rust doesn't support static strings)
  • Rate-limit plugin-messages (Plugin framework #3743 (comment))
  • Verify that plugin auto-updates are functional (Plugin framework #3743 (comment))
  • Add functionality to unload a plugin from the UI
  • Add functionality to trigger plugin installer from UI
  • Store name of API functions a given resource is allocated in so that this can be printed in the "leaked memory" message (Bug: Memory leaks reported by mumble hbeni/fgcom-mumble#81 (comment))

Fixes #2455
Fixes #2148
Fixes #1594
Fixes #2051
Fixes #3742
Fixes #4575
Fixes #4751

Links to comments describing changes in the API:

This was referenced Aug 13, 2019
Open
Open
@Krzmbrzl Krzmbrzl marked this pull request as ready for review September 1, 2019 18:30
@Krzmbrzl Krzmbrzl changed the title WIP: Plugin API Plugin API Sep 9, 2019
NouberNou
NouberNou reviewed Sep 9, 2019
View reviewed changes
Copy link

@NouberNou NouberNou left a comment

Choose a reason for hiding this comment

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

I have some fairly large questions about the audio callbacks in terms of where they occur within the audio processing system. I think we need to understand fully where these callbacks occur to make sure that they are affecting the sound data at a useful point in the system.

src/mumble/pluginAPI/Plugin.h Outdated Show resolved Hide resolved
src/mumble/pluginAPI/Plugin.h Outdated Show resolved Hide resolved
@NouberNou

This comment has been minimized.

Sign in to view
@davidebeatrici davidebeatrici force-pushed the plugin-API branch 2 times, most recently from b804d87 to 87277ff Compare September 20, 2019 22:07
@Krzmbrzl Krzmbrzl changed the title Plugin API WIP: Plugin framework Sep 21, 2019
@davidebeatrici davidebeatrici force-pushed the plugin-API branch 2 times, most recently from df56629 to c81b84c Compare September 21, 2019 17:44
@davidebeatrici davidebeatrici added this to the 1.4.0 milestone Sep 21, 2019
@davidebeatrici davidebeatrici mentioned this pull request Oct 15, 2019
Closed
3 tasks
@Krzmbrzl Krzmbrzl mentioned this pull request Oct 16, 2019
Closed
@Krzmbrzl Krzmbrzl mentioned this pull request Nov 24, 2019
Closed
This was referenced Dec 26, 2019
Closed
Closed
Open
@Krzmbrzl Krzmbrzl force-pushed the plugin-API branch 4 times, most recently from 53144e8 to 9adfa57 Compare December 27, 2019 17:56
@Krzmbrzl Krzmbrzl force-pushed the plugin-API branch 2 times, most recently from 5a6910b to 9a0a2e9 Compare January 3, 2020 16:41
@Krzmbrzl Krzmbrzl mentioned this pull request Jan 22, 2020
Closed
@Krzmbrzl Krzmbrzl mentioned this pull request Apr 1, 2021
Closed
@Krzmbrzl Krzmbrzl force-pushed the plugin-API branch 2 times, most recently from b01abfd to 18cdf85 Compare April 1, 2021 14:49
@Krzmbrzl Krzmbrzl added the feature-request This issue or PR deals with a new feature label Apr 1, 2021
@Krzmbrzl Krzmbrzl mentioned this pull request Apr 3, 2021
Closed
@Krzmbrzl
Copy link
Member Author

API change

The data and ID field for the API::sendData function are now hard-limited to 1KB and 100 bytes respectively (to avoid eating up all available bandwidth with them).

@Krzmbrzl
Copy link
Member Author

The deed is done!
After more than 1.5 years of development I consider this feature to be ready for merge. There are obviously still some things that can be improved upon and also lots of functionality that the framework could be extended to support, but this should be done at a later point in order to make sure that the first version of the plugin framework finally gets merged and is therefore available to our users.

I think the next milestone is the snapshot-phase in which users will test out the upcoming Mumble snapshots (that will include this framework) to see if the implementation broke any functionality.

Furthermore I will have to sit down and write some documentation on the entire thing so that it is as easy to use as possible.

Let's hope that it'll be put to good use! 🚀

@Krzmbrzl Krzmbrzl marked this pull request as ready for review April 14, 2021 18:06
@Krzmbrzl Krzmbrzl changed the title WIP: Plugin framework Plugin framework Apr 14, 2021
davidebeatrici
davidebeatrici reviewed Apr 14, 2021
View reviewed changes
.ci/azure-pipelines/install-environment_linux.bash Outdated Show resolved Hide resolved
docs/dev/build-instructions/cmake_options.md Outdated Show resolved Hide resolved
docs/dev/build-instructions/cmake_options.md Outdated Show resolved Hide resolved
plugins/testPlugin/testPlugin.cpp Outdated Show resolved Hide resolved
plugins/testPlugin/testPlugin.cpp Outdated Show resolved Hide resolved
src/mumble/API.h Outdated Show resolved Hide resolved
src/mumble/API.h Show resolved Hide resolved
@Krzmbrzl
Copy link
Member Author

API change

The API functions to get and set int settings have been changed to use int64_t instead of int in order to make sure that the size of the used parameter remains the same on all platforms.

Krzmbrzl added 2 commits April 16, 2021 20:15
@Krzmbrzl
FEAT(client): Plugin framework
27dbee8 
This commit introduces a new plugin framework into the codebase of the
Mumble client. Note that "plugin" here really refers to a (more or less)
general purpose plugin and is therefore not to be confused with the
previously available positional data plugins (only responsible for
fetching positional data from a running game and passing that to
Mumble).

The plugin interface is written in C, removing the compiler-dependence
the old "plugins" had. Instead plugins can now be written in an
arbitrary language as long as that language is capable of being compiled
into a shared library and also being capable of being C-compatible.

As already indicated a plugin is essentially a shared library that
provides certain functions that allow Mumble to interface with it.

Inside Mumble the so-called PluginManager is responsible for managing
the plugins and relaying events to the respective callbacks. Plugins
themselves can also interact with Mumble on their own initiative by
using the provided API functions.

Fixes mumble-voip#2455
Fixes mumble-voip#2148
Fixes mumble-voip#1594
Fixes mumble-voip#2051
Fixes mumble-voip#3742
Fixes mumble-voip#4575
Fixes mumble-voip#4751
@Krzmbrzl
TRANSLATION: Update translation files
5db2400
@Krzmbrzl Krzmbrzl merged commit d4a5fc1 into mumble-voip:master Apr 16, 2021
@Krzmbrzl Krzmbrzl deleted the plugin-API branch April 16, 2021 18:51
@hbeni
Copy link

hbeni commented Apr 16, 2021

Congratulations!

@Krzmbrzl
Copy link
Member Author

Krzmbrzl commented Apr 17, 2021 •
edited
Loading

Thanks!

Also thank you for your continous input and testing during the development @hbeni! :)

@zsawyer
Copy link

zsawyer commented Apr 17, 2021

Yes, great job!

Thank you all for this massive and valuable contribution.
I am looking forward to utilizing this in the future.

@Supermagnum Supermagnum mentioned this pull request Aug 5, 2021
Open
k3d3 pushed a commit to k3d3/mumble_one_ptt that referenced this pull request Oct 15, 2021
@radioactiveman
Add dependency for poco
47f5c4b 
See also: mumble-voip/mumble#3743
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@hbeni hbeni hbeni requested changes

@NouberNou NouberNou NouberNou left review comments

@Green-Sky Green-Sky Green-Sky left review comments

@Johni0702 Johni0702 Johni0702 left review comments

@eminence eminence eminence left review comments

@davidebeatrici davidebeatrici davidebeatrici approved these changes

Assignees
No one assigned
Labels
client feature-request This issue or PR deals with a new feature
Projects
None yet
Milestone
No milestone