View Issue Details

IDProjectCategoryView StatusLast Update
0007137ardourfeaturespublic2020-04-19 20:18
ReporterOnkelDead Assigned Toovenwerks  
PrioritynormalSeverityfeatureReproducibilityN/A
Status closedResolutionfixed 
Product Version5.4 
Summary0007137: Add OSC support to access plugins and their parameters
DescriptionAs already described in https://community.ardour.org/node/14083, I would like you to add some additional features to the OSC implementation of Ardour.

Intention is to get knowledge and control of the assigned plugins a strip owns.
Find a patch file attached, which contains my implementation.

It contains three new messages.

A) /strip/plugin/list
Resquest a list of the assinged plugins of a given strip.
The response contains:
0. An integer identifying the strip (sid) the list belongs to (once before the list of plugins)
1. An integer with the plugin index (needed bcause only PluginInserts gets listed)
2. A string with the plugin name
This list repeats from point 1. for each plugin

B) /strip/plugin/descriptor ssid piid
Requests a detailed structure describing a specific plugin
the response contains:
1. An integer identifying the strip (sid) the list belongs to (once before the list of parameters)
2. An integer with the plugin index (also once)
3. A string with the plugin name (again once)
4. An integer with the parameter index
5. A string with the parameter label
6. An integer where each bit represents on of the boolean members of ParameterDescriptor
7. An integer representing the parameters data type
8. A float for the lower bound
9. A float for the upper bound
10. A string containing the formating string (print_fmt)
11. An integer which indicates the count of scale points (zero if no scale points are in place)
12. An integer with the key value of the scale point map
13 A string with the scale point value representation
The list repeats from point 12 for each scale point
14. A double with the current parameter value

C) /strip/plugin/reset ssid piid
Resquest to reset a specific plugin to its default values.
Additional InformationSorry, but the attached patch file assumes the patch from issue 7136 have already been applied.
TagsNo tags attached.

Activities

OnkelDead

2016-11-23 11:24

reporter  

plugins.patch (6,980 bytes)   
diff -Naur 5.4.429.org/ardour/libs/surfaces/osc/osc.cc 5.4.429/ardour/libs/surfaces/osc/osc.cc
--- 5.4.429.org/ardour/libs/surfaces/osc/osc.cc	2016-11-23 11:33:11.505179116 +0100
+++ 5.4.429/ardour/libs/surfaces/osc/osc.cc	2016-11-23 11:35:36.980409229 +0100
@@ -603,6 +603,9 @@
                 REGISTER_CALLBACK(serv, "/strip/name", "is", route_rename);
                 REGISTER_CALLBACK(serv, "/strip/sends", "i", route_get_sends);
                 REGISTER_CALLBACK(serv, "/strip/receives", "i", route_get_receives);                
+                REGISTER_CALLBACK(serv, "/strip/plugin/list", "i", route_plugin_list);
+                REGISTER_CALLBACK(serv, "/strip/plugin/descriptor", "ii", route_plugin_descriptor);
+                REGISTER_CALLBACK(serv, "/strip/plugin/reset", "ii", route_plugin_reset);
                 
         
 		/* still not-really-standardized query interface */
@@ -2776,6 +2779,170 @@
 }
 
 int
