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.
