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

Status: RESOLVED FIXED

(Core :: Audio/Video: MediaStreamGraph, defect, P1)

Description

[noitidart]

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.

Comment 1

[William Lachance (:wlach)]

A regression window for this would be super helpful. Would you care to generate one with mozregression?

http://mozilla.github.io/mozregression/

Comment 2

[noitidart]

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?

Comment 3

[Jared Wein [:jaws] (please needinfo? me)]

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.

Comment 4

[Jared Wein [:jaws] (please needinfo? me)]

I ran mozregression and narrowed this down to these two changesets,
https://hg.mozilla.org/integration/mozilla-inbound/pushloghtml?fromchange=0e9913cfd157075621ce869da2f73454164eb70f&tochange=eb749a28eee0decab6c333ec283569f0e924ff7f

Comment 5

[noitidart]

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.

Comment 6

[Chia-hung Tai [:ctai :ctai_mozilla :cht]]

Bug 1266647 causes regression. Keep investigations....

Comment 7

[Chia-hung Tai [:ctai :ctai_mozilla :cht]]

Roll back below line can fix the regression.
https://dxr.mozilla.org/mozilla-central/source/dom/media/TrackUnionStream.cpp#329

Keep investigations....

Comment 8

[Chia-hung Tai [:ctai :ctai_mozilla :cht]]

Attachment: 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/

Comment 9

[Chia-hung Tai [:ctai :ctai_mozilla :cht]]

(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.

Comment 10

[Randell Jesup [:jesup] (needinfo me)]

Comment on attachment 8760560 [details]
Bug 1278027 - Reuse some codes of MediaEncoder::NotifyQueuedTrackChanges in MediaEncoder::NotifyQueuedAudioData.

https://reviewboard.mozilla.org/r/58134/#review54996

Comment 11

[Randell Jesup [:jesup] (needinfo me)]

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.)

Comment 12

[Chia-hung Tai [:ctai :ctai_mozilla :cht]]

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/

Comment 13

[Chia-hung Tai [:ctai :ctai_mozilla :cht]]

Bug 1266647 is landed in 49. Thanks for the review.

Comment 14

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/mozilla-inbound/rev/d13ee16778f1
Reuse some codes of MediaEncoder::NotifyQueuedTrackChanges in MediaEncoder::NotifyQueuedAudioData. r=jesup

Comment 15

[Carsten Book [:Tomcat]]

https://hg.mozilla.org/mozilla-central/rev/d13ee16778f1

Comment 17

[adifran]

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?

Comment 18

[Andreas Pehrson [:pehrsons]]

[Tracking Requested - why for this release]:
Regression in MediaRecorder. See comment 0 for the details.


ctai, can you request uplift?

Comment 19

[Chia-hung Tai [:ctai :ctai_mozilla :cht]]

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

Comment 20

[Liz Henry (:lizzard) (relman/hg->git project)]

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.

Comment 21

[Ryan VanderMeulen [:RyanVM]]

https://hg.mozilla.org/releases/mozilla-aurora/rev/8f2d601499b5