Closed Bug 825329 Opened 7 years ago Closed 6 years ago
Properly get the playback
Rate from the Audio Clock .
 is wrong. Setting the playback rate to 0.5 and then to 1.0 does not make the media play at it's intrinsic rate. : http://mxr.mozilla.org/mozilla-central/source/content/media/AudioStream.cpp#1138
This fixes the issue, remove the now useless mPlaybackRate member (not that it was super-useful anyways), and adds a test case. I've also reenabled the test on Linux to check if it was the cause of the super-frequent orange, we will see on try. And there is a bunch of dump()s that I'll have to remove.
Attachment #696407 - Flags: review?(kinetik)
Attachment #696407 - Flags: review?(kinetik) → review+
Push backed out for Windows pgo-only mochitest-1 timeouts in media tests, since the backout of just 1abf4c88f8f1 didn't work (see bug 793274 comment 12 for example logs): https://hg.mozilla.org/integration/mozilla-inbound/rev/f2912b7e727a
This changeset was the culprit of the orange when doing a PGO build. This patch is trivial, so I'm a bit sad. I'll try to find something (already tried the #pragma to disable optimizations, it did not work).
So, this fixes the issue, and does not trigger a PGO failure. I tested a bunch of things, and keeping a redundant |double mPlaybackRate| member in the class is the only thing that worked. This patch is functionnaly equivalent. We can revisit if/when we disable PGO.
Do you know where PGO is introducing the problem? I'd like to see some analysis like what bug 768333 has before introducing workarounds. At the very minimum, the patch needs a comment added so that some poor sucker doesn't come along later and remove mPlaybackRate while doing some innocent cleanup work.
So, I went and retriggered PGO builds on try with the original patch  to have a look at the functions, and it went green this time . I'll be landing the original patch. : https://bugzilla.mozilla.org/attachment.cgi?id=696407 : https://tbpl.mozilla.org/?tree=Try&rev=75e26f8270e0
Sorry, backed out for bug 844129. https://hg.mozilla.org/integration/mozilla-inbound/rev/a1c344441241
Pushed without the test, too much people are complaining about this bug, and I have no time as per now to make the test pass on try. Leaving this open so I can push the test sometime. https://hg.mozilla.org/integration/mozilla-inbound/rev/1bb461d484b2
Would this qualify for uplift to 22 beta? We're still early in the cycle, and it's a pretty glaring bug...
(In reply to Paul Adenot (:padenot) from comment #12) > Pushed without the test, too much people are complaining about this bug, and > I have no time as per now to make the test pass on try. Leaving this open so > I can push the test sometime. Since ongoing work for the test isn't here and it's been a while, can we close this bug and file a follow-up for the test?
No longer blocks: 883186
Status: NEW → RESOLVED
Closed: 6 years ago
Resolution: --- → FIXED
Setting target-milestone to mozilla23 since development of mozilla24 starting on May 13 and this patch got merged to m-c on May 6.
Target Milestone: --- → mozilla23
You need to log in before you can comment on or make changes to this bug.