mixer: handle control.ump property

Add a control.ump port property. When true, the port wants UMP and the
mixer will convert to it. When false, the port supports both UMP and
Midi1 and no conversions will happen. When unset, the mixer will always
convert UMP to midi1.

Remove the CONTROL_types property from the filter. This causes problems
because this is the format negotiated with peers, which might not
support the types but can still be linked because the mixer will
convert.

The control.ump port property is supposed to be a temporary fix until we
can negotiate the mixer ports properly with the CONTROL_types.

Remove UMP handling from bluetooth midi, just use the raw Midi1 events
now that the mixer will give those and we are supposed to output our
unconverted format.

Fix midi events in-place in netjack because we can.

Update docs and pw-mididump to note that we are back to midi1 as the
default format.

With this, most of the midi<->UMP conversion should be gone again and we
should be able to avoid conversion problems in ALSA and PipeWire.

Fixes #5183

doc/dox/internals/midi.dox

@@ -62,6 +62,13 @@ As of 1.4, SPA_CONTROL_UMP (Universal Midi Packet) is the prefered format
 for the MIDI 1.0 and 2.0 messages in the \ref spa_pod_sequence. Conversion
 to SPA_CONTROL_Midi is performed for legacy applications.
 
+As of 1.7 the prefered format is Midi1 again because most devices and
+applications are still Midi1 and conversions between Midi1 and UMP are not
+completely transparent in ALSA and PipeWire. UMP in the ALSA sequencer
+and consumers must be enabled explicitly. UMP in producers is supported
+still and will be converted to Midi1 by all consumers that did not explicitly
+enable UMP support.
+
 ## The PipeWire Daemon
 
 Nothing special is implemented for MIDI. Negotiation of formats
@@ -104,13 +111,14 @@ filtering out the \ref SPA_CONTROL_Midi, \ref SPA_CONTROL_OSC and
 \ref SPA_CONTROL_UMP types. On output ports, the JACK event stream is
 converted to control messages in a similar way.
 
-Normally, all MIDI and UMP messages are converted to MIDI1 jack events unless
-the JACK port was created with an explcit "32 bit raw UMP" format or with
-the JackPortIsMIDI2 flag, in which case the raw UMP is passed to the JACK
-application directly. For output ports,
-the JACK events are assumed to be MIDI1 and converted to UMP unless the port
-has the "32 bit raw UMP" format or the JackPortIsMIDI2 flag, in which case
-the UMP messages are simply passed on.
+Normally, all MIDI and UMP input messages are converted to MIDI1 jack
+events unless the JACK port was created with an explcit "32 bit raw UMP"
+format or with the JackPortIsMIDI2 flag, in which case the messages are
+converted to UMP or passed on directly.
+
+For output ports, the JACK events are assumed to be
+MIDI1 unless the port has the "32 bit raw UMP" format or the JackPortIsMIDI2
+flag, in which case the control messages are assumed to be UMP.
 
 There is a 1 to 1 mapping between the JACK events and control
 messages so there is no information loss or need for complicated

spa/plugins/alsa/alsa-seq-bridge.c