+OSC::route_plugin_list(int ssid, lo_message msg) {
+	if (!session) {
+		return -1;
+	}
+	
+	boost::shared_ptr<Route> r = boost::dynamic_pointer_cast<Route> (get_strip(ssid, get_address(msg)));
+
+	if (!r) {
+		PBD::error << "OSC: Invalid Remote Control ID '" << ssid << "'" << endmsg;
+		return -1;
+	}
+	int piid = 0;
+	
+	lo_message reply = lo_message_new();
+	lo_message_add_int32(reply, ssid);
+
+
+	for(;;) {
+		boost::shared_ptr<Processor> redi = r->nth_plugin(piid);
+		if( !redi ) {
+			break;
+		}
+
+		boost::shared_ptr<PluginInsert> pi;
+
+		if (!(pi = boost::dynamic_pointer_cast<PluginInsert>(redi))) {
+			PBD::error << "OSC: given processor # " << piid << " on RID '" << ssid << "' is not a Plugin." << endmsg;
+			continue;
+		}
+		lo_message_add_int32(reply, piid);   
+		
+		boost::shared_ptr<ARDOUR::Plugin> pip = pi->plugin();		
+		lo_message_add_string(reply, pip->name());
+		
+		piid++;
+	}
+
+	lo_send_message(lo_message_get_source(msg), "/strip/plugin/list", reply);
+	lo_message_free(reply);
+	return 0;
+}
+
+int
+OSC::route_plugin_descriptor(int ssid, int piid, lo_message msg) {
+	if (!session) {
+		return -1;
+	}
+	
+	boost::shared_ptr<Route> r = boost::dynamic_pointer_cast<Route> (get_strip(ssid, get_address(msg)));
+
+	if (!r) {
+		PBD::error << "OSC: Invalid Remote Control ID '" << ssid << "'" << endmsg;
+		return -1;
+	}
+
+	boost::shared_ptr<Processor> redi = r->nth_plugin(piid);
+
+	if (!redi) {
+		PBD::error << "OSC: cannot find plugin # " << piid << " for RID '" << ssid << "'" << endmsg;
+		return -1;
+	}
+
+	boost::shared_ptr<PluginInsert> pi;
+
+	if (!(pi = boost::dynamic_pointer_cast<PluginInsert>(redi))) {
+		PBD::error << "OSC: given processor # " << piid << " on RID '" << ssid << "' is not a Plugin." << endmsg;
+		return -1;
+	}
+
+	boost::shared_ptr<ARDOUR::Plugin> pip = pi->plugin();
+	bool ok = false;
+
+	lo_message reply = lo_message_new();
+	lo_message_add_int32(reply, ssid);
+	lo_message_add_int32(reply, piid);
+	lo_message_add_string(reply, pip->name());
+	for( uint32_t ppi = 0; ppi < pip->parameter_count(); ppi++) {
+		
+		uint32_t controlid = pip->nth_parameter(ppi, ok);
+		if (!ok) {
+			continue;
+		}  
+		if( pip->parameter_is_input(controlid) || pip->parameter_is_control(controlid) ) {
+			boost::shared_ptr<AutomationControl> c = pi->automation_control(Evoral::Parameter(PluginAutomation, 0, controlid));
+
+				lo_message_add_int32(reply, ppi);
+				ParameterDescriptor pd;
+				pi->plugin()->get_parameter_descriptor(controlid, pd);
+				lo_message_add_string(reply, pd.label.c_str());
+
+				// I've combined those binary descriptor parts in a bit-field to reduce lilo message elements
+				int flags = 0;
+				flags |= pd.enumeration ? 1 : 0;
+				flags |= pd.integer_step ? 2 : 0;
+				flags |= pd.logarithmic ? 4 : 0;
+				flags |= pd.max_unbound ? 8 : 0;
+				flags |= pd.min_unbound ? 16 : 0;
+				flags |= pd.sr_dependent ? 32 : 0;
+				flags |= pd.toggled ? 64 : 0;
+				flags |= c != NULL ? 128 : 0; // bit 7 indicates in input control
+				lo_message_add_int32(reply, flags);
+
+				lo_message_add_int32(reply, pd.datatype);
+				lo_message_add_float(reply, pd.lower);
+				lo_message_add_float(reply, pd.upper);
+				lo_message_add_string(reply, pd.print_fmt.c_str());
+				if( pd.scale_points ) {
+					lo_message_add_int32(reply, pd.scale_points->size());
+					for( ARDOUR::ScalePoints::const_iterator i = pd.scale_points->begin(); i != pd.scale_points->end(); ++i) {
+						lo_message_add_int32(reply, i->second);
+						lo_message_add_string(reply, ((std::string)i->first).c_str());
+					}
+				}
+				else {
+					lo_message_add_int32(reply, 0);
+				} 
+				if( c ) {
+					lo_message_add_double(reply, c->get_value());
+				}
+				else {
+					lo_message_add_double(reply, 0);
+			}
+		}
+	}
+	
+	lo_send_message(lo_message_get_source(msg), "/strip/plugin/descriptor", reply);
+	lo_message_free(reply);
+
+	return 0;
+}
+
+int
+OSC::route_plugin_reset(int ssid, int piid, lo_message msg) {
+	if (!session) {
+		return -1;
+	}
+	
+	boost::shared_ptr<Route> r = boost::dynamic_pointer_cast<Route> (get_strip(ssid, get_address(msg)));
+
+	if (!r) {
+		PBD::error << "OSC: Invalid Remote Control ID '" << ssid << "'" << endmsg;
+		return -1;
+	}
+
+	boost::shared_ptr<Processor> redi = r->nth_plugin(piid);
+
+	if (!redi) {
+		PBD::error << "OSC: cannot find plugin # " << piid << " for RID '" << ssid << "'" << endmsg;
+		return -1;
+	}
+
+	boost::shared_ptr<PluginInsert> pi;
+
+	if (!(pi = boost::dynamic_pointer_cast<PluginInsert>(redi))) {
+		PBD::error << "OSC: given processor # " << piid << " on RID '" << ssid << "' is not a Plugin." << endmsg;
+		return -1;
+	}
+
+	pi->reset_parameters_to_default();
+	
+	return 0;
+}
+
+int
 OSC::route_plugin_parameter (int ssid, int piid, int par, float val, lo_message msg)
 {
 	if (!session)
diff -Naur 5.4.429.org/ardour/libs/surfaces/osc/osc.h 5.4.429/ardour/libs/surfaces/osc/osc.h
--- 5.4.429.org/ardour/libs/surfaces/osc/osc.h	2016-11-23 11:33:11.506179125 +0100
+++ 5.4.429/ardour/libs/surfaces/osc/osc.h	2016-11-23 11:38:24.930776010 +0100
@@ -479,6 +479,11 @@
 	PATH_CALLBACK2_MSG(route_plugin_activate,i,i);
 	PATH_CALLBACK2_MSG(route_plugin_deactivate,i,i);
 
+	PATH_CALLBACK1_MSG(route_plugin_list,i);
+	PATH_CALLBACK2_MSG(route_plugin_descriptor,i,i);
+	PATH_CALLBACK2_MSG(route_plugin_reset,i,i);
+        
+        
 	int route_rename (int rid, char *s, lo_message msg);
 	int route_mute (int rid, int yn, lo_message msg);
 	int route_solo (int rid, int yn, lo_message msg);
@@ -507,6 +512,11 @@
 	int route_plugin_activate (int rid, int piid, lo_message msg);
 	int route_plugin_deactivate (int rid, int piid, lo_message msg);
 
+        int route_plugin_list(int ssid, lo_message msg);
+        int route_plugin_descriptor(int ssid, int piid, lo_message msg);
+        int route_plugin_reset(int ssid, int piid, lo_message msg);
+        
+        
 	//banking functions
 	int set_bank (uint32_t bank_start, lo_message msg);
 	int _set_bank (uint32_t bank_start, lo_address addr);
plugins.patch (6,980 bytes)   

x42

2016-11-23 19:39

administrator   ~0019033

This is very sensible API and the code is fine in general. Thanks for getting the ball rolling on this.
The only part to double check and refine is the PluginDescriptor enumeration-index and flags.

Some comments from a quick review:

Since Plugin::nth_parameter() only returns control ports, the check

 if( pip->parameter_is_input(controlid) || pip->parameter_is_control(controlid) )

is always true.

I'd also not expose max_unbound, min_unbound. Only some ancient LADSPA plugins use that and it pushes unnecessary complexity to the client which is not needed.

We should also remove some redundancies. e.g. plugin/list() already sends the plugin-name. Since one has to call plugin/list first to get the total count, there's no need to include it in plugin/descriptor() again.

ScalePoints are not limited to integer steps: lo_message_add_int32 is not correct (it ought to be a float).

lo_message_add_int32 (reply, pd.datatype); should use enum_2_string (pd.datatype) or a switch() case that guarantees a stable API.


Do you have an opinion if we should also offer a 2nd API that uses interface_to_internal(), internal_to_interface() and exposes all parameters in a normalized 0..1 range (and let Ardour do the logscale, sample-rate, integer, etc mapping)? compare to /strip/fader vs /strip/gain

OnkelDead

2016-11-24 12:58

reporter   ~0019036

Thanks for the quick reply!
Your doubts are fairly reasonable.

Redundancy...
Yes, I've removed the plugin name from plugin/descriptor(), and changed the OSC client accordingly. Works fine.

ScalePoints...
You're absolutely right. I'd missed to investigate it in deep, seems I was focused on the self-defined plugins currently in use. Also changed, works fine.

Datatype...
I've added the switch/case you've mentioned. I couldn't locate an EnumWriter for type ARDOUT::Variant::Type and for now I want to keep it simple.

max/min_unbound...
Right, this seems to be senseless. But regarding complexity, there is no additional payload and clients may ignore the two bits.

The-always-true-if...
Here I can't fully agree.
Yes, Plugin::nth_parameter() only returns control ports, but not all ports are input controls (the || must to be &&, my fault).
In my opinion exposing output ports makes no sense without the corresponding observers which pushes values to the client. This is unnecessary complexity for the client.

2nd API...
Yes, of cause. For more simple clients this could be enough.
My OSC client uses PluginDescriptor.print_fmt string (if exists) to display meaningful value representation; therefor the real parameter value is needed.


BTW: I got another issue regarding input parameters marked as hidden (lv2:portProperty pprop:notOnGUI). Until now I couldn't find a way to handle this.

x42

2016-11-24 13:59

administrator   ~0019037

ok. with || -> && the code makes sense.
  
and "lo_message_add_int32(reply, ppi);" does send an identifier that can be used with /strip/plugin/parameter as-is. All fine.


Still, I think we should plan ahead to allow surfaces to subscribe to output port changes (and/or poll them) some day. To do that the surface needs to know the parameter range and adding a separate API for each port-direction seems odd.

Is there a good reason to not simply add a flag to identify input/output?


In any case, the plugin parameter API itself will need an overhaul: expose automation state (write/read/touch/manual), send feedback if a parameter-change succeeded (it may fail if with automation read, or be modified by other control-uis concurrently), add touch support (grab control) etc etc. But all that's another story for later. It's just something to keep in mind when planning the API. Output-port notifications are similar.

x42

2016-11-24 14:06

administrator   ~0019038

Last edited: 2016-11-24 14:06

PS. Plugin::describe_parameter() returns "hidden" for LV2:notOnGUI

The same convention is used for LADSPA (and some VST plugins) as well. Ports with the literal (not translated) name "hidden" are not to be displayed.

ovenwerks

2016-11-25 16:13

reporter   ~0019043

error: patch failed: libs/surfaces/osc/osc.cc:603
error: libs/surfaces/osc/osc.cc: patch does not apply

I will try putting it in manually.

ovenwerks

2016-11-25 16:53

reporter   ~0019044

Added manually no problem. Patch works.

One thing I would like to change that will break (maybe) your controller. All control surface control numbering are 1 based. That is they are not strip number, but the first strip the second strip etc. Remembering that many users are not programmers. So I will be changing the plugin/PI control numbering to 1 based instead of 0 based. So it would be the first plugin rather than plugin number zero. It would be the first plugin parameter rather than parameter number zero.

ovenwerks

2016-11-25 18:14

reporter   ~0019045

added feature with commit 5bf8a5537b7771a7f523ab242f186e513c3a4c76. I have left PI and parameter numbering as is for now because the existing work for plugins has not been fixed yet.

OnkelDead

2016-11-26 08:50

reporter   ~0019046

Fine.

@ovenwerks: Don't hassle with my controllers behavior, it's still in an alpha state and will get rapidly changes anyway.
Meanwhile it self and the patch I've attached got some changes due to the discussion with x42 (see note 19036).
One-based numbering would make sense only if we change it in /strip/plugin/parameter also (see note 19067). This will have an impact to possible existing implementation of other users.

@x42: An input/output flag was already prepared (0x80). All fine.
"hidden" plugin parameters got solved, very good hint. The hidden state got its own flag (0x100).
So all parameters are notified and the client is able determine which should be offered and which not. Later it will be able to somehow subscribe/poll output port changes.

BTW: Playing around with feedback "Extra Select" (0x2000) and the corresponding /select/send_fader messages stuck for me. I was not able to un-select a strip with "/strip/select sid 0". The addressed strip is still selected and I get continuously the mentioned feedback messages. Maybe I didn't understand the intention of select and expand completely.

ovenwerks

2016-11-26 15:28

reporter   ~0019054

I opened another bug for changing counting from 0 based to 1 based. The values returned by your patches will work correctly when controlling the same parameters when it gets committed. (7142) So far as I know, no one has started playing with plug-in parameters since ver5.* so fixing this now is probably best. Your SW is the only one. Most controllers based on touchOSC kinds of things where the controllers are fixed, do not deal well with plug-ins.

/strip/select i 0 will not turn the select off on the currently selected track. The GUI has a selected track all the time which shows up as the editor mixer strip. Strip 0 does not exist and so selecting strip 0 is a no op instead (rather than crash). In /strip/expand 0 is special cased to turn expand off, but when expand is off, select is on. Select always deals with all strips (may be hidden). Expand always shows a strip that is contained with in the controllers current bank. So if two controllers are connected they can both have a different expanded strip.One of my future features is to allow two controllers to work as one and bank together, but each one would be able to expand a different strip.

system

2020-04-19 20:18

developer   ~0023681

Issue has been closed automatically, by Trigger Close Plugin.
Feel free to re-open with additional information if you think the issue is not resolved.

Issue History

Date Modified Username Field Change
2016-11-23 11:24 OnkelDead New Issue
2016-11-23 11:24 OnkelDead File Added: plugins.patch
2016-11-23 19:39 x42 Note Added: 0019033
2016-11-24 12:58 OnkelDead Note Added: 0019036
2016-11-24 13:59 x42 Note Added: 0019037
2016-11-24 14:06 x42 Note Added: 0019038
2016-11-24 14:06 x42 Note Edited: 0019038
2016-11-25 16:13 ovenwerks Note Added: 0019043
2016-11-25 16:53 ovenwerks Note Added: 0019044
2016-11-25 18:14 ovenwerks Note Added: 0019045
2016-11-25 18:14 ovenwerks Status new => resolved
2016-11-25 18:14 ovenwerks Resolution open => fixed
2016-11-25 18:14 ovenwerks Assigned To => ovenwerks
2016-11-26 08:50 OnkelDead Note Added: 0019046
2016-11-26 15:28 ovenwerks Note Added: 0019054
2020-04-19 20:18 system Note Added: 0023681
2020-04-19 20:18 system Status resolved => closed