1306594 - test_peerConnection_audioRenegotiationInactiveAnswer.html is permafailing when tested properly

Status: RESOLVED FIXED

(Core :: WebRTC: Signaling, defect, P2)

Description

[Andreas Pehrson [:pehrsons]]

There's a bug in AudioStreamHelper in our test framework so the checkAudioNotFlowing() path always passes.

This is because we set up the WebAudio graph with an analyser and then check that the analyser outputs all zeroes immediately. Even before the graph has started receiving any audio data.

When I wait a bit before starting to check the analyser I get a permafail.


Here's a run on try to show this: https://treeherder.mozilla.org/#/jobs?repo=try&revision=d092941072ef

I'll move the relevant patches to this bug.

Comment 1

[Andreas Pehrson [:pehrsons]]

Attachment: Bug 1306594 - Wait for the Analyser to start gathering data before analysing.

This is a real problem in for instance
test_peerConnection_verifyAudioAfterRenegotiation.html where we check that
disabled tracks don't output any audio. Locally the analyser calls the checking
function 3 times before it has gathered any output samples at all, meaning that
the disabled-track-checks always pass.

Review commit: https://reviewboard.mozilla.org/r/82356/diff/#index_header
See other reviews: https://reviewboard.mozilla.org/r/82356/

Comment 2

[Andreas Pehrson [:pehrsons]]

Nils, do you want to take a look at the failure? Reproduces nicely everywhere - just import the patch.

Comment 3

[Nils Ohlmeier [:drno]]

Cool catch!

The reason for the failure is pretty simple: Firefox continues to send RTP as you can see in the RTCP stats on about:webrtc on the wire.

Comment 4

[Nils Ohlmeier [:drno]]

Looking into why the offerer appears to ignore the a=inactive from the answer...

Comment 5

[Nils Ohlmeier [:drno]]

New bug 1306777 (to avoid confusing mozreview) contains the fix for this.

Comment 6

[Paul Adenot (:padenot)]

Comment on attachment 8796520 [details]
Bug 1306594 - Wait for the Analyser to start gathering data before analysing.

https://reviewboard.mozilla.org/r/82356/#review81348

Comment 7

[Nils Ohlmeier [:drno]]

Just pushed bug 1306777 to autoland. Once it's merged to mc I think you should be able to rebase this and land it.

Comment 8

[Nils Ohlmeier [:drno]]

Attachment: Bug 1306594 - Wait for the Analyser to start gathering data before analysing.

This is a real problem in for instance
test_peerConnection_verifyAudioAfterRenegotiation.html where we check that
disabled tracks don't output any audio. Locally the analyser calls the checking
function 3 times before it has gathered any output samples at all, meaning that
the disabled-track-checks always pass.

Review commit: https://reviewboard.mozilla.org/r/82948/diff/#index_header
See other reviews: https://reviewboard.mozilla.org/r/82948/

Comment 9

[Nils Ohlmeier [:drno]]

Obviously mozreview can't even handle the simple case of another person rebasing a changeset... (sigh). 
I'll kick off a try run and once that is green we can figure out who lands what.

Comment 10

[Andreas Pehrson [:pehrsons]]

I don't see anything obviously wrong in the try run. Landing.

Comment 11

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/13feb326ae28
Wait for the Analyser to start gathering data before analysing. r=padenot

Comment 12

[Wes Kocher (:KWierso) (Not reading bugmail; email directly if needed)]

https://hg.mozilla.org/mozilla-central/rev/13feb326ae28