Cannot play WAV file with u-law compression encoding

RESOLVED FIXED in Firefox 47

Status

()

defect
P5
normal
RESOLVED FIXED
7 years ago
4 years ago

People

(Reporter: chudinov, Assigned: lchristie)

Tracking

({dev-doc-needed})

19 Branch
mozilla47
x86_64
Windows 7
Points:
---
Dependency tree / graph

Firefox Tracking Flags

(firefox47 fixed)

Details

Attachments

(4 attachments, 5 obsolete attachments)

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.
Keywords: html5
Posted file WAV file to test
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.
related/dupe of bug 524109
Component: Untriaged → Video/Audio
Keywords: html5
Product: Firefox → Core
Depends on: 524109
Component: Audio/Video → Audio/Video: Playback
Summary: Cannot play WAV file of a specific format → Cannot play WAV file with u-law compression encoding
Assignee: nobody → lchristie
No longer depends on: 524109
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.
Depends on: 1231793
Posted patch bug-851530-fix.patch (obsolete) — Splinter Review
This code depends on the new code for the Wave Media Data Decoder in bug 1231793.
Attachment #8706191 - Flags: review?(cpearce)
Blocks: 1205580
Duplicate of this bug: 1205580
No longer blocks: 1205580
Posted patch bug-851530-fix.patch (obsolete) — Splinter Review
Added test cases, fixed one typo.
Attachment #8706191 - Attachment is obsolete: true
Attachment #8706191 - Flags: review?(cpearce)
Attachment #8706580 - Flags: review?(cpearce)
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.
Attachment #8706580 - Flags: review?(cpearce)
Posted patch bug-851530-fix.patch (obsolete) — Splinter Review
Attachment #8706580 - Attachment is obsolete: true
Attachment #8716014 - Flags: review?(cpearce)
Posted patch bug-851530-fix.patch (obsolete) — Splinter Review
Attachment #8716014 - Attachment is obsolete: true
Attachment #8716014 - Flags: review?(cpearce)
Attachment #8718202 - Flags: review?(cpearce)
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.
Attachment #8718202 - Flags: review?(cpearce) → review?(jyavenard)
Posted patch bug-851530-fix.patch (obsolete) — Splinter Review
Amended for changes in bug 1231793.
Attachment #8718202 - Attachment is obsolete: true
Attachment #8718202 - Flags: review?(jyavenard)
Attachment #8718516 - Flags: review?(jyavenard)
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
Attachment #8718516 - Flags: review?(jyavenard) → review+
Attachment #8718516 - Attachment is obsolete: true
Keywords: checkin-needed
https://hg.mozilla.org/mozilla-central/rev/1a0ee1b75cd8
https://hg.mozilla.org/mozilla-central/rev/daa046b2e17f
Status: UNCONFIRMED → RESOLVED
Closed: 4 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla47
As it's a feature improving the playback of audio WAVs for the user, I add dev-doc-needed.
Keywords: dev-doc-needed
You need to log in before you can comment on or make changes to this bug.