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

Initial Implementation of LV2 module #983

Open
wants to merge 3 commits into
base: master
Choose a base branch
from

Conversation

joinlaw
Copy link

@joinlaw joinlaw commented May 11, 2024

Can be tested with melt eg:

$ melt ~/path/to/audio/file.wav -attach "lv2.http<//kxstudio.sf.net/carla/plugins/audiogain_s" 5=0.3 -consumer avformat:processed.mp3 acodec=libmp3lame

(:) in http://kxstudio.sf.net/carla/plugins/audiogain_s replaced with (<) because melt and/or mlt doesn't accept (:) in plugin name and also uri should be prefiexed with "lv2."

currently only this extensions are supported:
http://lv2plug.in/ns/ext/buf-size#boundedBlockLength
http://lv2plug.in/ns/ext/urid#map
http://lv2plug.in/ns/ext/urid#unmap
http://lv2plug.in/ns/ext/options#options

https://lv2plug.in/pages/host-compatibility.html

Copy link
Member

@ddennedy ddennedy left a comment

Choose a reason for hiding this comment

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

The approach of copying the jackrack module and modifying it is not acceptable. That brings in a lot of baggage. jackrack code base was chosen at a time when I needed to support a rack XML file made with its GUI tool for a customer. This was a time when there were only some toy GUI tools for MLT. As a bonus it added support for JACK. I later modified it to support a single named LADSPA plugin for Kdenlive and friends. This ought to be a cleaner implementation without all of that baggage. I guess you can get there by continuing to modify, but I am not in a situation where I want to go through many PR reviews to get there. Alternatively, can the existing jackrack module be extended to support LV2?

src/modules/lv2/factory.c Outdated Show resolved Hide resolved
src/modules/lv2/factory.c Outdated Show resolved Hide resolved
src/modules/lv2/factory.c Outdated Show resolved Hide resolved
Comment on lines 231 to 235
s = calloc(1, strlen("lv2.") + strlen (desc->uri) + 1);

sprintf(s, "lv2.%s", desc->uri);

char *str_ptr = strchr(s, ':');
while (str_ptr != NULL)
{
*str_ptr++ = '<';
str_ptr = strchr(str_ptr, ':');
}
Copy link
Member

Choose a reason for hiding this comment

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

Regarding the plugin name, since it must be altered, please drop the broken scheme and just make it like "lv2//kxstudio.sf.net/carla/plugins/audiogain_s". Then, we can just say it is the schema-less URI. Notice I did not include a period after "lv2" because the "//" is enough delimiters but still following some URI grammar. Or, since we are breaking URI grammar use "lv2.kxstudio.sf.net/carla/plugins/audiogain_s". Your choice between the two, but I would rather not hear from people trying to use "http:" either in support issues or as bug reports.

Copy link
Author

Choose a reason for hiding this comment

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

As we said : can't be used in filter name for some reason.

Not all lv2 URIs contain http:// for example this one don't:
urn:distrho:vitalium

and < is disallowed in URI sytax so it can't conflict I there is chance there is another plugin URI with similar name but different delimiter and mlt/melt accept it so I choose it.

but still this is ugly there should be another solution.

src/modules/lv2/filter_lv2.c Outdated Show resolved Hide resolved
src/modules/lv2/filter_lv2.yml Outdated Show resolved Hide resolved
src/modules/lv2/filter_lv2.yml Outdated Show resolved Hide resolved
src/modules/lv2/producer_lv2.yml Outdated Show resolved Hide resolved
@joinlaw
Copy link
Author

joinlaw commented May 13, 2024

The approach of copying the jackrack module and modifying it is not acceptable. That brings in a lot of baggage. jackrack code base was chosen at a time when I needed to support a rack XML file made with its GUI tool for a customer. This was a time when there were only some toy GUI tools for MLT. As a bonus it added support for JACK. I later modified it to support a single named LADSPA plugin for Kdenlive and friends. This ought to be a cleaner implementation without all of that baggage. I guess you can get there by continuing to modify, but I am not in a situation where I want to go through many PR reviews to get there. Alternatively, can the existing jackrack module be extended to support LV2?

At first I tried to extend the jackrack module and even put both LADSPA and LV2 plugins in the same plugin manager but I fail to get working code for months until I tried copying and I get here.

Actually I have to think about the approach and take time to be more familiar with filter design in mlt. The problem is it was hard for me to use same structure of plugin/process/manager so I have to heavily alter it and only lock_free_fifo.c, lock_free_fifo.h files copied without modification.

@joinlaw
Copy link
Author

joinlaw commented May 17, 2024

with this commit aba53e9 now parameters metadata is more accurate and not causing issues and be readonly

kdenwork

And still I don't get dropdown options (enumerations) to work.

I am investigating ways to integrate this module in jackrack.

@joinlaw joinlaw force-pushed the lv2-module branch 2 times, most recently from 9646013 to 189bbf4 Compare May 24, 2024 13:20
@joinlaw
Copy link
Author

joinlaw commented May 24, 2024

By now this feature is reimplemented as part of jackrack module and the old implementation moved to lv2-module-old-way https://github.com/joinlaw/mlt/tree/lv2-module-old-way branch.

@ddennedy

src/modules/jackrack/filter_lv2.c Outdated Show resolved Hide resolved
src/modules/jackrack/filter_lv2.c Outdated Show resolved Hide resolved
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.

None yet

2 participants