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

aplaymidi uses the wrong tempo #241

Closed
PQCraft opened this issue Oct 27, 2023 · 2 comments
Closed

aplaymidi uses the wrong tempo #241

PQCraft opened this issue Oct 27, 2023 · 2 comments

Comments

@PQCraft
Copy link

PQCraft commented Oct 27, 2023

It seems to be locked to a tempo of 120 for some reason.

Version: 1.2.10

@jiyunomegami
Copy link

jiyunomegami commented Nov 3, 2023

This patch seems to fix it.

diff --git a/seq/aplaymidi/aplaymidi.c b/seq/aplaymidi/aplaymidi.c
index a7293d3..66038e2 100644
--- a/seq/aplaymidi/aplaymidi.c
+++ b/seq/aplaymidi/aplaymidi.c
@@ -819,6 +819,8 @@ static void play_midi(void)
                ev.time.tick = event->tick;
                ev.dest = ports[event->port];
                if (event->type == SND_SEQ_EVENT_TEMPO) {
+                       snd_seq_ev_set_fixed(&ev);
+                       ev.type = event->type;
                        ev.dest.client = SND_SEQ_CLIENT_SYSTEM;
                        ev.dest.port = SND_SEQ_PORT_SYSTEM_TIMER;
                        ev.data.queue.queue = queue;

with commit b399fb8, ev.type stopped being set to event->type

Edit: added the call to snd_seq_ev_set_fixed

freebsd-git pushed a commit to freebsd/freebsd-ports that referenced this issue Nov 29, 2023
Add temporary patch to fix an issue with tempo events

aplaymidi does not process tempo events because the part
that handles tempo events was not rewritten sufficiently.
It may be executed as a different event in songs that
have tempo changes in the middle of the song.

A similar fix was submitted to upstream as
alsa-project/alsa-utils#241

PR:		275349
Reported by:	Tatsuki Makino <tatsuki_makino@hotmail.com>
@jkl1337
Copy link
Contributor

jkl1337 commented Jan 7, 2024

Not surprisingly this bug also causes a segfault on most MIDI files where a SYSEX message or such as GS reset precedes a tempo event since the prior event is variable but the data buffer will be invalid for this event.

jkl1337 added a commit to jkl1337/alsa-utils that referenced this issue Jan 7, 2024
After UMP support was added in b399fb8 ev.type setting was inadvertently
dropped in the code path handling tempo meta event.
This is causing tempo meta events to not be handled at all.
Moreover, snd_seq_ev_set_fixed is also missing so MIDI files with
variable events such as SYSEX before the tempo meta event usually are
causing a segfault.

Fixes alsa-project#241
@tiwai tiwai closed this as completed in 4cb3e3a Jan 8, 2024
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 a pull request may close this issue.

3 participants