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

Florian Bomers javasound-dev at bome.com
Wed Jul 22 08:29:35 UTC 2015


>>  - 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 ?

it can, but by contract in AudioSystem's
getAudioInputStream(InputStream), the stream must be markable:

"The stream must point to valid audio file data. The implementation of
this method may require multiple parsers to examine the stream to
determine whether they support it. These parsers must be able to mark
the stream, read enough data to determine whether they support the
stream, and, if not, reset the stream's read pointer to its original
position. If the input stream does not support these operation, this
method may fail with an IOException."

Same for AudioSystem.getAudioFileFormat(InputStream).

Best,
Florian


> 
> 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. 
> 

-- 
Florian Bomers
Bome Software

everything sounds.
http://www.bome.com
__________________________________________________________________
Bome Software GmbH & Co KG        Gesellschafterin:
Dachauer Str.187                  Bome Komplementär GmbH
80637 München, Germany            Geschäftsführung: Florian Bömers
Amtsgericht München HRA95502      Amtsgericht München HRB185574



More information about the sound-dev mailing list