<Sound Dev> [9] Review Request: 8146239 Support of PCM_FLOAT for the AU file format

[Sergey Bylokhov]

Thanks Alex.
Any volunteers for the second review?

On 26.02.16 18:45, Alex Menkov wrote:
> ok, got it.
>
> Approved.
>
> --alex
>
> On 26.02.2016 18:20, Sergey Bylokhov wrote:
>> Hi, Alex.
>> Thanks for review.
>>
>> We have the same issue in most of our readers and writers(for ex
>> WaveFileWriter.java:198). In general our code in lots of place works
>> incorrectly when we convert unsigned integer to long/int and back.
>> I'll fix it in: https://bugs.openjdk.java.net/browse/JDK-6729836
>>
>> On 26.02.16 17:55, Alex Menkov wrote:
>>> Hi Sergey,
>>>
>>> Overall looks good, but could you please fix possible int overflow in
>>> AuFileWriter.java:
>>>
>>>   165         int fileSize;
>>>   166         if (stream.getFrameLength() !=
>>> AudioSystem.NOT_SPECIFIED) {
>>>   167             fileSize =
>>> (int)stream.getFrameLength()*streamFormat.getFrameSize() +
>>> AuFileFormat.AU_HEADERSIZE;
>>>   168         } else {
>>>   169             fileSize = AudioSystem.NOT_SPECIFIED;
>>>   170         }
>>>   171
>>>   172         return new AuFileFormat(Type.AU, fileSize, format,
>>>   173                                 (int) stream.getFrameLength());
>>>
>>> --alex
>>>
>>> On 26.02.2016 16:17, Sergey Bylokhov wrote:
>>>> Hello, Audio Guru.
>>>>
>>>> Please review the small fix for jdk9.
>>>>
>>>> According to specification [1] AU file format can contains the data in
>>>> the float encoding. When support of AU was added to the jdk the float
>>>> encoding was not supported. But in jdk 7 PCM_FLOAT was added(for WAV)
>>>> and it can be reused for AU as well.
>>>>
>>>> Changes description:
>>>>   1) AuFileFormat.java/AuFileReader.java: the pcm_float was added to
>>>> the
>>>> switches.
>>>>   2) AuFileWriter.java:
>>>>     - The "if" statement in getAudioFileFormat() was reworked. Its
>>>> purpose was to change the type PCM_UNSIGNED to PCM_SIGNED(because au
>>>> support only signed pcm). But it was written in some strange manner.
>>>> Note that this method returns the type which should be used in the
>>>> saved
>>>> audio file.
>>>>     - In the getFileStream() I removed the checks and always converts
>>>> the input stream to the AudioFileFormat which was returned from the
>>>> getAudioFileFormat() and the AudioSystem.getAudioInputStream() will
>>>> recognize conversion is needed or not
>>>>
>>>> [1] http://www-mmsp.ece.mcgill.ca/documents/audioformats/AU/AU.html
>>>>
>>>> Bug: https://bugs.openjdk.java.net/browse/JDK-8146239
>>>> Webrev can be found at:
>>>> http://cr.openjdk.java.net/~serb/8146239/webrev.00
>>>>
>>
>>


-- 
Best regards, Sergey.