Call ApplySettings in MediaEngineWebRTCMicrophoneSource::Start

RESOLVED FIXED in Firefox 64

Status

()

defect
P2
normal
RESOLVED FIXED
7 months ago
5 months ago

People

(Reporter: padenot, Assigned: padenot)

Tracking

({regression})

64 Branch
mozilla65
Points:
---
Bug Flags:
qe-verify -

Firefox Tracking Flags

(firefox-esr60 unaffected, firefox63 unaffected, firefox64+ fixed, firefox65+ fixed)

Details

Attachments

(2 attachments)

(Assignee)

Description

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

This was regressed by bug 1487057.
(Assignee)

Updated

7 months ago
Assignee: nobody → padenot
Priority: -- → P2
(Assignee)

Comment 2

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

Comment 4

7 months ago
Pushed by padenot@mozilla.com:
https://hg.mozilla.org/integration/autoland/rev/cdfaa36dcd55
Call ApplySettings in MediaEngineWebRTCMicrophoneSource::Start.  r=pehrsons

Comment 5

7 months ago
bugherder
https://hg.mozilla.org/mozilla-central/rev/cdfaa36dcd55
Status: NEW → RESOLVED
Last Resolved: 7 months ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla65
[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

7 months ago
Backout by rgurzau@mozilla.com:
https://hg.mozilla.org/mozilla-central/rev/53647b724d3b
Backed out changeset cdfaa36dcd55 for perma assertion failure at media/webrtc/MediaEngineWebRTCAudio.cpp a=backout
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]
Flags: needinfo?(padenot)
Status: RESOLVED → REOPENED
Resolution: FIXED → ---
Target Milestone: mozilla65 → ---
Keywords: regression
Version: Trunk → 64 Branch

Comment 9

7 months ago
Pushed by paul@paul.cx:
https://hg.mozilla.org/integration/mozilla-inbound/rev/eae4fa2210f5
Call ApplySettings in MediaEngineWebRTCMicrophoneSource::Start.  r=pehrsons

Comment 10

7 months ago
bugherder
https://hg.mozilla.org/mozilla-central/rev/eae4fa2210f5
Status: REOPENED → RESOLVED
Last Resolved: 7 months ago7 months ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla65
(Assignee)

Updated

6 months ago
Flags: needinfo?(padenot)
Please nominate this for Beta uplift when you're comfortable doing so.
Flags: needinfo?(padenot)
(Assignee)

Updated

6 months ago
Duplicate of this bug: 1506927
(Assignee)

Comment 13

6 months ago
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
Flags: needinfo?(padenot)
Attachment #9021474 - Flags: approval-mozilla-beta?
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
Attachment #9021474 - Flags: approval-mozilla-beta? → approval-mozilla-beta+
(Assignee)

Comment 15

6 months ago
I'll be providing a patch in a second.
(Assignee)

Comment 16

6 months ago
Posted patch beta patchSplinter Review
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.
Flags: qe-verify-
You need to log in before you can comment on or make changes to this bug.