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

Routing Rules Update #602

Closed
wants to merge 8 commits into from
Closed

Routing Rules Update #602

wants to merge 8 commits into from

Conversation

aboba
Copy link
Contributor

@aboba aboba commented Oct 4, 2016

Routing Rules update to address outstanding issues as well as to be more compatible with Section 6 of https://tools.ietf.org/html/draft-ietf-rtcweb-jsep-17#page-66

Addresses the following issues: #368, #546, #547

Routing Rules update to address compatibility with Section 7 of https://tools.ietf.org/html/draft-ietf-rtcweb-jsep

Addresses the following issues: #368, #546, #547
Cleanup
Cleanup
Cleanup
@ibc
Copy link
Contributor

ibc commented Oct 6, 2016

LGTM

Remove SSRC latching for PT table matches
Update rules for setting the PT table
@aboba aboba changed the title Routing Rules: Compatibility with WebRTC 1.0/JSEP Routing Rules Update Jan 4, 2017
@aboba
Copy link
Contributor Author

aboba commented Jan 4, 2017

@ibc Can you review?

@ibc
Copy link
Contributor

ibc commented Jan 4, 2017

Yes, I'll do it on next week (holidays now)

Revised rtx.ssrc unset algorithm
@ibc
Copy link
Contributor

ibc commented Jan 8, 2017

May be I miss something, but I see a problem within the following example:

codecs:
[
  { payloadType: 100, name: "audio/opus" }
  { payloadType: 101, name: "audio/CN" } 
],
encodings:
[
  { codecPayloadType: 100 }
]

AFAIU, with the new algorithm in the PR, "audio/CN" will be ignored. Am I wrong?

@aboba
Copy link
Contributor Author

aboba commented Jan 8, 2017

@ibc What if we set pt_table[parameters.codecs[j].payloadType] to receiver for all values of j from 0 to codecs.length-1, with the exception of entries for parameter.encodings[i].codecPayloadType where parameter.encodings[i].ssrc is set, for values of i from 0 to encodings.length-1?

@ibc
Copy link
Contributor

ibc commented Jan 8, 2017

That would create conflicts (InvalidParameter rejection) if the same transport is used for two RtpReceiver instances and both RtpParameters include CN or DTMF codecs with same PT values (which will happen when receiving two audio streams from a SFU via the same transport).

@ibc
Copy link
Contributor

ibc commented Jan 8, 2017

IMHO this should be more strict, something such as:

If there is any encodings[i].ssrc or encodings[i].rtx.ssrc or encodings[i].fec.ssrc for all values of i from 0 to encodings.length-1, then pt_table should not be filled at all.

This is, don't allow incomplete parameters. All or none.

@aboba
Copy link
Contributor Author

aboba commented Jan 8, 2017

@ibc With respect to your Issue #546 : if encodings[i].ssrc is filled but encodings[i].rtx is missing we still want the RTX payload type table entry to be filled, correct? The same would also be true of RED and FEC, right?

@ibc
Copy link
Contributor

ibc commented Jan 8, 2017

Ideally yes but, as noted above, there is no perfect solution. That's why I suggest "all or none", this is:

  • If SSRC are given, they all must be given (including ssrc for all the encodings[i] and encodings[i].rtx and encodings[i].fex) and hence, pt_table must not be filled.
  • If codecs[i].name is "audio/rtx" for some i, encodings[i].ssrc is given, and encodings[i].rtx.ssrc is not given, then I would consider that an error.

Update the markup
@aboba
Copy link
Contributor Author

aboba commented Jan 9, 2017

@ibc OK. Let me come up with a revised PR.

@aboba aboba closed this Jan 9, 2017
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants