<html>
<head>
<meta content="text/html; charset=windows-1252"
http-equiv="Content-Type">
</head>
<body bgcolor="#FFFFFF" text="#000000">
<div class="moz-cite-prefix">On 07/21/2015 08:17 AM, Sergey Bylokhov
wrote:<br>
</div>
<blockquote cite="mid:55AE626D.70107@oracle.com" type="cite">
<meta http-equiv="content-type" content="text/html;
charset=windows-1252">
<tt>Hello.<br>
Please review the fix for jdk9. Two issues were fixed:</tt><br>
<tt>
<meta name="qrichtext" content="1">
8013586: audioInputStream.close() does not release the resource<br>
8130305: AudioSystem behavior depends on order that providers
are located<br>
<meta http-equiv="Content-Type" content="text/html;
charset=windows-1252">
<style type="text/css">
p, li { white-space: pre-wrap; }</style><br>
Description:<br>
- Streams were not closed when necessary(ex: </tt><tt>
<meta name="qrichtext" content="1">
WaveFloatFileReader.
<meta http-equiv="Content-Type" content="text/html;
charset=windows-1252">
<style type="text/css">
p, li { white-space: pre-wrap; }
</style>getAudioInputStream(File)). In the fix they are closed always in
case of any exceptions.<br>
- Some of the readers do not reset the streams when they throw
an UnsupportedAudioFileException exception.<br>
</tt></blockquote>
<br>
<tt>Is this the issue behind the somewhat cryptic "</tt><tt>8130305:
AudioSystem behavior depends on order that providers are located"
?<br>
<br>
</tt>
<blockquote cite="mid:55AE626D.70107@oracle.com" type="cite"><tt> -
Some of the readers(like AiffFileReader) do not wrap
(FileInputStream, etc) in BufferedInputStream.</tt></blockquote>
<br>
<pre><span class="changed"> 80 public AudioInputStream getAudioInputStream(final InputStream stream)</span>
<span class="changed"> 81 throws UnsupportedAudioFileException, IOException {</span>
<span class="changed"> 82 stream.mark(200);</span>
<span class="changed"> 83 try {</span>
<span class="changed"> 84 final AudioFileFormat fileFormat = getAudioFileFormatImpl(stream);</span></pre>
<tt>You aren't wrapping the stream here. This is OK if it is called
via one of the other<br>
two methods that do, but can it not be called directly ?<br>
<br>
Same for</tt>
<pre><span class="changed">public final AudioFileFormat getAudioFileFormat(final InputStream stream)
but it may not matter on the methods that just read a few bytes of header anyway ..
</span><span class="changed">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.
</span></pre>
<tt> </tt>
<blockquote cite="mid:55AE626D.70107@oracle.com" type="cite"><tt> <br>
<br>
In the fix I merge all implementations of getAudioXXX to the
SunFileReader.<br>
<br>
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: <a moz-do-not-send="true"
class="moz-txt-link-freetext"
href="https://bugs.openjdk.java.net/browse/JDK-8131974">https://bugs.openjdk.java.net/browse/JDK-8131974</a><br>
</tt><br>
<tt>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?<br>
<br>
Bug: <a moz-do-not-send="true" class="moz-txt-link-freetext"
href="https://bugs.openjdk.java.net/browse/JDK-8013586">https://bugs.openjdk.java.net/browse/JDK-8013586</a><br>
Webrev can be found at: <a moz-do-not-send="true"
class="moz-txt-link-freetext"
href="http://cr.openjdk.java.net/%7Eserb/8013586/webrev.02">http://cr.openjdk.java.net/~serb/8013586/webrev.02</a></tt><br>
<pre class="moz-signature" cols="72">--
Best regards, Sergey. </pre>
</blockquote>
<br>
</body>
</html>