<Sound Dev> [9] Review Request: 8013586 and 8130305

[Sergey Bylokhov]

On 22.07.15 1:25, Phil Race wrote:
>
> Is this the issue behind the somewhat cryptic "8130305: AudioSystem 
> behavior depends on order that providers are located" ?
Yes, the order is changed, the test caused a problem in the latest 
reader, and the reset of stream was unnecessary, so the bug was not visible.
>
>   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 ..
In both cases this stream is passed to the method by the user, so I 
guess this is responsibility of the user what the stream it is passed. 
All old readers do not wrap it before.

>
>
> 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.
I select the biggest value which was used in WaveFloatFileReader, all 
other readers used smaller value.
>
>
>
> -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.
>


-- 
Best regards, Sergey.

-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://mail.openjdk.java.net/pipermail/sound-dev/attachments/20150722/e12b6027/attachment.html>