On 29.03.16 15:34, Alex Menkov wrote:
Why you don't want to fix 24 & 32+ bits right now?
Idea to have "partially fixed" does not look good to me (BTW maybe the
test should be extended to verify 24bit conversions too?)

I am still not 100% sure that I understand correctly the code for non-integral types:
- LSB: when the number of bits is not dividable by 8
- 24 bits/32+ bits

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



--
Best regards, Sergey.

Reply via email to