while loop does not seem to work when I delete System.out.println

Question

So this while loop pretty much do nothing until I change the value of bgmPlaying. It works fine. However, if I delete the parts that says //testing above it(without any line breaks), it does not work.

This block of code actually keeps checking whether a music is on or off.

Any idea why it stops working when I delete the System.out.println() parts???

Here is my code:

import java.io.File;

import javax.sound.sampled.AudioFormat;
import javax.sound.sampled.AudioInputStream;
import javax.sound.sampled.AudioSystem;
import javax.sound.sampled.Clip;
import javax.sound.sampled.DataLine;
import javax.sound.sampled.FloatControl;

/**
 * This class simply plays a background music in a seperate thread
 * @author Mohammad Nafis
 * @version 1.0
 * @since 04-03-2018
 *
 */
public class AudioPlayer implements Runnable{

    /**
     * this boolean indicates whether the background music is playing
     */
    private boolean bgmPlaying = true; 

    public void stopBGM() {
        bgmPlaying = false;
    }

    public void playBGM() {
        bgmPlaying = true;
    }

    /**
     * this is an overridden method from Runnable interface that executes when a thread starts
     */
    @Override
    public void run() {

        try {
            File soundFile = new File("sounds/epic_battle_music.wav");
            AudioInputStream ais = AudioSystem.getAudioInputStream(soundFile);
            AudioFormat format = ais.getFormat();
            DataLine.Info info = new DataLine.Info(Clip.class, format);
            Clip clip = (Clip)AudioSystem.getLine(info);
            clip.open(ais);
            clip.loop(Clip.LOOP_CONTINUOUSLY);

            //controlling the volume
            FloatControl gainControl = (FloatControl) clip.getControl(FloatControl.Type.MASTER_GAIN);
            gainControl.setValue(-20);
                clip.start();




                while(true) {

                    if(bgmPlaying) {
                        gainControl.setValue(-20);
                    } else {
                        gainControl.setValue(-80);
                    }

                    while(bgmPlaying) {

                        //testing
                        System.out.println("BGM is on: ");


                        if(bgmPlaying == false) {
                            gainControl.setValue(-80);


                            break;
                        }
                    }

                    while(!bgmPlaying) {

                        //testing
                        System.out.println("BGM is off: ");

                        if(bgmPlaying == true) {
                            gainControl.setValue(-20);


                            break;
                        }
                    }
                }

        } catch (Exception e) {
            e.printStackTrace();
        }

    }

}

This code is in my Controller class that calls the stop and play methods.

//adding action listener
    window.getpausebutton().addActionListener(new ActionListener() {
        public void actionPerformed(ActionEvent ev) {   
            new Thread(new SoundEffect("sounds/clickSound.wav")).start();
            bgm.stopBGM();
        }
    });
    window.getplaybutton().addActionListener(new ActionListener() {
        public void actionPerformed(ActionEvent ev) {
            new Thread(new SoundEffect("sounds/clickSound.wav")).start();
            bgm.playBGM();
        }
    });

Answer 1

Using volatile and Thread.sleep is really a work around for the problem, although it should work. But the larger problem is that the code is busy-waiting for the state of the variable to change, and depending on shared state between the two threads. With volatile or synchronized you can manage these problems, but there is a better way.

In the java.util.concurrent package there are power tools for dealing with concurrency. Threads, volatile, sleep (and also wait and notify) are like primitive hand tools by comparison. One of the power tools in there is the BlockingQueue and its various implementations, which would let you implement something like the Actor Model in Java.

In the Actor Model, actors send messages to one another to act on, but they never share memory. You could define some simple messages, like PLAY, MUTE, and STOP, and send these messages from your control thread to your player thread. If you use a BlockingQueue, there will be no problem with the player seeing the messages, and it won't have to busy-wait for a message to arrive. It can simply try to take a message from the queue, and if there's no message waiting, it blocks until a message becomes available.

Here's how you could implement that in your code:

public enum Command {
    PLAY, STOP, MUTE;
}

import java.io.File;

import javax.sound.sampled.AudioFormat;
import javax.sound.sampled.AudioInputStream;
import javax.sound.sampled.AudioSystem;
import javax.sound.sampled.Clip;
import javax.sound.sampled.DataLine;
import javax.sound.sampled.FloatControl;

import java.util.Objects;
import java.util.Scanner;
import java.util.concurrent.BlockingQueue;
import java.util.concurrent.LinkedBlockingQueue;

/**
 * This class simply plays a background music in a separate thread
 *
 * @author Mohammad Nafis
 * @version 1.0
 * @since 04-03-2018
 */
public class AudioPlayer implements Runnable {
    private final String filename;
    private final BlockingQueue<Command> commandQueue =
            new LinkedBlockingQueue<>();

    public final static int LOUD = -20;
    public final static int QUIET = -80;

    public AudioPlayer(String filename) {
        this.filename = Objects.requireNonNull(filename);
    }

    public void perform(Command command) throws InterruptedException {
        commandQueue.put(Objects.requireNonNull(command));
    }

    @Override
    public void run() {
        try {
            File soundFile = new File(filename);
            AudioInputStream ais = AudioSystem.getAudioInputStream(soundFile);
            AudioFormat format = ais.getFormat();
            DataLine.Info info = new DataLine.Info(Clip.class, format);
            Clip clip = (Clip)AudioSystem.getLine(info);
            clip.open(ais);
            clip.loop(Clip.LOOP_CONTINUOUSLY);

            //controlling the volume
            FloatControl gainControl = (FloatControl)
                    clip.getControl(FloatControl.Type.MASTER_GAIN);
            gainControl.setValue(LOUD);
            clip.start();

            forever: while (true) {
                switch (commandQueue.take()) {
                    case PLAY:
                        gainControl.setValue(LOUD);
                        break;
                    case MUTE:
                        gainControl.setValue(QUIET);
                        break;
                    case STOP:
                        break forever;
                }
            }

            clip.stop();
            clip.close();
        } catch (Exception e) {
            e.printStackTrace();
        }
    }

    public static void main(String[] args) throws InterruptedException {
        AudioPlayer player = new AudioPlayer(args[0]);
        Thread thread = new Thread(player);
        thread.start();

        Scanner in = new Scanner(System.in);
        String cmd = "";
        System.out.println("Type mute or play. Or stop to exit.");
        do {
            System.out.print(": ");
            System.out.flush();
            cmd = in.nextLine();
            if ("play".equals(cmd)) player.perform(Command.PLAY);
            else if ("mute".equals(cmd)) player.perform(Command.MUTE);
            else if ("stop".equals(cmd)) player.perform(Command.STOP);
            else System.out.println("I didn't understand that, sorry.");
        } while (!"stop".equals(cmd));

        player.perform(Command.STOP);
        thread.join();
        System.out.println("Be seeing you.");
    }
}

A few notes:

• I've added calls to clip.stop() and clip.close() after the player is stopped so that the audio system doesn't keep a background thread running that prevents the program from exiting.
• The perform(Command) method, while it is within the AudioPlayer class, will execute on the control thread that calls it, but that's okay. Because the queue is designed for concurrency, commands that are enqueued on the control thread will be immediately visible on the player thread. No need for Thread.sleep.
• I added constants for the two different gain levels.
• I made the name of the audio file a command-line parameter, since I don't have your epic battle music.
• Both the AudioPlayer constructor and the perform method will throw if you try to pass in null.