0

Simplified version of what I'm actually doing (triggering a buzzer sound when a timer expires), but this demonstrates my design well enough.

Once playback is started, Swing never needs to touch the audio clip again. I have been able to confirm that this code does play back the sound and does not block the event dispatch thread, but I want to make sure there isn't some other thread safety concern that I am unknowingly violating. Thanks!

import java.io.IOException;
import java.net.URL;

import javax.sound.sampled.AudioInputStream;
import javax.sound.sampled.AudioSystem;
import javax.sound.sampled.Clip;
import javax.sound.sampled.LineEvent;
import javax.sound.sampled.LineUnavailableException;
import javax.sound.sampled.UnsupportedAudioFileException;
import javax.swing.JButton;
import javax.swing.JFrame;
import javax.swing.SwingUtilities;

public class TriggeringSoundTest extends JFrame
{
    private static final long serialVersionUID = -4573437469199690850L;

    private boolean soundPlaying = false;

    public TriggeringSoundTest()
    {
        JButton button = new JButton("Play Sound");
        button.addActionListener(e -> new Thread(() -> playBuzzer()).start());
        getContentPane().add(button);
    }

    private void playBuzzer()
    {
        URL url = getClass().getResource("resources/buzzer.wav");
        try (AudioInputStream stream = AudioSystem.getAudioInputStream(url); 
                Clip clip = AudioSystem.getClip())
        {
            clip.open(stream);
            clip.addLineListener(e -> {
                if (e.getType() == LineEvent.Type.STOP)
                    soundPlaying = false;
            });
            soundPlaying = true;
            clip.start();
            while (soundPlaying)
            {
                try
                {
                    Thread.sleep(1000);
                }
                catch (InterruptedException exp)
                {
                    // TODO Auto-generated catch block
                    exp.printStackTrace();
                }
            }
        }
        catch (UnsupportedAudioFileException exp)
        {
            // TODO Auto-generated catch block
            exp.printStackTrace();
        }
        catch (IOException exp)
        {
            // TODO Auto-generated catch block
            exp.printStackTrace();
        }
        catch (LineUnavailableException exp)
        {
            // TODO Auto-generated catch block
            exp.printStackTrace();
        }
    }

    private static void createAndShowGUI()
    {
        JFrame frame = new TriggeringSoundTest();
        frame.setDefaultCloseOperation(DISPOSE_ON_CLOSE);
        frame.pack();
        frame.setVisible(true);
    }

    public static void main(String[] args)
    {
        SwingUtilities.invokeLater(() -> createAndShowGUI());
    }
}
Andrew Thompson
  • 168,117
  • 40
  • 217
  • 433
train1855
  • 300
  • 2
  • 5
  • 14
  • 1
    Looks good w.r.t. threading. You might be better off pre-loading the clip somehow, so that it plays a little sooner. Also, your waiting for `soundPlaying` to turn `false` seems completely unnecessary. What is that for? If you want to do something once the sounds has played, place it in the anonymous `LineListener`. – Hendrik Apr 27 '18 at 08:00
  • @hendrik *"You might be better off pre-loading the clip somehow,"* Like for instance declaring it as an attribute of the class, & reading it once on construction. – Andrew Thompson Apr 27 '18 at 09:45

1 Answers1

1

I've gotten kind of removed from all the in's and out's of working with Clips, as they come with so many annoying aspects.

But yes, as mentioned in the comments, you should initialize (open) each Clip once and only once at the beginning of your program. Then, restart the Clip each time you need it. The API will show you the exact commands for resetting the Clip to its starting frame and replaying it.

Managing the threads is irksome. If I understand correctly, Clip launches a daemon thread for the actual audio out. Daemon threads die when the parent thread terminates. Hence your solution of putting in the Thread.sleep() command and the LineListener to keep the thread alive for the duration of the SFX, so its daemon doesn't get chomped when parent thread terminates.

I worry though, that if you use this approach, the calling thread (the code launched by the button press) won't be able to do anything else during your "sleep" period. If the button code is only executing the playback, then you should be fine. But what if you later decide to have anything else be triggered with the same button press (like a buzzer animation)? It might become necessary to add another layer of complication, such as wrapping the clip in yet another thread whose only responsibility is the clip playback. Then your button press could launch this wrapper and do other things without itself being subject to the Thread.sleep() solution.

