AlsaMIDIDevice and CoreMIDIDevice small refactor

Author: charlesthobe

source/mididevices/music_alsa_mididevice.cpp

@@ -57,29 +57,30 @@ class AlsaMIDIDevice : public MIDIDevice
 	int GetTechnology() const override;
 	int SetTempo(int tempo) override;
 	int SetTimeDiv(int timediv) override;
-	int StreamOut(MidiHeader *data) override;
-	int StreamOutSync(MidiHeader *data) override;
+	int StreamOut(MidiHeader* data) override;
+	int StreamOutSync(MidiHeader* data) override;
 	int Resume() override;
 	void Stop() override;
-	bool FakeVolume() override { return true; }; //Not sure if we even can control the volume this way with Alsa, so make it fake.
+	bool FakeVolume() override { return true; }; // Not sure if we even can control the volume this way with Alsa, so make it fake.
 	bool Pause(bool paused) override;
 	void InitPlayback() override;
 	bool Update() override;
-	bool CanHandleSysex() const override { return true; } //Assume we can, let Alsa sort it out.
-	void PrecacheInstruments(const uint16_t *instruments, int count) override;
+	bool CanHandleSysex() const override { return true; } // Assume we can, let Alsa sort it out.
+	void PrecacheInstruments(const uint16_t* instruments, int count) override;
 
 protected:
 	bool PullEvent();
 	void PlayerLoop();
 	void HandleEvent(snd_seq_event_t &event, uint tick);
 
 	AlsaSequencer &sequencer;
-	MidiHeader *Events = nullptr;
+	MidiHeader* Events = nullptr;
 	snd_seq_event_t Event;
+	std::array<uint8_t, 3> ShortMsgBuffer;
 	snd_midi_event_t* Coder = nullptr;
 	uint32_t Position = 0;
 	uint32_t PositionOffset;
-	uint NextEventTickDelta;
+	uint32_t CurrentEventTickDelta;
 
 	const static int IntendedPortId = 0;
 	bool Connected = false;
@@ -91,12 +92,12 @@ class AlsaMIDIDevice : public MIDIDevice
 	int Technology;
 	bool Precache;
 
-	int InitialTempo = 480000;
+	int InitialTempo = 500000;
 	int Tempo;
-	int TimeDiv = 480;
+	int Division = 100; // PPQN
 
 	std::thread PlayerThread;
-	volatile bool Exit = false;
+	volatile bool Exit;
 	std::mutex Mutex;
 	std::condition_variable ExitCond;
 };
