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

Alex Menkov alexey.menkov at oracle.com
Thu Feb 18 11:35:12 UTC 2016

Looks ok to me.


On 15.02.2016 18:48, Sergey Bylokhov wrote:
> 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

More information about the sound-dev mailing list