Java Sound GUI using MVC model

Question

I have made a Java Swing application that detects and displays audio pitch and level from an input (e.g microphone). I would like feedback on the current structure of my project, I'm attempting to follow the modified MVC pattern found here, diagram of my project below:

I'm seeking feedback on the structure and any other criticisms. Some thoughts: I don't know if I need the PitchLabel class, it's quite small and doesn't do much, so could be needless. I start a thread for the audio dispatcher (from a public pitch detection API "TarsosDSP"), but I'm not sure if this is the best way to implement it. I only have one user interface on the App's JFrame, that being the record button, but should I add another I think I would need to change the model, to encapsulate the controllers.

App.java - GUI

import com.formdev.flatlaf.FlatLightLaf; import javax.swing.*; import javax.swing.border.EmptyBorder; import java.awt.*; public class App { public static void main(String[] args) { EventQueue.invokeLater(new Runnable() { @Override public void run() { FlatLightLaf.setup(); JFrame mainFrame = new JFrame("JavaTuner"); mainFrame.setDefaultCloseOperation(WindowConstants.EXIT_ON_CLOSE); mainFrame.setSize(1024, 600); JPanel panel = new JPanel(new BorderLayout()); panel.setBorder(new EmptyBorder(25, 50, 25, 50)); AudioBar bar = new AudioBar(); bar.setPreferredSize(new Dimension(20, 100)); panel.add(bar, BorderLayout.EAST); PitchLabel pitchLabel = new PitchLabel("Pitch: ", SwingConstants.CENTER); Action recordBtnAction = new RecordButtonAction(bar, pitchLabel); recordBtnAction.putValue(Action.NAME, "Start Recording"); JToggleButton recordBtn = new JToggleButton(recordBtnAction); panel.add(pitchLabel, BorderLayout.CENTER); mainFrame.setContentPane(panel); mainFrame.add(recordBtn, BorderLayout.WEST); mainFrame.setVisible(true); } }); } } 

RecordButtonAction.java - Acts when the 'Start Recording' button is pressed

import javax.swing.*; import java.awt.event.ActionEvent; public class RecordButtonAction extends AbstractAction { private final AudioBar bar; private final PitchLabel label; private Mic m; public RecordButtonAction(AudioBar bar, PitchLabel label){ this.bar = bar; this.label = label; } @Override public void actionPerformed(ActionEvent e) { JToggleButton toggle = (JToggleButton)e.getSource(); if(toggle.isSelected()){ toggle.setText("Stop Recording"); System.out.println("Streaming Started"); m = new Mic(RecordButtonAction.this); } else{ toggle.setText("Start Recording"); System.out.println("Recording Ended"); m.stopRecording(); updateLabel("Stopped Recording"); } } public void updateLabel(String str){ label.setText(str); } public void updateAudioBar(float rms) { bar.setAmplitude(rms); } } 

Mic.java - Responsible for recording audio from input

import be.tarsos.dsp.AudioDispatcher; import be.tarsos.dsp.AudioEvent; import be.tarsos.dsp.io.jvm.JVMAudioInputStream; import be.tarsos.dsp.pitch.*; import javax.sound.sampled.AudioFormat; import javax.sound.sampled.AudioInputStream; import javax.sound.sampled.AudioSystem; import javax.sound.sampled.DataLine; import javax.sound.sampled.LineUnavailableException; import javax.sound.sampled.TargetDataLine; import java.util.*; public class Mic{ private final RecordButtonAction recordButtonAction; private static AudioDispatcher dispatcher; private TargetDataLine targetLine; private AudioInputStream inputStream; public Mic(RecordButtonAction recordBtnAction) { this.recordButtonAction = recordBtnAction; this.initDataLines(); this.startRecording(); } private void initDataLines() { AudioFormat format = new AudioFormat(44100.0F, 16, 2, true, false); try { DataLine.Info info = new DataLine.Info(TargetDataLine.class, format); targetLine = (TargetDataLine)AudioSystem.getLine(info); targetLine.open(); inputStream = new AudioInputStream(targetLine); } catch (LineUnavailableException e) { e.printStackTrace(); } } private void startRecording() { JVMAudioInputStream audioInputStream = new JVMAudioInputStream(inputStream); targetLine.start(); float sampleRate = 44100; int bufferSize = 6000; int overlap = 0; dispatcher = new AudioDispatcher(audioInputStream, bufferSize, overlap); dispatcher.addAudioProcessor(new PitchProcessor(PitchProcessor.PitchEstimationAlgorithm.YIN, sampleRate, bufferSize, new Pitch(recordButtonAction))); new Thread(dispatcher, "Mic Audio Dispatch Thread").start(); } public void stopRecording(){ dispatcher.stop(); targetLine.close(); targetLine.stop(); } } 

