How do I stop mp3 files being played multiple times at once?

Question

I am trying to play an mp3 file on button press or selection from a list (which I have managed successfully). However, I cannot seem to stop the song being played multiple times on the same button press.

What I would like to do is play the song in a new thread, disable playing the song again until the thread has closed, then allow playing again.

My code is as follows:

public class SoundFactory {

private Player player;
private static boolean running = false;

private String getFile(String name) {
    String f = "sound" + File.separator + name + ".mp3";
    return f;
}

public void playMP3(String name) {


    if (!running) {
        running = true;

        try {
            FileInputStream fis = new FileInputStream(getFile(name));
            BufferedInputStream bis = new BufferedInputStream(fis);
            player = new Player(bis);
        } catch (Exception e) {
            System.out.println("Problem playing file " + name);
            System.out.println(e);
        }

        // run in new thread to play in background
        new Thread() {
            public void run() {
                try {
                    player.play();
                } catch (Exception e) {
                    System.out.println(e);
                }
            }
        }.start();
        //running = false;
    }
}


public void close() {
    if (player != null) player.close();
}

}

The file is played via:

    SoundFactory sf = new SoundFactory();
    sf.playMp3("song name");

on a JButton click

I am new to threading so I apologise in advance if this has an obvious solution!

Answer 1

It sounds to me like you are getting multiple click events fired at once instead of just one. A little logging should verify this. Your method as is, is wide open to race conditions.

The two events can be so close together that when the one checks running it see !running as true. Before that one can do running = true, the second event also sees !running as true and enters the if clause. They then both set running to true and spawn a thread to play the mp3.

What you need to do is make your method synchronized.

public synchronized void playMP3(String name)

http://docs.oracle.com/javase/tutorial/essential/concurrency/syncmeth.html

If count is an instance of SynchronizedCounter, then making these methods synchronized has two effects:

• First, it is not possible for two invocations of synchronized methods on the same object to interleave. When one thread is executing a synchronized method for an object, all other threads that invoke synchronized methods for the same object block (suspend execution) until the first thread is done with the object.
• Second, when a synchronized method exits, it automatically establishes a happens-before relationship with any subsequent invocation of a synchronized method for the same object. This guarantees that changes to the state of the object are visible to all threads.

Just to clarify my last comment, here is a test program showing where running = false should be placed.

public class Test {

  public static boolean running = false;

  public synchronized void runner() {
    if(!running) {
      running = true;
      System.out.println("I'm running!");
      new Thread() {
        public void run() {
          for(int i=0; i<10000; i++) {} // Waste some time
          running = false; // This is only changed once the thread completes its execution.
        }
      }.start();
    } else {
      System.out.println("Already running.");
    }
  }

  public static void main(String[] args) {
    Test tester = new Test();
    tester.runner();
    tester.runner(); // The loop inside the Thread should still be running so this should fail.
    for(int i=0; i<20000; i++) {} // Waste even more time.
    tester.runner(); // The loop inside the Thread should be done so this will work.
  }
}

It outputs:

I'm running! 
Already running. 
I'm running!

It's been years since I've worked with Swing and had forgotten that its event dispatcher is single threaded. So your issue is more likely this than a race condition. It still doesn't hurt to get into writing things to be thread safe from the beginning as it gets you used to it and thinking that way.

Definite warning on using the synchronized method... It can be horrible on performance if only a small part of your method needs to be synchronized. In this case your whole method needs to be thread safe.

If only a small part needs to be thread safe you need to use synchronized blocks.

Thread safe per instance:

public class myClass {
   public void myFunc() {
      // bunch of code that doesn't need to be thread safe.
      synchronized(this) {
         // Code that needs to be thread safe per instance
      }
      // More code that doesn't need thread safety.
   }
}

Thread safe across all instances.

public class myClass {
   static Object lock = new Object();
   public void myFunc() {
      // bunch of code that doesn't need to be thread safe.
      synchronized(lock) {
         // Code that needs to be thread safe across all instances.
      }
      // More code that doesn't need thread safety.
   }
}

Thread safe in a static method.

public class myClass {
   public static void myFunc() {
      // bunch of code that doesn't need to be thread safe.
      synchronized(MyClass.class) {
         // Code that needs to be thread safe.
      }
      // More code that doesn't need thread safety.
   }
}

Probably way more information than you want, but I've just seen threaded programming taught so poorly many, many times.

Answer 2

You need to call JButton.setEnabled(false); right before you start playing the mp3, and then call JButton.setEnabled(true); when the mp3 finishes playing.

Obviously, you should replace JButton with your button's object (eg: playButton.setEnabled()).