Closed
Bug 829761
Opened 12 years ago
Closed 12 years ago
WebRTC should accept stated direction for ip=0.0.0.0, port != 0
Categories
(Core :: WebRTC: Signaling, defect, P1)
Tracking
()
RESOLVED
FIXED
mozilla21
People
(Reporter: abr, Assigned: abr)
Details
(Whiteboard: [WebRTC] [blocking-webrtc-interop] [blocking-webrtc+] [qa-])
Attachments
(1 file, 1 obsolete file)
9.21 KB,
patch
|
ehugg
:
checkin+
|
Details | Diff | Splinter Review |
In order to facilitate trickle ice as it's currently being used, we need the ability to receive SDP with an IP of 0.0.0.0 and still treat the stream as active (sendonly, recvonly, or sendrecv, as appropriate), so long as the corresponding m= section has a non-zero port. This is necessary due to the fact that the RTCWEB and Trickle ICE work is still in flight. This may or may not be the final behavior that we want to exhibit.
Assignee | ||
Updated•12 years ago
|
Summary: WebRTC should accept stated direction ip=0.0.0.0, port != 0 → WebRTC should accept stated direction for ip=0.0.0.0, port != 0
Assignee | ||
Comment 1•12 years ago
|
||
The code in question is in gsm_sdp.c: /* * To support legacy way of signaling remote hold, we will interpret * c=0.0.0.0 to be a=inactive */ if (dest_addr->type == CPR_IP_ADDR_IPV4 && dest_addr->u.ip4 == 0) { direction = SDP_DIRECTION_INACTIVE; } else { //todo IPv6: reject the request. }
Assignee | ||
Updated•12 years ago
|
Whiteboard: [WebRTC] [blocking-webrtc-interop] [blocking-webrtc+]
Assignee | ||
Updated•12 years ago
|
Priority: -- → P1
Assignee | ||
Updated•12 years ago
|
Assignee: nobody → adam
Assignee | ||
Comment 2•12 years ago
|
||
Assignee | ||
Updated•12 years ago
|
Attachment #701942 -
Flags: review?(ekr)
Assignee | ||
Updated•12 years ago
|
Status: NEW → ASSIGNED
Comment 3•12 years ago
|
||
Comment on attachment 701942 [details] [diff] [review] Remove inactive check for sdpmode if port != 0 Review of attachment 701942 [details] [diff] [review]: ----------------------------------------------------------------- ::: media/webrtc/signaling/test/signaling_unittests.cpp @@ +591,5 @@ > > // Now call CreateOffer as JS would > pObserver->state = TestObserver::stateNoResponse; > ASSERT_EQ(pc->CreateOffer(constraints), NS_OK); > + ASSERT_TRUE_WAIT(pObserver->state != TestObserver::stateNoResponse, Can you explain why this is needed? @@ +745,5 @@ > > private: > void SDPSanityCheck(std::string sdp, uint32_t flags, bool offer) > { > + ASSERT_TRUE(pObserver->state == TestObserver::stateSuccess); I thought ASSERT_TRUE() did a return if it failed.
Attachment #701942 -
Flags: review?(ekr) → review+
Assignee | ||
Comment 4•12 years ago
|
||
(In reply to Eric Rescorla (:ekr) from comment #3) > Comment on attachment 701942 [details] [diff] [review] > Remove inactive check for sdpmode if port != 0 > > Review of attachment 701942 [details] [diff] [review]: > ----------------------------------------------------------------- > > ::: media/webrtc/signaling/test/signaling_unittests.cpp > @@ +591,5 @@ > > > > // Now call CreateOffer as JS would > > pObserver->state = TestObserver::stateNoResponse; > > ASSERT_EQ(pc->CreateOffer(constraints), NS_OK); > > + ASSERT_TRUE_WAIT(pObserver->state != TestObserver::stateNoResponse, > > Can you explain why this is needed? It used to only check/wait for it to change to success. This allows it to return immediately if the state changes from "no response" to "failed." > @@ +745,5 @@ > > > > private: > > void SDPSanityCheck(std::string sdp, uint32_t flags, bool offer) > > { > > + ASSERT_TRUE(pObserver->state == TestObserver::stateSuccess); > > I thought ASSERT_TRUE() did a return if it failed. Ah, yes. This is overkill. I'll remove the second check.
Assignee | ||
Comment 5•12 years ago
|
||
Assignee | ||
Updated•12 years ago
|
Attachment #701942 -
Attachment is obsolete: true
Assignee | ||
Comment 6•12 years ago
|
||
Comment on attachment 701988 [details] [diff] [review] Remove inactive check for sdpmode if port != 0, Carrying forward r=ekr
Attachment #701988 -
Flags: checkin?(ethanhugg)
Comment 7•12 years ago
|
||
Comment on attachment 701988 [details] [diff] [review] Remove inactive check for sdpmode if port != 0, https://hg.mozilla.org/integration/mozilla-inbound/rev/b93357d16604
Attachment #701988 -
Flags: checkin?(ethanhugg) → checkin+
Comment 8•12 years ago
|
||
https://hg.mozilla.org/mozilla-central/rev/b93357d16604
Status: ASSIGNED → RESOLVED
Closed: 12 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla21
Updated•12 years ago
|
Whiteboard: [WebRTC] [blocking-webrtc-interop] [blocking-webrtc+] → [WebRTC] [blocking-webrtc-interop] [blocking-webrtc+] [qa-]
You need to log in
before you can comment on or make changes to this bug.
Description
•