Sound Fixes for Linux/OpenJDK #252
Conversation
…at already exists because the pulseaudio implementation is a little split-brained. This fixes a problem where many sound effects were not playing on Linux systems using pulseaudio.
| DataLine.Info info = new DataLine.Info(Clip.class, format); | ||
| c = (Clip) AudioSystem.getLine(info); | ||
| if (format != null) | ||
| c.open(format, audioData, 0, audioData.length); | ||
| // This is a little hacky, but we can't do an instanceof check and | ||
| // there's no reason to add the listener unless the system is using | ||
| // PulseAudioClip et al. |
There was a problem hiding this comment.
What is the "et al." referring to here?
There was a problem hiding this comment.
I meant the other classes related to to the pulse audio backend, but all that really matters is the PulseAudioClip check so I'll remove that bit in the comment.
| // there's no reason to add the listener unless the system is using | ||
| // PulseAudioClip et al. | ||
| if (c.getClass().getSimpleName().equals("PulseAudioClip")) { | ||
| c.addLineListener(new PulseAudioFixerListener(c)); |
There was a problem hiding this comment.
I'd prefer if you inline the listener instead of defining a new class for it.
Also, it'd be helpful to comment on any specific version numbers (at least for your system) where this is applicable.
There was a problem hiding this comment.
When I first wrote the listener, I had it inlined, but it resulted in a proliferation of threads. I moved the listener to a defined class and switch to a static cached thread pool which works really well at keeping extraneous threads low. I also like keeping the ugliness tied to a particular Clip implementation separate.
That being said, I can move the static cached thread pool to the Multiclip class and inline the listener or I can do something like keep the class declaration and just move it to the MutlipClip file. What do you think?
| @@ -186,10 +195,17 @@ private Clip getClip() throws LineUnavailableException { | |||
| } else { | |||
| // create a new clip | |||
| // NOTE: AudioSystem.getClip() doesn't work on some Linux setups. | |||
| Log.debug("creating a new clip"); | |||
There was a problem hiding this comment.
Could you get rid of these? :P
|
Looks good, thank you for doing this! :) Left a few comments in the diff. |
|
@itdelatrisu will you merge this PR? Thank you! |
Signed-off-by: Jeffrey Han <itdelatrisu@gmail.com>
The PR of mine prior to this fixed the loading of audio clips on Linux, but they would only play once. After lots of digging into the internals of the icedtea-sound library, I've finally gotten all hit sounds and sound effects to play consistently using the OpenJDK and pulseaudio on Linux. I've documented most things that aren't obvious, but let me know if you want more details or clarification.