On 07/21/2015 08:17 AM, Sergey Bylokhov wrote:
Hello.
Please review the fix for jdk9. Two issues were fixed:
8013586: audioInputStream.close() does not release the resource
8130305: AudioSystem behavior depends on order that providers are located
Description:
- Streams were not closed when necessary(ex: WaveFloatFileReader.
getAudioInputStream(File)). In the fix they are closed always in case
of any exceptions.
- Some of the readers do not reset the streams when they throw an
UnsupportedAudioFileException exception.
Is this the issue behind the somewhat cryptic "8130305: AudioSystem
behavior depends on order that providers are located" ?
- Some of the readers(like AiffFileReader) do not wrap
(FileInputStream, etc) in BufferedInputStream.
80 public AudioInputStream getAudioInputStream(final InputStream stream)
81 throws UnsupportedAudioFileException, IOException {
82 stream.mark(200);
83 try {
84 final AudioFileFormat fileFormat =
getAudioFileFormatImpl(stream);
You aren't wrapping the stream here. This is OK if it is called via one
of the other
two methods that do, but can it not be called directly ?
Same for
public final AudioFileFormat getAudioFileFormat(final InputStream stream)
but it may not matter on the methods that just read a few bytes of header
anyway ..
stream.mark(200);
seems arbitrary and I assume it was your calculation of the max likely to
be needed by any subclass but it would be good to add a comment about this.
-phil.
In the fix I merge all implementations of getAudioXXX to the
SunFileReader.
Note that I added a comment to the
SunFileReader.getAudioFileFormat(InputStream) that implementation
contradicts the specification, and it seems the javadoc should be
updated? And I created a new issue about EOFException:
https://bugs.openjdk.java.net/browse/JDK-8131974
Also I have a related question about WaveExtensibleFileReader why it
was not added to the spi.AudioFileReader? Because of this
WaveExtensibleFileReader actually is never used. Is that intentionally?
Bug: https://bugs.openjdk.java.net/browse/JDK-8013586
Webrev can be found at: http://cr.openjdk.java.net/~serb/8013586/webrev.02
--
Best regards, Sergey.
