Closed
Bug 1323723
Opened 7 years ago
Closed 7 years ago
RTCPeerConnection: stop sending media when applying a second SDP offer
Categories
(Core :: WebRTC: Networking, defect, P2)
Tracking
()
RESOLVED
FIXED
mozilla55
Tracking | Status | |
---|---|---|
firefox55 | --- | fixed |
backlog | webrtc/webaudio+ |
People
(Reporter: mparisdiaz, Assigned: drno)
References
(Depends on 1 open bug)
Details
Attachments
(6 files, 1 obsolete file)
User Agent: Mozilla/5.0 (X11; Ubuntu; Linux x86_64; rv:50.0) Gecko/20100101 Firefox/50.0 Build ID: 20161209095719 Steps to reproduce: Having the next 2 SDP negotiations: Negotiation 1 - LOCAL SDP OFFER v=0 o=mozilla...THIS_IS_SDPARTA-50.1.0 2543689248089967985 0 IN IP4 0.0.0.0 s=- t=0 0 a=fingerprint:sha-256 26:09:35:29:4A:CC:DB:53:08:B9:82:F7:FF:21:A9:FD:A5:7B:DF:58:68:56:4E:3A:55:9D:9F:29:01:20:8A:A1 a=group:BUNDLE sdparta_0 sdparta_1 a=ice-options:trickle a=msid-semantic:WMS * m=audio 9 UDP/TLS/RTP/SAVPF 109 9 0 8 c=IN IP4 0.0.0.0 a=recvonly a=extmap:1 urn:ietf:params:rtp-hdrext:ssrc-audio-level a=fmtp:109 maxplaybackrate=48000;stereo=1;useinbandfec=1 a=ice-pwd:104efa9005ab9a78ae3436942cdd11e6 a=ice-ufrag:3d69bb57 a=mid:sdparta_0 a=rtcp-mux a=rtpmap:109 opus/48000/2 a=rtpmap:9 G722/8000/1 a=rtpmap:0 PCMU/8000 a=rtpmap:8 PCMA/8000 a=setup:actpass a=ssrc:2072412225 cname:{1a9156b4-1a09-4d51-9fba-316325af0062} m=video 9 UDP/TLS/RTP/SAVPF 120 126 97 c=IN IP4 0.0.0.0 a=sendrecv a=fmtp:126 profile-level-id=42e01f;level-asymmetry-allowed=1;packetization-mode=1 a=fmtp:97 profile-level-id=42e01f;level-asymmetry-allowed=1 a=fmtp:120 max-fs=12288;max-fr=60 a=ice-pwd:104efa9005ab9a78ae3436942cdd11e6 a=ice-ufrag:3d69bb57 a=mid:sdparta_1 a=msid:{fac09d71-a62d-464c-bc01-e3b2f564580e} {e13d494d-9bfc-4fb1-a6b5-e656bd82c1e0} a=rtcp-fb:120 nack a=rtcp-fb:120 nack pli a=rtcp-fb:120 ccm fir a=rtcp-fb:120 goog-remb a=rtcp-fb:126 nack a=rtcp-fb:126 nack pli a=rtcp-fb:126 ccm fir a=rtcp-fb:126 goog-remb a=rtcp-fb:97 nack a=rtcp-fb:97 nack pli a=rtcp-fb:97 ccm fir a=rtcp-fb:97 goog-remb a=rtcp-mux a=rtpmap:120 VP8/90000 a=rtpmap:126 H264/90000 a=rtpmap:97 H264/90000 a=setup:actpass a=ssrc:251820637 cname:{1a9156b4-1a09-4d51-9fba-316325af0062} Negotiation 1 - REMOTE SDP ANSWER v=0 o=- 3690792591 3690792591 IN IP4 0.0.0.0 s=VM76da225fd9594c06b60d05a7bee52203 t=0 0 a=ice-options:trickle a=msid-semantic:WMS * a=group:BUNDLE sdparta_0 sdparta_1 a=msid-semantic:WMS * a=ice-lite m=audio 13330 UDP/TLS/RTP/SAVPF 109 0 c=IN IP4 172.22.2.175 a=inactive a=mid:sdparta_0 a=rtcp:13330 IN IP4 172.22.2.175 a=rtpmap:109 opus/48000/2 a=rtpmap:0 PCMU/8000 a=fmtp:109 maxplaybackrate=48000;stereo=1;useinbandfec=1 a=rtcp-mux a=setup:active a=ice-ufrag:HLBq a=ice-pwd:ru2tvQBt7TMLAYKIUQoVCQ a=fingerprint:sha-256 07:94:59:74:19:9C:57:D6:0A:9A:7A:7B:CE:0B:4B:A1:31:80:72:C2:EF:6C:61:4A:EA:71:9E:50:28:1D:35:82 a=candidate:1 1 UDP 2013266431 fe80::897:43ff:fe24:2b3a 13849 typ host a=candidate:2 1 UDP 2013266431 54.88.69.222 13330 typ host a=candidate:1 2 UDP 2013266430 fe80::897:43ff:fe24:2b3a 13036 typ host a=candidate:2 2 UDP 2013266430 54.88.69.222 10568 typ host m=video 13330 UDP/TLS/RTP/SAVPF 120 c=IN IP4 172.22.2.175 b=AS:2000 a=recvonly a=mid:sdparta_1 a=rtcp:13330 IN IP4 172.22.2.175 a=rtpmap:120 VP8/90000 a=rtcp-fb:120 nack a=rtcp-fb:120 nack pli a=rtcp-fb:120 ccm fir a=rtcp-fb:120 goog-remb a=fmtp:120 max-fs=12288;max-fr=60 a=rtcp-mux a=setup:active a=ice-ufrag:HLBq a=ice-pwd:ru2tvQBt7TMLAYKIUQoVCQ a=fingerprint:sha-256 07:94:59:74:19:9C:57:D6:0A:9A:7A:7B:CE:0B:4B:A1:31:80:72:C2:EF:6C:61:4A:EA:71:9E:50:28:1D:35:82 a=candidate:1 1 UDP 2013266431 fe80::897:43ff:fe24:2b3a 13849 typ host a=candidate:2 1 UDP 2013266431 54.88.69.222 13330 typ host a=candidate:1 2 UDP 2013266430 fe80::897:43ff:fe24:2b3a 13036 typ host a=candidate:2 2 UDP 2013266430 54.88.69.222 10568 typ host Negotiation 2 - REMOTE SDP OFFER v=0 o=- 3690792591 3690792592 IN IP4 0.0.0.0 s=VM76da225fd9594c06b60d05a7bee52203 c=IN IP4 0.0.0.0 t=0 0 a=group:BUNDLE sdparta_0 sdparta_1 a=msid-semantic:WMS * a=ice-lite m=audio 1 UDP/TLS/RTP/SAVPF 109 0 a=rtpmap:109 opus/48000/2 a=rtpmap:0 PCMU/8000 a=fmtp:109 maxplaybackrate=48000;stereo=1;useinbandfec=1 a=rtcp:9 IN IP4 0.0.0.0 a=rtcp-mux a=recvonly a=mid:sdparta_0 a=ice-ufrag:HLBq a=ice-pwd:ru2tvQBt7TMLAYKIUQoVCQ a=candidate:1 1 UDP 2013266431 fe80::897:43ff:fe24:2b3a 13849 typ host a=candidate:2 1 UDP 2013266431 54.88.69.222 13330 typ host a=candidate:1 2 UDP 2013266430 fe80::897:43ff:fe24:2b3a 13036 typ host a=candidate:2 2 UDP 2013266430 54.88.69.222 10568 typ host a=fingerprint:sha-256 07:94:59:74:19:9C:57:D6:0A:9A:7A:7B:CE:0B:4B:A1:31:80:72:C2:EF:6C:61:4A:EA:71:9E:50:28:1D:35:82 m=video 1 UDP/TLS/RTP/SAVPF 120 b=AS:2000 a=rtpmap:120 VP8/90000 a=fmtp:120 max-fs=12288;max-fr=60 a=rtcp:9 IN IP4 0.0.0.0 a=rtcp-mux a=sendrecv a=msid:{ac3bf23b-3a5f-4277-a185-07a1f1519bec} {41a81f09-51ec-4931-b0d7-1c8ae72ff4ed} a=ssrc:1116397011 cname:user3472025491@host-b2c6baed a=mid:sdparta_1 a=rtcp-fb:120 nack a=rtcp-fb:120 nack pli a=rtcp-fb:120 goog-remb a=rtcp-fb:120 ccm fir a=ice-ufrag:HLBq a=ice-pwd:ru2tvQBt7TMLAYKIUQoVCQ a=candidate:1 1 UDP 2013266431 fe80::897:43ff:fe24:2b3a 13849 typ host a=candidate:2 1 UDP 2013266431 54.88.69.222 13330 typ host a=candidate:1 2 UDP 2013266430 fe80::897:43ff:fe24:2b3a 13036 typ host a=candidate:2 2 UDP 2013266430 54.88.69.222 10568 typ host a=fingerprint:sha-256 07:94:59:74:19:9C:57:D6:0A:9A:7A:7B:CE:0B:4B:A1:31:80:72:C2:EF:6C:61:4A:EA:71:9E:50:28:1D:35:82 Negotiation 2 - LOCAL SDP ANSWER v=0 o=mozilla...THIS_IS_SDPARTA-50.1.0 2543689248089967985 1 IN IP4 0.0.0.0 s=- t=0 0 a=fingerprint:sha-256 26:09:35:29:4A:CC:DB:53:08:B9:82:F7:FF:21:A9:FD:A5:7B:DF:58:68:56:4E:3A:55:9D:9F:29:01:20:8A:A1 a=group:BUNDLE sdparta_1 a=ice-options:trickle a=msid-semantic:WMS * m=audio 0 UDP/TLS/RTP/SAVPF 0 c=IN IP4 0.0.0.0 a=inactive a=rtpmap:0 PCMU/8000 m=video 9 UDP/TLS/RTP/SAVPF 120 c=IN IP4 0.0.0.0 a=sendrecv a=fmtp:120 max-fs=12288;max-fr=60 a=ice-pwd:104efa9005ab9a78ae3436942cdd11e6 a=ice-ufrag:3d69bb57 a=mid:sdparta_1 a=msid:{fac09d71-a62d-464c-bc01-e3b2f564580e} {e13d494d-9bfc-4fb1-a6b5-e656bd82c1e0} a=rtcp-fb:120 nack a=rtcp-fb:120 nack pli a=rtcp-fb:120 ccm fir a=rtcp-fb:120 goog-remb a=rtcp-mux a=rtpmap:120 VP8/90000 a=setup:active a=ssrc:251820637 cname:{1a9156b4-1a09-4d51-9fba-316325af0062} Actual results: After "Negotiation 2" Firefox stops sending video and I do not see any error in console. I am attaching a file with the Firefox log. Expected results: Firefox should continue sending video and should start receiving video.
Assignee | ||
Comment 1•7 years ago
|
||
Thanks for the ICE log. But I think what we need in this case is a log file from the signaling layer. Could please provide us with the NSPR logging described here https://wiki.mozilla.org/Media/WebRTC/Logging#Signaling_.28SDP_offer.2Fanswer_handling.29
Whiteboard: [needinfo 2016/12/16 to reporter]
Assignee | ||
Comment 2•7 years ago
|
||
Forgot to actually set the NeedInfo to the reporter.
Flags: needinfo?(mparisdiaz)
Reporter | ||
Comment 3•7 years ago
|
||
Nils, first of all thanks for the pointer, I didn't know it ;). I have attached the requested log and here you have the standard output of firefox: Timecard created 1482148724.563581 Timestamp | Delta | Event | File | Function ====================================================================================================================== 0.000424 | 0.000424 | Constructor Completed | PeerConnectionImpl.cpp:344 | PeerConnectionImpl 0.003457 | 0.003033 | Initializing PC Ctx | PeerConnectionImpl.cpp:672 | Initialize 0.020280 | 0.016823 | Create Offer | PeerConnectionImpl.cpp:1515 | CreateOffer 0.023471 | 0.003191 | Set Local Description | PeerConnectionImpl.cpp:1681 | SetLocalDescription 0.049872 | 0.026401 | Ice gathering state: gathering | PeerConnectionImpl.cpp:3301 | IceGatheringStateChange 0.722054 | 0.672182 | Set Remote Description | PeerConnectionImpl.cpp:2029 | SetRemoteDescription 0.784972 | 0.062918 | Ice state: checking | PeerConnectionImpl.cpp:3249 | IceConnectionStateChange 0.915665 | 0.130693 | Ice state: connected | PeerConnectionImpl.cpp:3252 | IceConnectionStateChange 13.444991 | 12.529326 | Ice gathering state: complete | PeerConnectionImpl.cpp:3304 | IceGatheringStateChange 17.889691 | 4.444700 | Set Remote Description | PeerConnectionImpl.cpp:2029 | SetRemoteDescription 17.898593 | 0.008902 | Create Answer | PeerConnectionImpl.cpp:1572 | CreateAnswer 17.899576 | 0.000983 | Set Local Description | PeerConnectionImpl.cpp:1681 | SetLocalDescription 17.939186 | 0.039610 | Ice gathering state: gathering | PeerConnectionImpl.cpp:3301 | IceGatheringStateChange 17.939678 | 0.000492 | Ice state: checking | PeerConnectionImpl.cpp:3249 | IceConnectionStateChange 18.049821 | 0.110143 | Ice state: connected | PeerConnectionImpl.cpp:3252 | IceConnectionStateChange 31.836831 | 13.787010 | Ice gathering state: complete | PeerConnectionImpl.cpp:3304 | IceGatheringStateChange 46.037341 | 14.200510 | Destructor Invoked | PeerConnectionImpl.cpp:358 | ~PeerConnectionImpl
Flags: needinfo?(mparisdiaz)
Assignee | ||
Comment 4•7 years ago
|
||
Thanks for the log file. That helped me to get an idea what is going on here. The problem has multiple aspects to it: - the renegotiation appears to restart ICE - not sure if that is intended or not - the Negotiation 2 - REMOTE SDP OFFER is missing the a=setup attribute, that appears to be a bug on your implementation - Firefox answer includes the a=setup attribute which switches the DTLS client server roles, which is actually not allowed - but the logs show that Firefox DTLS implementation is actually still configured as server and therefore waits for the other side to send the client hello which I guess never happens
Status: UNCONFIRMED → NEW
backlog: --- → webrtc/webaudio+
Rank: 25
Ever confirmed: true
Priority: -- → P2
Assignee | ||
Updated•7 years ago
|
Whiteboard: [needinfo 2016/12/16 to reporter]
Comment hidden (mozreview-request) |
Assignee | ||
Updated•7 years ago
|
Attachment #8820074 -
Flags: review?(docfaraday)
Assignee | ||
Comment 6•7 years ago
|
||
@bwc: mostly looking for feedback initially if you think these are the right places to add these checks. Once we have the right places we should add some unit tests.
Assignee: nobody → drno
Comment hidden (mozreview-request) |
Assignee | ||
Comment 8•7 years ago
|
||
Miguel once the try run here https://treeherder.mozilla.org/#/jobs?repo=try&revision=b13147742c6f has finished building you should be able to download the build for your platform and test it against your Kurento server. It should reject the re-offer from the Kurento server with the missing setup attribute.
Comment 9•7 years ago
|
||
mozreview-review |
Comment on attachment 8820074 [details] Bug 1323723: added setup attribute checks. https://reviewboard.mozilla.org/r/99600/#review100028 LGTM (I think byron is on PTO this week; so you can decide if you want to take my r+)
Attachment #8820074 -
Flags: review+
Reporter | ||
Comment 10•7 years ago
|
||
Hello Nils, thanks for the great support!! You are right and I missed the detail of a=setup attribute, good point!! After fixing that and adding a=setup attribute properly, it continues not working. Is this caused because of the ICE restart? (what is NOT intended). I have attached a new log of this new case. In addition, I have tried to verify if the patch you did for Firefox detects the a=setup in the former case, but I didn't find a way to download and install firefox from the link you passed. I would like to contribute verifying that, so could you please tell me how to download the binary associated to your patch?
Comment hidden (mozreview-request) |
Assignee | ||
Comment 12•7 years ago
|
||
Miguel, my first patch was not good. I started a new try run for my new patch. You can download the Linux build for example from here https://archive.mozilla.org/pub/firefox/try-builds/drno@ohlmeier.org-2321f7cc8da018b378c90051598163bc3e2c275a/try-linux/ You can either navigate from the directory up and then down into the other OS's. Or you can click on the green 'B' on this treeherder page https://treeherder.mozilla.org/#/jobs?repo=try&revision=2321f7cc8da0 and then click on the link in the lower left corner next to the word 'Build'. Would be great to know if my patch properly rejects the re-offer with the missing 'a=setup' from your initial setup! I looked at your new log file and I think it is actually not an ICE restart, but for some reason Fx behaves as if there is no pre-existing ICE transport. I need to check in more details and probably ask you for more log files with a different log string.
Assignee | ||
Comment 13•7 years ago
|
||
Miguel, I'm pretty sure I now understand what the problem is: Because in the initial negotiation the first m-line gets set to inactive by your server Firefox establishes the ICE transport kind of bound to the second m-line which has real RTP traffic on it. But when your server send the re-offer it actually set the first m-line to recvonly. And even though Firefox then sends it back as inactive again my guess is that this causes internally the bundled transport to "move" to the first m-line.
Reporter | ||
Comment 14•7 years ago
|
||
Nils, after testing the nightly associated to your patch the missed a=setup attr is detected: 2016-12-21 11:04:52.410568 UTC - 1211201408[7f3246b6c5c0]: [main|PeerConnectionImpl] PeerConnectionImpl.cpp:2162: SetRemoteDescription: pc = 09d5ed92b2588c71, error = Missing setup attribute in m-line 0 Great work!! In relation to the "ICE problem", I support your assumption because if the first m-line is sendrecv in the second offer sent by Kurento, it works. What could we do to fix this issue? - Do you think if it is a bug in Firefox? - Do you think if Kurento is doing something wrong?
Flags: needinfo?(drno)
Comment 15•7 years ago
|
||
mozreview-review |
Comment on attachment 8820074 [details] Bug 1323723: added setup attribute checks. https://reviewboard.mozilla.org/r/99600/#review101906 ::: media/webrtc/signaling/src/jsep/JsepSessionImpl.cpp:927 (Diff revision 3) > + if (mCurrentLocalDescription) { > + if (mCurrentLocalDescription->GetMediaSectionCount() > mlineIndex) { > + auto& msection = mCurrentLocalDescription->GetMediaSection(mlineIndex); > + if (!mSdpHelper.MsectionIsDisabled(msection) && > + msection.GetAttributeList().GetSetup().mRole != role) { > + JSEP_SET_ERROR("Remote side attempted to switch DTLS roles"); > + return NS_ERROR_FAILURE; > + } > + } > + } Hmm. Won't the a=setup in the current local description be actpass if we were the initial offerer? That will never be returned by DetermineAnswererSetupRole.
Attachment #8820074 -
Flags: review?(docfaraday)
Reporter | ||
Comment 16•7 years ago
|
||
Related discussions: https://github.com/rtcweb-wg/jsep/issues/448 https://groups.google.com/forum/#!msg/discuss-webrtc/DfpIMwvUfeM/VgCtnJ0cEgAJ
Comment hidden (mozreview-request) |
Assignee | ||
Updated•7 years ago
|
Attachment #8820074 -
Attachment is obsolete: true
Assignee | ||
Comment 18•7 years ago
|
||
@bwc the new patch is only to require the presence of the setup attribute (which would have prevented the initial problem). I'll open a separate bug for adding more code to fix and enforce the setup roles per https://tools.ietf.org/html/draft-ietf-mmusic-dtls-sdp-15 Miguel the ICE transport thing is a Firefox issue. I'll probably open a new bug for that as well.
Flags: needinfo?(drno)
Comment 19•7 years ago
|
||
mozreview-review |
Comment on attachment 8823964 [details] Bug 1323723: enforce a=setup in SDP offers. https://reviewboard.mozilla.org/r/102442/#review102964 ::: media/webrtc/signaling/src/jsep/JsepSessionImpl.cpp:1718 (Diff revision 1) > - "\"holdconn\" at level " > + "\"holdconn\" at level " > - << i); > + << i); > - return NS_ERROR_INVALID_ARG; > + return NS_ERROR_INVALID_ARG; > - } > + } > + } else { > + JSEP_SET_ERROR("Missing setup attribute in m-line " << i); "m-section" I guess.
Attachment #8823964 -
Flags: review?(docfaraday) → review+
Comment hidden (mozreview-request) |
Assignee | ||
Comment 21•7 years ago
|
||
(In reply to Nils Ohlmeier [:drno] from comment #18) > @bwc the new patch is only to require the presence of the setup attribute > (which would have prevented the initial problem). I'll open a separate bug > for adding more code to fix and enforce the setup roles per > https://tools.ietf.org/html/draft-ietf-mmusic-dtls-sdp-15 See bug 1329028
Assignee | ||
Comment 22•7 years ago
|
||
(In reply to Nils Ohlmeier [:drno] from comment #18) > Miguel the ICE transport thing is a Firefox issue. I'll probably open a new > bug for that as well. I created bug 1332183 for the ICE transport issue. Once we have that landed I would land the small fix here. And the more strict checking of a=setup in bug 1329028 can land later.
Assignee | ||
Comment 23•7 years ago
|
||
Miguel, can you get me the full offer answer when Kurento includes a=setup in it's re-offer? So what's happening is: - Kurento sends the re-offer with the first m-section recvonly - as Fx has no audio stream for the m-section it rejects it by setting the port to 0 and also removing the mid from the bundle group - this makes the mid of the video m-section the new bundle master - therefore new ICE and DTLS need to happen - but since Kurento omitted the a=setup in it's re-offer Fx just makes up a default a=setup attribute (active in this case), but in fact the DTLS part of Fx is still in the original server mode and waits for Kurento to start the DTLS handshake So that's why I'm curious what happens if Kurento properly puts a=setup in the re-offer.
Assignee | ||
Updated•7 years ago
|
Flags: needinfo?(mparisdiaz)
Reporter | ||
Comment 24•7 years ago
|
||
Hello Nils!! The re-offer is the same that in the report, but adding "a=setup:active" in each m-line.
Reporter | ||
Comment 25•7 years ago
|
||
Hello again Nils, after the discussion in JSEP, the conclusion [1] is that "a=setup" value MUST be always "actpass" [2]. So, if I am not wrong this should be fixed in Firefox, shouldn't it? Refs [1] https://github.com/rtcweb-wg/jsep/issues/448#issuecomment-278495950 [2] http://rtcweb-wg.github.io/jsep/#rfc.section.5.2.1
Updated•7 years ago
|
Flags: needinfo?(drno)
Assignee | ||
Comment 26•7 years ago
|
||
The main work for this will happen in bug 1329028.
Depends on: 1329028
Comment hidden (mozreview-request) |
Comment hidden (mozreview-request) |
Comment hidden (mozreview-request) |
Comment hidden (mozreview-request) |
Comment 31•7 years ago
|
||
mozreview-review |
Comment on attachment 8846982 [details] Bug 1323723: reject setup value actpass in answers. https://reviewboard.mozilla.org/r/120024/#review122040
Attachment #8846982 -
Flags: review?(docfaraday) → review+
Comment 32•7 years ago
|
||
mozreview-review |
Comment on attachment 8823964 [details] Bug 1323723: enforce a=setup in SDP offers. https://reviewboard.mozilla.org/r/102442/#review122064 ::: media/webrtc/signaling/src/jsep/JsepSessionImpl.cpp:1711 (Diff revision 3) > + } else { > + JSEP_SET_ERROR("Missing setup attribute in m-section at level " << i); > + return NS_ERROR_INVALID_ARG; > + } Hang on. In answers a=setup has a default value of "passive", which is fine with the JSEP spec (it recommends "active" though). In offers, the default is "active", which is _not_ ok with the JSEP spec.
Attachment #8823964 -
Flags: review+ → review-
Comment 33•7 years ago
|
||
mozreview-review |
Comment on attachment 8846983 [details] Bug 1323723: update a=setup tests. https://reviewboard.mozilla.org/r/120026/#review122066 So we need to test the following: "actpass" in offers is the only thing that is ok; all other values (including missing a=setup) are an error. "active", "passive", and missing a=setup in answers is ok, everything else is not.
Comment hidden (mozreview-request) |
Comment hidden (mozreview-request) |
Comment hidden (mozreview-request) |
Comment 37•7 years ago
|
||
mozreview-review |
Comment on attachment 8823964 [details] Bug 1323723: enforce a=setup in SDP offers. https://reviewboard.mozilla.org/r/102442/#review122232
Attachment #8823964 -
Flags: review?(docfaraday) → review+
Comment 38•7 years ago
|
||
mozreview-review |
Comment on attachment 8846983 [details] Bug 1323723: update a=setup tests. https://reviewboard.mozilla.org/r/120026/#review122236 ::: media/webrtc/signaling/gtest/jsep_session_unittest.cpp:5063 (Diff revision 3) > } > > // In this test we will change the answer SDP's a=setup value > // from active to passive. This will make both sides do > // active and should not connect. > +// TODO jsep unittest can't verify that this does not connect?! If this worries you, you could inspect the JsepTransports on each side and verify that they both think they're active.
Attachment #8846983 -
Flags: review?(docfaraday) → review+
Comment hidden (mozreview-request) |
Comment 40•7 years ago
|
||
Pushed by drno@ohlmeier.org: https://hg.mozilla.org/integration/autoland/rev/e5fc51a36f97 enforce a=setup in SDP offers. r=bwc https://hg.mozilla.org/integration/autoland/rev/36223085d3d5 reject setup value actpass in answers. r=bwc https://hg.mozilla.org/integration/autoland/rev/b954d11db347 update a=setup tests. r=bwc
Comment 41•7 years ago
|
||
bugherder |
https://hg.mozilla.org/mozilla-central/rev/e5fc51a36f97 https://hg.mozilla.org/mozilla-central/rev/36223085d3d5 https://hg.mozilla.org/mozilla-central/rev/b954d11db347
Status: NEW → RESOLVED
Closed: 7 years ago
status-firefox55:
--- → fixed
Resolution: --- → FIXED
Target Milestone: --- → mozilla55
Reporter | ||
Comment 42•7 years ago
|
||
Great to see that you have fixed it. Thank you very much!! :D
Flags: needinfo?(mparisdiaz)
Assignee | ||
Comment 43•7 years ago
|
||
Miguel, it is probably worth pointing out that the patches from this bug might not be enough actually fix the whole issue. These were just the easy checks. I'm still working in bug 1329028 on adding more checks.
Flags: needinfo?(drno)
Reporter | ||
Comment 44•7 years ago
|
||
Great!! If you need help from my side to test newest versions of Firefox with you patch, please let me know, I will be happy to do it ;).
You need to log in
before you can comment on or make changes to this bug.
Description
•