1231507 - Mochitest for simulcast

Status: RESOLVED FIXED

(Core :: WebRTC, defect, P1)

Description

[Byron Campen [:bwc]]

The basic idea here is as follows:

1. Have some chrome-only JS API on PeerConnection that selects which SSRC to let through MediaPipeline on to webrtc.org.

2. Write a mochitest that configures send simulcast on one side, and rewrites SDP to make it appear as though simulcast was actually negotiated. This will result in multiple SSRCs of video being sent to the receiver, when it expects only one. (We also need to disable bundle to prevent the bundle filter from eating these extra SSRCs) This mochitest then uses the API in part 1 to switch through all the expected SSRCs, and make sure each is receiving frames.

Comment 1

[Nils Ohlmeier [:drno]]

Attachment: MozReview Request: Bug 1231507: added mochitest SimulCast offer test case

Bug 1231507 (WIP): testing - function to transfer simulcast attributes from one SDP to another

Comment 2

[Byron Campen [:bwc]]

Attachment: MozReview Request: Bug 1231507 - Part 1: selectSsrc chrome-only API for SSRC-based filtering of receive tracks. r=drno, r=smaug

Bug 1231507 - Part 1: selectSsrc chrome-only API for SSRC-based filtering of receive tracks.

Comment 3

[Byron Campen [:bwc]]

Comment on attachment 8697334 [details]
MozReview Request: Bug 1231507 - Part 1: selectSsrc chrome-only API for SSRC-based filtering of receive tracks. r=drno, r=smaug

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/27565/diff/1-2/

Comment 4

[Nils Ohlmeier [:drno]]

Comment on attachment 8697111 [details]
MozReview Request: Bug 1231507: added mochitest SimulCast offer test case

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/27513/diff/1-2/

Comment 5

[Nils Ohlmeier [:drno]]

Comment on attachment 8697111 [details]
MozReview Request: Bug 1231507: added mochitest SimulCast offer test case

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/27513/diff/2-3/

Comment 6

[Nils Ohlmeier [:drno]]

Looks like the Chrome only API access actually works with SpecialPowers. So this now depends on availability of RTPReceivers in bug 1232082.

Comment 7

[Nils Ohlmeier [:drno]]

Comment on attachment 8697111 [details]
MozReview Request: Bug 1231507: added mochitest SimulCast offer test case

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/27513/diff/3-4/

Comment 8

[Nils Ohlmeier [:drno]]

https://reviewboard.mozilla.org/r/27561/#review25699

::: media/webrtc/signaling/src/peerconnection/PeerConnectionImpl.cpp:2199
(Diff revision 2)
> +      auto& pipelines = mMedia->GetLocalStreamByIndex(i)->GetPipelines();

I think this is a bug: it should be mMedia->GetRemoteStreamByIndex(i)->GetPipelines() to be alinged with the if condition before and because this crashes right now in recvonly mode

Comment 9

[Nils Ohlmeier [:drno]]

Comment on attachment 8697111 [details]
MozReview Request: Bug 1231507: added mochitest SimulCast offer test case

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/27513/diff/4-5/

Comment 10

[Nils Ohlmeier [:drno]]

If I apply the fix mentioned in comment #8 the mochitest is stuck when it tries to verify that it receives the first stream, because the RTCP stats don't indicate that RTP is actually being received. Which matches that only three video streams get rendered on the screen. I assume the remote video from the Simulcast stream is missing.

But I do see on the wire two different RTP streams going from the Simulcast sender to the receiver.

Comment 11

[Nils Ohlmeier [:drno]]

If I comment out the first call to selectSsrc() in the test then video starts rendering and the test appears to be happy. Even when later then selectSsrc() gets called everything appears to be fine. But I'm afraid that waitForMedia() call in this case actually gets fooled either through the Promises being used in the waitForMedia() call or by looking at the RTCP stats for the first stream.

