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

IPC interface for control ports #21

Open
wants to merge 4 commits into
base: main
Choose a base branch
from

Conversation

twischer-adit
Copy link

This PR is depending on #2. Therefore #2 has to be merged before this PR should be merged.

This PR introduces a feature to change the Lv2 control ports from other processes. A user can write its own application to control a Lv2 plugin. The API is defined in the jalv.h header file. It already contains all required synchronization and locking mechanism.

Timo Wischer added 2 commits November 10, 2018 11:53
This function can be used to round any address to any chosen boundary
e.g. 8 byte aligned

Signed-off-by: Timo Wischer <[email protected]>
This ID is unique for each new period which will be processed.
Therefore it can be used to check if a function was already called for the
current period. This is benificial for some optimization.

Signed-off-by: Timo Wischer <[email protected]>
@drobilla drobilla force-pushed the ipc_interface_for_control_ports branch from 992277b to d399039 Compare November 10, 2018 10:57
@drobilla
Copy link
Owner

I am pretty skeptical of this, to be honest. I've idly considered adding IPC to Jalv, but in a more generally useful way via a pipe or socket or something along those lines (which would be internally somewhat similar to how the console interface works). SHM is complicated, error-prone, and pretty much only useful for this "use jalv to put an LV2 host inside jackd" use case. I am especially not a fan of things like adding bespoke API for things like scale points on an as-needed basis when LV2 already has an API for doing any kind of plugin introspection (lilv).

Overall this is a huge amount of complexity for what seems like pretty limited functionality. What is the use case here on a higher level?

@twischer-adit
Copy link
Author

@drobilla

which would be internally somewhat similar to how the console interface works

With this solution I would like to avoid additional conversion between strings and float/integer to minimize the overhead.

SHM is complicated, error-prone, and pretty much only useful for this "use jalv to put an LV2 host inside jackd" use case.

In general it is independent if JALV is executed as a separate process or inside of JACK.

I am especially not a fan of things like adding bespoke API for things like scale points on an as-needed basis when LV2 already has an API for doing any kind of plugin introspection (lilv).

I tried to provide one API which can handle all required parts. But may be we can adapt the scale-point part. So the new JALV API will return an lilv instance and the user can access the scale points via lilv. But I think the user should somehow be blocked to execute the Lv2 plugin (call run()) via this instance.

Overall this is a huge amount of complexity for what seems like pretty limited functionality. What is the use case here on a higher level?

We have a lot of Lv2 plugins and control ports which has to be able to be changed in sync. There is a requirement to change up to 100 control ports in the same execution cycle (audio period) without a big processing overhead. Currently we are working with a period time of 2ms.
The change of control ports should not block the real time audio processing thread. Therefore I could not use a mutex or semaphore and had to use some atomic solution. (Therefore I used a similar solution as of JACK. It is also using an atomic version counter which is used to verify if the read memory region is valid or currently changed by another thread/process.)
This is the main reason for the complexity.
This implementation uses a named pipe to write to control ports and uses the shared memory to read from control ports. Sockets were not an option because it would require an additional thread which listens on a port and at the end an atomic synchronization between the listener thread and the real time audio thread is required anyway. Without a shared memory the real time audio thread has always to answer a control port read request. With the current implementation each client can directly read the current value of a control port from the shared memory without interacting with JALV.

what seems like pretty limited functionality

Do you have any feature in your mind which could be interesting for such an API in future?

I have also attached the Doxygen documentation of JALV
jalv_doc.tar.gz
In group___a_p_i.html you can find some diagrams which describe the communication and synchronization mechanism. I hope it helps to understand the implementation.

@twischer-adit
Copy link
Author

@drobilla

I am thinking about an additional API which make it easier to replace the IPC interface. So the IPC interface has to implement the following functions to be able to used by JALV.

  • float* get_control_data_pointer() - Returns a pointer to the data section where the control data is located
  • lock_control_data() - Has to be called before reading or writing to the pointer
  • unlock_control_data()
  • atomic_read_control_data(float values[], size_t count) - can be used to read the value of a control port without using lock and unlock - performance optimization
  • atomic_write_control_data(float value) - can be used to write a value to a control port without using lock and unlock - performance optimization

A simple implementation of this functions could be the current solution without IPC mechanism. Another implementation could be the one of this PR using shared memory. Another implementation could be one using sockets.

If this is also not a good approach for you. May be I could adapt this PR to use sockets but keep the shared memory for high performance read access. But I would like to keep the binary format inside the socket to avid the float to string conversion.

What do you think?

