<Sound Dev> [9] Review Request: 8152501 closed/javax/sound/sampled/FileWriter/WaveBigEndian.java failing

[Alex Menkov]

On 29.03.2016 16:19, Sergey Bylokhov wrote:
> 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

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).

> - 24 bits/32+ bits

24bits - the same as 8/16/32bit data

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.

--alex

>
> 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
>>>
>
>