Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Make offer generated by "createOffer" spec compliant #62

Closed
stefhak opened this issue Oct 27, 2014 · 33 comments
Closed

Make offer generated by "createOffer" spec compliant #62

stefhak opened this issue Oct 27, 2014 · 33 comments

Comments

@stefhak
Copy link
Contributor

stefhak commented Oct 27, 2014

Offers generated now miss ICE-ufrag and password as well as DTLS fingerprint, meaning they can't be sent to the other side (you have to do setLocal and fetch the local config and send that one).

According to the spec you should be able to send the offer generated by createOffer (it should be complete with ufrag, pw and fingerprint). We should fix this.

@longsleep
Copy link

I am investigating if we can get https://github.com/strukturag/spreed-webrtc working with Bowser. This was one of the first issues i saw.

@sheepsleeper
Copy link

Any progress?

@stefhak
Copy link
Contributor Author

stefhak commented Nov 19, 2014

At least I've not heard of any progress so far.

@AndrewJDR
Copy link

It has been a few more months, any more word on this?

@stefanalund
Copy link
Contributor

@adam-be @pererikb would you like to comment?

@stefhak
Copy link
Contributor Author

stefhak commented Feb 17, 2015

I think an update to libnice is needed. I don't know if anyone looked into
that.

On Tue, Feb 17, 2015 at 6:51 AM, Stefan Ålund notifications@github.com
wrote:

@adam-be https://github.com/adam-be @pererikb
https://github.com/pererikb would you like to comment?


Reply to this email directly or view it on GitHub
#62 (comment)
.

@stefanalund
Copy link
Contributor

Maybe Olivier Crete knows. Describe the issue in an email an I can forward it.

@stefhak
Copy link
Contributor Author

stefhak commented Feb 17, 2015

I asked on the libnice list: http://lists.freedesktop.org/archives/nice/2015-February/001072.html
Olivier has already responded, and I'm trying to gather info to be able to make a reasonable response. Please chime in if you have anything to add!

@ijsf
Copy link
Contributor

ijsf commented Mar 13, 2015

Somebody submitted a patch on the libnice list, see link above.

@dborovik
Copy link

Current (from App store) version of Bowser run on IPhone6.
Offer does not contain DTLS fingerpring:

sdp: "v=0
o=- 1427208527661020400 1 IN IP4 127.0.0.1
s=-
t=0 0
a=msid-semantic:WMS 2iQLycjj5ZgOpXY3ztfUxCpNiQ0vltk1gg5P
m=audio 1 RTP/SAVPF 111 8 0
c=IN IP4 0.0.0.0
a=rtcp-mux
a=sendrecv
a=rtpmap:111 OPUS/48000/2
a=rtpmap:8 PCMA/8000
a=rtpmap:0 PCMU/8000
a=msid:2iQLycjj5ZgOpXY3ztfUxCpNiQ0vltk1gg5P NjgqaQfdGpUz8Wvk2EVOSGM8Y1VZTZGAfNfK
a=setup:actpass
m=video 1 RTP/SAVPF 103 100 120
c=IN IP4 0.0.0.0
a=rtcp-mux
a=sendrecv
a=rtpmap:103 H264/90000
a=rtpmap:100 VP8/90000
a=rtpmap:120 RTX/90000
a=fmtp:103 packetization-mode=1
a=fmtp:120 apt=100;rtx-time=200
a=rtcp-fb:100 nack
a=rtcp-fb:103 nack pli
a=rtcp-fb:100 nack pli
a=rtcp-fb:103 ccm fir
a=rtcp-fb:100 ccm fir
a=msid:2iQLycjj5ZgOpXY3ztfUxCpNiQ0vltk1gg5P 1egNvfTh3Mlq0EeJvGljCjNjxUP1Ix9M645B
a=setup:actpass
"

@superdump
Copy link
Contributor

Now being tracked here: https://bugs.freedesktop.org/show_bug.cgi?id=89839

@stefhak
Copy link
Contributor Author

stefhak commented Apr 1, 2015

Follow up: the freedesktop bug is part of the problem, but there is more to it (e.g. the offer created should contain ssrc info, mid, ....)

@zalmoxisus
Copy link

Any updates of this issue? I see that the bug from freedesktop was fixed.

Just tested in Safari (with owr) and the following offer is generated:
m=video 1 RTP/SAVPF 100
↵c=IN IP4 0.0.0.0
↵a=sendrecv
↵a=rtpmap:100 VP8/90000
↵a=rtcp-fb:100 nack
↵a=rtcp-fb:100 ccm fir
↵a=msid:MlIIetGUkwgXOAJwDZDJRcQatW3xnr0Bi9lA 4UTGksHhkDIUdsgcHQw9g+c4M1QaKWmBbQg3
↵a=setup:active

