Bug (crash in SRC handling), and fix for it

Submitted by franky on Wed, 2021-05-19 19:09

Forums:

MOC version: 
Current SVN trunk as of May 19, 2021

Hi,

I had noticed for a while that playback of certain WAV files would make moc crash for me. This only happened in conjunction with JACK playback, and only for certain (short?) files.
A server dump would look like this:

...
May 19 20:52:45.440253: jack.c:223 moc_jack_open(): jack open
May 19 20:52:45.440258: audio.c:732 audio_open(): Conversion of the sound is needed.
May 19 20:52:45.440264: server.c:1770 server_loop(): Got 'wake up'
May 19 20:52:45.440270: server.c:698 send_events(): Flushing events for client 0
May 19 20:52:45.440280: audio.c:743 audio_open(): Requested sound parameters: float, 1 channels, 44100Hz
May 19 20:52:45.440288: audio.c:747 audio_open(): Driver sound parameters: float, 2 channels, 48000Hz
May 19 20:52:45.440293: player.c:471 decode_loop(): loop...
May 19 20:52:45.440299: player.c:580 decode_loop(): putting into the buffer 7068 bytes
May 19 20:52:45.440305: server.c:1770 server_loop(): Got 'wake up'
double free or corruption (!prev)
Aborted (core dumped)

Now I dug deeper into this with printf, gdb and valgrind, and found that it crashes while doing SRC - sample rate conversion.

In my case, jackd is running at 48kHz and the files I play back are 44100 Hz. Furthermore, it seems to affect just mono files, but this might be coincidence.

Long story short, turned out that moc is using the wrong channel count in two places in audio_conversion.c (it uses the channel count of the destination ("to"), not that of the source ("from")).

This little patch for audio_conversion.c has fixed the problem for me:

--- audio_conversion.c.orig 2021-05-19 20:53:33.600406400 +0200 +++ audio_conversion.c 2021-05-19 20:54:52.685159507 +0200 @@ -477,7 +477,7 @@ else fatal ("Bad ResampleMethod option: %s", method); - conv->src_state = src_new (resample_type, to->channels, &err); + conv->src_state = src_new (resample_type, from->channels, &err); if (!conv->src_state) { error ("Can't resample from %dHz to %dHz: %s", from->rate, to->rate, src_strerror (err)); @@ -691,7 +691,7 @@ if (conv->from.rate != conv->to.rate) { char *new_sound = (char *)resample_sound (conv, (float *)curr_sound, - *conv_len / sizeof(float), conv->to.channels, + *conv_len / sizeof(float), conv->from.channels, conv_len); *conv_len *= sizeof(float); if (curr_sound != buf)

I still have a problem that very short files are not audible when played back standalone, but
that's a minor issue still to be analyzed.

Greetings, and thanks for moc - I like it :-),
Frank

tomaszg

Wed, 2021-05-19 19:59

Makes sense. While the code seems indeed to be wrong, I can't make MOC crash. ALSA usually reports being able to handle absurd channel counts (like 1-10000). With hw devices it is something like 2-6, so it should be possible to trigger this bug as well. I tried but I failed. I couldn't even make it crash with jack. Maybe it happens only with some particular audio format, setting of resampling parameters or src library version.

As for the short files not being audible, I had similar thing happen due to the amplifier I used. It needed a while to "prepare" for incoming audio.

franky

Wed, 2021-05-19 20:24

To reproduce the crash, you might try this tiny 44.1kHz mono WAV file: moog-clave3.wav
Start your jackd at 48kHz, and run moc against it and attempt to play back that file. Results in an immediate crash for me.

About the short/inaudible file, I am sure it's not my speakers (they are real "on-is-on" studio monitor speakers). I'll do some jack_capture tests now to see what comes out of mocp compared to some other JACK file player.

tomaszg

Wed, 2021-05-19 20:46

All right, that does the trick :) I had a similar file, but maybe it was too long. With this file it crashes, but not every time. Your patch seems to resolve the issue for me.