Re-assignment of audioInputStream removed #57
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters. Learn more about bidirectional Unicode characters
Some unit tests are improved, but there are more to be done.
Re-assignment of audioInputStream removed by changing audioInputStream = AudioSystem.getAudioInputStream(targetFormat, audioInputStream); to assignment of a new variable. Variable name changes: Initial audioInputStream changed to encodedAudioInputStream audioInputStream after the re-assignment now assigned to a field called decodedAudioInputStream. Old encodedAudioInputStream renamed to encodedAudioInputStreamCopy.
…tile. According to Sonar, Non-primitive fields should not be "volatile" For primitive fields volatile inhibits caching. This is only of interest of the field changes value, but this one doesn't. Since the field is an object reference, value change in this context means that it gets replaced with a different object, but it will not happen. See also https://wiki.sei.cmu.edu/confluence/display/java/CON50-J.+Do+not+assume+that+declaring+a+reference+volatile+guarantees+safe+publication+of+the+members+of+the+referenced+object
StreamPlayerMethodsTest still contains tests that don't pass. The tested methods are candidates for removal from StreamPlayer. All tests in StreamPlayerMethodsTest must be reviewed: Do they actually verify anything that need to be verified? Or are they too coupled with the current implementation? StreamPlayerFutureImprovementTest contains tests that currently fail. Failures are caused by behavior in StreamPlayer which I think is wrong or bad. But I may have misinterpreted the intended behavior.
Improve unit testing
| I finally had time to check this pull request . Also by looking on the folder https://github.com/goxr3plus/java-stream-player/tree/master/src/main/java/com/goxr3plus/streamplayer/application we are having many demos , why we need all of them : )? |
AudioSystem.getMixerInfo() will not return null, so we don't have to …
| The demo applications are outside the scope of this pull request, I think. |
Re-assignment of audioInputStream removed by changing audioInputStream = AudioSystem.getAudioInputStream(targetFormat, audioInputStream); to assignment of a new variable. Variable name changes: Initial audioInputStream changed to encodedAudioInputStream audioInputStream after the re-assignment now assigned to a field called decodedAudioInputStream. Old encodedAudioInputStream renamed to encodedAudioInputStreamCopy.
…tile. According to Sonar, Non-primitive fields should not be "volatile" For primitive fields volatile inhibits caching. This is only of interest of the field changes value, but this one doesn't. Since the field is an object reference, value change in this context means that it gets replaced with a different object, but it will not happen. See also https://wiki.sei.cmu.edu/confluence/display/java/CON50-J.+Do+not+assume+that+declaring+a+reference+volatile+guarantees+safe+publication+of+the+members+of+the+referenced+object
…nstrom/java-stream-player into decodedAudioInputStream
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Add this suggestion to a batch that can be applied as a single commit. This suggestion is invalid because no changes were made to the code. Suggestions cannot be applied while the pull request is closed. Suggestions cannot be applied while viewing a subset of changes. Only one suggestion per line can be applied in a batch. Add this suggestion to a batch that can be applied as a single commit. Applying suggestions on deleted lines is not supported. You must change the existing code in this line in order to create a valid suggestion. Outdated suggestions cannot be applied. This suggestion has been applied or marked resolved. Suggestions cannot be applied from pending reviews. Suggestions cannot be applied on multi-line comments. Suggestions cannot be applied while the pull request is queued to merge. Suggestion cannot be applied right now. Please check back later.
Description
Related to #55
Re-assignment of audioInputStream removed by changing
audioInputStream = AudioSystem.getAudioInputStream(targetFormat, audioInputStream);
to assignment of a new variable.
Variable name changes: Initial audioInputStream changed to encodedAudioInputStream
audioInputStream after the re-assignment now assigned to a field called decodedAudioInputStream.
Old encodedAudioInputStream renamed to encodedAudioInputStreamCopy.
encodedAudioInputStream doesn't change, so it doesn't have to be volatile.
According to Sonar, Non-primitive fields should not be "volatile"
For primitive fields volatile inhibits caching. This is only of interest of the field changes value, but this one doesn't. Since the field is an object reference, value change in this context means that it gets replaced with a different object, but it will not happen.
See also https://wiki.sei.cmu.edu/confluence/display/java/CON50-J.+Do+not+assume+that+declaring+a+reference+volatile+guarantees+safe+publication+of+the+members+of+the+referenced+object
There are no dependencies for this change.
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: