1503536 - Call ApplySettings in MediaEngineWebRTCMicrophoneSource::Start

Status: RESOLVED FIXED

(Core :: WebRTC: Audio/Video, defect, P2)

Description

[Paul Adenot (:padenot)]

Otherwise, some bits of processing won't be enabled/disabled correctly.

This was regressed by bug 1487057.

Comment 1

[Paul Adenot (:padenot)]

Attachment: Bug 1503536 - Call ApplySettings in MediaEngineWebRTCMicrophoneSource::Start. r?pehrsons

Comment 2

[Paul Adenot (:padenot)]

We want this uplifted to 64, this will apply just fine.

Comment 3

[Paul Adenot (:padenot)]

https://treeherder.mozilla.org/#/jobs?repo=try&revision=7f560945a11565b62df0b0e2b24f9fc294c976c3

Comment 4

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/cdfaa36dcd55
Call ApplySettings in MediaEngineWebRTCMicrophoneSource::Start.  r=pehrsons

Comment 5

[Cosmin Sabou [:CosminS]]

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

Comment 6

[Jan-Ivar Bruaroey [:jib] (needinfo? me)]

[Tracking Requested - why for this release]: Nasty echo feedback in WebRTC calls; intolerable noise in local loop WebRTC demos. 

Regression range points confirms bug 1487057:

12:12.64 INFO: Last good revision: 9268ec81fb01ae1fd1162fc6866ef8f74cd3ea09
12:12.64 INFO: First bad revision: 0190c5793ffeeb0eec281cf377435c7a592415f3
12:12.65 INFO: Pushlog:
https://hg.mozilla.org/integration/mozilla-inbound/pushloghtml?fromchange=9268ec81fb01ae1fd1162fc6866ef8f74cd3ea09&tochange=0190c5793ffeeb0eec281cf377435c7a592415f3

Comment 7

[Pulsebot]

Backout by [e-mail]:
https://hg.mozilla.org/mozilla-central/rev/53647b724d3b
Backed out changeset cdfaa36dcd55 for perma assertion failure at media/webrtc/MediaEngineWebRTCAudio.cpp a=backout

Comment 8

[Raul Gurzau (:RaulG)]

Backed out changeset cdfaa36dcd55 (bug 1503536) for perma assertion failure at media/webrtc/MediaEngineWebRTCAudio.cpp a=backout

Backout link: https://hg.mozilla.org/mozilla-central/rev/53647b724d3bbc77055bd9167fa125c5d52e4eda

Push with failures: https://treeherder.mozilla.org/#/jobs?repo=mozilla-inbound&resultStatus=pending%2Crunning%2Csuccess%2Ctestfailed%2Cbusted%2Cexception&group_state=expanded&searchStr=mochitest%2Cmedia%2Cdebug&platform=linux&job_type_symbol=mda3&selectedJob=209856624&revision=4115e3cf7ad0054a3846926eb7b85afb019ff385

Log link: https://treeherder.mozilla.org/logviewer.html#?job_id=209856624&repo=mozilla-inbound&lineNumber=9719

Log snippet: [task 2018-11-05T16:58:11.324Z] 16:58:11     INFO - GECKO(1065) | Assertion failure: aStream->GraphImpl()->IterationEnd() > mLastCallbackAppendTime, at /builds/worker/workspace/build/src/dom/media/webrtc/MediaEngineWebRTCAudio.cpp:1024
[task 2018-11-05T16:59:34.769Z] 16:59:34     INFO - GECKO(1065) | #01: mozilla::MediaDevice::Pull(RefPtr<mozilla::SourceMediaStream> const&, int, long, nsMainThreadPtrHandle<nsIPrincipal> const&) [mfbt/RefPtr.h:84]
[task 2018-11-05T16:59:34.770Z] 16:59:34     INFO - 
[task 2018-11-05T16:59:34.771Z] 16:59:34     INFO - GECKO(1065) | #02: mozilla::SourceListener::NotifyPull(mozilla::MediaStreamGraph*, long) [mfbt/UniquePtr.h:324]
[task 2018-11-05T16:59:34.771Z] 16:59:34     INFO - 
[task 2018-11-05T16:59:34.772Z] 16:59:34     INFO - GECKO(1065) | #03: mozilla::SourceMediaStream::PullNewData(long) [xpcom/threads/Mutex.h:250]
[task 2018-11-05T16:59:34.773Z] 16:59:34     INFO - 
[task 2018-11-05T16:59:34.774Z] 16:59:34     INFO - GECKO(1065) | #04: mozilla::MediaStreamGraphImpl::UpdateGraph(long) [dom/media/MediaStreamGraph.cpp:1282]
[task 2018-11-05T16:59:34.775Z] 16:59:34     INFO - 
[task 2018-11-05T16:59:34.776Z] 16:59:34     INFO - GECKO(1065) | #05: mozilla::MediaStreamGraphImpl::OneIteration(long) [dom/media/MediaStreamGraph.cpp:1475]
[task 2018-11-05T16:59:34.777Z] 16:59:34     INFO - 
[task 2018-11-05T16:59:34.778Z] 16:59:34     INFO - GECKO(1065) | #06: mozilla::AudioCallbackDriver::DataCallback(float const*, float*, long) [dom/media/GraphDriver.cpp:1008]
[task 2018-11-05T16:59:34.779Z] 16:59:34     INFO - 
[task 2018-11-05T16:59:34.781Z] 16:59:34     INFO - GECKO(1065) | #07: <futures::future::lazy::Lazy<F, R> as futures::future::Future>::poll [media/audioipc/client/src/stream.rs:110]
[task 2018-11-05T16:59:34.783Z] 16:59:34     INFO - 
[task 2018-11-05T16:59:34.785Z] 16:59:34     INFO - GECKO(1065) | #08: std::panicking::try::do_call [third_party/rust/futures/src/future/catch_unwind.rs:32]
[task 2018-11-05T16:59:34.787Z] 16:59:34     INFO - 
[task 2018-11-05T16:59:34.791Z] 16:59:34     INFO - GECKO(1065) | #09: __rust_maybe_catch_panic [/rustc/da5f414c2c0bfe5198934493f04c676e2b23ff2e/src/libpanic_abort/lib.rs:43]

Comment 9

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/mozilla-inbound/rev/eae4fa2210f5
Call ApplySettings in MediaEngineWebRTCMicrophoneSource::Start.  r=pehrsons

Comment 10

[Daniel Varga [:dvarga]]

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

Comment 11

[Ryan VanderMeulen [:RyanVM]]

Please nominate this for Beta uplift when you're comfortable doing so.

Comment 13

[Paul Adenot (:padenot)]

Comment on attachment 9021474 [details]
Bug 1503536 - Call ApplySettings in MediaEngineWebRTCMicrophoneSource::Start.  r?pehrsons

[Beta/Release Uplift Approval Request]

Feature/Bug causing the regression: Bug 1487057

User impact if declined: No Echo Cancelation on webrtc calls

Is this code covered by automated tests?: No

Has the fix been verified in Nightly?: Yes

Needs manual test from QE?: No

If yes, steps to reproduce: 

List of other uplifts needed: None

Risk to taking this patch: Low

Why is the change risky/not risky? (and alternatives if risky): The code is simple, we're just calling a method that we forgot to call.

String changes made/needed: none

Comment 14

[Julien Cristau [:jcristau] (back April 22)]

Comment on attachment 9021474 [details]
Bug 1503536 - Call ApplySettings in MediaEngineWebRTCMicrophoneSource::Start.  r?pehrsons

webrtc regression fix, spent a week in nightly; approved for 64.0b10

Comment 15

[Paul Adenot (:padenot)]

I'll be providing a patch in a second.

Comment 16

[Paul Adenot (:padenot)]

Attachment: beta patch

This is the correct beta patch.

It effectively backs bug 1499426 out from beta, that was uplifted earlier, and adds the fix for this bug.

Comment 17

[Ryan VanderMeulen [:RyanVM]]

https://hg.mozilla.org/releases/mozilla-beta/rev/929f848d3a54