Keyboard music maker

Question

I enjoy making silly programs every once in a while. This is one of them. It makes nice sounding notes that vary slightly depending on the key being entered into a TextArea. When you press enter, it plays back all the notes you have entered. That's pretty much all it does. I'm looking for constructive criticism regarding the coding style, and perhaps usability.

package boop;

import java.awt.TextArea;
import java.awt.event.KeyEvent;
import java.awt.event.KeyListener;
import java.util.HashMap;
import javax.sound.sampled.AudioFormat;
import javax.sound.sampled.AudioSystem;
import javax.sound.sampled.LineUnavailableException;
import javax.sound.sampled.SourceDataLine;
import javax.swing.JFrame;

public class Boop extends JFrame implements KeyListener {
    private final TextArea textArea;

    private class Noise implements Runnable {

        private int noise[];
        public boolean sound = true;
        private final int freq;
        private int ms = 100;
        private int samples = (int) ((ms * SAMPLE_RATE) / 1000);
        private byte[] output = new byte[samples];
        private int i = 0;
        private double period;

        public Noise(int freq) {
            this.freq = freq;
            period = (double) SAMPLE_RATE / freq;
            new Thread(this).start();
        }

        @Override
        public void run() {
            SourceDataLine line;
            try {
                line = AudioSystem.getSourceDataLine(af);
                line.open(af, SAMPLE_RATE);
            } catch (LineUnavailableException ex) {
                ex.printStackTrace();
                return;
            }
            line.start();
            while (i < output.length) {
                double angle = 2.0 * Math.PI * i / period * 1.0 + 0.08;
                output[i++] += (byte) (Math.sin(angle) * 127f);
            }
            while (sound) {
                line.write(output, 0, output.length);
            }
            line.close();
        }
    }

    private class Reader implements Runnable {
        private final String sz;
        Reader(String s) {
            sz = s;
            new Thread(this).start();
        }
        public void run() {
            Noise n;
            for (char c: sz.toCharArray()) {
                n = new Noise(c % 20 * 110);
                try {
                    Thread.sleep(100);
                } catch (InterruptedException ex) {
                    continue;
                }
                n.sound = false;
            }
        }
    }

    private static final int SAMPLE_RATE = 16 * 1024;
    private final AudioFormat af = 
            new AudioFormat(SAMPLE_RATE, 8, 1, true, true);
    private HashMap<Character, Noise> noises;

    public static void main(String[] args) {
        new Boop();
    }

    public Boop()  {
        noises = new HashMap<>();
        textArea = new TextArea(20, 80);
        textArea.addKeyListener(this);
        add(textArea);
        pack();
        setVisible(true);
        setDefaultCloseOperation(JFrame.EXIT_ON_CLOSE);
    }

    @Override
    public void keyTyped(KeyEvent ke) {
        try {
            keyReleased(ke);
        } finally {
            char ch = ke.getKeyChar();
            Noise t = new Noise(ch % 20 * 110);
            noises.put(ch, t);
        }
    }

    @Override
    public void keyPressed(KeyEvent ke) {
        if (ke.getKeyCode() == KeyEvent.VK_ENTER)
            new Reader(textArea.getText().toString());
    }

    @Override
    public void keyReleased(KeyEvent ke) {
        try {
            noises.get(ke.getKeyChar()).sound = false;
        } finally {
            return;
        }
    }
}

Answer 1

1) What do the magic numbers represent and why did you choose exactly these values for them?

double angle = 2.0 * Math.PI * i / period * 1.0 + 0.08;
output[i++] += (byte) (Math.sin(angle) * 127f);

(BTW, isn't (2.0 * Math.PI * i) * 1.0 the same than just 2.0 * Math.PI * i?)

n = new Noise(c % 20 * 110);

new AudioFormat(SAMPLE_RATE, 8, 1, true, true);

I'd make them finals and document them.

2) SAMPLE_RATE and af being used before their declaration might confuse a reader of your code at first sight.

3) I'd separate the actual "workhorse" from the UI, i.e. make Noise a separate class.

4) I'd also make Reader a separate class.

Answer 2

Key-to-frequency mapping

Characters with an ASCII code that is a multiple of 20 (such as d) are mapped to 0 Hz, resulting in silence. You wrote new Noise(c % 20 * 110) in two places, so fixing this problem will require changing both call sites. To avoid such duplication, it would be better to have a Noise(char c) constructor that chains to the Noise(int freq) constructor.

Since the frequencies are spaced at uniform intervals of 110 Hz, the higher frequencies will sound similar in pitch. From e to f is one octave; going from v to w is a half step. That's fine, if that's the effect you want.

