In Firefox Nightly, WebRTC recording is slowed by 100%. So duration is 2x the actual duration.

RESOLVED FIXED in Firefox 49

Status

()

P1
normal
Rank:
12
RESOLVED FIXED
2 years ago
2 years ago

People

(Reporter: noitidart, Assigned: ctai)

Tracking

({regression})

49 Branch
mozilla50
regression
Points:
---

Firefox Tracking Flags

(firefox48 unaffected, firefox49+ fixed, firefox50+ fixed)

Details

Attachments

(1 attachment)

(Reporter)

Description

2 years ago
User Agent: Mozilla/5.0 (Windows NT 10.0; WOW64; rv:47.0) Gecko/20100101 Firefox/47.0
Build ID: 20160531183335

Steps to reproduce:

Go here in nightly - https://www.webrtc-experiment.com/RecordRTC/

Pick "Audio"

Do a recording.

Press stop.

It will play it back. In nightly it will be slowed down.

Do the same in stable or beta firefox and you will hear it play back at actual speed.

I actually discovered this because I used WebRtc Mic in a song identification addon I made here, and I hit this problem - https://addons.mozilla.org/en-US/firefox/addon/songifier/


Actual results:

In Firefox Nightly, WebRTC recording is slowed by 100%. So duration is 2x the actual duration.


Expected results:

Recording should not be slowed.
(Reporter)

Updated

2 years ago
Component: Untriaged → Video/Audio Controls
OS: Unspecified → Windows 10
Product: Firefox → Toolkit
Version: 47 Branch → 49 Branch
A regression window for this would be super helpful. Would you care to generate one with mozregression?

http://mozilla.github.io/mozregression/
Has Regression Range: --- → no
Has STR: --- → yes
Flags: needinfo?(noitidart)
(Reporter)

Comment 2

2 years ago
Hi William never did one of these before, I had a question about it. I know for sure it works in Fx 47 though. And it works in 49. So should I just test 48 and say yes or no?
The mozregression tool will help determine the actual change that caused this to break, so it's much more helpful than just knowing which version of Firefox it stopped working in. It also handles downloading, unpacking, and executing each build so it's actually much faster to use mozregression than it is to download random builds manually.
Flags: needinfo?(noitidart)

Updated

2 years ago
Rank: 12
Keywords: regression
Priority: -- → P1
Component: Video/Audio Controls → Audio/Video
Product: Toolkit → Core
Component: Audio/Video → Audio/Video: MediaStreamGraph
(Reporter)

Comment 5

2 years ago
Thanks Jared for the help on that one! I'll make sure to have mozRegression understood for next time someone asks me to do it.
Bug 1266647 causes regression. Keep investigations....
Flags: needinfo?(ctai)
Roll back below line can fix the regression.
https://dxr.mozilla.org/mozilla-central/source/dom/media/TrackUnionStream.cpp#329

Keep investigations....
Created attachment 8760560 [details]
Bug 1278027 - Reuse some codes of MediaEncoder::NotifyQueuedTrackChanges in MediaEncoder::NotifyQueuedAudioData.

