goxr3plus / java-stream-player

🌌Java Advanced Audio Controller Library (WAV, AU, AIFF, MP3, OGG VORBIS, FLAC, MONKEY's AUDIO and SPEEX audio formats )
GNU General Public License v3.0
147 stars 33 forks source link

added setMixerName #65

Closed JellevanAbbema closed 4 years ago

JellevanAbbema commented 4 years ago

Description

Alright this is my first pull request so I hope I'm doing everything right. (Any feedback is welcome!) I came across issue #64 which is something I fixed a while back for a program I'm writing (note: I'm not a software engineer and just doing this as a hobby) at that time I didn't know GitHub that well and had no idea how to share that code the right way, but thought it would be useful to share now for @RinAndShizuka

The only changes I made was add a function to set the mixerName and remove 1 line of code that overwrites the SourceDataLine (which I believe was useless anyway). Overall quite simple since almost all the necessary code is already there. The setMixerName function should be called BEFORE creating a line and thus before opening a file.

Fixes #64

What kind of change does this PR introduce? (check at least one)

Does this PR introduce a breaking change? (check one)

Has This Been Tested?

Checklist:

goxr3plus commented 4 years ago

Great job guys @AObuchow when you feel this will be ready you can merge it :). I am thinking of remaking the repository code more simple as it was before... There are too many classes now and no documentation.

JellevanAbbema commented 4 years ago

Alright so I changed the things that @AObuchow mentioned, I'm trying to write a test (I just recently started to use tests myself so just still figuring it out). Right now (I think) there is no possibility to check the mixer that is currently being used, so also no way to check if it correspond to the set mixer. Should I add a 'getCurrentMixer' function? (or something like that?)

goxr3plus commented 4 years ago

@JellevanAbbema Yes add that function it will be useful.

AObuchow commented 4 years ago

This looks good to me but @HelgeStenstrom might want to have a look.

One thing I noticed is there's no actual dedicated test for get/set mixer? It seems the implementation of the StreamPlayerTest was just modified. It might be nicer to setup a dedicated JUnit test for get/set mixer, so that if it fails we know what "feature" is broken.

goxr3plus commented 4 years ago

Looks good to me also. @JellevanAbbema have you tested that it works correctly, you runned some files and all good? If yes I merge.

JellevanAbbema commented 4 years ago

Yes I did do a couple of tests, played it on the speakers of my second monitor (which is not the default audio device) without a problem.

goxr3plus commented 4 years ago

You the best :)

goxr3plus commented 4 years ago

I think many improvements can be done on this library.

goxr3plus commented 4 years ago

I just made a new release feel free to check and use :)

https://github.com/goxr3plus/java-stream-player/releases

goxr3plus commented 4 years ago

10.0.2

AObuchow commented 4 years ago

Thanks @goxr3plus ! :)