818714 - Offer for peer connection always contains video and audio stream even if only one of those has been requested by gUM

Status: VERIFIED FIXED

(Core :: WebRTC: Signaling, defect)

Description

[Henrik Skupin [:whimboo][⌚️UTC+2]]

When building up a peer connection between two parties with only video or audio requested by gUM, the offer always contains audio and video streams. But we should only offer those streams which really have been requested.

Attached you find a testcase you can run inside your browser which dumps all the necessary information to the terminal (console)

You will see that onaddstream gets called 4 times while it should only be called twice:

******* step 1: {"type":"offer","sdp":"v=0\r\no=Mozilla-SIPUA 7710 0 IN IP4 0.0.0.0\r\ns=SIP Call\r\nt=0 0\r\na=ice-ufrag:bf1a0af8\r\na=ice-pwd:baf53806a1d6b5c57b838de7d704eab0\r\na=fingerprint:sha-256 68:04:28:F0:9F:0D:28:CB:24:55:63:93:79:FC:A8:E5:9E:38:BE:DD:F1:F2:B5:B1:75:12:ED:B6:9D:A8:8A:81\r\nm=audio 65529 RTP/SAVPF 109 0 8 101\r\nc=IN IP4 XYZ.34.51\r\na=rtpmap:109 opus/48000/2\r\na=ptime:20\r\na=rtpmap:0 PCMU/8000\r\na=rtpmap:8 PCMA/8000\r\na=rtpmap:101 telephone-event/8000\r\na=fmtp:101 0-15\r\na=sendrecv\r\na=candidate:0 1 UDP 2113601791 192.168.123.16 51189 typ host\r\na=candidate:1 1 UDP 1694236671 XYZ.34.51 51189 typ srflx raddr 192.168.123.16 rport 51189\r\na=candidate:2 1 UDP 2113667327 192.168.123.3 65529 typ host\r\na=candidate:3 1 UDP 1694302207 XYZ.34.51 65529 typ srflx raddr 192.168.123.3 rport 65529\r\na=candidate:4 1 UDP 2111832319 10.254.250.38 55766 typ host\r\na=candidate:6 1 UDP 2111766783 10.2.21.142 50095 typ host\r\na=candidate:0 2 UDP 2113601790 192.168.123.16 49399 typ host\r\na=candidate:1 2 UDP 1694236670 XYZ.34.51 49399 typ srflx raddr 192.168.123.16 rport 49399\r\na=candidate:2 2 UDP 2113667326 192.168.123.3 53951 typ host\r\na=candidate:3 2 UDP 1694302206 XYZ.34.51 53951 typ srflx raddr 192.168.123.3 rport 53951\r\na=candidate:4 2 UDP 2111832318 10.254.250.38 55613 typ host\r\na=candidate:6 2 UDP 2111766782 10.2.21.142 61720 typ host\r\nm=video 58399 RTP/SAVPF 120\r\nc=IN IP4 XYZ.34.51\r\na=rtpmap:120 VP8/90000\r\na=recvonly\r\na=candidate:0 1 UDP 2113601791 192.168.123.16 49456 typ host\r\na=candidate:1 1 UDP 1694236671 XYZ.34.51 49456 typ srflx raddr 192.168.123.16 rport 49456\r\na=candidate:2 1 UDP 2113667327 192.168.123.3 58399 typ host\r\na=candidate:3 1 UDP 1694302207 XYZ.34.51 58399 typ srflx raddr 192.168.123.3 rport 58399\r\na=candidate:4 1 UDP 2111832319 10.254.250.38 50047 typ host\r\na=candidate:6 1 UDP 2111766783 10.2.21.142 53463 typ host\r\na=candidate:0 2 UDP 2113601790 192.168.123.16 51149 typ host\r\na=candidate:1 2 UDP 1694236670 XYZ.34.51 51149 typ srflx raddr 192.168.123.16 rport 51149\r\na=candidate:2 2 UDP 2113667326 192.168.123.3 64521 typ host\r\na=candidate:3 2 UDP 1694302206 XYZ.34.51 64521 typ srflx raddr 192.168.123.3 rport 64521\r\na=candidate:4 2 UDP 2111832318 10.254.250.38 57614 typ host\r\na=candidate:6 2 UDP 2111766782 10.2.21.142 63188 typ host\r\nm=application 64527 SCTP/DTLS 5000 \r\nc=IN IP4 XYZ.34.51\r\na=fmtp:5000 protocol=webrtc-datachannel;streams=16\r\na=sendrecv\r\na=candidate:0 1 UDP 2113601791 192.168.123.16 62578 typ host\r\na=candidate:1 1 UDP 1694236671 XYZ.34.51 62578 typ srflx raddr 192.168.123.16 rport 62578\r\na=candidate:2 1 UDP 2113667327 192.168.123.3 64527 typ host\r\na=candidate:3 1 UDP 1694302207 XYZ.34.51 64527 typ srflx raddr 192.168.123.3 rport 64527\r\na=candidate:4 1 UDP 2111832319 10.254.250.38 58788 typ host\r\na=candidate:6 1 UDP 2111766783 10.2.21.142 53204 typ host\r\na=candidate:0 2 UDP 2113601790 192.168.123.16 55764 typ host\r\na=candidate:1 2 UDP 1694236670 XYZ.34.51 55764 typ srflx raddr 192.168.123.16 rport 55764\r\na=candidate:2 2 UDP 2113667326 192.168.123.3 55006 typ host\r\na=candidate:3 2 UDP 1694302206 XYZ.34.51 55006 typ srflx raddr 192.168.123.3 rport 55006\r\na=candidate:4 2 UDP 2111832318 10.254.250.38 61954 typ host\r\na=candidate:6 2 UDP 2111766782 10.2.21.142 54092 typ host\r\n"}
********** type: audio
********** type: video
******* step 4: {"type":"answer","sdp":"v=0\r\no=Mozilla-SIPUA 1141 0 IN IP4 0.0.0.0\r\ns=SIP Call\r\nt=0 0\r\na=ice-ufrag:0a1eb514\r\na=ice-pwd:0843bb329461cb1a23bde2ea90688d4b\r\na=fingerprint:sha-256 0B:C4:E4:C3:DE:90:49:85:6A:CB:98:2E:7C:0D:7D:7E:8B:CC:78:DB:1F:2D:DA:D4:DE:32:F3:D0:0C:14:DF:77\r\nm=audio 63976 RTP/SAVPF 109 101\r\nc=IN IP4 XYZ.34.51\r\na=rtpmap:109 opus/48000/2\r\na=ptime:20\r\na=rtpmap:101 telephone-event/8000\r\na=fmtp:101 0-15\r\na=sendrecv\r\na=candidate:0 1 UDP 2113601791 192.168.123.16 56990 typ host\r\na=candidate:1 1 UDP 1694236671 XYZ.34.51 56990 typ srflx raddr 192.168.123.16 rport 56990\r\na=candidate:2 1 UDP 2113667327 192.168.123.3 63976 typ host\r\na=candidate:3 1 UDP 1694302207 XYZ.34.51 63976 typ srflx raddr 192.168.123.3 rport 63976\r\na=candidate:4 1 UDP 2111832319 10.254.250.38 51440 typ host\r\na=candidate:6 1 UDP 2111766783 10.2.21.142 56038 typ host\r\na=candidate:0 2 UDP 2113601790 192.168.123.16 53466 typ host\r\na=candidate:1 2 UDP 1694236670 XYZ.34.51 53466 typ srflx raddr 192.168.123.16 rport 53466\r\na=candidate:2 2 UDP 2113667326 192.168.123.3 55292 typ host\r\na=candidate:3 2 UDP 1694302206 XYZ.34.51 55292 typ srflx raddr 192.168.123.3 rport 55292\r\na=candidate:4 2 UDP 2111832318 10.254.250.38 64806 typ host\r\na=candidate:6 2 UDP 2111766782 10.2.21.142 51827 typ host\r\nm=video 65182 RTP/SAVPF 120\r\nc=IN IP4 XYZ.34.51\r\na=rtpmap:120 VP8/90000\r\na=inactive\r\na=candidate:0 1 UDP 2113601791 192.168.123.16 49347 typ host\r\na=candidate:1 1 UDP 1694236671 XYZ.34.51 49347 typ srflx raddr 192.168.123.16 rport 49347\r\na=candidate:2 1 UDP 2113667327 192.168.123.3 65182 typ host\r\na=candidate:3 1 UDP 1694302207 XYZ.34.51 65182 typ srflx raddr 192.168.123.3 rport 65182\r\na=candidate:4 1 UDP 2111832319 10.254.250.38 65175 typ host\r\na=candidate:6 1 UDP 2111766783 10.2.21.142 56978 typ host\r\na=candidate:0 2 UDP 2113601790 192.168.123.16 51602 typ host\r\na=candidate:1 2 UDP 1694236670 XYZ.34.51 51602 typ srflx raddr 192.168.123.16 rport 51602\r\na=candidate:2 2 UDP 2113667326 192.168.123.3 60399 typ host\r\na=candidate:3 2 UDP 1694302206 XYZ.34.51 60399 typ srflx raddr 192.168.123.3 rport 60399\r\na=candidate:4 2 UDP 2111832318 10.254.250.38 60980 typ host\r\na=candidate:6 2 UDP 2111766782 10.2.21.142 52231 typ host\r\nm=application 60364 SCTP/DTLS 5001 \r\nc=IN IP4 XYZ.34.51\r\na=sendrecv\r\na=candidate:0 1 UDP 2113601791 192.168.123.16 61324 typ host\r\na=candidate:1 1 UDP 1694236671 XYZ.34.51 61324 typ srflx raddr 192.168.123.16 rport 61324\r\na=candidate:2 1 UDP 2113667327 192.168.123.3 60364 typ host\r\na=candidate:3 1 UDP 1694302207 XYZ.34.51 60364 typ srflx raddr 192.168.123.3 rport 60364\r\na=candidate:4 1 UDP 2111832319 10.254.250.38 61934 typ host\r\na=candidate:6 1 UDP 2111766783 10.2.21.142 50930 typ host\r\na=candidate:0 2 UDP 2113601790 192.168.123.16 59101 typ host\r\na=candidate:1 2 UDP 1694236670 XYZ.34.51 59101 typ srflx raddr 192.168.123.16 rport 59101\r\na=candidate:2 2 UDP 2113667326 192.168.123.3 54067 typ host\r\na=candidate:3 2 UDP 1694302206 XYZ.34.51 54067 typ srflx raddr 192.168.123.3 rport 54067\r\na=candidate:4 2 UDP 2111832318 10.254.250.38 56166 typ host\r\na=candidate:6 2 UDP 2111766782 10.2.21.142 51861 typ host\r\n"}

