Closed Bug 787788 Opened 8 years ago Closed 8 years ago

Downmixing weights for 6.1 Opus files are wrong


(Core :: Audio/Video, defect)

17 Branch
Not set



Tracking Status
firefox15 --- unaffected
firefox16 --- unaffected
firefox17 --- verified
firefox18 --- verified
firefox-esr10 --- unaffected


(Reporter: derf, Assigned: derf)




(1 file)

-        /*7*/{ {0.4553f,0}, {0.322f,0.322f}, {0,4553}, {0.3943f,0.2277f}, {0.2277f,0.3943f}, {0.2788f,0.2788f}, {0.322f,0.322f}},
+        /*7*/{ {0.4553f,0}, {0.322f,0.322f}, {0,0.4553f}, {0.3943f,0.2277f}, {0.2277f,0.3943f}, {0.2788f,0.2788f}, {0.322f,0.322f}},

Assignee: nobody → tterribe
Attachment #657665 - Flags: review?(giles)
Depends on: 748144
Comment on attachment 657665 [details] [diff] [review]
Fix downmixing weights for 6.1 Opus files

Oops indeed!
Attachment #657665 - Flags: review?(giles) → review+
Greenish on try:
Keywords: checkin-needed
OS: Linux → All
Hardware: x86_64 → All
Target Milestone: --- → mozilla18
Comment on attachment 657665 [details] [diff] [review]
Fix downmixing weights for 6.1 Opus files

[Approval Request Comment]
Bug caused by (feature/regressing bug #): 748144
User impact if declined: 6.1 channel Opus files will produce very loud/nasty clipping/static
Testing completed (on m-c, etc.): Tested locally, greenish on Try, pushed to m-i
Risk to taking this patch (and alternatives if risky): Low. This changes a single constant in a table.
String or UUID changes made by this patch: None.
Attachment #657665 - Flags: approval-mozilla-aurora?
Closed: 8 years ago
Resolution: --- → FIXED
Let's let this bake a bit more on nightly and then revisit for branch uplift.
Attachment #657665 - Flags: approval-mozilla-aurora? → approval-mozilla-aurora+
Thanks for reviewing!
Timothy, can you please verify if this is now fixed for you in Firefox 17 and 18?
Verified fixed on MacOS X 10.7.5:

 Aurora 18.0a2 (2012-10-17)
 Beta 17.0b2
For reference, I have a 6.1 test file here:
Thanks Ralph and Timothy. Given we have a test file, is this something we can have an automated test for? If not, do we require a manual regression test?
Blocks: 782091
For now this requires a manual regression test, since someone needs to listen to the test file and verify correct playback.

I've thought about adding some sort of automated verification of decoding quality, but that's likely to be both expensive to run, and require lossless reference audio in the test suite, to catch anything but the most egregious errors.
You need to log in before you can comment on or make changes to this bug.