-
-
Notifications
You must be signed in to change notification settings - Fork 231
feat(playout): add audio preprocessing plugin support #2919
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
base: main
Are you sure you want to change the base?
Conversation
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## main #2919 +/- ##
==========================================
+ Coverage 70.36% 70.70% +0.33%
==========================================
Files 149 151 +2
Lines 4033 4090 +57
==========================================
+ Hits 2838 2892 +54
- Misses 1195 1198 +3
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
cdd1226
to
39bac8e
Compare
I know, this is a complex PR and it introduces a 3rd party dependency (optional on the installer, included in the docker build), but can i get a general opinion on how to proceed with this? The external SO will likely never hit a debian or ubuntu package since it's a really specialized software, however, liquidsoap 1.0 and newer had a very broad support for this typ of plugin, so at least we don't have to differentiate between the liquidsoap versions. |
I think the proposed addition is sensible, but I prefer to keep this outside of LibreTime. Every radio station has different needs and if we merge this, we would open ourselves to adding a looot of 3rd party tools. That said, I do acknowledge that LibreTime currenlty does not provide an easy way to customize liquidsoap. (#229) What we can work on is to provide an interface to power users so they can plug any tools into liquidsoap: My idea was to provide a list of filepath configuration field that can refer to liquidsoap scripts that will be hooked at different moment of the liquidsoap run script. e.g. stream:
hooks:
before_output: # naming can be improved I think
- path/to/my/custom/liquidsoap.script.liq
- path/to/another/custom/liquidsoap.script.liq
output: [] This way you can plug master_me without custom code. The only downside is that we do not have an UI to tweak some hooks settings, but maybe we find a way to handle this. I think power users don't necessarily need the UI, but maybe you have a usecase? I also think that its sad to implement only the "easy" mode for master_me, I would definitly want to play around with more settings at some point. |
The downside to this approach, which i considered alternatively is the lack of plugins in the docker module. This can be alleviated by a possibility to spin your own liquidsoap container (possibly also with a different version of liquidsoap itself) and replace the stock one in the docker compose file. I wanted to give people the possibility to use this without digging into docker too deeply.
These tie in to each other. My reasoning was to put the easy mode in the GUI as a first step to see if the PR gets accepted, and afterwards create an 'advanced' preset where a power user could tweak every value not by GUI elements but by editing an advanced.jq2 file. |
What do you think about having a volume binding from the host to the liquidsoap container, that contains both the binary/.so/younameit and the related liquidsoap script? Should be easy enough for users, and could also be configured by default, users would only have to drop their files in the folder? |
Thats a great idea... This should be not too difficult to add. Suggestion: we can keep the easy mode GUI but hide it if the plugin is not loaded. This way people can experiment with the features wihtout too much scripting. |
Hmm, not sure if I want to ship the GUI at all, maybe also as plugin (using a .php file) ? Maybe some really good documentation could do the trick for most users? |
Yes, sure, if somebody really wants this one can do everything in a text file. Documentation is key. I can understand if you don't want to ship a plugin GUI, especially since we want to get rid of the zend stuff anyway,,, |
fc65cea
to
6efa056
Compare
* removed unused code and refactored * correct zoom level on expand * fix concurrency
Main Merge
… an optional file in the liquidsoap LADSPA plugin dir.
@paddatrapper @jooola i changed the implementation to a very basic mapping of a directory where you can put the *.so and the j2 file. We can add more spots in the template like this, but this will allow to plug in the preprocessor in the right spot. If this is ok, i will create documentation to go with the feature. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The idea looks good, but still needs some documentation work and less hard coded values.
# create plugin dir for liquidsoap | ||
RUN mkdir -p /usr/lib/ladspa |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I don't think we should create this directory in the image, if users want to mount the directory at runtime, they are free to do so.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
this path is the hardcoded LADSPA plugin location on debian type distros. I was even pondering to make it a required mount but no other image uses this technique. I included the path since it took me a bit of googling to find the correct location for the plugins when i started developing this feature. But agree, this can be moved to the documentation.
@@ -39,6 +39,8 @@ input_fade_transition = interactive.float("input_fade_transition", {{ preference | |||
|
|||
%include "{{ paths.lib_filepath }}" | |||
|
|||
{% include "/usr/lib/ladspa/processor_plugin.liq.j2" ignore missing %} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I really like this, you can really harness the power of liquidsoap.
That said, could we make this configurable?
For example:
liquidsoap:
hooks:
before_output:
# include single file
- /var/lib/libretime/before_output/plugin.liq.j2
# include a whole directory? File order matters, not sure we should do this, single files might be enough.
- /var/lib/libretime/before_output/
# More hooks may be defined, not sure where those should be injected.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Also could we document a tiny bit, and have a rough idea what the jinja2 context holds?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Haha, I almost had the same comment in #2919 (comment)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
yes, noticed that too ... :-) How would the yaml file fit into the j2 templating? i am not too familiar with the system. is that a feature that is built into j2 or would there need to be additional logic to translate the yaml to the templating mounts? My idea here was to provide a few of these 'hard coded' template names to tie into different spots in the liquidsoap flow.
The hardcoded dir was selected out of practicality. If the user needs to create the mount to plug in the SO he might as well add the config snippet in the same spot.
Sample jinja context for Master Me would be (this is the 'music general' preset from easy mode) :
s = ladspa.master_me(
bypass = false,
target = -16,
brickwall_bypass = false,
brickwall_ceiling = -1.0,
brickwall_release = 75.0,
eq_bypass = false,
eq_highpass_freq = 5.0,
eq_side_bandwidth = 1.0,
eq_side_freq = 600.0,
eq_side_gain = 1.0,
eq_tilt_gain = 0.0,
gate_attack = 0.0,
gate_bypass = true,
gate_hold = 50.0,
gate_release = 430.5,
gate_threshold = -90.0,
kneecomp_attack = 20.0,
kneecomp_bypass = false,
kneecomp_dry_wet = 50,
kneecomp_ff_fb = 50,
kneecomp_knee = 6.0,
kneecomp_link = 60,
kneecomp_makeup = 0.0,
kneecomp_release = 340.0,
kneecomp_strength = 20,
kneecomp_tar_thresh = -4.0,
leveler_brake_threshold = -10.0,
leveler_bypass = false,
leveler_max = 10.0,
leveler_max__ = 10.0,
leveler_speed = 20,
limiter_attack = 3.0,
limiter_bypass = false,
limiter_ff_fb = 50,
limiter_knee = 3.0,
limiter_makeup = 0.0,
limiter_release = 40.0,
limiter_strength = 80,
limiter_tar_thresh = 6.0,
mscomp_bypass = false,
high_attack = 8.0,
high_crossover = 8000.0,
high_knee = 12.0,
high_link = 30,
high_release = 30.0,
high_strength = 30,
high_tar_thresh = -12.0,
low_attack = 15.0,
low_crossover = 60.0,
low_knee = 12.0,
low_link = 70,
low_release = 150.0,
low_strength = 10,
low_tar_thresh = -3.0,
makeup = 1.0,
dc_blocker = false,
input_gain = 0.0,
mono = false,
phase_l = false,
phase_r = false,
stereo_correct = false,
s
)
The idea behind the plugin is really to chain the processing with the s context and pass it on.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
How would the yaml file fit into the j2 templating?
The yaml is a snippet the represent what could be written inside the libretime config file at /etc/libretime/config.yml
to inject the liquidsoap or liquidsoap+jinja files.
Description
Similar to non-free Stereotool this is a free broadcast compressor that is supported as a standard linux plugin to liquidsoap. Initial support adds 5 original presets (Easy mode) and a LUFS slider to the GUI/Legacy streaming presets. Disabled (default) skips the plugin completely.
Warning: this adds a considerable amount of software processing (e.g. CPU and Memory Usage). Also the current PR assumes X86_64 system and will not work on Raspberry PI systems.
Are there documentation changes required as a result of these changes? See
https://github.com/libretime/libretime/wiki/Documentation-Requirements
Yes, Documentation is still missing.
Testing Notes
Change compressor settings and reload libretime-liquidsoap. After playback resumes notice increased CPU and memory usage, and non-linear sound.