While in chrome the offer is:
m=video 9 RTP/SAVPF 100 116 117
↵c=IN IP4 0.0.0.0
↵a=rtcp:9 IN IP4 0.0.0.0
↵a=ice-ufrag:k0QA2sdLpdu5mCLM
↵a=ice-pwd:EjnDQZlWo8dWLdi76VkbRfc3
↵a=fingerprint:sha-256 78:09:3D:DB:AF:D3:CC:D1:FA:3A:81:5A:0D:B5:B3:66:39:90:E4:56:07:D3:D6:C4:58:9B:89:1C:FA:04:5F:07
↵a=setup:active
↵a=mid:video
↵a=extmap:2 urn:ietf:params:rtp-hdrext:toffset
↵a=extmap:3 http://www.webrtc.org/experiments/rtp-hdrext/abs-send-time
↵a=sendrecv
↵a=rtpmap:100 VP8/90000
↵a=rtcp-fb:100 ccm fir
↵a=rtcp-fb:100 nack
↵a=rtcp-fb:100 goog-remb
↵a=rtpmap:116 red/90000
↵a=rtpmap:117 ulpfec/90000
↵a=ssrc:1181138693 cname:lkjR0pUPXzMtQOOT
↵a=ssrc:1181138693 msid:664a6a7c-77fb-4050-9410-7cd05c48ad2c 85f02b9e-642a-4944-b99c-938691c8ae84
↵a=ssrc:1181138693 mslabel:664a6a7c-77fb-4050-9410-7cd05c48ad2c
↵a=ssrc:1181138693 label:85f02b9e-642a-4944-b99c-938691c8ae84

We need at least ICE-ufrag and password.

@superdump
Copy link
Contributor

@stefhak - if we update libnice to pull in the _set_local_credentials patch, do you think you can implement the remaining parts in OpenWebRTC?

@stefhak
Copy link
Contributor Author

stefhak commented Apr 26, 2015

With some help I think I could manage that.

On Sun, Apr 26, 2015 at 12:17 PM, Robert Swain notifications@github.com
wrote:

@stefhak https://github.com/stefhak - if we update libnice to pull in
the _set_local_credentials patch, do you think you can implement the
remaining parts in OpenWebRTC?


Reply to this email directly or view it on GitHub
#62 (comment)
.

@superdump
Copy link
Contributor

Then I'll look into updating libnice after the release tomorrow.

@superdump
Copy link
Contributor

@stefhak, libnice is updated in cerbero to 0.1.12 which includes the _set_local_credentials API.

@superdump
Copy link
Contributor

@stefhak any luck with this?

@stefhak
Copy link
Contributor Author

stefhak commented May 22, 2015

I have not done anything yet. Promise to get to it next week.

@stefhak
Copy link
Contributor Author

stefhak commented May 31, 2015

So after some discussions with people who know stuff, we've concluded that a possible way forward is to divide this into the following parts (all of which are needed to make the offer spec compliant):

@superdump
Copy link
Contributor

SSRC should be set in the capsfilter caps after the RTP payloader.

cname can be set on the internal RTPSession's sdes property. That property is a GstStructure which has key=value fields as follows: http://cgit.freedesktop.org/gstreamer/gst-plugins-good/tree/gst/rtpmanager/rtpsource.c#n99 When requesting a new pad on rtpbin, the RTPSource and RTPSession will be created. After that, you can use the get-internal-session action signal ( http://gstreamer.freedesktop.org/data/doc/gstreamer/head/gst-plugins-good-plugins/html/gst-plugins-good-plugins-rtpbin.html#GstRtpBin-get-internal-session ) to get the RTPSession object and then create the appropriate SDES GstStructure and set it on the RTPSession. If the cname is the same for all sessions, the structure can be set on the rtpbin sdes property.

@superdump superdump added this to the 0.4.0 milestone Jun 1, 2015
@superdump
Copy link
Contributor

Make ssrc and cname properties writable: #398 Though I still need to look at RTX SSRC stuff (whether we're signalling it correctly and how to set it.)

@superdump
Copy link
Contributor

So, I think we need an rtx-ssrc property somewhere and then we need to correctly handle the ssrc-map signal from rtprtxsend and rtprtxreceive to map the payload types to ssrcs.

@superdump
Copy link
Contributor

@pererikb has implemented and tested the credentials handling parts: #400

@sdroege
Copy link
Contributor

sdroege commented Jun 4, 2015

@superdump, see the ssrc-map property on rtprtxsend.

@stefanalund
Copy link
Contributor

Done #427

@superdump
Copy link
Contributor

I think the only part remaining is to check and fix is the RTX SSRC bit.

@stefanalund
Copy link
Contributor

Should we reopen or create a separate, new, issue?

@superdump
Copy link
Contributor

I'll reopen this as the information is above.

@superdump superdump reopened this Oct 22, 2015
@stefhak
Copy link
Contributor Author

stefhak commented Oct 22, 2015

We can do that, but I think being able to set MID has higher prio.

On Thu, Oct 22, 2015 at 6:43 PM, Stefan Ålund notifications@github.com
wrote:

Should we reopen or create a separate, new, issue?


Reply to this email directly or view it on GitHub
#62 (comment)
.

@superdump
Copy link
Contributor

@stefhak: do you have any list of compliance issues?

@stefhak
Copy link
Contributor Author

stefhak commented Oct 22, 2015

I'll work on one.

On Thu, Oct 22, 2015 at 6:48 PM, Robert Swain notifications@github.com
wrote:

@stefhak https://github.com/stefhak: do you have any list of compliance
issues?


Reply to this email directly or view it on GitHub
#62 (comment)
.

@superdump
Copy link
Contributor

OK, I've changed my mind. I think we can close this and consider the original issue with createOffer compliance completed. The RTX stuff can be added later.

See here: #492

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

10 participants