@@ -125,7 +126,7 @@ int AlsaMIDIDevice::Open()
 
 	if (PortId < 0)
 	{
-		snd_seq_port_info_t *pinfo;
+		snd_seq_port_info_t* pinfo;
 		snd_seq_port_info_alloca(&pinfo);
 
 		snd_seq_port_info_set_port(pinfo, IntendedPortId);
@@ -200,12 +201,12 @@ int AlsaMIDIDevice::SetTempo(int tempo)
 
 int AlsaMIDIDevice::SetTimeDiv(int timediv)
 {
-	TimeDiv = timediv;
+	Division = timediv;
 	return 0;
 }
 
 // This is meant to mirror WinMIDIDevice::PrecacheInstruments
-void AlsaMIDIDevice::PrecacheInstruments(const uint16_t *instruments, int count)
+void AlsaMIDIDevice::PrecacheInstruments(const uint16_t* instruments, int count)
 {
 	// Setting snd_midiprecache to false disables this precaching, since it
 	// does involve sleeping for more than a miniscule amount of time.
@@ -215,7 +216,6 @@ void AlsaMIDIDevice::PrecacheInstruments(const uint16_t *instruments, int count)
 	}
 	uint8_t bank[16] = {0};
 	uint8_t i, chan;
-	std::array<uint8_t, 3> message;
 
 	for (i = 0, chan = 0; i < count; ++i)
 	{
@@ -227,29 +227,29 @@ void AlsaMIDIDevice::PrecacheInstruments(const uint16_t *instruments, int count)
 		{
 			if (bank[9] != banknum)
 			{
-				message = { MIDI_CTRLCHANGE | 9, 0, banknum };
-				snd_midi_event_encode(Coder, message.data(), 3, &Event);
+				ShortMsgBuffer = { MIDI_CTRLCHANGE | 9, 0, banknum };
+				snd_midi_event_encode(Coder, ShortMsgBuffer.data(), 3, &Event);
 				HandleEvent(Event, 0);
 				bank[9] = banknum;
 			}
-			message = { MIDI_NOTEON | 9, instr, 1 };
-			snd_midi_event_encode(Coder, message.data(), 3, &Event);
+			ShortMsgBuffer = { MIDI_NOTEON | 9, instr, 1 };
+			snd_midi_event_encode(Coder, ShortMsgBuffer.data(), 3, &Event);
 			HandleEvent(Event, 0);
 		}
 		else
 		{ // Melodic
 			if (bank[chan] != banknum)
 			{
-				message = { MIDI_CTRLCHANGE | 9, 0, banknum };
-				snd_midi_event_encode(Coder, message.data(), 3, &Event);
+				ShortMsgBuffer = { MIDI_CTRLCHANGE | 9, 0, banknum };
+				snd_midi_event_encode(Coder, ShortMsgBuffer.data(), 3, &Event);
 				HandleEvent(Event, 0);
 				bank[chan] = banknum;
 			}
-			message = { (uint8_t)(MIDI_PRGMCHANGE | chan), (uint8_t)instruments[i] };
-			snd_midi_event_encode(Coder, message.data(), 2, &Event);
+			ShortMsgBuffer = { (uint8_t)(MIDI_PRGMCHANGE | chan), (uint8_t)instruments[i] };
+			snd_midi_event_encode(Coder, ShortMsgBuffer.data(), 2, &Event);
 			HandleEvent(Event, 0);
-			message = { (uint8_t)(MIDI_NOTEON | chan), 60, 1 };
-			snd_midi_event_encode(Coder, message.data(), 3, &Event);
+			ShortMsgBuffer = { (uint8_t)(MIDI_NOTEON | chan), 60, 1 };
+			snd_midi_event_encode(Coder, ShortMsgBuffer.data(), 3, &Event);
 			HandleEvent(Event, 0);
 			if (++chan == 9)
 			{ // Skip the percussion channel
@@ -265,8 +265,8 @@ void AlsaMIDIDevice::PrecacheInstruments(const uint16_t *instruments, int count)
 			for (chan = 15; chan-- != 0; )
 			{
 				// Turn all notes off
-				message = { (uint8_t)(MIDI_CTRLCHANGE | chan), 123, 0 };
-				snd_midi_event_encode(Coder, message.data(), 3, &Event);
+				ShortMsgBuffer = { (uint8_t)(MIDI_CTRLCHANGE | chan), 123, 0 };
+				snd_midi_event_encode(Coder, ShortMsgBuffer.data(), 3, &Event);
 				HandleEvent(Event, 0);
 			}
 			// And now chan is back at 0, ready to start the cycle over.
@@ -277,13 +277,11 @@ void AlsaMIDIDevice::PrecacheInstruments(const uint16_t *instruments, int count)
 	{
 		if (bank[i] != 0)
 		{
-			message = { MIDI_CTRLCHANGE | 9, 0, 0 };
-			snd_midi_event_encode(Coder, message.data(), 3, &Event);
+			ShortMsgBuffer = { MIDI_CTRLCHANGE | 9, 0, 0 };
+			snd_midi_event_encode(Coder, ShortMsgBuffer.data(), 3, &Event);
 			HandleEvent(Event, 0);
 		}
 	}
-	// Wait until all events are processed
-	snd_seq_sync_output_queue(sequencer.handle);
 }
 
 bool AlsaMIDIDevice::PullEvent()
@@ -299,10 +297,10 @@ bool AlsaMIDIDevice::PullEvent()
 	}
 
 	if (Position >= Events->dwBytesRecorded)
-	{	// All events in the "Events" buffer were used, point to next buffer
+	{	// All events in the buffer were used, point to next buffer
 		Events = Events->lpNext;
 		Position = 0;
-		if (Callback != NULL)
+		if (Callback)
 		{	// This ensures that we always have 2 unused buffers after 1 is used up.
 			// omit this nested "if" block if you want to use up the 2 buffers before requesting new buffers
 			Callback(CallbackData);
@@ -314,51 +312,46 @@ bool AlsaMIDIDevice::PullEvent()
 		return false;
 	}
 
-	uint32_t *event = (uint32_t *)(Events->lpData + Position);
-	NextEventTickDelta = event[0];
+	uint32_t* event = (uint32_t*)(Events->lpData + Position);
+	CurrentEventTickDelta = event[0]; // First 4 bytes of event
 
 	// Get event size to advance Position
-	if (event[2] < 0x80000000)
-	{	// Short message
-		PositionOffset = 12;
+	if (event[2] < 0x80000000) // Short message (event[2] is the combined status/data bytes)
+	{
+		PositionOffset = 12; // 4 bytes delta time, 4 bytes reserved, 4 bytes MIDI message (up to 3 bytes + padding)
 	}
-	else
-	{	// Long message
+	else // Long message or meta-event (event[2] holds type and parameter length)
+	{
 		PositionOffset = 12 + ((MEVENT_EVENTPARM(event[2]) + 3) & ~3);
 	}
 
 	// Pulling event out of buffer
 	switch (MEVENT_EVENTTYPE(event[2]))
 	{
 	case MEVENT_TEMPO:
-	{
-		int tempo = MEVENT_EVENTPARM(event[2]);
-		snd_seq_ev_set_queue_tempo(&Event, QueueId, tempo);
+		snd_seq_ev_set_queue_tempo(&Event, QueueId, MEVENT_EVENTPARM(event[2]));
 		break;
-	}
-	case MEVENT_LONGMSG: // SysEx messages...
+	case MEVENT_LONGMSG: // SysEx message...
 	{
-		uint8_t* data = (uint8_t *)&event[3];
-		int len = MEVENT_EVENTPARM(event[2]);
-		if (len > 2 && data[0] == 0xF0 && data[len - 1] == 0xF7)
+		int long_msg_len = MEVENT_EVENTPARM(event[2]);
+		uint8_t* long_msg_data = (uint8_t*)&event[3];
+		// Ensure valid sysex message
+		if (long_msg_len > 2 && long_msg_data[0] == 0xF0 && long_msg_data[long_msg_len - 1] == 0xF7)
 		{
-			snd_seq_ev_set_sysex(&Event, len, (void*)data);
+			snd_seq_ev_set_sysex(&Event, long_msg_len, (void*)long_msg_data);
 			break;
 		}
 	}
 	case 0: // Short MIDI event
-	{
-		uint8_t status = event[2] & 0xFF;
-		uint8_t param1 = (event[2] >> 8) & 0x7f;
-		uint8_t param2 = (event[2] >> 16) & 0x7f;
-		uint8_t message[] = {status, param1, param2};
-		// This silently ignore extra bytes, so no message length logic is needed.
-		snd_midi_event_encode(Coder, message, 3, &Event);
+		ShortMsgBuffer = {	(uint8_t)(event[2] & 0xff), // Status
+							(uint8_t)((event[2] >> 8) & 0xff), // Data 1
+							(uint8_t)((event[2] >> 16) & 0xff) }; // Data 2
+
+		// This silently ignores extra bytes, so no message length logic is needed.
+		snd_midi_event_encode(Coder, ShortMsgBuffer.data(), 3, &Event);
 		break;
-	}
 	default: // We didn't really recognize the event, treat it as a NOP
 		Event.type = SND_SEQ_EVENT_NONE;
-		snd_seq_ev_set_fixed(&Event);
 	}
 	return true;
 }
@@ -374,10 +367,10 @@ void AlsaMIDIDevice::PlayerLoop()
 	const std::chrono::microseconds buffer_step(40000);
 
 	// TODO: fill in error handling throughout this.
-	snd_seq_queue_tempo_t *tempo;
+	snd_seq_queue_tempo_t* tempo;
 	snd_seq_queue_tempo_alloca(&tempo);
 	snd_seq_queue_tempo_set_tempo(tempo, InitialTempo);
-	snd_seq_queue_tempo_set_ppq(tempo, TimeDiv);
+	snd_seq_queue_tempo_set_ppq(tempo, Division);
 	snd_seq_set_queue_tempo(sequencer.handle, QueueId, tempo);
 
 	snd_seq_start_queue(sequencer.handle, QueueId, NULL);
@@ -386,7 +379,7 @@ void AlsaMIDIDevice::PlayerLoop()
 	Tempo = InitialTempo;
 	int buffered_ticks = 0;
 
-	snd_seq_queue_status_t *status;
+	snd_seq_queue_status_t* status;
 	snd_seq_queue_status_malloc(&status);
 
 	while (!Exit)
@@ -399,34 +392,43 @@ void AlsaMIDIDevice::PlayerLoop()
 		}
 
 		// Figure out if we should sleep (the event is too far in the future for us to care), and for how long
-		int next_event_tick = buffered_ticks + NextEventTickDelta;
+		int current_event_tick = buffered_ticks + CurrentEventTickDelta;
 		snd_seq_get_queue_status(sequencer.handle, QueueId, status);
 		int queue_tick = snd_seq_queue_status_get_tick_time(status);
-		int tick_delta = next_event_tick - queue_tick;
-		auto usecs = std::chrono::microseconds(tick_delta * Tempo / TimeDiv);
-		auto schedule_time = std::max(std::chrono::microseconds(0), usecs - buffer_step);
+		int ticks_until_current_ev = current_event_tick - queue_tick;
+		auto time_until_current_ev = std::chrono::microseconds(ticks_until_current_ev * Tempo / Division);
+		auto schedule_time = time_until_current_ev - buffer_step;
 		if (schedule_time >= buffer_step)
 		{
-			ExitCond.wait_for(lock, schedule_time);
-			continue;
+			if (ExitCond.wait_for(lock, schedule_time) == std::cv_status::no_timeout)
+			{
+				break;
+			}
 		}
-		if (tick_delta < 0)
-		{	// Can be triggered on rare occasions on playback start.
+		if (ticks_until_current_ev < 0)
+		{	// Can be triggered on playback start.
 			// Message shouldn't be shown by default like other midi backends here.
-			ZMusic_Printf(ZMUSIC_MSG_NOTIFY, "Alsa sequencer underrun: %d ticks!\n", tick_delta);
+			ZMusic_Printf(ZMUSIC_MSG_NOTIFY, "Alsa sequencer underrun: %d ticks!\n", ticks_until_current_ev);
 		}
 
 		// We found an event worthy of sending to the sequencer
-		HandleEvent(Event, next_event_tick);
-		buffered_ticks = next_event_tick;
+		if (Event.type != SND_SEQ_EVENT_NONE)
+		{
+			HandleEvent(Event, current_event_tick);
+		}
+		else
+		{	// No reason to run NOP events through event handling logic; just clear "Event" for the next event pulling.
+			snd_seq_ev_clear(&Event);
+		}
+		buffered_ticks = current_event_tick;
 		Position += PositionOffset;
 	}
 
 	snd_seq_ev_clear(&Event); // Event is cleared to be used in reset messages in Stop()
 	snd_seq_queue_status_free(status);
 }
 
-// Requires QueueId to be started first for non-zero tick position
+// Requires QueueId to be started first for non-zero tick positioned events.
 void AlsaMIDIDevice::HandleEvent(snd_seq_event_t &event, uint tick)
 {
 	snd_seq_ev_set_source(&event, PortId);
@@ -468,7 +470,10 @@ void AlsaMIDIDevice::Stop()
 {
 	Exit = true;
 	ExitCond.notify_all();
-	PlayerThread.join();
+	if (PlayerThread.joinable())
+	{
+		PlayerThread.join();
+	}
 	snd_seq_drop_output(sequencer.handle); // This drops events in the sequencer, the sequencer is still usable
 
 	// Reset all channels to prevent hanging notes
@@ -489,37 +494,36 @@ bool AlsaMIDIDevice::Pause(bool paused)
 }
 
 
-int AlsaMIDIDevice::StreamOut(MidiHeader *header)
+int AlsaMIDIDevice::StreamOut(MidiHeader* data)
 {
-	header->lpNext = NULL;
-	if (Events == NULL)
+	data->lpNext = nullptr;
+	if (Events == nullptr)
 	{
-		Events = header;
+		Events = data;
 		Position = 0;
 	}
 	else
 	{
-		MidiHeader **p;
-
-		for (p = &Events; *p != NULL; p = &(*p)->lpNext)
+		MidiHeader** p;
+		for (p = &Events; *p != nullptr; p = &(*p)->lpNext)
 		{ }
-		*p = header;
+		*p = data;
 	}
 	return 0;
 }
 
 
-int AlsaMIDIDevice::StreamOutSync(MidiHeader *header)
+int AlsaMIDIDevice::StreamOutSync(MidiHeader* data)
 {
-	return StreamOut(header);
+	return StreamOut(data);
 }
 
 bool AlsaMIDIDevice::Update()
 {
 	return true;
 }
 
-MIDIDevice *CreateAlsaMIDIDevice(int mididevice)
+MIDIDevice* CreateAlsaMIDIDevice(int mididevice)
 {
 	return new AlsaMIDIDevice(mididevice, miscConfig.snd_midiprecache);
 }