Clip/Sound is using too much CPU?

[kedxu]

I'm probably doing this wrong, because there has to be a way to do this.

Here is my current code:

import javax.sound.sampled.*;

public class soundHandler{

    soundHandler()
    {

    }
    public void newSound(String sound)
    {
        new soundThread(sound).start();
    }
}



class soundThread extends Thread
{
    String soundName;
    Clip clip = null;
    soundThread(String i_soundName)
    {
        soundName = i_soundName;
    }
    public void run()
    {
        try{
            try{
                try{
                    AudioInputStream stream = AudioSystem.getAudioInputStream(ClassLoader.getSystemClassLoader().getResource(soundName));

                    DataLine.Info info = new DataLine.Info(Clip.class, stream.getFormat());
                    clip = (Clip)AudioSystem.getLine(info);
                    clip.open(stream);
                }catch(Exception e){System.out.println("err= soundThread_errorLoadingSound ("+soundName+")");}

                clip.start();
            }catch(Exception e){System.out.println("err = soundThread_errorPlayingSound ("+soundName+")");}

            Thread.sleep(150);

            //wait for it to finish, then close
            while(clip.isActive()){}
            clip.close();
        }catch(Exception e){System.out.println("err= soundThread_error");e.printStackTrace();return;}
    }
}

In soundHandler, the method newSound() is called very frequently, perhaps multiple times per second. This causes it to use a significant amount of CPU for some reason.

Any idea what's going on, or how I can fix it?

[JamesCherrill]

Maybe your problem isn't the clip, it's this horror on line 43

while(clip.isActive()){}

that's going to take 100% of the available CPU until it terminates
I suspect there must be a better way to be notified when a clip is finished, but at the very least you could try something like

while(clip.isActive()){Thread.sleep(250);}

[~s.o.s~]

Two problems with your approach.

First the major one: As mentioned by James, never use busy loops (i.e. while(true) {} in your application code. It will always result in CPU spinning at 100%. There is almost always a better way of waiting for some event than to loop over it continuously. If you ever feel that there is no alternative left, at least sleep for an acceptable amount of time (e.g. 100ms) before testing the condition again. Also, the Thread.sleep call in your code is not really required. Explicit sleeps in application logic is almost always a sign of bad design. Try to find better way to solve the same stuff.

The second minor issue is that you are always creating a thread whenever there is a request to play a new sound. This is expensive since Java has OS threads and OS threads have a significant (comparatively) overhead during creation. Plus, if you are using a recent Java version (e.g. Java 5 and above), give preference to using Executor and ExecutorService over raw threads if possible.

Here is one possile way of how I would do it, not the best but still works good.

public class Noise {

    /**
     * @param args
     */
    public static void main(String[] args) throws Exception {
        final SoundHandler handler = new SoundHandler();
        handler.newSound("gunshots.mid");
        Thread.sleep(1000 * 100);
    }

}

class SoundHandler {

    private final Executor executor = Executors.newCachedThreadPool();

    public void newSound(String sound) {
        executor.execute(new SoundRunnable(sound));
    }

    class SoundRunnable implements Runnable {
        final String soundName;

        SoundRunnable(String soundName) {
            this.soundName = soundName;
        }

        public void run() {
            try {
                AudioInputStream stream = AudioSystem.getAudioInputStream(this.getClass().
                        getClassLoader().getResourceAsStream(soundName));

                DataLine.Info info = new DataLine.Info(Clip.class, stream.getFormat());
                final Clip clip = (Clip) AudioSystem.getLine(info);
                clip.open(stream);
                clip.addLineListener(new LineListener() {
                    @Override
                    public void update(LineEvent event) {
                        if (event.getType() == Type.STOP) {
                            System.out.println("STOP");
                            clip.close();
                        }
                    }
                });
                clip.start();
            } catch (Exception e) {
                e.printStackTrace();
            }
        }
    }

}

Just make sure you create a single handler instance and keep adding sounds to it rather than creation multiple instances. I am not really aware of javax.* packages so if you feel a need to tap into other events, just browse the API. I came up with the above code after a brief look at the sound API.