Review commit: https://reviewboard.mozilla.org/r/58134/diff/#index_header
See other reviews: https://reviewboard.mozilla.org/r/58134/
Attachment #8760560 - Flags: review?(rjesup)
Attachment #8760560 - Flags: review?(pehrsons)
(In reply to Chia-hung Tai [:ctai :ctai_mozilla :cht] from comment #8)
> Created attachment 8760560 [details]
> Bug 1278027 - Reuse some codes of MediaEncoder::NotifyQueuedTrackChanges in
> MediaEncoder::NotifyQueuedAudioData.
> 
> Review commit: https://reviewboard.mozilla.org/r/58134/diff/#index_header
> See other reviews: https://reviewboard.mozilla.org/r/58134/

I think the root cause is the mistake of keeping notify queued audio data in direct connected mode. That will cause the double of encoded data.

Updated

2 years ago
Attachment #8760560 - Flags: review?(rjesup) → review+
Comment on attachment 8760560 [details]
Bug 1278027 - Reuse some codes of MediaEncoder::NotifyQueuedTrackChanges in MediaEncoder::NotifyQueuedAudioData.

https://reviewboard.mozilla.org/r/58134/#review54996
Comment on attachment 8760560 [details]
Bug 1278027 - Reuse some codes of MediaEncoder::NotifyQueuedTrackChanges in MediaEncoder::NotifyQueuedAudioData.

Pehrsons is unavailable this week.  We shouldn't need his review.

Since this code has likely uplifted to Aurora/49, we'll probably need to nominate this for 49 once we land and verify the fix.  (Please mark the flags appropriately for what versions are affected.)
Flags: needinfo?(ctai)
Attachment #8760560 - Flags: review?(pehrsons)
Comment on attachment 8760560 [details]
Bug 1278027 - Reuse some codes of MediaEncoder::NotifyQueuedTrackChanges in MediaEncoder::NotifyQueuedAudioData.

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/58134/diff/1-2/
Bug 1266647 is landed in 49. Thanks for the review.
status-firefox49: --- → affected
Flags: needinfo?(ctai)
OS: Windows 10 → All
Hardware: Unspecified → All
Keywords: checkin-needed

Comment 14

2 years ago
Pushed by cbook@mozilla.com:
https://hg.mozilla.org/integration/mozilla-inbound/rev/d13ee16778f1
Reuse some codes of MediaEncoder::NotifyQueuedTrackChanges in MediaEncoder::NotifyQueuedAudioData. r=jesup
Keywords: checkin-needed
Assignee: nobody → ctai

Comment 15

2 years ago
bugherder
https://hg.mozilla.org/mozilla-central/rev/d13ee16778f1
Status: NEW → RESOLVED
Last Resolved: 2 years ago
status-firefox50: --- → fixed
Resolution: --- → FIXED
Target Milestone: --- → mozilla50

Updated

2 years ago
Duplicate of this bug: 1280002

Comment 17

2 years ago
Hi Paul, I confirm here that yes the 1280002 was a duplicate and it works fine on Nightly. One question: in comment #11 #13 it is noted that this is also in 49/Aurora. So is it going to be fixed on the 49 before moving to stable?
[Tracking Requested - why for this release]:
Regression in MediaRecorder. See comment 0 for the details.


ctai, can you request uplift?
status-firefox48: --- → unaffected
tracking-firefox49: --- → ?
Flags: needinfo?(ctai)
Comment on attachment 8760560 [details]
Bug 1278027 - Reuse some codes of MediaEncoder::NotifyQueuedTrackChanges in MediaEncoder::NotifyQueuedAudioData.

Approval Request Comment
[Feature/regressing bug #]:
[User impact if declined]:
[Describe test coverage new/current, TreeHerder]:
[Risks and why]: 
[String/UUID change made/needed]:

[Feature/regressing bug #]: regression bug Bug 1266647
[User impact if declined]: The duration of recorded blob/file will be 2x long. Break MediaRecorder functionality.
[Describe test coverage new/current, TBPL]: Manual test on website https://www.webrtc-experiment.com/RecordRTC/. Also in Comment 18, adifran also verified this bug. 
[Risks and why]: Low risk because it just reuse some existing code.
[String/UUID change made/needed]: none
Flags: needinfo?(ctai)
Attachment #8760560 - Flags: approval-mozilla-aurora?
Comment on attachment 8760560 [details]
Bug 1278027 - Reuse some codes of MediaEncoder::NotifyQueuedTrackChanges in MediaEncoder::NotifyQueuedAudioData.

Recent regression, let's uplift this fix! 
Thanks for catching the bug, noitidart.
Attachment #8760560 - Flags: approval-mozilla-aurora? → approval-mozilla-aurora+
tracking-firefox49: ? → +
tracking-firefox50: --- → +

Comment 21

2 years ago
bugherderuplift
https://hg.mozilla.org/releases/mozilla-aurora/rev/8f2d601499b5
status-firefox49: affected → fixed
You need to log in before you can comment on or make changes to this bug.