@@ -227,7 +227,7 @@ static void emit_port_info(struct seq_state *this, struct seq_port *port, bool f
 	if (full)
 		port->info.change_mask = port->info_all;
 	if (port->info.change_mask) {
-		struct spa_dict_item items[6];
+		struct spa_dict_item items[7];
 		uint32_t n_items = 0;
 		int card_id;
 		snd_seq_port_info_t *info;
@@ -284,6 +284,9 @@ static void emit_port_info(struct seq_state *this, struct seq_port *port, bool f
 			snprintf(card, sizeof(card), "%d", card_id);
 			items[n_items++] = SPA_DICT_ITEM_INIT(SPA_KEY_API_ALSA_CARD, card);
 		}
+		if (this->ump)
+			items[n_items++] = SPA_DICT_ITEM_INIT("control.ump", "true");
+
 		port->info.props = &SPA_DICT_INIT(items, n_items);
 
 		spa_node_emit_port_info(&this->hooks,

spa/plugins/bluez5/midi-node.c

@@ -23,7 +23,6 @@
 #include <spa/utils/ringbuffer.h>
 #include <spa/monitor/device.h>
 #include <spa/control/control.h>
-#include <spa/control/ump-utils.h>
 
 #include <spa/node/node.h>
 #include <spa/node/utils.h>
@@ -450,7 +449,7 @@ static void midi_event_recv(void *user_data, uint16_t timestamp, uint8_t *data,
 	struct impl *this = user_data;
 	struct port *port = &this->ports[PORT_OUT];
 	struct time_sync *sync = &port->sync;
-	uint64_t time, state = 0;
+	uint64_t time;
 	int res;
 
 	spa_assert(size > 0);
@@ -460,19 +459,11 @@ static void midi_event_recv(void *user_data, uint16_t timestamp, uint8_t *data,
 	spa_log_trace(this->log, "%p: event:0x%x size:%d timestamp:%d time:%"PRIu64"",
 			this, (int)data[0], (int)size, (int)timestamp, (uint64_t)time);
 
-	while (size > 0) {
-		uint32_t ump[4];
-		int ump_size = spa_ump_from_midi(&data, &size,
-					ump, sizeof(ump), 0, &state);
-		if (ump_size <= 0)
-			break;
-
-		res = midi_event_ringbuffer_push(&this->event_rbuf, time, (uint8_t*)ump, ump_size);
-		if (res < 0) {
-			midi_event_ringbuffer_init(&this->event_rbuf);
-			spa_log_warn(this->log, "%p: MIDI receive buffer overflow: %s",
-					this, spa_strerror(res));
-		}
+	res = midi_event_ringbuffer_push(&this->event_rbuf, time, data, size);
+	if (res < 0) {
+		midi_event_ringbuffer_init(&this->event_rbuf);
+		spa_log_warn(this->log, "%p: MIDI receive buffer overflow: %s",
+				this, spa_strerror(res));
 	}
 }
 
@@ -713,7 +704,7 @@ static int process_output(struct impl *this)
 			offset = time * this->rate / SPA_NSEC_PER_SEC;
 			offset = SPA_CLAMP(offset, 0u, this->duration - 1);
 
-			spa_pod_builder_control(&port->builder, offset, SPA_CONTROL_UMP);
+			spa_pod_builder_control(&port->builder, offset, SPA_CONTROL_Midi);
 			buf = spa_pod_builder_reserve_bytes(&port->builder, size);
 			if (buf) {
 				midi_event_ringbuffer_pop(&this->event_rbuf, buf, size);
@@ -786,37 +777,28 @@ static int write_data(struct impl *this, struct spa_data *d)
 	time = 0;
 
 	while (spa_pod_parser_get_control_body(&parser, &c, &c_body) >= 0) {
-		int size;
-		uint8_t event[32];
-		const uint32_t *ump = c_body;
-		size_t ump_size = c.value.size;
-		uint64_t state = 0;
+		const uint8_t *event = c_body;
+		uint32_t size = c.value.size;
 
-		if (c.type != SPA_CONTROL_UMP)
+		if (c.type != SPA_CONTROL_Midi)
 			continue;
 
 		time = SPA_MAX(time, this->current_time + c.offset * SPA_NSEC_PER_SEC / this->rate);
 
-		while (ump_size > 0) {
-			size = spa_ump_to_midi(&ump, &ump_size, event, sizeof(event), &state);
-			if (size <= 0)
-				break;
+		spa_log_trace(this->log, "%p: output event:0x%x time:%"PRIu64, this,
+				(size > 0) ? event[0] : 0, time);
 
-			spa_log_trace(this->log, "%p: output event:0x%x time:%"PRIu64, this,
-					(size > 0) ? event[0] : 0, time);
-
-			do {
-				res = spa_bt_midi_writer_write(&this->writer,
-						time, event, size);
-				if (res < 0) {
-					return res;
-				} else if (res) {
-					int res2;
-					if ((res2 = flush_packet(this)) < 0)
-						return res2;
-				}
-			} while (res);
-		}
+		do {
+			res = spa_bt_midi_writer_write(&this->writer,
+					time, event, size);
+			if (res < 0) {
+				return res;
+			} else if (res) {
+				int res2;
+				if ((res2 = flush_packet(this)) < 0)
+					return res2;
+			}
+		} while (res);
 	}
 
 	if ((res = flush_packet(this)) < 0)

spa/plugins/control/mixer.c

@@ -72,6 +72,7 @@ struct impl {
 	struct spa_node node;
 
 	uint32_t quantum_limit;
+	uint32_t control_types;
 
 	struct spa_log *log;
 
@@ -473,9 +474,9 @@ static int port_set_format(void *object,
 		if (!port->have_format) {
 			this->n_formats++;
 			port->have_format = true;
-			port->types = types;
-			spa_log_debug(this->log, "%p: set format on port %d:%d",
-					this, direction, port_id);
+			port->types = types == 0 ? this->control_types : types;
+			spa_log_debug(this->log, "%p: set format on port %d:%d types:%08x %08x",
+					this, direction, port_id, port->types, this->control_types);
 		}
 	}
 	port->info.change_mask |= SPA_PORT_CHANGE_MASK_PARAMS;
@@ -955,6 +956,8 @@ impl_init(const struct spa_handle_factory *factory,
 	}
 
 	this->quantum_limit = 8192;
+	/* by default we convert to midi1 */
+	this->control_types = 1u<<SPA_CONTROL_Midi;
 
 	for (i = 0; info && i < info->n_items; i++) {
 		const char *k = info->items[i].key;
@@ -962,6 +965,14 @@ impl_init(const struct spa_handle_factory *factory,
 		if (spa_streq(k, "clock.quantum-limit")) {
 			spa_atou32(s, &this->quantum_limit, 0);
 		}
+		else if (spa_streq(k, "control.ump")) {
+			if (spa_atob(s))
+				/* we convert to UMP when forced */
+				this->control_types = 1u<<SPA_CONTROL_UMP;
+			else
+				/* when unforced we let both midi1 and UMP through */
+				this->control_types = 0;
+		}
 	}
 
 	spa_hook_list_init(&this->hooks);

src/modules/module-client-node/client-node.c

@@ -641,7 +641,6 @@ node_port_enum_params(struct impl *impl, int seq,
 		if (count == num)
 			break;
 	}
-
 	return 0;
 }
 

src/modules/module-netjack2/peer.c

@@ -235,16 +235,13 @@ struct data_info {
 	bool filled;
 };
 
-static inline bool fix_midi_event(const uint8_t *data, size_t size, uint8_t tmp[3])
+static inline void fix_midi_event(uint8_t *data, size_t size)
 {
 	/* fixup NoteOn with vel 0 */
 	if (size > 2 && (data[0] & 0xF0) == 0x90 && data[2] == 0x00) {
-		tmp[0] = 0x80 + (data[0] & 0x0F);
-		tmp[1] = data[1];
-		tmp[2] = 0x40;
-		return true;
+		data[0] = 0x80 + (data[0] & 0x0F);
+		data[2] = 0x40;
 	}
-	return false;
 }
 
 static inline void *n2j_midi_buffer_reserve(struct nj2_midi_buffer *buf,
@@ -276,39 +273,18 @@ static inline void *n2j_midi_buffer_reserve(struct nj2_midi_buffer *buf,
 }
 
 static inline void n2j_midi_buffer_write(struct nj2_midi_buffer *buf,
-		uint32_t offset, const void *data, uint32_t size)
+		uint32_t offset, const void *data, uint32_t size, bool fix)
 {
-	void *ptr = n2j_midi_buffer_reserve(buf, offset, size);
-	if (ptr != NULL)
+	uint8_t *ptr = n2j_midi_buffer_reserve(buf, offset, size);
+	if (ptr != NULL) {
 		memcpy(ptr, data, size);
+		if (fix)
+			fix_midi_event(ptr, size);
+	}
 	else
 		buf->lost_events++;
 }
 
-static inline void n2j_midi_buffer_append(struct nj2_midi_buffer *buf,
-		void *data, uint32_t size)
-{
-	struct nj2_midi_event *ev;
-	uint32_t old_size;
-	uint8_t *old_ptr, *new_ptr;
-
-	ev = &buf->event[--buf->event_count];
-	old_size = ev->size;
-	if (old_size <= MIDI_INLINE_MAX) {
-		old_ptr = ev->buffer;
-	} else {
-		buf->write_pos -= old_size;
-		old_ptr = SPA_PTROFF(buf, ev->offset, void);
-	}
-	new_ptr = n2j_midi_buffer_reserve(buf, ev->time, old_size + size);
-	if (new_ptr == NULL) {
-		buf->lost_events++;
-	} else {
-		memmove(new_ptr, old_ptr, old_size);
-		memcpy(new_ptr+old_size, data, size);
-	}
-}
-
 static void midi_to_netjack2(struct netjack2_peer *peer,
 		struct nj2_midi_buffer *buf, float *src, uint32_t n_samples)
 {
@@ -336,7 +312,6 @@ static void midi_to_netjack2(struct netjack2_peer *peer,
 	while (spa_pod_parser_get_control_body(&parser, &c, &c_body) >= 0) {
 		uint32_t size = c.value.size;
 		const uint8_t *data = c_body;
-		uint8_t tmp[3];
 
 		if (c.type != SPA_CONTROL_Midi)
 			continue;
@@ -345,11 +320,7 @@ static void midi_to_netjack2(struct netjack2_peer *peer,
 			buf->lost_events++;
 			continue;
 		}
-		if (peer->fix_midi && fix_midi_event(data, size, tmp)) {
-			data = tmp;
-			size = 3;
-		}
-		n2j_midi_buffer_write(buf, c.offset, data, size);
+		n2j_midi_buffer_write(buf, c.offset, data, size, peer->fix_midi);
 	}
 	if (buf->write_pos > 0)
 		memmove(SPA_PTROFF(buf, sizeof(*buf) + buf->event_count * sizeof(struct nj2_midi_event), void),

src/pipewire/filter.c

@@ -1793,11 +1793,13 @@ static void add_control_dsp_port_params(struct filter *impl, struct port *port,
 		SPA_FORMAT_mediaType,      SPA_POD_Id(SPA_MEDIA_TYPE_application),
 		SPA_FORMAT_mediaSubtype,   SPA_POD_Id(SPA_MEDIA_SUBTYPE_control),
 		0);
+#if 0
 	if (types != 0) {
 		spa_pod_builder_add(&b,
 			SPA_FORMAT_CONTROL_types, SPA_POD_CHOICE_FLAGS_Int(types),
 			0);
 	}
+#endif
 	add_param(impl, port, SPA_PARAM_EnumFormat, PARAM_FLAG_LOCKED,
 			spa_pod_builder_pop(&b, &f[0]));
 }
@@ -1857,10 +1859,13 @@ void *pw_filter_add_port(struct pw_filter *filter,
 			add_video_dsp_port_params(impl, p);
 		else if (spa_streq(str, "8 bit raw midi"))
 			add_control_dsp_port_params(impl, p, 1u << SPA_CONTROL_Midi);
-		else if (spa_streq(str, "8 bit raw control"))
+		else if (spa_streq(str, "8 bit raw control")) {
 			add_control_dsp_port_params(impl, p, 0);
+			pw_properties_set(props, "control.ump", "false");
+		}
 		else if (spa_streq(str, "32 bit raw UMP")) {
 			add_control_dsp_port_params(impl, p, 1u << SPA_CONTROL_UMP);
+			pw_properties_set(props, "control.ump", "true");
 			pw_properties_set(props, PW_KEY_FORMAT_DSP, "8 bit raw midi");
 		}
 	}

src/pipewire/impl-port.c

@@ -1094,11 +1094,11 @@ int pw_impl_port_set_mix(struct pw_impl_port *port, struct spa_node *node, uint3
 
 static int setup_mixer(struct pw_impl_port *port, const struct spa_pod *param)
 {
-	uint32_t media_type, media_subtype;
+	uint32_t media_type, media_subtype, n_items;
 	int res;
-	const char *fallback_lib, *factory_name;
+	const char *fallback_lib, *factory_name, *str;
 	struct spa_handle *handle;
-	struct spa_dict_item items[3];
+	struct spa_dict_item items[4];
 	char quantum_limit[16];
 	void *iface;
 	struct pw_context *context = port->node->context;
@@ -1150,14 +1150,17 @@ static int setup_mixer(struct pw_impl_port *port, const struct spa_pod *param)
 		return -ENOTSUP;
 	}
 
-	items[0] = SPA_DICT_ITEM_INIT(SPA_KEY_LIBRARY_NAME, fallback_lib);
+	n_items = 0;
+	items[n_items++] = SPA_DICT_ITEM_INIT(SPA_KEY_LIBRARY_NAME, fallback_lib);
 	spa_scnprintf(quantum_limit, sizeof(quantum_limit), "%u",
 			context->settings.clock_quantum_limit);
-	items[1] = SPA_DICT_ITEM_INIT("clock.quantum-limit", quantum_limit);
-	items[2] = SPA_DICT_ITEM_INIT(PW_KEY_NODE_LOOP_NAME, port->node->data_loop->name);
+	items[n_items++] = SPA_DICT_ITEM_INIT("clock.quantum-limit", quantum_limit);
+	items[n_items++] = SPA_DICT_ITEM_INIT(PW_KEY_NODE_LOOP_NAME, port->node->data_loop->name);
+	if ((str = pw_properties_get(port->properties, "control.ump")) != NULL)
+		items[n_items++] = SPA_DICT_ITEM_INIT("control.ump", str);
 
 	handle = pw_context_load_spa_handle(context, factory_name,
-			&SPA_DICT_INIT_ARRAY(items));
+			&SPA_DICT_INIT(items, n_items));
 	if (handle == NULL)
 		return -errno;
 

src/tools/pw-mididump.c

@@ -33,7 +33,7 @@ struct data {
 	struct pw_filter *filter;
 	struct port *in_port;
 	int64_t clock_time;
-	bool opt_midi1;
+	bool force_ump;
 };
 
 
@@ -174,7 +174,8 @@ static int dump_filter(struct data *data)
 			PW_FILTER_PORT_FLAG_MAP_BUFFERS,
 			sizeof(struct port),
 			pw_properties_new(
-				PW_KEY_FORMAT_DSP, data->opt_midi1 ? "8 bit raw midi" : "32 bit raw UMP",
+				PW_KEY_FORMAT_DSP,
+					data->force_ump ? "32 bit raw UMP" : "8 bit raw midi",
 				PW_KEY_PORT_NAME, "input",
 				NULL),
 			NULL, 0);
@@ -198,7 +199,7 @@ static void show_help(const char *name, bool error)
 		"  -h, --help                            Show this help\n"
 		"      --version                         Show version\n"
 		"  -r, --remote                          Remote daemon name\n"
-	        "  -M, --force-midi                      Force midi format, one of \"midi\" or \"ump\",(default ump)\n",
+	        "  -M, --force-midi                      Force midi format, one of \"midi\" or \"ump\",(default midi)\n",
 		name);
 }
 
@@ -238,9 +239,9 @@ int main(int argc, char *argv[])
 
 		case 'M':
 			if (spa_streq(optarg, "midi"))
-				data.opt_midi1 = true;
+				data.force_ump = false;
 			else if (spa_streq(optarg, "ump"))
-				data.opt_midi1 = false;
+				data.force_ump = true;
 			else {
 				fprintf(stderr, "error: bad force-midi %s\n", optarg);
 				show_help(argv[0], true);