Looks ok to me.
--alex
On 30.03.2016 19:33, Sergey Bylokhov wrote:
Hi, Alex.
On 29.03.16 17:13, Alex Menkov wrote:
- LSB: when the number of bits is not dividable by 8
when sampleSize % 8 != 0 we need to clean unused bits, then we can
handle the data as 8/16/24/etc samples (least significant bits are
zeroes).
32+ bits - as far as I see the converter uses only 32 most significant
bits - you pointed that for 32bit <-> float conversion rounding error is
tiny, so for 32+ bit this way produces accurate enough results too.
I spent some time to understand why it works differently in 32bits vs
32+bits, I finally realize that there is a typo :
AudioFloatConversion32xUB.toByteArray {
....
int x = (int) (in_buff[ix++] * 2147483647.0);
....
}
Note that the literal is double, so
-1.0 * 2147483647.0 is -2147483647, but in case of float literal
-1.0 * 2147483647.0f is -2147483648 which is minimal int value.
Here is a new version, 24 and 32+ bits were fixed:
http://cr.openjdk.java.net/~serb/8152501/webrev.01
I also tested the code when the bits are not dividable by 8, and I see
that the current implementation uses the algorithm you described and
this cause of some lost of precision (so I cannot convert float to
integral then to float again and compare results) I can change the logic
but in separate CR.
My experiments show that there are possible issues related to
overflow/underflow. So I postpones that change until I'll prove that the
current code is ok and only the same change (like in this fix) is
necessary.
On 29.03.2016 14:33, Sergey Bylokhov wrote:
Hello,
Please review the fix for jdk9.
Description:
After the jigsaw integration the closed test WaveBigEndian fails.
The
reason is that the order of service providers is not specified in the
jigsaw(I guess it will be changed in the future).
The test verify that the audio data(pcm signed) during conversion is
not
changed corrupted. The java sound has two codecs which can convert pcm
signed: PCMtoPCMCodec(base on int) and AudioFloatConverter(based on
float). It was found that the test fails when AudioFloatConverter is
used. This is because PCMtoPCMCodec converts data of
pcm_signed/pcm_unsigned directly, and AudioFloatConverter converts
data
to the intermediate floats[-1.0; 1.0] and then converts it to the
target
format.
Example of formulas are used by AudioFloatConverter.
- Signed byte to/from float: byte / 127.0f <--> byte * 127. Has a
problem with a minimum byte "-128". it is outside of "(float) -1"
- Signed byte to/from unsigned: byte + 127 <--> byte - 127. Converts
"-128" to "-1" instead of zero, and 127 to "254" instead of 255.
Similar issues exists for 16 bits/32bits.
The code was change to:
- Signed byte to/from float: byte > 0 ? byte / 127.0f : byte /
128.0f
- Signed byte to/from unsigned: byte + 128 <--> byte - 128.
Note that for 32 bits only the second step is changed,
division/multiplication error is quite tiny in this case.
Also note that PCMtoPCMCodec can be removed after this fix, I'll do
that
later if the performance of two codecs is similar. I guess other
conversions(LSB, 24 bit, 32+) in AudioFloatConverter should be
investigated also, I'll do that later.
Bug: https://bugs.openjdk.java.net/browse/JDK-8152501
Webrev can be found at:
http://cr.openjdk.java.net/~serb/8152501/webrev.00