Timo Wischer added 2 commits December 4, 2018 14:52
Lv2 control ports from external processes

Signed-off-by: Timo Wischer <[email protected]>
Lv2 control ports from external processes

Signed-off-by: Timo Wischer <[email protected]>
@twischer-adit twischer-adit force-pushed the ipc_interface_for_control_ports branch from d399039 to 83fa080 Compare December 4, 2018 13:54
@twischer-adit
Copy link
Author

I fixed an issue when providing an empty group name via the IPC API. This includes the following diff:

-------------------------------- src/zix/sem.h --------------------------------
index 1bba36a..74696ed 100644
@@ -211,7 +211,7 @@ zix_sem_clear(ZixSem* const sem)
 static inline ZixStatus
 zix_sem_create_internal(ZixSem* sem, const char* const name, const int oflag, const mode_t mode, unsigned initial, const char* group)
 {
-    gid_t gid = -1;
+	gid_t gid = -1;
 
 	zix_sem_clear(sem);
 
@@ -223,25 +223,10 @@ zix_sem_create_internal(ZixSem* sem, const char* const name, const int oflag, co
 	const size_t space = sizeof(name_with_slash) - strlen(name_with_slash);
 	strncat(name_with_slash, name, space);
 
-	if ((oflag & O_CREAT) && (NULL != group)) {
-		if (isdigit(*group)) {
-			const long gid_long = atol(group);
-			if (gid_long < 0 || gid_long > UINT_MAX) {
-				ERR("Group '%s' was converted to group ID %ld. This is an invalid ID.", group, gid_long);
-				return ZIX_STATUS_ERROR;
-			}
-			gid = gid_long;
-		} else {
-			/* as mentioned in the manpage of getgrnam()
-			 * errno has to be set to 0 when reading it
-			 */
-			errno = 0;
-			const struct group* const g = getgrnam(group);
-			if (!g) {
-				perror("getgrnam() failed");
-				return ZIX_STATUS_ERROR;
-			}
-			gid = g->gr_gid;
+	if (oflag & O_CREAT) {
+		const ZixStatus ret = zix_group_id(group, &gid);
+		if (ret != ZIX_STATUS_SUCCESS && ret != ZIX_STATUS_EMPTY) {
+			return ret;
 		}
 	}
------------------------------- src/zix/common.h -------------------------------
index 4c70397..834172c 100644
@@ -192,25 +192,23 @@ static inline size_t zix_round_up(const size_t value, const size_t boundary)
 }
 
 
+#ifndef _WIN32
 static inline ZixStatus
-zix_group_set(const int fd, const char* const group)
+zix_group_id(const char* const group, gid_t* const gid)
 {
+	*gid = -1;
+
 	if (!group || strlen(group) == 0) {
 		return ZIX_STATUS_EMPTY;
 	}
 
-#ifdef _WIN32
-	INFO("User groups are not supported for this operating system. (fd %d, group %s)", fd, group);
-#else
-	gid_t gid = -1;
-
 	if (isdigit(*group)) {
 		const long gid_long = atol(group);
 		if (gid_long < 0 || gid_long > UINT_MAX) {
 			ERR("Group '%s' was converted to group ID %ld. This is an invalid ID.", group, gid_long);
 			return ZIX_STATUS_ERROR;
 		}
-		gid = gid_long;
+		*gid = gid_long;
 	} else {
 		/* as mentioned in the manpage of getgrnam()
 		 * errno has to be set to 0 when reading it
@@ -221,9 +219,24 @@ zix_group_set(const int fd, const char* const group)
 			perror("getgrnam() failed");
 			return ZIX_STATUS_ERROR;
 		}
-		gid = g->gr_gid;
+		*gid = g->gr_gid;
 	}
 
+	return ZIX_STATUS_SUCCESS;
+}
+#endif
+
+static inline ZixStatus
+zix_group_set(const int fd, const char* const group)
+{
+#ifdef _WIN32
+	INFO("User groups are not supported for this operating system. (fd %d, group %s)", fd, group);
+#else
+	gid_t gid = -1;
+	ZixStatus ret = zix_group_id(group, &gid);
+	if (ret != ZIX_STATUS_SUCCESS)
+		return ret;
+
 	if (fchown(fd, (uid_t)-1, gid) < 0) {
 		perror("fchown() failed");
 		return ZIX_STATUS_ERROR;

@twischer-adit
Copy link
Author

@drobilla
May be I could also use the OSC syntax (http://opensoundcontrol.org/spec-1_0) for the messages sent via the JalvAPI. Would this help to merge this PR?

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.

2 participants