1144087 - Audio glitches on OpenBSD caused by rounding errors.

Status: RESOLVED FIXED

(Core :: Audio/Video, defect)

Description

[Alexandre Ratchov]

Attachment: diff to fix glitches

Audio decoders may produce samples slightly outside the [-1:1] interval (eg while decoding sound of youtube videos). This causes integer overflows during convertion from floats to signed integers. In turn this produces audible
glitches.

The attached diff fixes glitches by clipping samples to avoid overflows.

Comment 1

[Chris Pearce [:cpearce (Not reading bugmail)]]

Comment on attachment 8578598 [details] [diff] [review]
diff to fix glitches

Review of attachment 8578598 [details] [diff] [review]:
-----------------------------------------------------------------

Matthew should handle this.

Comment 2

[Matthew Gregan [:kinetik]]

Comment on attachment 8578598 [details] [diff] [review]
diff to fix glitches

Review of attachment 8578598 [details] [diff] [review]:
-----------------------------------------------------------------

Clamping during the conversion is definitely a good idea.  Thanks for the patch.

Comment 3

[Jean-Yves Avenard [:jya]]

the rounding from float to int introduces unnecessary noise.
float -> int cast works as ceil on negative number and floor on positive number rather than rounding to the nearest int.

use lrintf on the result of the float multiplication and then clip.

Comment 4

[Yoshihisa Matsushita]

Alternatively, it could compile with MOZ_SAMPLE_TYPE_S16 instead of MOZ_SAMPLE_TYPE_FLOAT32.

Comment 5

[Alexandre Ratchov]

(In reply to Jean-Yves Avenard [:jya] from comment #3)
> 
> use lrintf on the result of the float multiplication and then clip.

i agree, thanks for the tip

Comment 6

[Alexandre Ratchov]

Attachment: version 2 diff, with call to lrintf

New diff, with call to lrintf() to reduce quatization noise, as Jean-Yves suggested. This is an independent problem, but let's improve the noise level as we're at it. Tested on OpenBSD/amd64.

Comment 7

[Matthew Gregan [:kinetik]]

Comment on attachment 8579545 [details] [diff] [review]
version 2 diff, with call to lrintf

Thanks for updating the patch, looks good!

Comment 8

[Yoshihisa Matsushita]

(In reply to Alexandre Ratchov from comment #6)
> Created attachment 8579545 [details] [diff] [review]
> version 2 diff, with call to lrintf
> 
> New diff, with call to lrintf() to reduce quatization noise, as Jean-Yves
> suggested. This is an independent problem, but let's improve the noise level
> as we're at it. Tested on OpenBSD/amd64.

Also tested on OpenBSD/amd64, OK for me. Thanks!

Comment 9

[Carsten Book [:Tomcat]]

https://hg.mozilla.org/integration/mozilla-inbound/rev/535571bc0164

Comment 10

[Wes Kocher (:KWierso) (Not reading bugmail; email directly if needed)]

https://hg.mozilla.org/mozilla-central/rev/535571bc0164