Comment 1

[Henrik Skupin [:whimboo][⌚️UTC+2]]

Attachment: testcase

Comment 2

[Eric Rescorla (:ekr)]

So in this case, I notice that the video in the offer is recvonly, so it would appear that what's going on is that we are defaulting to OfferToReceiveVideo. I believe that this is not spec compliant if no video stream has been added.

Comment 3

[Henrik Skupin [:whimboo][⌚️UTC+2]]

(In reply to Eric Rescorla (:ekr) from comment #2)
> So in this case, I notice that the video in the offer is recvonly, so it
> would appear that what's going on is that we are defaulting to
> OfferToReceiveVideo. I believe that this is not spec compliant if no video
> stream has been added.

Well, but the same happens if you only request video via gUM. onaddstream is always fired twice. Could this be related to bug 816780?

Comment 4

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

(from IRC):
You could always add an a=x-temp-myapp: video: true/false and then search the incoming SDP string for that. It's a huge hack, but that's ok. In any case, the app is responsible for sending and receiving the offers/answers, and can encapsulate them anyway they want. So you could also send the video state outside of the SDP

I.e. add an "a=x-temp-blahapp: video=true" to the SDP, then on reception search for that (or with video=false).  You can leave it in the SDP; it will get ignored.  Or send the SDP over, and include separately (different attr in json object, etc) a private indication of whether video was intended to be offered.  The application controls how to send the signaling; it can send more than just a bare SDP string.

This obviously only helps when your app is talking to your app (or other things that support the same hack); inter-app calls won't be helped by this.  If that's important, it's a different solution: edit the m=video section - either remove m=video (down to the next m= line or the end) or edit the port number to 0.  (a=inactive in place of a=recvonly should work, but I suspect it hits the same bug)

Comment 5

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

From IRC:
	flo-retina	jesup: the hack you just described is similar to what I did. On the audio-only calling side I have: offer.sdp = offer.sdp.split("m=").filter(function(s) { return !s.startsWith("video"); }).join("m=");
[12:14]	flo-retina	jesup: on the receiving side I have offer.sdp = offer.sdp.split("m=").filter(function(s) { return !s.startsWith("video") || s.indexOf("a=recvonly") == -1; }).join("m=");

Comment 6

[Adam Roach [:abr]]

For ease of reading, the reported SDP from comment 1 looks like this when you unwrap the lines:
> v=0
> o=Mozilla-SIPUA 7710 0 IN IP4 0.0.0.0
> s=SIP Call
> t=0 0
> a=ice-ufrag:bf1a0af8
> a=ice-pwd:baf53806a1d6b5c57b838de7d704eab0
> a=fingerprint:sha-256 68:04:28:F0:9F:0D:28:CB:24:55:63:93:79:FC:A8:E5:9E:38:BE:DD:F1:F2:B5:B1:75:12:ED:B6:9D:A8:8A:81
> m=audio 65529 RTP/SAVPF 109 0 8 101
> c=IN IP4 XYZ.34.51
> a=rtpmap:109 opus/48000/2
> a=ptime:20
> a=rtpmap:0 PCMU/8000
> a=rtpmap:8 PCMA/8000
> a=rtpmap:101 telephone-event/8000
> a=fmtp:101 0-15
> a=sendrecv
> a=candidate:0 1 UDP 2113601791 192.168.123.16 51189 typ host
> a=candidate:1 1 UDP 1694236671 XYZ.34.51 51189 typ srflx raddr 192.168.123.16 rport 51189
> a=candidate:2 1 UDP 2113667327 192.168.123.3 65529 typ host
> a=candidate:3 1 UDP 1694302207 XYZ.34.51 65529 typ srflx raddr 192.168.123.3 rport 65529
> a=candidate:4 1 UDP 2111832319 10.254.250.38 55766 typ host
> a=candidate:6 1 UDP 2111766783 10.2.21.142 50095 typ host
> a=candidate:0 2 UDP 2113601790 192.168.123.16 49399 typ host
> a=candidate:1 2 UDP 1694236670 XYZ.34.51 49399 typ srflx raddr 192.168.123.16 rport 49399
> a=candidate:2 2 UDP 2113667326 192.168.123.3 53951 typ host
> a=candidate:3 2 UDP 1694302206 XYZ.34.51 53951 typ srflx raddr 192.168.123.3 rport 53951
> a=candidate:4 2 UDP 2111832318 10.254.250.38 55613 typ host
> a=candidate:6 2 UDP 2111766782 10.2.21.142 61720 typ host
> m=video 58399 RTP/SAVPF 120
> c=IN IP4 XYZ.34.51
> a=rtpmap:120 VP8/90000
> a=recvonly
> a=candidate:0 1 UDP 2113601791 192.168.123.16 49456 typ host
> a=candidate:1 1 UDP 1694236671 XYZ.34.51 49456 typ srflx raddr 192.168.123.16 rport 49456
> a=candidate:2 1 UDP 2113667327 192.168.123.3 58399 typ host
> a=candidate:3 1 UDP 1694302207 XYZ.34.51 58399 typ srflx raddr 192.168.123.3 rport 58399
> a=candidate:4 1 UDP 2111832319 10.254.250.38 50047 typ host
> a=candidate:6 1 UDP 2111766783 10.2.21.142 53463 typ host
> a=candidate:0 2 UDP 2113601790 192.168.123.16 51149 typ host
> a=candidate:1 2 UDP 1694236670 XYZ.34.51 51149 typ srflx raddr 192.168.123.16 rport 51149
> a=candidate:2 2 UDP 2113667326 192.168.123.3 64521 typ host
> a=candidate:3 2 UDP 1694302206 XYZ.34.51 64521 typ srflx raddr 192.168.123.3 rport 64521
> a=candidate:4 2 UDP 2111832318 10.254.250.38 57614 typ host
> a=candidate:6 2 UDP 2111766782 10.2.21.142 63188 typ host
> m=application 64527 SCTP/DTLS 5000 
> c=IN IP4 XYZ.34.51
> a=fmtp:5000 protocol=webrtc-datachannel;streams=16
> a=sendrecv
> a=candidate:0 1 UDP 2113601791 192.168.123.16 62578 typ host
> a=candidate:1 1 UDP 1694236671 XYZ.34.51 62578 typ srflx raddr 192.168.123.16 rport 62578
> a=candidate:2 1 UDP 2113667327 192.168.123.3 64527 typ host
> a=candidate:3 1 UDP 1694302207 XYZ.34.51 64527 typ srflx raddr 192.168.123.3 rport 64527
> a=candidate:4 1 UDP 2111832319 10.254.250.38 58788 typ host
> a=candidate:6 1 UDP 2111766783 10.2.21.142 53204 typ host
> a=candidate:0 2 UDP 2113601790 192.168.123.16 55764 typ host
> a=candidate:1 2 UDP 1694236670 XYZ.34.51 55764 typ srflx raddr 192.168.123.16 rport 55764
> a=candidate:2 2 UDP 2113667326 192.168.123.3 55006 typ host
> a=candidate:3 2 UDP 1694302206 XYZ.34.51 55006 typ srflx raddr 192.168.123.3 rport 55006
> a=candidate:4 2 UDP 2111832318 10.254.250.38 61954 typ host
> a=candidate:6 2 UDP 2111766782 10.2.21.142 54092 typ host

Comment 7

[Adam Roach [:abr]]

Attachment: Set media enabled to FALSE unless added using addStream

Comment 8

[Adam Roach [:abr]]

Comment on attachment 694093 [details] [diff] [review]
Set media enabled to FALSE unless added using addStream

Review of attachment 694093 [details] [diff] [review]:
-----------------------------------------------------------------

::: media/webrtc/signaling/src/sipcc/core/gsm/fsmdef.c
@@ +3062,5 @@
> +    cause = gsmsdp_negotiate_media_lines(fcb, dcb->sdp, 
> +            /* initial_offer */       TRUE, 
> +            /* offer */               TRUE, 
> +            /* notify_stream_added */ FALSE, 
> +            /* create_answer */       TRUE);

Oops -- I see the trailing spaces here. I'll fix them along with any other review comments.

::: media/webrtc/signaling/src/sipcc/core/gsm/gsm_sdp.c
@@ +147,5 @@
> +         * the W3C hasn't specified the proper behavior here anyway, so
> +         * we would only be implementing speculatively) -- so we'll
> +         * always offer data channels until the standard is
> +         * a bit more set.
> +         */

Ditto on these trailing spaces.

::: media/webrtc/signaling/test/signaling_unittests.cpp
@@ +1172,5 @@
>    answerconstraints.setBooleanConstraint("OfferToReceiveAudio", true, false);
>    answerconstraints.setBooleanConstraint("OfferToReceiveVideo", true, false);
>    OfferAnswer(offerconstraints, answerconstraints, OFFER_AUDIO | ANSWER_AV,
> +              false, SHOULD_SENDRECV_AUDIO | SHOULD_OMIT_VIDEO, 
> +              SHOULD_SENDRECV_AUDIO | SHOULD_OMIT_VIDEO);

...and this one.

Comment 9

[Ethan Hugg [:ehugg]]

Comment on attachment 694093 [details] [diff] [review]
Set media enabled to FALSE unless added using addStream


r+ with whitespace fixed.  Built/tested on Ubuntu64.

Comment 10

[Adam Roach [:abr]]

Attachment: Set media enabled to FALSE unless added using addStream,

Comment 11

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

https://hg.mozilla.org/integration/mozilla-inbound/rev/60f6033378d4

Comment 12

[Henrik Skupin [:whimboo][⌚️UTC+2]]

The landing of this patch will cause unexpected passes on the alder branch once it gets merged. Sadly we do not run the mochitests on m-c given the current list of leaks, but I would like that we proof code stability by running tests in the future.

 2:31.73 TEST-UNEXPECTED-PASS| unknown test url | No remote video stream has been attached to the local peer - 0 should equal 0
 2:31.73 TEST-PASS | unknown test url | A remote audio stream has been attached to the remote peer
 2:31.73 TEST-UNEXPECTED-PASS| unknown test url | No remote video stream has been attached to the remote peer - 0 should equal 0

