851530 - Cannot play WAV file with u-law compression encoding

Status: RESOLVED FIXED

(Core :: Audio/Video: Playback, defect, P5)

Description

[Mikael]

Attachment: tycisco.wav WAV sample to test

User Agent: Mozilla/5.0 (Windows NT 6.1; WOW64; rv:19.0) Gecko/20100101 Firefox/19.0
Build ID: 20130307023931

Steps to reproduce:

I play WAV files in an <audio> tag.
These WAV files have the following format
    Audio sample size: 8 bit
    Channels: 1 (mono)
    Audio sample rate: 8 kHz
    Audio format: CCITT u-Law

This WAV format is in use in prompts in some calling systems.

Samples sound files can be downloaded here
http://chudinov.net/mix/ciscowav/tycisco.wav
http://chudinov.net/mix/ciscowav/beep.wav




Actual results:

Nothing happened.


Expected results:

FireFox should play sound in the <audio> tag.
It works with other WAV files. But not with this low rate mono u-Law WAVs.

Comment 1

[Mikael]

Attachment: WAV file to test

Comment 2

[Mikael]

Here you can test some WAV files 
http://chudinov.net/mix/ciscowav/audio.html

FireFox plays high rate WAVs 22kHz with no problem.
But it can't play low rate CCITT u-Law WAVs.

Comment 3

[Matthias Versen [:Matti]]

related/dupe of bug 524109

Comment 4

[Louis Christie [:lchristie]]

With u-Law encoding the conversion is between uncompressed signed 16bit integers and compressed 8bit integers, so there is no need for support of 24bit wave files to play these.

Comment 5

[Louis Christie [:lchristie]]

Attachment: bug-851530-fix.patch

This code depends on the new code for the Wave Media Data Decoder in bug 1231793.

Comment 7

[Louis Christie [:lchristie]]

https://treeherder.mozilla.org/#/jobs?repo=try&revision=e8c3ba5f5e1e

Comment 8

[Louis Christie [:lchristie]]

Attachment: bug-851530-fix.patch

Added test cases, fixed one typo.

Comment 9

[Louis Christie [:lchristie]]

https://treeherder.mozilla.org/#/jobs?repo=try&revision=243b986eb82e

Comment 10

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

Comment on attachment 8706580 [details] [diff] [review]
bug-851530-fix.patch

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

Please re-request review when the WaveDemuxer patches are stable.

Comment 11

[Louis Christie [:lchristie]]

Attachment: bug-851530-fix.patch

Comment 12

[Louis Christie [:lchristie]]

Attachment: bug-851530-fix.patch

Comment 13

[Louis Christie [:lchristie]]

https://treeherder.mozilla.org/#/jobs?repo=try&revision=47100b30ea57

Comment 14

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

Comment on attachment 8718202 [details] [diff] [review]
bug-851530-fix.patch

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

Given that jya's been reviewing other Wave MediaDataDecoder stuff, I think it makes sense to pass this review onto him.

Comment 15

[Louis Christie [:lchristie]]

Attachment: bug-851530-fix.patch

Amended for changes in bug 1231793.

Comment 16

[Jean-Yves Avenard [:jya]]

Comment on attachment 8718516 [details] [diff] [review]
bug-851530-fix.patch

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

I would split this patch in two:
one introducing the change
and another with the mochitest.

one patch == one scope.

::: dom/media/platforms/agnostic/WAVDecoder.cpp
@@ +39,5 @@
> +int16_t
> +DecodeULawSample(uint8_t aValue)
> +{
> +  aValue = aValue ^ 0xFF;
> +  uint8_t sign = (aValue & 0x80) >> 7;

why not make that a bool ? or better a value of either 1 or -1 so you only need to return sign * sample

@@ +43,5 @@
> +  uint8_t sign = (aValue & 0x80) >> 7;
> +  uint8_t exponent = (aValue & 0x70) >> 4;
> +  uint8_t mantissa = aValue & 0x0F;
> +  int16_t sample = (33 + 2 * mantissa) * (2 << (exponent + 1)) - 33;
> +  return (sign != 0) ? sample : -sample;

return sign ? sample : -sample

Comment 17

[Louis Christie [:lchristie]]

Attachment: bug-851530-part-1.patch

Comment 18

[Louis Christie [:lchristie]]

Attachment: bug-851530-part-2.patch

Comment 19

[Louis Christie [:lchristie]]

https://treeherder.mozilla.org/#/jobs?repo=try&revision=04a67f8e93d9

Comment 20

[Pulsebot]

https://hg.mozilla.org/integration/mozilla-inbound/rev/1a0ee1b75cd8
https://hg.mozilla.org/integration/mozilla-inbound/rev/daa046b2e17f

Comment 21

[Carsten Book [:Tomcat]]

https://hg.mozilla.org/mozilla-central/rev/1a0ee1b75cd8
https://hg.mozilla.org/mozilla-central/rev/daa046b2e17f

Comment 22

[Loic]

As it's a feature improving the playback of audio WAVs for the user, I add dev-doc-needed.