Your sampling frequency, 16 * 1024 Hz, is sufficiently high to provide good sound quality for the tones you want to generate. However, it's still a magic number pulled from nowhere. I suggest that you stick to conventional values, such as 44100 Hz (CD audio quality) or 48000 Hz (DVD audio quality).

Continuity

When you hold down a key, you'll hear something that sounds like machine gun fire, at 10 rounds per second. It appears that your intention is to play the tone continuously (possibly simultaneously with other tones) until the key is released, in which case a smooth sinusoid would be preferable.

The main cause for the machine-gun effect is that you are writing sounds to the audio device without regard to what is already playing. A secondary problem is that you write 2¹⁴ samples, which is in general not an integral number of wave periods.

The way to play a waveform repeatedly is not to have your own thread pump the audio data to the output, but to use Clip.loop(Clip.LOOP_CONTINUOUSLY).

Your KeyListener handlers need adjusting as well. I would use just keyPressed() and keyReleased(), not keyTyped(). In keyReleased(), the finally is superfluous.

Constructors

Constructors should just construct objects. I don't expect that instantiating a Noise also starts playing it, since new Thread() doesn't imply starting the thread. Similarly, instantiating a Boop shouldn't also cause the frame to be shown, since that's not how JFrames behave. Automatically calling .show() in the constructor violates the substitutability principle for subclasses.

As per the documentation, Swing code should run on the Event Dispatch, not the main thread.

Noise should be a static inner class. af should be a static field of Boop.

Suggested solution

import java.awt.TextArea;
import java.awt.event.KeyEvent;
import java.awt.event.KeyListener;
import java.util.HashMap;
import javax.sound.sampled.AudioFormat;
import javax.sound.sampled.AudioSystem;
import javax.sound.sampled.Clip;
import javax.sound.sampled.LineUnavailableException;
import javax.sound.sampled.SourceDataLine;
import javax.swing.JFrame;
import javax.swing.SwingUtilities;

public class Boop extends JFrame implements KeyListener {
    private static class Noise {
        private int freq;
        private Clip clip;
        private static Clip currentClip;

        public Noise(char c) {
            this((c % 20 + 1) * 110);
        }

        public Noise(int freq) {
            this.freq = freq;
            double slice = 2 * Math.PI * freq / SAMPLE_RATE;
            byte[] waveform = new byte[(int)(SAMPLE_RATE / freq + 1)];
            for (int i = 0; i < waveform.length; i++) {
                waveform[i] = (byte)(127 * Math.sin(i * slice));
            }

            try {
                this.clip = AudioSystem.getClip();
                this.clip.open(af, waveform, 0, waveform.length);
            } catch (LineUnavailableException ex) {
                ex.printStackTrace();
            }
        }

        public void play(int msec) {
            if (this.clip != null) {
                if (Noise.currentClip != null) {
                    Noise.currentClip.drain();
                }
                Noise.currentClip = this.clip;
                this.clip.loop(msec * this.freq / 1000);
            }
        }

        public void start() {
            if (this.clip != null) {
                this.clip.loop(Clip.LOOP_CONTINUOUSLY);
            }
        }

        public void stop() {
            if (this.clip != null) {
                this.clip.stop();
            }
        }
    }

    private static final int SAMPLE_RATE = 44100;
    private static final AudioFormat af = 
            new AudioFormat(SAMPLE_RATE, 8, 1, true, true);
    private HashMap<Character, Noise> noises = new HashMap<>();

    private final TextArea textArea;

    public Boop() {
        textArea = new TextArea(20, 80);
        textArea.addKeyListener(this);
        add(textArea);
        pack();
        setDefaultCloseOperation(JFrame.EXIT_ON_CLOSE);
    }

    public void playText(String sz) {
        for (char c: sz.toCharArray()) {
            (new Noise(c)).play(100);
        }
    }

    @Override
    public void keyTyped(KeyEvent ke) {
    }

    @Override
    public void keyPressed(KeyEvent ke) {
        if (ke.getKeyCode() == KeyEvent.VK_ENTER) {
            playText(textArea.getText().toString());
        } else {
            char ch = ke.getKeyChar();
            if (!noises.containsKey(ch)) {
                Noise t = new Noise(ch);
                noises.put(ch, t);
                t.start();
            }
        }
    }

    @Override
    public void keyReleased(KeyEvent ke) {
        Noise n = noises.get(ke.getKeyChar());
        if (n != null) {
            n.stop();
            noises.remove(ke.getKeyChar());
        }
    }

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