For this time I will update the tests on bug 796890 but in the future changes have to be also made in the actual patch.

Comment 14

[Henrik Skupin [:whimboo][⌚️UTC+2]]

Comment on attachment 694190 [details] [diff] [review]
Set media enabled to FALSE unless added using addStream,

I had to back this out because it introduces a new sigabort crash. Details will follow in a couple of minutes.

Backout:
https://hg.mozilla.org/integration/mozilla-inbound/rev/7d7600c0d5d0

Comment 15

[Henrik Skupin [:whimboo][⌚️UTC+2]]

Steps to follow:

1. Add 'export MOZ_WEBRTC_LEAKING_TESTS=1' to your mozconfig
2. Rebuild by doing a clobber build
3. Run ./mach mochitest-plain dom/media/tests/mochitest/test_peerConnection_basicVideo.html --debugger=gdb

Crash details:

Assertion failure: PR_FALSE, at /Volumes/data/code/firefox/nightly/media/webrtc/signaling/src/media/VcmSIPCCBinding.cpp:1283

Program received signal SIGABRT, Aborted.
0x00007fff8c792ce2 in __pthread_kill ()

#0  0x00007fff8c792ce2 in __pthread_kill ()
#1  0x00007fff894a57d2 in pthread_kill ()
#2  0x00007fff89496a7a in abort ()
#3  0x0000000100308f8d in PR_Assert (s=0x104dad895 "PR_FALSE", file=0x104dacefe "/Volumes/data/code/firefox/nightly/media/webrtc/signaling/src/media/VcmSIPCCBinding.cpp", ln=1283) at /Volumes/data/code/firefox/nightly/nsprpub/pr/src/io/prlog.c:554
#4  0x0000000104281542 in vcmRxStartICE_m (mcap_id=1, group_id=0, stream_id=5, level=1, pc_stream_id=1, pc_track_id=0, call_handle=131074, peerconnection=0x10c294bcc "931e8f412c4fee4e", num_payloads=1, payloads=0x115ee79b0, fingerprint_alg=0x1069e9b30 "sha-256", fingerprint=0x1069e9b3a "78:F2:48:0B:F7:90:55:27:D4:03:B3:36:E7:0E:23:FA:35:98:CB:31:FF:61:D3:5C:F5:66:98:08:F6:C0:D7:24", attrs=0x1193fb888) at VcmSIPCCBinding.cpp:1283
#5  0x0000000104285fc5 in mozilla::runnable_args_nm_13_ret<int (*)(unsigned short, unsigned short, unsigned short, int, int, int, unsigned int, char const*, int, vcm_payload_info_t const*, char const*, char const*, vcm_attrs_t_*), unsigned short, unsigned short, unsigned short, int, int, int, unsigned int, char const*, int, vcm_payload_info_t const*, char const*, char const*, vcm_attrs_t_*, int>::Run (this=0x111aa1a10) at runnable_utils_generated.h:1291
#6  0x00000001035ff532 in nsThreadSyncDispatch::Run (this=0x11929b020) at /Volumes/data/code/firefox/nightly/xpcom/threads/nsThread.cpp:774
#7  0x00000001035fec24 in nsThread::ProcessNextEvent (this=0x10bd08780, mayWait=false, result=0x7fff5fbfca33) at /Volumes/data/code/firefox/nightly/xpcom/threads/nsThread.cpp:627
#8  0x0000000103566f22 in NS_ProcessPendingEvents_P (thread=0x10bd08780, timeout=20) at nsThreadUtils.cpp:187
#9  0x0000000102f5a85f in nsBaseAppShell::NativeEventCallback (this=0x10c90b8f0) at /Volumes/data/code/firefox/nightly/widget/xpwidgets/nsBaseAppShell.cpp:97
#10 0x0000000102eed04c in nsAppShell::ProcessGeckoEvents (aInfo=0x10c90b8f0) at /Volumes/data/code/firefox/nightly/widget/cocoa/nsAppShell.mm:387
#11 0x00007fff820884f1 in __CFRUNLOOP_IS_CALLING_OUT_TO_A_SOURCE0_PERFORM_FUNCTION__ ()
#12 0x00007fff82087d5d in __CFRunLoopDoSources0 ()
#13 0x00007fff820aeb49 in __CFRunLoopRun ()
#14 0x00007fff820ae486 in CFRunLoopRunSpecific ()
#15 0x00007fff844d52bf in RunCurrentEventLoopInMode ()
#16 0x00007fff844dc4bf in ReceiveNextEventCommon ()
#17 0x00007fff844dc3fa in BlockUntilNextEventMatchingListInMode ()
#18 0x00007fff83632779 in _DPSNextEvent ()
#19 0x00007fff8363207d in -[NSApplication nextEventMatchingMask:untilDate:inMode:dequeue:] ()
#20 0x0000000102eeb8b7 in -[GeckoNSApplication nextEventMatchingMask:untilDate:inMode:dequeue:] (self=0x10bd2c690, _cmd=0x7fff83f0a458, mask=18446744073709551615, expiration=0x422d63c37f00000d, mode=0x7fff7038cae0, flag=1 '\001') at /Volumes/data/code/firefox/nightly/widget/cocoa/nsAppShell.mm:164
#21 0x00007fff8362e9b9 in -[NSApplication run] ()
#22 0x0000000102eedacc in nsAppShell::Run (this=0x10c90b8f0) at /Volumes/data/code/firefox/nightly/widget/cocoa/nsAppShell.mm:741
#23 0x0000000102b56eb7 in nsAppStartup::Run (this=0x10c90b790) at /Volumes/data/code/firefox/nightly/toolkit/components/startup/nsAppStartup.cpp:289
#24 0x0000000101011228 in XREMain::XRE_mainRun (this=0x7fff5fbfe748) at /Volumes/data/code/firefox/nightly/toolkit/xre/nsAppRunner.cpp:3824
#25 0x00000001010119ab in XREMain::XRE_main (this=0x7fff5fbfe748, argc=6, argv=0x7fff5fbff338, aAppData=0x1000081c0) at /Volumes/data/code/firefox/nightly/toolkit/xre/nsAppRunner.cpp:3891
#26 0x0000000101011ddf in XRE_main (argc=6, argv=0x7fff5fbff338, aAppData=0x1000081c0, aFlags=0) at /Volumes/data/code/firefox/nightly/toolkit/xre/nsAppRunner.cpp:4089
#27 0x0000000100001d3e in do_main (argc=6, argv=0x7fff5fbff338) at /Volumes/data/code/firefox/nightly/browser/app/nsBrowserApp.cpp:174
#28 0x00000001000015b7 in main (argc=6, argv=0x7fff5fbff338) at /Volumes/data/code/firefox/nightly/browser/app/nsBrowserApp.cpp:279

