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

save SDP unknown fields #9

Merged
merged 1 commit into from
Jun 28, 2020
Merged

save SDP unknown fields #9

merged 1 commit into from
Jun 28, 2020

Conversation

moggle-mog
Copy link

Save SDP unknown/unrealized fields as Other field in order to improve SDP scalability.
People can easily implement his own SDP library.

@jart
Copy link
Owner

jart commented Jun 28, 2020

If I understand correctly, this change would cause unknown SDP fields to be transparently forwardable. Your change looks good. Could you help me understand why it's needed? AFAIK SDP doesn't do what HTTP does with X-Foo: Bar headers.

Would it be possible to put JSON or something in the Session Info field? https://tools.ietf.org/html/rfc4566#section-5.4 The SDP standard says i= is arbitrary UTF-8 and u= is an arbitrary URL for anything we want.

If there's existing systems productionized that use non-RFC fields, could you provide details on what they are?

@moggle-mog
Copy link
Author

Yes, one protocol named gb28181 which can control camera uses non-rfc SDP. It uses y= to describe ssrc and f= to describe camera stream(IPC like the sip phone so we can call it and the video will return back).So I think that maybe it is a good way to add strange fields into Other

@jart
Copy link
Owner

jart commented Jun 28, 2020

I'm happy to merge this as a workaround. I can't promise it in the long term because RFC 4566 defines its BNF in such a way as to not allow custom SDP fields. Easiest way for @GB28181 to help us is to publish an RFC explaining how these new fields work.

@jart jart merged commit 8ccbc0a into jart:master Jun 28, 2020
@moggle-mog
Copy link
Author

Thanks, perhaps a better way is to maintain a copy of SDP.However this SIP library and SDP are interdependent(https://github.com/jart/gosip/blob/master/sip/msg_parse.rl#L68).
&MiscPayload{T: ctype, D: data[p:len(data)]} is better

@moggle-mog
Copy link
Author

moggle-mog commented Jun 29, 2020

I'm happy to merge this as a workaround. I can't promise it in the long term because RFC 4566 defines its BNF in such a way as to not allow custom SDP fields. Easiest way for @GB28181 to help us is to publish an RFC explaining how these new fields work.

Although supporting custom non-RPC SDP helped me a lot, I think the standard SIP library should strictly follow the RFC regulations.
Should we decouple SIP and SDP instead of supporting custom SDP?

@jart
Copy link
Owner

jart commented Jun 30, 2020

For posterity, we've moved this discussion into #11.

zwczou added a commit to zwczou/gosip that referenced this pull request Jul 5, 2021
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

Successfully merging this pull request may close these issues.

None yet

2 participants