Pitch.java - Responsible for pitch detection, implements interface from TarsosDSP public pitch detection API

import be.tarsos.dsp.AudioEvent; import be.tarsos.dsp.pitch.PitchDetectionHandler; import be.tarsos.dsp.pitch.PitchDetectionResult; import java.util.ArrayDeque; import java.util.Queue; public class Pitch implements PitchDetectionHandler { private final int RMS_STORE_SIZE = 10; // For testing purposes private final double MIC_AMBIENCE_RMS = 0.000711; private final RecordButtonAction recordBtnAction; private final Queue<Double> previousRMSUnits; private double avgPreviousRMSUnits = 0; public Pitch(RecordButtonAction recordBtnAction){ this.recordBtnAction = recordBtnAction; previousRMSUnits = new ArrayDeque<Double>(); } // handlePitch is called after the Mic.startRecording() method, and runs continuously // until Mic.stopRecording() is called @Override public void handlePitch(PitchDetectionResult pitchDetectionResult, AudioEvent audioEvent) { if(previousRMSUnits.size() == RMS_STORE_SIZE){ previousRMSUnits.remove(); previousRMSUnits.add(audioEvent.getRMS()); double sum = previousRMSUnits.stream().reduce(Double::sum).get(); avgPreviousRMSUnits = sum / RMS_STORE_SIZE; } else{ previousRMSUnits.add(audioEvent.getTimeStamp()); } recordBtnAction.updateAudioBar((float) (avgPreviousRMSUnits + MIC_AMBIENCE_RMS) * 5); if(pitchDetectionResult.getPitch() != -1){ double timeStamp = audioEvent.getTimeStamp(); float pitch = pitchDetectionResult.getPitch(); recordBtnAction.updateLabel((String.format("Pitch detected at %.2fs: %.2fHz\n", timeStamp, pitch*2))); } } } 

AudioBar.java - Graphics component, a volume bar

import javax.swing.*; import java.awt.*; // Using implementation from Radiodef's stackoverflow answer // Link: https://stackoverflow.com/questions/26574326/how-to-calculate-the-level-amplitude-db-of-audio-signal-in-java public class AudioBar extends JComponent { private int meterWidth = 10; private float amp = 0f; public void setAmplitude(float amp) { this.amp = Math.abs(amp); repaint(); } public void setMeterWidth(int meterWidth) { this.meterWidth = meterWidth; } @Override protected void paintComponent(Graphics g) { int w = Math.min(meterWidth, getWidth()); int h = getHeight(); int x = getWidth() / 2 - w / 2; int y = 0; g.setColor(Color.LIGHT_GRAY); g.fillRect(x, y, w, h); g.setColor(Color.BLACK); g.drawRect(x, y, w - 1, h - 1); int a = Math.round(amp * (h - 2)); g.setColor(Color.GREEN); g.fillRect(x + 1, y + h - 1 - a, w - 2, a); } @Override public Dimension getMinimumSize() { Dimension min = super.getMinimumSize(); if (min.width < meterWidth) min.width = meterWidth; if (min.height < meterWidth) min.height = meterWidth; return min; } @Override public Dimension getPreferredSize() { Dimension pref = super.getPreferredSize(); pref.width = meterWidth; return pref; } @Override public void setPreferredSize(Dimension pref) { super.setPreferredSize(pref); setMeterWidth(pref.width); } } 

PitchLabel.java - Graphics component, a label that displays the pitch of audio streaming

import javax.swing.*; import java.awt.*; public class PitchLabel extends JLabel { private final Font f = new Font("Segoe UI", Font.PLAIN, 22); public PitchLabel(String text, int horizontalAlignment) { super(text, horizontalAlignment); this.setFont(f); } @Override public void setText(String text) { super.setText(text); } } 

Code in git repo here

Answer 1

It's not a bad implementation. I did find the architecture a bit questionable at times, but it's definitely not awful.

That said, I do find the communication between components to be a bit muddled, and the RecordButtonAction class in particular feels a bit overloaded.

But this leaves the question: How do we update the display (audiobar and label) upon receiving an event?

An intuitive way to do that would be to either send that event to each of those components directly by making them implement PitchDetectionHandler, or to have some parent component which does that. Since there is a single JPanel which contains all presentation-related components, making that panel handle the events isn't unreasonable, but some might argue that it could be more future-proof to use multiple PitchDetectionHandlers instead.

Either way, Mic could then take a PitchDetectionHandler (or perhaps a Collection of them), which it attaches to the dispatcher. That way, once the Mic has been created, the record button needn't know who actually listens to the mic - the mic knows who to talk to, and the record button knows about the mic, and that's enough. Makes for a fairly neat data flow in my eyes.

I also have a few other comments about a few of the classes

RecordButtonAction

The idea behind Action is that the action provides state that the button respects, while the action itself needn't know about the button (and can thus be attached to multiple buttons at once if need be). This action, however, explicitly checks the state of the button it's attached to and directly modifies the button as well.

Conventionally, the state would be stored on the action rather than the button, and instead of updating the button directly, the action would update its own properties (NAME and SELECTED_KEY seem appropriate here)

And, briefly going back to the subject of communication, updateLabel and updateAudioBar kind of feel like they don't belong here. The Action itself never actually touches the AudioBar, so expecting others to go through this Action to get to it feels a bit strange to me.

Mic

initDataLines swallows an exception and returns successfully, but if that exception was thrown in the first place, the Mic object will not have a valid audio line, and future uses of the Mic will probably fail with something akin to a NullPointerException or IllegalStateException. I'd argue that initDataLines (and, by extension, the constructor itself) should be declared as throws LineUnavailableException, or at the very least the exception could be wrapped in an unchecked exception and re-thrown to make sure it fails sooner rather than later.

Pitch

handlePitch adding timestamps instead of RMSs at the start looks like it might be a mistake.

I'm also not seeing an obvious reason to not calculate the average while there are fewer than 10 entries in the RMS list.

PitchLabel

In its current state, I don't think this is terribly useful. All it is is a JLabel that sets its own font to Segoe UI. It's only used in one place. It might be easier to just create a regular JLabel and set its font manually.

Put that all together, what do you get?

Well, there are many ways to do it, but it might look a bit like the following:

App.java

import java.awt.BorderLayout; import java.awt.EventQueue; import javax.sound.sampled.LineUnavailableException; import javax.swing.JFrame; import javax.swing.JToggleButton; import javax.swing.JOptionPane; import javax.swing.WindowConstants; import javax.swing.border.EmptyBorder; public class App { public static void main(String[] args) throws LineUnavailableException { EventQueue.invokeLater(() -> { try { JFrame mainFrame = new JFrame("JavaTuner"); mainFrame.setDefaultCloseOperation(WindowConstants.EXIT_ON_CLOSE); mainFrame.setSize(1024, 600); PitchPanel displayPanel = new PitchPanel(); JToggleButton recordBtn = new JToggleButton(new RecordButtonAction(new Mic(displayPanel))); mainFrame.setContentPane(displayPanel); mainFrame.add(recordBtn, BorderLayout.WEST); mainFrame.setVisible(true); } catch (LineUnavailableException ex) { JOptionPane.showMessageDialog(null, "Mic not available."); } }); } } 

AudioBar.java

(unchanged)

Mic.java

import be.tarsos.dsp.AudioDispatcher; import be.tarsos.dsp.io.jvm.JVMAudioInputStream; import be.tarsos.dsp.pitch.PitchDetectionHandler; import be.tarsos.dsp.pitch.PitchDetectionResult; import be.tarsos.dsp.pitch.PitchProcessor; import javax.sound.sampled.AudioFormat; import javax.sound.sampled.AudioInputStream; import javax.sound.sampled.AudioSystem; import javax.sound.sampled.DataLine; import javax.sound.sampled.LineUnavailableException; import javax.sound.sampled.TargetDataLine; public class Mic { private static final AudioFormat AUDIO_FORMAT = new AudioFormat(44100.0F, 16, 2, true, false); private AudioDispatcher dispatcher; private AudioInputStream inputStream; private final TargetDataLine line; private final PitchDetectionHandler onDetection; public Mic(PitchDetectionHandler onDetection) throws LineUnavailableException { this.onDetection = onDetection; DataLine.Info info = new DataLine.Info(TargetDataLine.class, AUDIO_FORMAT); line = (TargetDataLine) AudioSystem.getLine(info); this.startRecording(); } public void startRecording() throws LineUnavailableException { line.open(); JVMAudioInputStream audioInputStream = new JVMAudioInputStream(new AudioInputStream(line)); line.start(); float sampleRate = 44100; int bufferSize = 6000; int overlap = 0; dispatcher = new AudioDispatcher(audioInputStream, bufferSize, overlap); dispatcher.addAudioProcessor(new PitchProcessor(PitchProcessor.PitchEstimationAlgorithm.YIN, sampleRate, bufferSize, onDetection)); new Thread(dispatcher, "Mic Audio Dispatch Thread").start(); } public void stopRecording() { dispatcher.stop(); line.stop(); line.close(); } } 

PitchPanel.java

import be.tarsos.dsp.AudioEvent; import be.tarsos.dsp.pitch.PitchDetectionHandler; import be.tarsos.dsp.pitch.PitchDetectionResult; import java.awt.BorderLayout; import java.awt.Dimension; import java.awt.Font; import java.util.ArrayDeque; import java.util.Queue; import javax.sound.sampled.LineUnavailableException; import javax.swing.border.EmptyBorder; import javax.swing.JLabel; import javax.swing.JPanel; import javax.swing.SwingConstants; public class PitchPanel extends JPanel implements PitchDetectionHandler { private static final int RMS_STORE_SIZE = 10; // For testing purposes private static final double MIC_AMBIENCE_RMS = 0.000711; private static final Font PITCH_LABEL_FONT = new Font("Segoe UI", Font.PLAIN, 22); private final JLabel pitchLabel; private final AudioBar bar; private final Queue<Double> previousRMSUnits = new ArrayDeque<>(); public PitchPanel() throws LineUnavailableException { super(new BorderLayout()); setBorder(new EmptyBorder(25, 50, 25, 50)); bar = new AudioBar(); bar.setPreferredSize(new Dimension(20, 100)); add(bar, BorderLayout.EAST); pitchLabel = new JLabel("Pitch: ", SwingConstants.CENTER); pitchLabel.setFont(PITCH_LABEL_FONT); add(pitchLabel, BorderLayout.CENTER); } @Override public void handlePitch(PitchDetectionResult result, AudioEvent event) { double avgPreviousRMSUnits = 0; previousRMSUnits.add(event.getRMS()); if (previousRMSUnits.size() > RMS_STORE_SIZE) { previousRMSUnits.remove(); double sum = previousRMSUnits.stream().reduce(Double::sum).get(); avgPreviousRMSUnits = sum / RMS_STORE_SIZE; } bar.setAmplitude((float) (avgPreviousRMSUnits + MIC_AMBIENCE_RMS) * 5); if (result.getPitch() != -1){ double timeStamp = event.getTimeStamp(); float pitch = result.getPitch(); pitchLabel.setText(String.format("Pitch detected at %.2fs: %.2fHz\n", timeStamp, pitch * 2)); } } } 

RecordButtonAction.java

import javax.sound.sampled.LineUnavailableException; import java.awt.event.ActionEvent; import javax.swing.AbstractAction; import javax.swing.JOptionPane; public class RecordButtonAction extends AbstractAction { private final Mic mic; private boolean isRecording; { isRecording = false; updateValues(); } public RecordButtonAction(Mic mic) throws LineUnavailableException { this.mic = mic; } @Override public void actionPerformed(ActionEvent event) { try { if (isRecording) { mic.stopRecording(); } else { mic.startRecording(); } isRecording = ! isRecording; updateValues(); } catch (LineUnavailableException ex) { JOptionPane.showMessageDialog(null, "Mic not available, try again later.", "Error", JOptionPane.ERROR_MESSAGE); } } private void updateValues() { putValue(SELECTED_KEY, isRecording); putValue(NAME, isRecording ? "Stop recording" : "Record"); } } 

Answer 2

Without deep review I have to point out, that this static declaration is potentially very dangerous and can lead into hard-to-detect bugs. It also doesn't allow multiple usages of your Mic class no matter how unlikely would that be:

private static AudioDispatcher dispatcher;