Now, it should be possible to avoid all this! Let's say you made a Clip called buzzer and opened it early in your program, holding it in an instance variable (as suggested by Andrew). Then, let's say you call the buzzer.start() from within a thread that stays alive indefinitely, such as a classic game loop. I think in this the Clip start() will launch it's daemon thread and play as long as that game loop thread continues to live. In this way, you would be able to dispense with the sleep and line-listening. But I'm not 100% certain without actually writing an example and trying it myself.

(How does the button thread tell the game-loop thread to play the sound? Perhaps the button thread sets a flag, and the game-loop thread inspects that flag as part of its normal "update" cycle. Hmm. Maybe I really do need to try coding this before offering it as advice.)

As an alternative, feel free to examine the code, and to possibly make use of AudioCue. It is similar to a Clip in loading and playing, but underneath it makes use of SourceDataLine for the output instead of Clip. If you look at the code, you will see the SourceDataLine being maintained in its own thread (which stays alive as part of the AudioCue being opened). This eliminates a lot of complication.

A nice advantage that comes with this approach is that it makes it possible to provide for concurrent playbacks--for example if you want to start a second instance of a buzzer while the first is still playing. AudioCue also allows you to do things like play your buzzer at different speeds, which might be useful for leveraging your SFX into sounding like multiple cues. I did my best to provide documentation and examples. The license allows you to cut and paste the code as best works for your particular needs.

Phil Freihofner
  • 7,645
  • 1
  • 20
  • 41
  • I thought about trying to load the clip earlier, but I don't know how to ensure that it gets properly closed if I do that (since Java doesn't allow destructors). Also, the clip is *already* wrapped in a thread whose only responsibility is the clip playback. – train1855 Apr 29 '18 at 01:06
  • UPDATE: Shutdown hooks (see [here](https://www.javatpoint.com/ShutdownHook-thread) and [here](https://stackoverflow.com/questions/2921945/useful-example-of-a-shutdown-hook-in-java)) were the piece I was missing. This allowed me to load the `Clip` and the `AudioInputStream` as static fields and close them in the shutdown hook. This also removed the need for a wrapper thread and the need for the `Thread.sleep` business as Phil described above - the audio playing in the daemon thread does not block the event dispatch thread, and `sleep` is not needed because the thread is not about to die. – train1855 Apr 29 '18 at 16:41
  • FWIW, with JavaFX GUI, there's stage.setOnCloseRequest for clean shutdowns. If I have something that persists, like an Executor providing threads for audio, I close it at that time. But I'm not clear what you intend when you say "properly close". Can always set a Clip variable to null or reinitialize it, if having GC deal with it when it goes out of scope is not enough. The AudioInputStream (data source) is a potential memory leak and should be closed, but I think once the Clip is successfully opened, the ais is no longer needed. I could be wrong. – Phil Freihofner Apr 29 '18 at 22:36
  • 1
    Just checked this in my test program. The AIS can be closed as soon as the Clip is opened (it can actually be local to the static initializer block where I open the Clip). Re: "properly close," I figure that since Clip has a close method, I should make sure I invoke it even if (to your point) it might not be strictly necessary. Also, my GUI is in Swing rather than JavaFX. Do you know if there's a Swing equivalent to the JavaFX command you mentioned, or am I still looking at a shutdown hook if I use Swing? – train1855 Apr 30 '18 at 10:39
  • Thanks for verifying that about the AIS! I spent some time looking in a Swing application I wrote a few years ago with sound, and I can't find what you are looking for. The only shutdown provision I have for this program is the close button (top right) but there is nothing that I can find that hooks into this. So at the moment, I'm puzzled. Fortunately, everything shuts down just fine anyway in this program. I'll poke around a little more. I recall the topic coming up at Java-gaming.org forum at some point. – Phil Freihofner May 01 '18 at 06:51
  • You might look at using a WindowListener. In the API for JFrame, DefaultCloseOperation, I use the option "EXIT_ON_CLOSE" which seems to simple shut down everything. But you could also >> DO_NOTHING_ON_CLOSE (defined in WindowConstants): Don't do anything; require the program to handle the operation in the windowClosing method of a registered WindowListener object. << Here is a link> https://docs.oracle.com/javase/7/docs/api/javax/swing/JFrame.html#setDefaultCloseOperation(int) – Phil Freihofner May 01 '18 at 06:57