<Sound Dev> [9] Review Request: 8038139 AudioInputStream.getFrameLength() returns wrong value for floating-point WAV

[Sergey Bylokhov]

Hi, Alex.
Thanks for a review, it took some time to understand why it was not 
found by the test, see my comments inline.

On 08.02.16 13:28, Alex Menkov wrote:
> It seems to me that a-law & m-law can be only 8bit,

It is not necessary m-law 8bit, because we also support PCM_SIGNED and 
different samplesize.

but if you updated
> calculation in getFileStream method, I suppose write method should be
> updated too:

Yes, you are right:
http://cr.openjdk.java.net/~serb/8038139/webrev.02

This fix have a few new changes:
  - The calculation in the write() method was updated(as you suggested).
  - The test now verifies the write to the real file, when the size of 
the stream is unknown/unspecified. This will hit uncovered issue.
  - Aiff file format contains the number of frames in two places(in two 
different chunks).
  a) The first place contain it as is: numSampleFrames, Your comment 
about incorrect code in write() method was about this place.
  b) The second place contains the sound data size + offset of data in 
bytes which should be divided by the samplesize.
When I wrote a test I found that our code use the second place but 
actually calculate the size incorrectly(was mentioned in comment). So I 
switched the code to use the first place and filed the separate CR to 
fix the second place:
https://bugs.openjdk.java.net/browse/JDK-8149649

I adds a few todo which will be fixed in JDK-8149649/JDK-6729836/JDK-8132782

> On 05.02.2016 20:20, Sergey Bylokhov wrote:
>> Hello, Audio Guru.
>>
>> Please review the fix for jdk9.
>> There are two bugs in this area:
>>   - in the WaveExtensibleFileReader.java/WaveFloatFileReader.java we
>> create the AudioInputStream using the data size in bytes instead of the
>> the size in frames. In the fix the size in bytes is divided by the size
>> of the frame.
>>   - in AiffFileWriter.java we incorrectly calculate the data size in
>> bytes(for example if the sample size is less than 8). instead of
>>      numFrames * channels * sampleSize / 8
>> it should be
>>      numFrames * channels * ((sampleSize + 7) / 8)
>>
>> And when later we read this data in AiffFilereader we incorrectly
>> calculate the FrameLength.
>>
>> In the test I created some fake stream and converted it to a different
>> formats with expectation that FrameLength will be preserved.
>>
>> Bug: https://bugs.openjdk.java.net/browse/JDK-8038139
>> Webrev can be found at:
>> http://cr.openjdk.java.net/~serb/8038139/webrev.00
>>


-- 
Best regards, Sergey.