Start using UDP/TLS/RTP/SAVPF in offers once it interops

RESOLVED FIXED in Firefox 43

Status

()

Core
WebRTC: Signaling
P3
normal
Rank:
30
RESOLVED FIXED
4 years ago
3 years ago

People

(Reporter: bwc, Assigned: bwc)

Tracking

Trunk
mozilla43
Points:
---

Firefox Tracking Flags

(firefox43 fixed)

Details

MozReview Requests

()

Submitter Diff Changes Open Issues Last Updated
Loading...
Error loading review requests:

Attachments

(1 attachment)

(Assignee)

Description

4 years ago
Current specs say we should use UDP/TLS/RTP/SAVPF instead of RTP/SAVPF, but this does not play well with currently deployed firefox, and possibly other implementations. Bug 1091242 will allow us to accept this in an offer, and once this work is on all supported branches (or stopgaps are put in place), we should investigate making this change.
(Assignee)

Updated

3 years ago
Duplicate of this bug: 1058006
(Assignee)

Updated

3 years ago
backlog: --- → webRTC+
Rank: 50
Priority: -- → P4
(Assignee)

Comment 2

3 years ago
Looks like the Chrome guys have committed the same fix on their end:

https://chromium.googlesource.com/external/webrtc.git/+/f39382943449b7e44ac563e05a14203534591acf

We should go ahead and do this now.
Assignee: nobody → docfaraday
(Assignee)

Updated

3 years ago
Rank: 50 → 30
Priority: P4 → P3
(Assignee)

Comment 3

3 years ago
Created attachment 8655682 [details]
MozReview Request: Bug 1094447: Use UDP/TLS/RTP/SAVPF for audio/video m-lines

Bug 1094447: Use UDP/TLS/RTP/SAVPF for audio/video m-lines
Comment on attachment 8655682 [details]
MozReview Request: Bug 1094447: Use UDP/TLS/RTP/SAVPF for audio/video m-lines

https://reviewboard.mozilla.org/r/17977/#review16111

::: media/webrtc/signaling/test/signaling_unittests.cpp:3390
(Diff revision 1)
>    // TODO(bug 1099351): Use commented out code instead.

Can we update this comment to point to bug 814227 please?
Attachment #8655682 - Flags: review+
Comment on attachment 8655682 [details]
MozReview Request: Bug 1094447: Use UDP/TLS/RTP/SAVPF for audio/video m-lines

https://reviewboard.mozilla.org/r/17977/#review16115

::: media/webrtc/signaling/test/signaling_unittests.cpp:3240
(Diff revision 1)
> -    "m=audio 56187 RTP/SAVPF 109 0 8 101\r\n"
> +    "m=audio 56187 UDP/TLS/RTP/SAVPF 109 0 8 101\r\n"

We probably want at least one test case which keeps using just 'RTP/SAVPF' to ensure we don;t loos the ability to handle that properly and answer with 'RTP/SAVPF' in that case.
Attachment #8655682 - Flags: review+
(Assignee)

Comment 6

3 years ago
https://reviewboard.mozilla.org/r/17977/#review16115

> We probably want at least one test case which keeps using just 'RTP/SAVPF' to ensure we don;t loos the ability to handle that properly and answer with 'RTP/SAVPF' in that case.

I can add such a test to jsep_session_unittest
(Assignee)

Comment 7

3 years ago
Comment on attachment 8655682 [details]
MozReview Request: Bug 1094447: Use UDP/TLS/RTP/SAVPF for audio/video m-lines

Bug 1094447: Use UDP/TLS/RTP/SAVPF for audio/video m-lines
Attachment #8655682 - Flags: review?(drno)
(Assignee)

Comment 8

3 years ago
Check back on try.
Flags: needinfo?(docfaraday)
(Assignee)

Updated

3 years ago
Flags: needinfo?(docfaraday)

Updated

3 years ago
Attachment #8655682 - Flags: review?(drno) → review+
Comment on attachment 8655682 [details]
MozReview Request: Bug 1094447: Use UDP/TLS/RTP/SAVPF for audio/video m-lines

https://reviewboard.mozilla.org/r/17977/#review16207

We could go as crazy as testing for switching protocols mid-flight. But I don't think we actually want to support that.
(Assignee)

Updated

3 years ago
Keywords: checkin-needed

Updated

3 years ago
Status: NEW → RESOLVED
Last Resolved: 3 years ago
status-firefox43: --- → fixed
Resolution: --- → FIXED
Target Milestone: --- → mozilla43
(Assignee)

Comment 12

3 years ago
I've already sent mail to mozilla-dev-media about this, and we need to make sure this is mentioned in the release notes as well.
You need to log in before you can comment on or make changes to this bug.