Comment 16

[Adam Roach [:abr]]

The patch on bug 796890 is required to avoid failures in the mochitests.

Comment 17

[Adam Roach [:abr]]

Attachment: Set media enabled to FALSE unless added using addStream,

Comment 18

[Adam Roach [:abr]]

Okay; this patch passes unit tests (including one I added to repro the problem whimboo describes above). When applied in combination with the patch for bug 796890, it passes test_peerConnection_basicVideo.html and introduces no *additional* mochitest or crashtest failures.

However, mochitests on head of m-i presently aborts in the Deadlock Detector (both with and without this patch), and crashtests aborts in the destructor for sigslot::lock_block<> (again, both with and without this patch). As a consequence, pushing to try on top of Alder would almost certainly be a waste of resources.

I'm asking for re-review on the patch because I've had to make functional changes to the code.

Comment 19

[Adam Roach [:abr]]

Comment on attachment 694565 [details] [diff] [review]
Set media enabled to FALSE unless added using addStream,

Reassigning to jesup for re-review. The key change is in lsm_rx_start. Here's the interdiff from the version ehugg looked at: https://bugzilla.mozilla.org/attachment.cgi?oldid=694190&action=interdiff&newid=694565&headers=1

Comment 20

[Adam Roach [:abr]]

To be complete, I've verified that the mochi tests and crash tests currently both crash (in the same way as mozilla-inbound) on the Alder branch, even without this patch.