To me it looks like the selectSsrc() function is not really working 100% as intended :-(

Comment 12

[Byron Campen [:bwc]]

Did you disable bundle in the test? You'll need to do that for this to work, since the initial bundle filter used in bundle will prevent the MediaPipeline from learning about all the SSRCs. I might be able to adjust the code in part 1 to not have this limitation, but we'd still end up with problems demuxing audio/video.

Comment 13

[Byron Campen [:bwc]]

https://treeherder.mozilla.org/#/jobs?repo=try&revision=9fcdbfb1eb1f

Comment 14

[Nils Ohlmeier [:drno]]

(In reply to Byron Campen [:bwc] from comment #13)
> https://treeherder.mozilla.org/#/jobs?repo=try&revision=9fcdbfb1eb1f

Looks like that try run generated a new perma orange test failure for dom/events/test/test_all_synthetic_events.html. No idea why though.

Comment 15

[Byron Campen [:bwc]]

That's interesting...

Comment 16

[Byron Campen [:bwc]]

From what I can tell, that perma-orange is happening before our tests run. Maybe adding the test caused something to get moved?

Comment 17

[Byron Campen [:bwc]]

https://treeherder.mozilla.org/#/jobs?repo=try&revision=5bbf88fec108

Comment 18

[Byron Campen [:bwc]]

Seeing some intermittent timeouts in the new test. Investigating.

Comment 19

[Byron Campen [:bwc]]

https://treeherder.mozilla.org/#/jobs?repo=try&revision=9294cb35df4a

Comment 20

[Byron Campen [:bwc]]

Comment on attachment 8697334 [details]
MozReview Request: Bug 1231507 - Part 1: selectSsrc chrome-only API for SSRC-based filtering of receive tracks. r=drno, r=smaug

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/27565/diff/2-3/

Comment 21

[Byron Campen [:bwc]]

Attachment: MozReview Request: Bug 1231507 - Part 2: added mochitest SimulCast offer test case r=jib

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

Comment 22

[Byron Campen [:bwc]]

Comment on attachment 8697334 [details]
MozReview Request: Bug 1231507 - Part 1: selectSsrc chrome-only API for SSRC-based filtering of receive tracks. r=drno, r=smaug

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/27565/diff/3-4/

Comment 23

[Byron Campen [:bwc]]

Comment on attachment 8707959 [details]
MozReview Request: Bug 1231507 - Part 2: added mochitest SimulCast offer test case r=jib

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/30935/diff/1-2/

Comment 24

[Olli Pettay [:smaug][[e-mail]]]

Comment on attachment 8697334 [details]
MozReview Request: Bug 1231507 - Part 1: selectSsrc chrome-only API for SSRC-based filtering of receive tracks. r=drno, r=smaug

https://reviewboard.mozilla.org/r/27565/#review27897

r+ for the ChromeOnly webidl change

Comment 25

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

Comment on attachment 8707959 [details]
MozReview Request: Bug 1231507 - Part 2: added mochitest SimulCast offer test case r=jib

https://reviewboard.mozilla.org/r/30935/#review27951

::: dom/canvas/test/captureStream_common.js:15
(Diff revision 2)
> +  this.cout.id = "canvasMJF1-"+serialNum++;

This looks like Michael's code? Did you mean for me to review it, or did other code sneak in here? There's also lots of what looks like logging cruft rather than ready for primetime in here?

Comment 26

[Byron Campen [:bwc]]

Comment on attachment 8697334 [details]
MozReview Request: Bug 1231507 - Part 1: selectSsrc chrome-only API for SSRC-based filtering of receive tracks. r=drno, r=smaug

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/27565/diff/4-5/

Comment 27

[Byron Campen [:bwc]]

Comment on attachment 8707959 [details]
MozReview Request: Bug 1231507 - Part 2: added mochitest SimulCast offer test case r=jib

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/30935/diff/2-3/

Comment 28

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

Comment on attachment 8707959 [details]
MozReview Request: Bug 1231507 - Part 2: added mochitest SimulCast offer test case r=jib

https://reviewboard.mozilla.org/r/30935/#review28153

A question on the double createOffer, and a possible intermittent. Otherwise just nits.

::: dom/media/tests/mochitest/mochitest.ini:141
(Diff revision 3)
> +[test_peerConnection_simulcastOffer.html]
> +skip-if = toolkit == 'gonk' || buildapp == 'mulet' || android_version # b2g(Bug 960442, video support for WebRTC is disabled on b2g), no simulcast support on android

Why do all the other tests seemingly use (android_version == '18') to test for android?

::: dom/media/tests/mochitest/sdpUtils.js:56
(Diff revision 3)
> +transferSimulcastProperties: function(offer, answer) {

Perhaps offerSdp and answerSdp ? A bit confusing otherwise, given the context.

::: dom/media/tests/mochitest/sdpUtils.js:57
(Diff revision 3)
> +  if (!offer.includes("a=simulcast:")) {
> +    return answer;

Does this happen in this test? If not, should we assert instead?

::: dom/media/tests/mochitest/sdpUtils.js:60
(Diff revision 3)
> +  var new_answer = answer;
> +  var o_simul = offer.match(/simulcast: send rid=(.*)([\n$])*/i);
> +  var new_answer = new_answer.concat("a=simulcast: recv rid=").concat(o_simul[1]).concat("\r\n");

Nits: Redundant use of 'var' is confusing.

Also, it seems simpler to use + here (perhaps even marginally faster, by not resizing the full answer 3 times?) E.g.:

    new_answer += "a=simulcast: recv rid=" + o_simul[1] + "\r\n";

::: dom/media/tests/mochitest/sdpUtils.js:64
(Diff revision 3)
> +  for (var i=0; i< o_rid.length; i++) {
> +    var new_answer = new_answer.concat(o_rid[i].replace(/send/, "recv")).concat("\r\n");
> +  }
> +  return new_answer;

Could use reduce here, or at least forEach. Otherwise, spacing should be i = 0 etc., and redundant var is confusing.

::: dom/media/tests/mochitest/test_peerConnection_simulcastOffer.html:19
(Diff revision 3)
> +  function alternateRedAndGreen(helper, canvas, stream) {
> +    var i = 0;
> +    setInterval(function() {
> +      if (i) {
> +        helper.drawColor(canvas, helper.green);
> +      } else {
> +        helper.drawColor(canvas, helper.red);
> +      }
> +      i = 1 - i;
> +      stream.requestFrame();
> +    }, 500);

What happens if stream.requestFrame() is called after the stream has shut down?

Since the interval set up by setInterval() is never cleared, might there be an intermittent here?

::: dom/media/tests/mochitest/test_peerConnection_simulcastOffer.html:22
(Diff revision 3)
> +      if (i) {
> +        helper.drawColor(canvas, helper.green);
> +      } else {
> +        helper.drawColor(canvas, helper.red);
> +      }

Or maybe helper.drawColor(canvas, i ? helper.green : helper.red); ?

::: dom/media/tests/mochitest/test_peerConnection_simulcastOffer.html:34
(Diff revision 3)
> +    ok(receivers.length > 0, "We have at least one RTP receiver");

Is there ever more than 1 here?

::: dom/media/tests/mochitest/test_peerConnection_simulcastOffer.html:37
(Diff revision 3)
> +    pc._chromePc = SpecialPowers.wrap(pc._pc);
> +    pc._chromePc.mozSelectSsrc(receiver, index);

Nit: _chromePc is unused outside of this function, so its scope is broader than needed. e.g.:
    SpecialPowers.wrap(pc._pc).mozSelectSsrc(receiver, index);

::: dom/media/tests/mochitest/test_peerConnection_simulcastOffer.html:41
(Diff revision 3)
> +  function waitForColorChange(helper, canvas, context) {
> +    return Promise.all([

Tip: could also be written as:

    var waitForColorChange = (helper, canvas, context) => Promise.all([

That way there's no chance of forgetting the crucial return (you didn't, I'm just full of advice, sorry). Cool use of Promise.all btw.

::: dom/media/tests/mochitest/test_peerConnection_simulcastOffer.html:52
(Diff revision 3)
> +      var stream1;
> +      var h1 = new CaptureStreamTestHelper2D(352,288);
> +      var canvas1 = h1.createAndAppendElement('canvas', 'source_canvas1');

There's no stream2, h2, or canvas2, so maybe just stream, h, and canvas?

::: dom/media/tests/mochitest/test_peerConnection_simulcastOffer.html:55
(Diff revision 3)
> +      h1.drawColor(canvas1, h1.green); // Make sure this is initted

inited?

::: dom/media/tests/mochitest/test_peerConnection_simulcastOffer.html:57
(Diff revision 3)
> +      var options = {};
> +      options.bundle = false;
> +      test = new PeerConnectionTest(options);
> +      test.setMediaConstraints([{video: true}], []);

Nit of nits: Inconsistent param setup for options vs. constraints. E.g. why not:

    test = new PeerConnectionTest({ bundle: false});
or
    var constraints;
    constraints.video = false;
    test.setMediaConstraints([constraints], []);

?

::: dom/media/tests/mochitest/test_peerConnection_simulcastOffer.html:72
(Diff revision 3)
> +      test.chain.insertBefore('PC_LOCAL_CREATE_OFFER', [
> +        // TODO(): Hack to work around the fact that GMP init blocks completion
> +        // of addTrack temporarily. PC_LOCAL_CREATE_OFFER is blocked from
> +        // completing in the same way, but we get a callback that tells us it is
> +        // done.
> +        function PC_LOCAL_CREATE_OFFFER(test) {

I'm confused. Did you mean to replace PC_LOCAL_CREATE_OFFER here?

Since this inserts steps *before* PC_LOCAL_CREATE_OFFER, doesn't this mean that PC_LOCAL_CREATE_OFFER still runs after this code, and doesn't that trounce on the description set up (and modified?) here, or is it harmless?

I wasn't able to gather from this comment why we call createOffer twice here, or how the hack works. Some clarification in the comment would be great.

::: dom/media/tests/mochitest/test_peerConnection_simulcastOffer.html:77
(Diff revision 3)
> +        function PC_LOCAL_CREATE_OFFFER(test) {

OFFFER?

::: dom/media/tests/mochitest/test_peerConnection_simulcastOffer.html:82
(Diff revision 3)
> +          ok(senders.length == 1, "We have exactly one RTP sender");

is(senders.length, 1, "We have exactly one RTP sender");

::: dom/media/tests/mochitest/test_peerConnection_simulcastOffer.html:95
(Diff revision 3)
> +        function PC_LOCAL_ADD_RIDS_TO_ANSWER(test) {
> +          test._remote_answer.sdp = sdputils.transferSimulcastProperties(
> +            test.originalOffer.sdp, test._remote_answer.sdp);
> +          info("Answer with RIDs: " + JSON.stringify(test._remote_answer));

Should we verify that _remote_answer.sdp is actually changed here?

::: dom/media/tests/mochitest/test_peerConnection_simulcastOffer.html:109
(Diff revision 3)
> +          var vremote1 = document.getElementById('pcRemote_remote1_video');
> +          ok(!!vremote1, "Should have remote video element for pcRemote");
> +          return waitForColorChange(h1, vremote1, "pcRemote's remote");

Idea: Instead of passing separate label into waitForColorChange, could we use the label from the passed-in vremote1 here?

::: dom/media/tests/mochitest/test_peerConnection_simulcastOffer.html:110
(Diff revision 3)
> +          ok(!!vremote1, "Should have remote video element for pcRemote");

nit: !! is superfluous.

::: dom/media/tests/mochitest/test_peerConnection_simulcastOffer.html:115
(Diff revision 3)
> +      test.chain.append([
> +        function PC_REMOTE_SET_RTP_SECOND_RID(test) {
> +          selectRecvSsrc(test.pcRemote, 1);
> +        },
> +        function PC_REMOTE_WAIT_FOR_SECOND_MEDIA_FLOW(test) {
> +          return test.pcRemote.waitForMediaFlow();
> +        }
> +      ]);
> +
> +      test.chain.append([
> +        function PC_REMOTE_WAIT_FOR_COLOR_CHANGE_2() {

Could you explain to me a bit what setting the RecvSsrc here accomplishes, and what this actually verifies? A comment may help.

My concern is that if something broke and setRecvSsrc for some reason ended up being a no-op, wouldn't this test still succeed? Are we merely exercising the simulcast code? I guess it's tricky to measure bitrate?

Also, a nit: Given that append takes an array, why call it multiple times in a row?

::: dom/media/tests/mochitest/test_peerConnection_simulcastOffer.html:127
(Diff revision 3)
> +          ok(!!vremote1, "Should have remote video element for pcRemote");

!! superfluous.

Comment 29

[Byron Campen [:bwc]]

https://reviewboard.mozilla.org/r/30935/#review28153

> Why do all the other tests seemingly use (android_version == '18') to test for android?

The simulcast stuff doesn't seem to work on any version of android. We have not determined why, but decided to figure that out later.

> What happens if stream.requestFrame() is called after the stream has shut down?
> 
> Since the interval set up by setInterval() is never cleared, might there be an intermittent here?

I am not sure whether this would cause an exception, but I suppose if it did it wouldn't have anywhere to propagate. I can wrap this in a try/catch though.

> Is there ever more than 1 here?

Should be only one.

> There's no stream2, h2, or canvas2, so maybe just stream, h, and canvas?

Sure.

> I'm confused. Did you mean to replace PC_LOCAL_CREATE_OFFER here?
> 
> Since this inserts steps *before* PC_LOCAL_CREATE_OFFER, doesn't this mean that PC_LOCAL_CREATE_OFFER still runs after this code, and doesn't that trounce on the description set up (and modified?) here, or is it harmless?
> 
> I wasn't able to gather from this comment why we call createOffer twice here, or how the hack works. Some clarification in the comment would be great.

Nope, I'm putting an extra createOffer in order to wait for GMP to init, which guarantees that the previous addTrack has completed (because it doesn't until GMP is initted).

> Could you explain to me a bit what setting the RecvSsrc here accomplishes, and what this actually verifies? A comment may help.
> 
> My concern is that if something broke and setRecvSsrc for some reason ended up being a no-op, wouldn't this test still succeed? Are we merely exercising the simulcast code? I guess it's tricky to measure bitrate?
> 
> Also, a nit: Given that append takes an array, why call it multiple times in a row?

If selectRecvSsrc was a no-op, this test would pass I think. We could test whether passing too large of an index would cause the video stream to stop, I suppose. Another thing we could try doing is renegotiate simulcast off and verify that only one of the two ssrcs is receiving anything.

Comment 30

[Byron Campen [:bwc]]

Comment on attachment 8707959 [details]
MozReview Request: Bug 1231507 - Part 2: added mochitest SimulCast offer test case r=jib

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/30935/diff/3-4/

Comment 31

[Byron Campen [:bwc]]

https://reviewboard.mozilla.org/r/30935/#review28153

> Does this happen in this test? If not, should we assert instead?

None of the other rewriting functions in here assert/throw, so probably not.

> inited?

I've seen this both ways, and it isn't a real word anyhow.

> Should we verify that _remote_answer.sdp is actually changed here?

I have added verification that the resulting answer has an a=simulcast: and a=rid: line.

Comment 32

[Byron Campen [:bwc]]

Comment on attachment 8707959 [details]
MozReview Request: Bug 1231507 - Part 2: added mochitest SimulCast offer test case r=jib

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/30935/diff/4-5/

Comment 33

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

Comment on attachment 8707959 [details]
MozReview Request: Bug 1231507 - Part 2: added mochitest SimulCast offer test case r=jib

https://reviewboard.mozilla.org/r/30935/#review28363

Ship it (with larger timeout).

::: dom/media/tests/mochitest/sdpUtils.js:56
(Diff revisions 3 - 4)
> -transferSimulcastProperties: function(offer, answer) {
> -  if (!offer.includes("a=simulcast:")) {
> -    return answer;
> +transferSimulcastProperties: function(offer_sdp, answer_sdp) {
> +  if (!offer_sdp.includes("a=simulcast:")) {
> +    return answer_sdp;

Nit: A bit of a mix of styles in this file. offer_sdp vs. offerSdp. See existing `expectedType` for instance.

camelCase seems to be (vaguely) preferred in style guide FWIW: https://developer.mozilla.org/en-US/docs/Mozilla/Developer_guide/Coding_Style#Methods

::: dom/media/tests/mochitest/test_peerConnection_simulcastOffer.html:22
(Diff revisions 3 - 4)
> -      if (i) {
> +      helper.drawColor(canvas, i ? helper.green : helper.red);
> -        helper.drawColor(canvas, helper.green);
> -      } else {
> -        helper.drawColor(canvas, helper.red);
> -      }
>        i = 1 - i;
> +      try {
> -      stream.requestFrame();
> +        stream.requestFrame();

Shouldn't the try be around the whole function, i.e. including the hitting of canvas to be safe?

::: dom/media/tests/mochitest/test_peerConnection_simulcastOffer.html:49
(Diff revisions 3 - 4)
> +  var ensureNoColorChange = (helper, canvas) => Promise.race([
> +      waitForColorChange(helper, canvas).then(() => ok(false, "Color should not change")),
> +      wait(1000).then(() => ok(true, "No color change"))

Isn't one second aggressive for any test that might run on connected devices? Smells like orange-fodder to me.

Also, with the switching interval being half a second, isn't one second the smallest time that waitForColorChange can possibly take? If so, then it may never beat the wait(1000) in this race, amounting to dead code.

Comment 34

[Byron Campen [:bwc]]

https://reviewboard.mozilla.org/r/30935/#review28363

> Shouldn't the try be around the whole function, i.e. including the hitting of canvas to be safe?

I'm ok with that.

> Isn't one second aggressive for any test that might run on connected devices? Smells like orange-fodder to me.
> 
> Also, with the switching interval being half a second, isn't one second the smallest time that waitForColorChange can possibly take? If so, then it may never beat the wait(1000) in this race, amounting to dead code.

So the one second is how long we're willing to watch for an invalid transition; in the event that something bogs down performance wise, no orange will result, but we might not notice a transition if selectRecvSsrc doesn't work.

Absent nasty delays/performance problems, waitForColorChange should take at most 500ms, since the Promise.all waits until it has seen both colors, in either order.

I can increase the timeout to 2000ms, but I don't want to increase too much, since that'll waste a lot of time.

Comment 35

[Byron Campen [:bwc]]

Comment on attachment 8707959 [details]
MozReview Request: Bug 1231507 - Part 2: added mochitest SimulCast offer test case r=jib

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/30935/diff/5-6/

Comment 36

[Byron Campen [:bwc]]

Comment on attachment 8697334 [details]
MozReview Request: Bug 1231507 - Part 1: selectSsrc chrome-only API for SSRC-based filtering of receive tracks. r=drno, r=smaug

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/27565/diff/5-6/

Comment 37

[Byron Campen [:bwc]]

Comment on attachment 8707959 [details]
MozReview Request: Bug 1231507 - Part 2: added mochitest SimulCast offer test case r=jib

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/30935/diff/6-7/

Comment 38

[Nils Ohlmeier [:drno]]

Comment on attachment 8697334 [details]
MozReview Request: Bug 1231507 - Part 1: selectSsrc chrome-only API for SSRC-based filtering of receive tracks. r=drno, r=smaug

https://reviewboard.mozilla.org/r/27565/#review28587

LGTM

::: media/webrtc/signaling/src/mediapipeline/MediaPipeline.cpp:209
(Diff revision 6)
> +  }

Do we want a log message here in case a test cases tries to select something which we haven't received?

Comment 39

[Byron Campen [:bwc]]

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

Comment 40

[Byron Campen [:bwc]]

Comment on attachment 8697334 [details]
MozReview Request: Bug 1231507 - Part 1: selectSsrc chrome-only API for SSRC-based filtering of receive tracks. r=drno, r=smaug

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/27565/diff/6-7/

Comment 41

[Byron Campen [:bwc]]

Comment on attachment 8707959 [details]
MozReview Request: Bug 1231507 - Part 2: added mochitest SimulCast offer test case r=jib

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/30935/diff/7-8/

Comment 42

[Pulsebot]

https://hg.mozilla.org/integration/mozilla-inbound/rev/d2c568567473
https://hg.mozilla.org/integration/mozilla-inbound/rev/ad4931f85249

Comment 43

[Phil Ringnalda (:philor)]

https://hg.mozilla.org/mozilla-central/rev/d2c568567473
https://hg.mozilla.org/mozilla-central/rev/ad4931f85249