Using sounds in Java

Question

I'm trying to apply sound today, and the goal in mind was a simple violin tuner. The actionListener seems repetitive/ How might I optimize it (what I tried just broke everything)? I also noticed the sounds bleed into each other. I don't mind that so much for the different keys (though I'm thinking of a toggle option for this) but it also is the case for the same key, which is undesirable. I'm not sure how to handle that.

/* Author: Luigi Vincent
A simple violin tuner, emits G, D, A, and E sounds to facilitate violin tuning
*/

import javax.swing.*;
import javax.sound.sampled.*;
import java.awt.*;
import java.awt.event.*;
import java.io.*;

public class ViolinTuner {
    public static void main(String[] args) {
        JFrame frame = new JFrame("Violin Tuner");
        // get and set icon for the program
        ImageIcon icon = new ImageIcon("Images/Icon.png");
        frame.setIconImage(icon.getImage());
        // Buttons
        final JButton G_KEY = new JButton("G");
        final JButton D_KEY = new JButton("D");
        final JButton A_KEY = new JButton("A");
        final JButton E_KEY = new JButton("E");
        // Listener to play sounds on click
        ActionListener violinSounds = new ActionListener() {
            @Override
            public void actionPerformed(ActionEvent e) {
                if (e.getSource() == G_KEY) {
                    try {
                        AudioInputStream audioInputStream = AudioSystem.getAudioInputStream(new File("Sounds/G.wav").getAbsoluteFile());
                        Clip clip = AudioSystem.getClip();
                        clip.open(audioInputStream);
                        clip.start();
                    }catch(Exception x) { x.printStackTrace(); }
                }
                else if (e.getSource() == D_KEY) {
                    try {
                        AudioInputStream audioInputStream = AudioSystem.getAudioInputStream(new File("Sounds/D.wav").getAbsoluteFile());
                        Clip clip = AudioSystem.getClip();
                        clip.open(audioInputStream);
                        clip.start();
                    }catch(Exception x) { x.printStackTrace(); }
                }
                else if (e.getSource() == A_KEY) {
                    try {
                        AudioInputStream audioInputStream = AudioSystem.getAudioInputStream(new File("Sounds/A.wav").getAbsoluteFile());
                        Clip clip = AudioSystem.getClip();
                        clip.open(audioInputStream);
                        clip.start();
                    }catch(Exception x) { x.printStackTrace(); }
                }
                else if (e.getSource() == E_KEY) {
                    try {
                        AudioInputStream audioInputStream = AudioSystem.getAudioInputStream(new File("Sounds/E.wav").getAbsoluteFile());
                        Clip clip = AudioSystem.getClip();
                        clip.open(audioInputStream);
                        clip.start();
                    } catch(Exception x) { x.printStackTrace(); }
                }
            }
        };
        // Register buttons to listener
        G_KEY.addActionListener(violinSounds);
        D_KEY.addActionListener(violinSounds);
        A_KEY.addActionListener(violinSounds);
        E_KEY.addActionListener(violinSounds);
        // buttons to panel
        JPanel p = new JPanel();
        p.add(G_KEY);
        p.add(D_KEY);
        p.add(A_KEY);
        p.add(E_KEY);
        // Panel to frame
        frame.add(p);
        frame.setDefaultCloseOperation(JFrame.EXIT_ON_CLOSE);
        frame.setSize(225, 75);
        frame.setLocationRelativeTo(null);
        frame.setVisible(true);
    }
};

Answer 1

Use MIDI

Why play a WAV file, when you could synthesize the tones using MIDI? The tones can be played with indefinite length. You wouldn't need to distribute supplementary files with the program. You can also easily modify the program to generate other pitches without having to obtain new WAV recordings.

import java.awt.BorderLayout;
import java.awt.Dimension;
import java.awt.GridLayout;
import javax.sound.midi.*;
import javax.swing.*;
import javax.swing.event.ChangeEvent;
import javax.swing.event.ChangeListener;

public class ViolinTuner {
    private static enum Note {
        G(55), D(62), A(69), E(76);

        public final int midiNoteNumber;

        Note(int midiNoteNumber) {
            this.midiNoteNumber = midiNoteNumber;
        }
    }

    // Orchestras tune to oboes.
    // http://music.stackexchange.com/q/22133/1941
    // http://www.midi.org/techspecs/gm1sound.php
    private static final int OBOE = 69;

    private final MidiChannel channel;
    private final AbstractButton[] buttons = new AbstractButton[Note.values().length];

    public ViolinTuner() throws MidiUnavailableException {
        this.channel = initMidiChannel(OBOE);

        JFrame frame = new JFrame("Violin Tuner");
        frame.setIconImage(new ImageIcon("Images/Icon.png").getImage());

        // buttons to panel
        JPanel notePanel = new JPanel(new GridLayout(2, 0, 8, 4));
        notePanel.setBorder(BorderFactory.createEmptyBorder(4, 8, 4, 8));
        int i = 0;
        for (Note note : Note.values()) {
            notePanel.add(buttons[i++] = makeButton(note));
        }

        // Component to impose a minimum width on the JFrame to ensure that the
        // window title is not truncated
        JPanel widener = new JPanel();
        widener.setPreferredSize(new Dimension(250, 0));

        // Panels to frame
        frame.setLayout(new BorderLayout());
        frame.add(notePanel, BorderLayout.CENTER);
        frame.add(widener, BorderLayout.SOUTH);
        frame.pack();

        frame.setDefaultCloseOperation(JFrame.EXIT_ON_CLOSE);
        frame.setLocationRelativeTo(null);
        frame.setVisible(true);
    }

    private static MidiChannel initMidiChannel(int instrument) throws MidiUnavailableException {
        Synthesizer synth = MidiSystem.getSynthesizer();
        synth.open();
        MidiChannel channel = synth.getChannels()[0];
        // MIDI instruments are traditionally numbered from 1,
        // but the javax.midi API numbers them from 0
        channel.programChange(instrument - 1);
        channel.setChannelPressure(5);  // optional vibrato
        return channel;
    }

    private AbstractButton makeButton(final Note note) {
        AbstractButton button = new JToggleButton(note.name());
        button.addChangeListener(new ChangeListener() {
            @Override
            public void stateChanged(ChangeEvent e) {
                if (((AbstractButton)e.getSource()).isSelected()) {
                    for (AbstractButton b : ViolinTuner.this.buttons) {
                        // Turn off any note that might be being played
                        if (b != e.getSource()) {
                            b.setSelected(false);
                        }
                    }
                    ViolinTuner.this.play(note);
                } else {
                    ViolinTuner.this.silence();
                }
            }
        });
        return button;
    }

    public void play(Note note) {
        this.channel.noteOn(note.midiNoteNumber, 127); 
    }

    public void silence() {
        this.channel.allNotesOff();
    }

    public static void main(String[] args) {
        SwingUtilities.invokeLater(new Runnable() {
            public void run() {
                try {
                    new ViolinTuner();
                } catch (MidiUnavailableException noMidi) {
                    noMidi.printStackTrace();
                    System.exit(1);
                }
            }
        });
    }
}

Answer 2

Layout

Calling .setSize() with fixed dimensions isn't portable. For example, this is what happens on Mac OS X:

Instead, call frame.pack() to automatically set the frame size according to its contents.

If you want the 2 × 2 layout, then use a suitable LayoutManager — for example, p.setLayout(new GridLayout(2, 0, 10, 6));

Don't Repeat Yourself

The code is awfully repetitive. You should avoid repeating yourself, since repetition makes maintenance difficult: cut-and-paste programming leads to cut-and-paste bugs. I recommend using an Enum to represent the four notes.

Explicitness

It's bad practice to catch (Exception x). I have no clue what kind of exception would likely cause the flow of execution to jump to the exception handler. You, as the author, might intend for certain exceptions to be caught, but you don't know what will actually happen. Starting with Java 7, you can list multiple exceptions to be handled by the same catch block, and that's what I recommend that you do.

Avoid overuse of wildcard imports. There exists a small probability that a future version of some package could add a class that clashes and breaks your program. Mostly, using too many wildcard imports makes it hard for human maintainers to resolve identifiers — the compiler may be good at searching the classpath, but I'm not. My suggested guidelines are:

• It's OK to use wildcard imports with the standard Java packages that almost every Java programmer is familiar with (such as java.io.* or java.util.*).
• It's OK to use one wildcard import per file, maybe two, but not much more.
• If in doubt, import individual classes explicitly.
• List your imports alphabetically.

By the way, you never used anything in java.awt.*.

Suggested implementation

import java.awt.BorderLayout;
import java.awt.Dimension;
import java.awt.GridLayout;
import java.awt.event.ActionEvent;
import java.awt.event.ActionListener;
import java.io.File;
import java.io.IOException;
import javax.sound.sampled.*;
import javax.swing.BorderFactory;
import javax.swing.JButton;
import javax.swing.JFrame;
import javax.swing.JPanel;
import javax.swing.ImageIcon;
import javax.swing.SwingUtilities;

public class ViolinTuner {
    private static enum Note {
        G, D, A, E;

        public final File file;

        Note() {
            this.file = new File("Sounds", this.name() + ".wav").getAbsoluteFile();
        }
    }

    private static JButton makeJButton(final Note note) {
        JButton button = new JButton(note.name());
        button.addActionListener(new ActionListener() {
            @Override
            public void actionPerformed(ActionEvent e) {
                try {
                    AudioInputStream audioInputStream = AudioSystem.getAudioInputStream(note.file);
                    Clip clip = AudioSystem.getClip();
                    clip.open(audioInputStream);
                    clip.start();
                } catch (IOException | UnsupportedAudioFileException | LineUnavailableException x) {
                    x.printStackTrace();
                }
            }
        });
        return button;
    }

    private static void createAndShowGUI() {
        JFrame frame = new JFrame("Violin Tuner");
        frame.setIconImage(new ImageIcon("Images/Icon.png").getImage());

        // buttons to panel
        JPanel notePanel = new JPanel(new GridLayout(2, 0, 8, 4));
        notePanel.setBorder(BorderFactory.createEmptyBorder(4, 8, 4, 8));
        for (Note note : Note.values()) {
            notePanel.add(makeJButton(note));
        }

        // Component to impose a minimum width on the JFrame to ensure that the
        // window title is not truncated
        JPanel widener = new JPanel();
        widener.setPreferredSize(new Dimension(250, 0));

        // Panels to frame
        frame.setLayout(new BorderLayout());
        frame.add(notePanel, BorderLayout.CENTER);
        frame.add(widener, BorderLayout.SOUTH);
        frame.pack();

        frame.setDefaultCloseOperation(JFrame.EXIT_ON_CLOSE);
        frame.setLocationRelativeTo(null);
        frame.setVisible(true);
    }

    public static void main(String[] args) {
        SwingUtilities.invokeLater(new Runnable() {
            public void run() {
                createAndShowGUI();
            }
        }
    }
}

Answer 3

You can clean up the repetition by moving the ActionListener's code into a custom class, like this:

static class KeyButtonListener implements ActionListener {
    private final JButton button;
    private final String filename;

    KeyButtonListener(JButton button, String filename) {
        this.button = button;
        this.filename = filename;
    }

    @Override
    public void actionPerformed(ActionEvent e) {
        AudioInputStream audioInputStream = null;
        try {
            audioInputStream = AudioSystem.getAudioInputStream(new File(filename));
            Clip clip = AudioSystem.getClip();
            clip.open(audioInputStream);
            clip.start();
        } catch (IOException | UnsupportedAudioFileException | LineUnavailableException x) {
            x.printStackTrace();
        } finally {
            if (audioInputStream != null) {
                try {
                    audioInputStream.close();
                } catch (IOException e1) {
                    e1.printStackTrace();
                }
            }
        }
    }
}

You can then create your listeners like this:

ActionListener gListener = new KeyButtonListener(G_KEY, "Sounds/G.wav");
ActionListener dListener = new KeyButtonListener(D_KEY, "Sounds/D.wav");
ActionListener aListener = new KeyButtonListener(A_KEY, "Sounds/A.wav");
ActionListener eListener = new KeyButtonListener(E_KEY, "Sounds/E.wav");

This cleans up the multiple if-else, because now there are 4 listeners instead of 1, and each is dedicated to one key. And there's no more need for checking the source of the event, as the listeners are dedicated to a single button.

Another change I did there, instead of catching Exception, I spell out all the possible exceptions that might be thrown there. This is a recommended practice in general. Often you might want to handle each exception a little bit differently, but in this simple case it just helps me see what exactly can go wrong in that block of code.

Finally, remember to close all streams that you opened, in this example the AudioInputStream.

---

Notice that the code still has much repetition: for each button, you repeat the following:

• create the button
• create a listener
• add the listener to the button
• add the button to the panel

You can do these in a loop:

JPanel panel = new JPanel();

for (String key : new String[]{"G", "D", "A", "E"}) {
    JButton keyButton = new JButton(key);
    ActionListener listener = new KeyButtonListener(keyButton, String.format("Sounds/%s.wav", key));
    keyButton.addActionListener(listener);
    panel.add(keyButton);
}

Minor things

My IDE doesn't like this line:

frame.setDefaultCloseOperation(JFrame.EXIT_ON_CLOSE);

It seems the correct constant to use is different:

frame.setDefaultCloseOperation(WindowConstants.EXIT_ON_CLOSE);

Finally, all the comments in your code are redundant. It's better to remove them. Try to avoid comments as much as possible. If the code is not self-explanatory, try to improve it so it becomes self-explanatory. Here's a nice article on the topic:

http://blog.codinghorror.com/coding-without-comments/

Answer 4

Initializing Swing on the Event Dispatch Thread

As noted in the Concurrency in Swing Java tutorial, Swing code must run in the Event Dispatch Thread, not the main thread. Therefore, you need to wrap the entire contents of your main() function inside SwingUtilities.invokeLater():

private static void createAndShowGUI() {
    JFrame frame = new JFrame("Violin Tuner");
    …
}

public static void main(String[] args) {
    SwingUtilities.invokeLater(new Runnable() {
        public void run() {
            createAndShowGUI();
        }
    });
}

(I stumbled upon this problem the hard way.)

Adding a keyboard handler

While holding a violin in the left hand and a bow in the right hand, it can be difficult to grab a mouse and click on the appropriate button. You can, however, tap a key on a keyboard relatively easily. Therefore, I suggest adding a KeyListener to handle G, D, A, and E keys. I also suggest mapping Space to A = 440 Hz, since it is the easiest key to hit and the standard pitch to tune to.

Answer 5

Most of the really good feedback is already given, but I have one last recommendation:

If you're going to use audio clips instead of using MIDI, why not use a simple library like tinysound?

It takes care of so much, and is available under a BSD-2 clause license. https://github.com/finnkuusisto/TinySound

That way, you don't have to care about creating clips, making sure the Wave-files are in a correct form, etc.

It may not be the thing for you if you want to make a product to take on Cubase, but it's probably exactly the type of library you'd want now.