Comment 21

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

https://hg.mozilla.org/integration/mozilla-inbound/rev/8667a82f1bf2

Comment 22

[Ryan VanderMeulen [:RyanVM]]

https://hg.mozilla.org/mozilla-central/rev/8667a82f1bf2

Comment 23

[Henrik Skupin [:whimboo][⌚️UTC+2]]

The patch on bug 796890 should have been landed on inbound together with this one. As result the mochitests are broken now. I will go ahead and land my tests from bug 796890.

Comment 24

[Henrik Skupin [:whimboo][⌚️UTC+2]]

(In reply to Adam Roach [:abr] from comment #18)
> However, mochitests on head of m-i presently aborts in the Deadlock Detector

Yes, that should be bug 821292 which should also be fixed now.

> (both with and without this patch), and crashtests aborts in the destructor
> for sigslot::lock_block<> (again, both with and without this patch). As a

Interesting. I haven't seen this before. Might be worth filing if it hasn't been done so yet.

Comment 25

[Eric Rescorla (:ekr)]

I'd like to see someone rerun them with the current m-i and verify that they still exist.

Comment 26

[Florian Quèze [:florian]]

(In reply to Randell Jesup [:jesup] from comment #5)
> From IRC:
> 	flo-retina	jesup: the hack you just described is similar to what I did. On
> the audio-only calling side I have: offer.sdp =
> offer.sdp.split("m=").filter(function(s) { return !s.startsWith("video");
> }).join("m=");

I've just verified that I no longer need this hack when using a current nightly. Thanks!

Comment 27

[Henrik Skupin [:whimboo][⌚️UTC+2]]

Randell, would you mind doing another merge with my test changes included? Those are on m-c now and will become part of alder.

For this specific bug I can agree that it is fixed now. Thanks a lot!