Closed
Bug 1300871
Opened 8 years ago
Closed 8 years ago
MediaRecorder video and audio are very out of sync on win10 if setting media.navigator.audio.full_duplex=false
Categories
(Core :: Audio/Video: MediaStreamGraph, defect, P1)
Tracking
()
RESOLVED
FIXED
mozilla51
Tracking | Status | |
---|---|---|
firefox48 | --- | unaffected |
firefox49 | --- | unaffected |
firefox50 | --- | unaffected |
firefox51 | + | fixed |
People
(Reporter: alice0775, Assigned: ctai)
References
Details
(Keywords: regression)
Attachments
(2 files)
[Tracking Requested - why for this release]:
+++ This bug was initially created as a clone of Bug #1300650 +++
Build Identifier:
https://hg.mozilla.org/releases/mozilla-aurora/rev/2231ab09a51c3b21dae6fa1627c1a64cfd43b3b0
Mozilla/5.0 (Windows NT 10.0; WOW64; rv:50.0) Gecko/20100101 Firefox/50.0 ID:20160905004005
Steps to reproduce:
1. Open https://www.webrtc-experiment.com/RecordRTC/
2. "Start Recording" and shear video/audio.
3. After for a while, Click "Stop Recording"
Actual results:
The audio lags behind the video by about 1 second all evidence of how far off it is.
Expected results:
The audio and video should be in sync.
Regression window on Nightly51.01 with setting media.navigator.audio.full_duplex=false :
https://hg.mozilla.org/integration/mozilla-inbound/pushloghtml?fromchange=e0a401691ee7b6c0979d228bc55f6c8db291663d&tochange=492d8382a49c86b16f758edb67b46d061adac5fc
Regressed by: 492d8382a49c Terrence Cole — Bug 1291928 - Expose JSSavedStack's stack before returning; r=bz
Flags: needinfo?(terrence)
Flags: needinfo?(bzbarsky)
Comment 1•8 years ago
|
||
Flags: needinfo?(bzbarsky)
Reporter | ||
Comment 2•8 years ago
|
||
I forgot a step.
0. setting media.navigator.audio.full_duplex=false
Reporter | ||
Comment 3•8 years ago
|
||
Recorded video : https://youtu.be/7yy8kNVoU34
Reference video(for calibration) : https://www.youtube.com/watch?v=rzF6bNSXZac
Comment 4•8 years ago
|
||
Moreover, we should only be using JSSavedStack at all when profiling, which I don't think is active here.
Comment 5•8 years ago
|
||
More to the point: if the increased latency from running a single extra barrier permanently delays A/V sync by over a second, our A/V component has major, major problems. The sort of problems that a GC expert is not going to be able to address in any way.
Flags: needinfo?(terrence)
Comment 6•8 years ago
|
||
> Moreover, we should only be using JSSavedStack at all when profiling
That's certainly false. It's used all over the place in Promise code, in exceptions, etc. But yes, comment 5 is basically what I was thinking...
Alice0775, how confident are you about this being the patch that triggers the problem?
Reporter | ||
Comment 7•8 years ago
|
||
(In reply to Boris Zbarsky [:bz] from comment #6)
>
> Alice0775, how confident are you about this being the patch that triggers
> the problem?
Okay.
I reinstalled Windows10... :)
And I re-checked the range without running foreground application except WMP.
I got
m-c:
https://hg.mozilla.org/mozilla-central/pushloghtml?fromchange=975ba208687a97ecb9fd439c1ee52bfa3350e25b&tochange=d320ef56876f52db9bc0eb79554c7332d4793769
m-i
https://hg.mozilla.org/integration/mozilla-inbound/pushloghtml?fromchange=79415ce5af65c6b207ae185825770ac039af13a8&tochange=a02925fe4ded04e2f5523fbd7e5ddfa46b190d39
Regressed by: Bug 1201363
(If possible, cross check)
status-firefox48:
--- → unaffected
status-firefox49:
--- → unaffected
Comment 9•8 years ago
|
||
Thanks Alice; that makes far more sense than JS stuff :-). ctai is going to look at it in the morning; we've already discussed it on IRC
Flags: needinfo?(rjesup)
Comment 10•8 years ago
|
||
Removing bz and terrence since this shouldn't be anything they have to care about
Comment hidden (mozreview-request) |
Assignee | ||
Updated•8 years ago
|
Assignee: nobody → ctai
Flags: needinfo?(ctai)
Comment hidden (mozreview-request) |
Assignee | ||
Comment 16•8 years ago
|
||
Comment 17•8 years ago
|
||
mozreview-review |
Comment on attachment 8789221 [details]
Bug 1300871 - Rollback to use original duration in video case.
https://reviewboard.mozilla.org/r/77518/#review75928
r+ -- is it reasonable to look to re-land the new version of this after bugfixing, or should we just go back to the old way permanently? And have you tested it?
Attachment #8789221 -
Flags: review?(rjesup) → review+
Assignee | ||
Comment 18•8 years ago
|
||
(In reply to Randell Jesup [:jesup] from comment #17)
> Comment on attachment 8789221 [details]
> Bug 1300871 - Rollback to use original duration in video case.
>
> https://reviewboard.mozilla.org/r/77518/#review75928
>
> r+ -- is it reasonable to look to re-land the new version of this after
> bugfixing, or should we just go back to the old way permanently? And have
> you tested it?
Yea, I have tested it. I don't think we need to re-land the new one. But it is reasonable to change to use timestamp only(bug 1290777). Because each encoder will change the duration into timestamp in their implementation. That will be redundant if we already have timestamp. The root cause of this bug is the skipping the duration of null image in the beginning.
Assignee | ||
Updated•8 years ago
|
Keywords: checkin-needed
Comment 19•8 years ago
|
||
Pushed by ryanvm@gmail.com:
https://hg.mozilla.org/integration/autoland/rev/4e77ec88d85a
Rollback to use original duration in video case. r=jesup
Keywords: checkin-needed
Comment 20•8 years ago
|
||
bugherder |
Status: NEW → RESOLVED
Closed: 8 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla51
Assignee | ||
Comment 21•8 years ago
|
||
You need to log in
before you can comment on or make changes to this bug.
Description
•