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

[Phil Race]

So .. I suggest to add a comment about where "200" came from. Otherwise OK

-phil.
On 7/21/15 6:12 PM, Sergey Bylokhov wrote:
> 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/20150724/62c702f5/attachment.html>