DavidGriffith / frotz

Infocom-style interactive fiction player for Unix and DOS (moved to https://gitlab.com/DavidGriffith/frotz)
GNU General Public License v2.0
209 stars 64 forks source link

Audio works but only for one sound at a time #27

Open DavidGriffith opened 8 years ago

DavidGriffith commented 8 years ago

I've managed to get audio playback working for AIFF, OGGV, and MOD, but this only works for one sound being played at a time. According to the specs, AIFF can be played at the same time as an OGGV or MOD, but at no time may an OGGV and MOD play at the same time. My code seems to enforce this restriction, but I am having lots of trouble getting AIFF and OGGV/MOD streams mixed. When I start a long-lasting OGGV or MOD clip, then cause an AIFF to play, I get distortion, slowdown, and finally a segfault. I can't figure out what the problem is or even any reference code on doing this the right way.

@erikd, this is what I described to you around a month ago in #xiph. Please take a look at it. The relevant branch is ao-curses.

erikd commented 8 years ago

I've submitted PR https://github.com/DavidGriffith/frotz/pull/28 . When you fix all compiler warnings I'll take another look.

erikd commented 8 years ago

The way I added CFLAGS means you can also add more CFLAGS on the command line so that after make clean you can do:

CFLAGS=-Werror make

which will promote all warnings to errors that terminate compilation.

DavidGriffith commented 8 years ago

Right after I approved the pull, I noticed that it was for the master branch. My audio trouble is in the ao-curses branch. I'm working on the warnings right now.

erikd commented 8 years ago

That patch should be on all branches :).

DavidGriffith commented 8 years ago

I've fixed most of -Wall's complaints. There remain two in src/curses/ux_text.c having to do with the Z-machine's handling of character sets. I don't believe they have anything to do with audio.

A test game is at http://661.org/if/soundtest.blb. In the game, the C64, when turned on, will play an OGG file. The Amiga will play a MOD file. The red, green, and blue buttons on the little handheld device will play AIFF samples. All of these files are embedded in the .blb (Blorb) file. The "off" button is supposed to stop all sounds. The dial is a volume control. Any one of these noises will play alone, but if you turn on the Amiga and then press a colored button, then you'll hear distortion, slowdown, and Frotz will segfault.

erikd commented 8 years ago

In my own code, I usually work to remove all warnings unless those warnings occur due to header files from outside my project.

Ok, so I'd like to test this. What do I do to reproduce the problem you are seeing?

DavidGriffith commented 8 years ago

Download http://661.org/if/soundtest.blb and type "frotz soundtest.blb". Then press the colored buttons on the handheld device first so you know what they sound like alone. Then turn on one of the computers. Then press one of the colored buttons. The audio will get distorted for a second or two and then Frotz will crash with a segfault. I presume the distortion is related to the segfault.

erikd commented 8 years ago

I built frotz with the thread sanitizer using:

CFLAGS="-fsanitize=thread -g" LIB="-fsanitize=thread -g" make clean frotz

and then ran it using:

./frotz soundtest.blb 2> thread-errors.txt

Once running, I start either the Amiga or the C64 emulation, let it run for a while and then stop it. The "thread-errors.txt" then contains:

==================
WARNING: ThreadSanitizer: data race (pid=6413)
  Read of size 1 at 0x00000062c3c2 by main thread:
    #0 os_stop_sample src/curses/ux_audio.c:262 (frotz+0x00000041fd1b)
    #1 os_reset_screen src/curses/ux_init.c:486 (frotz+0x000000419805)
    #2 main src/common/main.c:193 (frotz+0x000000402e45)

  Previous write of size 1 at 0x00000062c3c2 by thread T2:
    [failed to restore the stack]

  Location is global 'music_playing' of size 1 at 0x00000062c3c2 (frotz+0x00000062c3c2)

  Thread T2 (tid=6416, running) created by main thread at:
    #0 pthread_create <null> (libtsan.so.0+0x000000027d7d)
    #1 os_start_sample src/curses/ux_audio.c:237 (frotz+0x00000041fc46)
    #2 start_sample src/common/sound.c:79 (frotz+0x000000409efd)
    #3 z_sound_effect src/common/sound.c:192 (frotz+0x00000040a233)
    #4 interpret src/common/process.c:301 (frotz+0x0000004033a8)
    #5 main src/common/main.c:189 (frotz+0x000000402e3b)

SUMMARY: ThreadSanitizer: data race src/curses/ux_audio.c:262 in os_stop_sample
==================
==================
WARNING: ThreadSanitizer: data race (pid=6413)
  Read of size 4 at 0x00000062ceb0 by main thread:
    #0 os_stop_sample src/curses/ux_audio.c:262 (frotz+0x00000041fd30)
    #1 os_reset_screen src/curses/ux_init.c:486 (frotz+0x000000419805)
    #2 main src/common/main.c:193 (frotz+0x000000402e45)

  Previous write of size 4 at 0x00000062ceb0 by thread T2:
    [failed to restore the stack]

  Location is global 'musicnum' of size 4 at 0x00000062ceb0 (frotz+0x00000062ceb0)

  Thread T2 (tid=6416, running) created by main thread at:
    #0 pthread_create <null> (libtsan.so.0+0x000000027d7d)
    #1 os_start_sample src/curses/ux_audio.c:237 (frotz+0x00000041fc46)
    #2 start_sample src/common/sound.c:79 (frotz+0x000000409efd)
    #3 z_sound_effect src/common/sound.c:192 (frotz+0x00000040a233)
    #4 interpret src/common/process.c:301 (frotz+0x0000004033a8)
    #5 main src/common/main.c:189 (frotz+0x000000402e3b)

SUMMARY: ThreadSanitizer: data race src/curses/ux_audio.c:262 in os_stop_sample
==================
==================
WARNING: ThreadSanitizer: thread leak (pid=6413)
  Thread T2 (tid=6416, finished) created by main thread at:
    #0 pthread_create <null> (libtsan.so.0+0x000000027d7d)
    #1 os_start_sample src/curses/ux_audio.c:237 (frotz+0x00000041fc46)
    #2 start_sample src/common/sound.c:79 (frotz+0x000000409efd)
    #3 z_sound_effect src/common/sound.c:192 (frotz+0x00000040a233)
    #4 interpret src/common/process.c:301 (frotz+0x0000004033a8)
    #5 main src/common/main.c:189 (frotz+0x000000402e3b)

SUMMARY: ThreadSanitizer: thread leak (/usr/lib/x86_64-linux-gnu/libtsan.so.0+0x27d7d) in __interceptor_pthread_create
==================
ThreadSanitizer: reported 3 warnings

The "thread leak" is probably caused by the main thread not waiting for the other leaked thread to terminate before exiting. Its worth fixing but probably not a major issue. The two data races are important and should be fixed.

, start either the Amga or the C64 emulation, let it run for a while and then stop it, when the program exits curses, there are a large number of threading errors in the terminal.

erikd commented 8 years ago

Thread sanitizer said musicnum and music_playing were subject to race conditions so I added locks around them (PR https://github.com/DavidGriffith/frotz/pull/30) and now turning on either the Amgia or C64 emulations, letting it run for a while and then quitting results in zero thread sanitizer errors.

After fixing musicnum and music_playing there are still thread sanitizer errors when you press one of the colored buttons.

DavidGriffith commented 7 years ago

Partial fix applied by @fundamental in d88e730333712805babe34b7ac8582e009c39b50. Thanks!