Closed
Bug 928797
Opened 11 years ago
Closed 11 years ago
<video> framerate regression on mac after cubeb audiounit latency change
Categories
(Core :: Audio/Video, defect)
Tracking
()
VERIFIED
FIXED
mozilla28
People
(Reporter: alessandro.d, Assigned: padenot)
References
Details
Attachments
(3 files)
1.30 KB,
patch
|
padenot
:
review-
|
Details | Diff | Splinter Review |
2.67 KB,
patch
|
kinetik
:
review+
|
Details | Diff | Splinter Review |
2.80 KB,
patch
|
Details | Diff | Splinter Review |
https://hg.mozilla.org/integration/mozilla-inbound/pushloghtml?fromchange=616980966644&tochange=6ca71aca6e9e caused a big regression in <video> framerate on mac. For example playing http://download.blender.org/peach/trailer/trailer_400p.ogg I get 312 rendered frames over 813 decoded frames. The culprit seems to be in the following change to media/cubeb/src/cubeb_audiounit.c: /* Set the maximum number of frame that the render callback will ask for, * effectively setting the latency of the stream. This is process-wide. */ r = AudioUnitSetProperty(stm->unit, kAudioDevicePropertyBufferFrameSize, kAudioUnitScope_Output, 0, &buffer_size, sizeof(buffer_size)); I am no AudioUnit expert but by googling a little I found a kAudioUnitProperty_MaximumFramesPerSlice that seems to do what the comment above says. Setting that property seems to fix the issue.
Reporter | ||
Comment 1•11 years ago
|
||
Reporter | ||
Comment 2•11 years ago
|
||
Oh and forgot to mention that we probably need a test to check that we aren't dropping more than 10% of the frames or so...
Assignee | ||
Comment 3•11 years ago
|
||
Comment on attachment 819568 [details] [diff] [review] 0001-Bug-928797-framerate-regression-on-mac-after-cubeb-a.patch Setting myself as a reviewer, I'll double check that when I arrive at the office, but it seems ok.
Attachment #819568 -
Flags: review?(paul)
Assignee | ||
Comment 4•11 years ago
|
||
Comment on attachment 819568 [details] [diff] [review] 0001-Bug-928797-framerate-regression-on-mac-after-cubeb-a.patch Review of attachment 819568 [details] [diff] [review]: ----------------------------------------------------------------- Reading [1], we see that this new call has more likely no effect (because this particular audio unit is an IO unit), and thus this patch reverts to the previous behavior. I'm going to write a patch to address this. [1]: https://developer.apple.com/library/mac/documentation/AudioUnit/Reference/AudioUnitPropertiesReference/Reference/reference.html#jumpTo_54
Attachment #819568 -
Flags: review?(paul) → review-
Assignee | ||
Comment 5•11 years ago
|
||
This fixes the situation.
Attachment #821044 -
Flags: review?(kinetik)
Assignee | ||
Updated•11 years ago
|
Assignee: nobody → paul
Reporter | ||
Comment 6•11 years ago
|
||
Interesting. Did you find out why framerate drops if latency is increased? Is it the clock synchronization in the decoder state machine?
Assignee | ||
Comment 7•11 years ago
|
||
Maybe, I just wanted to get a working patch. I'll now investigate the causes.
Comment 8•11 years ago
|
||
Comment on attachment 821044 [details] [diff] [review] Only set the audio output latency on mac if it is lower than the default. r= Oh, yeah. Because the audio "clock" is driven by updates in the render callback (stm->frames_played), causing the render callback to run less frequently causes the clock resolution to drop.
Attachment #821044 -
Flags: review?(kinetik) → review+
Assignee | ||
Comment 9•11 years ago
|
||
https://hg.mozilla.org/integration/mozilla-inbound/rev/38414f29834c
Comment 10•11 years ago
|
||
Backed out for causing bug 70138 -esque failures: eg: https://tbpl.mozilla.org/php/getParsedLog.php?id=29605638&tree=Mozilla-Inbound https://tbpl.mozilla.org/php/getParsedLog.php?id=29606123&tree=Mozilla-Inbound https://tbpl.mozilla.org/php/getParsedLog.php?id=29608807&tree=Mozilla-Inbound remote: https://hg.mozilla.org/integration/mozilla-inbound/rev/348904f9fc0c
Comment 11•11 years ago
|
||
Sorry, bug 701384 even.
Assignee | ||
Comment 12•11 years ago
|
||
Pushed to try with some instrumentation (this has always been green locally): https://tbpl.mozilla.org/?tree=Try&rev=f17d14b61bba
Assignee | ||
Comment 13•11 years ago
|
||
The failure was because I forgot to initialize the `size` variable when getting the property.
Comment 14•11 years ago
|
||
https://github.com/kinetiknz/cubeb/commit/93e51e70e978420c745ec22503fa8e121cbb7aa5
Assignee | ||
Comment 15•11 years ago
|
||
Landed while re-syncing cubeb in bug 939593 https://hg.mozilla.org/integration/mozilla-inbound/rev/e5063d54b289
Assignee | ||
Comment 16•11 years ago
|
||
e5063d54b289 has been merged to mozilla-central, closing.
Status: NEW → RESOLVED
Closed: 11 years ago
Resolution: --- → FIXED
Comment 17•11 years ago
|
||
Confirmed. Playback is smooth with all frames reported as presented in Nightly 28.0a1 (2013-11-18). Thanks, Paul!
Status: RESOLVED → VERIFIED
Updated•11 years ago
|
Target Milestone: --- → mozilla28
You need to log in
before you can comment on or make changes to this bug.
Description
•