<Sound Dev> [9] Review Request: 6459818 Audio A-law and law decoder skip() method not implemented

[Alex Menkov]

Hi Sergey,

Using of bitwise operators to multiplication and arithmetic for division 
looks a bit inconsistent:
return encode ? super.skip(n << 1) / 2 : super.skip(n / 2) << 1;

I think it would be better to use arithmetic for both. The code becomes 
clearer (and compilers are smart enough to do optimizations):
return encode ? super.skip(n * 2) / 2 : super.skip(n / 2) * 2;

regards
Alex

On 03.01.2016 01:52, Sergey Bylokhov wrote:
> Hello, Audio Guru.
>
> Please review the fix for jdk9.
>
> Description of the problem copied from the bug:
> "The AudioInputStream objects returned by the A-Law encoding converters
> do not implement the skip() method correctly. The skip() method is not
> reimplemented and so calls to the skip method of the decoded PCM stream
> skip the underlying original A-law stream. This results in skipping the
> double amount of bytes as intended."
> In short this means that if we have a pcm(16bit) stream on top of the
> alaw(8bit) stream and call the skip of the pcm stream the incorrect
> number of bytes will be skipped.
>
> The bug submitter suggest such patch:
> +
> +    public long skip(long n) throws IOException {
> +            if(n<0){
> +                throw new IOException("Cannot skip negative value!");
> +            }
> +            long skipped=0;
> +            if (encode){
> +                long toSkip = n * 2;
> +                long srcSkipped=0;
> +                do{
> +                    srcSkipped+=super.skip(toSkip-srcSkipped);
> +                }while(srcSkipped % 2 !=0);
> +                skipped=srcSkipped / 2;
> +            }else{
> +                long toSkip = n / 2;
> +                long srcSkipped=super.skip(toSkip);
> +                skipped = srcSkipped * 2;
> +            }
> +            return skipped;
> +        }
>
> But I simplified it a little bit and moved the repeat skip to the parent
> class. Now it is implemented in the similar way like was done in:
> http://cr.openjdk.java.net/~serb/8135160/webrev.01/src/java.desktop/share/classes/com/sun/media/sound/RIFFReader.java.sdiff.html
>
>
>
> Bug: https://bugs.openjdk.java.net/browse/JDK-6459818
> Webrev can be found at: http://cr.openjdk.java.net/~serb/6459818/webrev.00
>