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

Questions regarding translation #27

Open
jensengar opened this issue Sep 5, 2019 · 4 comments
Open

Questions regarding translation #27

jensengar opened this issue Sep 5, 2019 · 4 comments

Comments

@jensengar
Copy link

@legastero Can you provide some insight on the following?

  1. Here: https://github.com/otalk/sdp-jingle-json/blob/master/lib/tojson.js#L20 it looks like the port is getting changed? I'm seeing something like m=audio 9 ... (from the offer) be changed to m=audio 1 .... Is there a reason for that? I would think the goal would be to convert sdp to json, and be able to get the original sdp back when converting the json back to sdp.

  2. Here: https://github.com/otalk/sdp-jingle-json/blob/master/lib/tosdp.js#L55 It looks like it's basically hard coding the sdp for data channels but this is blowing up for me in chrome with the following error:

"Failed to execute 'setRemoteDescription' on 'RTCPeerConnection': Failed to parse SessionDescription. m=application 1 DTLS/SCTP Expects at least 4 fields."

The sdp being generated for the offer looks like

m=application 9 UDP/DTLS/SCTP webrtc-datachannel

but what's being received looks like this:

m=application 1 DTLS/SCTP

I assume this is a chrome change of some sort because this used to not be a problem. I'm also seeing issues in firefox:

Failed to parse SDP: SDP Parse Error on line 74: No sctp port specified in m= media line, parse failed.

Can you weigh in on these issues?

@xdumaine
Copy link

xdumaine commented Sep 5, 2019

cc @fippo

@fippo
Copy link
Member

fippo commented Sep 6, 2019

(1): port 1 should be translated to port 9 these days
(2): this is the result of the format change since Chrome just started sending the new UDP/DTLS/SCTP format. Since the new format should be parsed by all recent (~3 years) browsers it probably makes sense to update.

@jensengar
Copy link
Author

Thanks for the info @fippo, I'll work on updating this lib and open a pr. I've also come across this:

a=extmap:1/sendrecv urn:ietf:params:rtp-hdrext:ssrc-audio-level

where the original looked like this:

a=extmap:1 urn:ietf:params:rtp-hdrext:ssrc-audio-level

Should this be changed also to match the original?

@fippo
Copy link
Member

fippo commented Sep 6, 2019

no, those lines can have a direction. Which defaults to sendrecv so sendrecv may be omitted but it should be parsed properly in the current state

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

No branches or pull requests

3 participants