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

RTP matching rules when no encoding.rtx.ssrc is given #546

Open
ibc opened this issue May 16, 2016 · 11 comments
Open

RTP matching rules when no encoding.rtx.ssrc is given #546

ibc opened this issue May 16, 2016 · 11 comments

Comments

@ibc
Copy link
Contributor

ibc commented May 16, 2016

Imagine receive() is called without muxId, with "vp8" (PT 100) and "rtx" (PT 98) in codecs, and with a single encoding with ssrc but no rtx.

According to the rules:

payload type table: If parameters.muxId is unset and parameters.encodings[i].ssrc is unset for all values of i from 0 to the number of encodings, then add entries to pt_table by setting pt_table[parameters.codecs[j].payloadType] to receiver

But in the above case encoding.ssrc is given, so the PT of "rtx" won't be added to the pt_table and thus "rtx" packets will be discarded.

One may say that encoding.rtx.ssrc is not required (as the spec states) but in the above case RTX would not work.

@aboba aboba added the 1.1 label May 19, 2016
@aboba
Copy link
Contributor

aboba commented May 19, 2016

@ibc What if we always fill the pt_table regardless of whether parameters.muxId is set or parameters.encodings[i].ssrc is set? If we did that, then the "rtx" packets wouldn't be discarded.

@ibc
Copy link
Contributor Author

ibc commented May 19, 2016

Exactly. Not filling the pt_table in certain cases provides no benefit and becomes a problem if the previously given SSRC changes in runtime.

@ibc
Copy link
Contributor Author

ibc commented Jul 14, 2016

I must add a concern. Not filling the pt_table when ssrcs or muxId is given does have a value:

If multiple media tracks are carried over the same transport it's likely they will share most of the codecs and PTs. If all those PT values are inserted into the pt_table of the corresponding transport they will conflict.

Finally, in my implementation I followed the original text regarding filling tables, with the following changes/steps:

  • First remove from the the listener tables all the entries pointing to given RtpReceiver.
  • Add entries into the ssrc_table for each encoding (encoding.ssrc, encoding.fec.ssrc and encoding.rtx.ssrc).
  • Add entries into muxId_table.
  • Add entries into pt_table just if:
    • Not all the encoding.ssrc are given, or
    • Not all the encoding.rtx.ssrc (for encodings with rtx field) are given, or
    • Not all the encoding.fec.ssrc (for encodings with fecfield) are given.

This works fine so far.

@aboba
Copy link
Contributor

aboba commented Sep 13, 2016

Routing Rules PR in JSEP: rtcweb-wg/jsep#320

@aboba
Copy link
Contributor

aboba commented Sep 30, 2016

@ibc The JSEP Section 7 rules always add entries to the pt_table, but never remove entries or latch. The rules do not say what happens when pt_table entries conflict, but presumably this is not considered an error (e.g. one of the pt_table entries wins). This has the advantage of perhaps handling the case where all the SSRCs are given for encoding.ssrc, encoding.rtx.ssrc and encoding.fec.ssrc, but there is an SSRC change that is not signaled.

aboba added a commit that referenced this issue Oct 4, 2016
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
@aboba
Copy link
Contributor

aboba commented Oct 4, 2016

@ibc @robin-raymond - Can you check whether this issue is resolved by PR #602 (which is based on JSEP Section 7)?

@aboba
Copy link
Contributor

aboba commented Oct 4, 2016

It looks to me like this Issue is addressed by the JSEP Section 7 rules, since the proposed algorithm does fill the pt_table even when ssrcs or muxId have values. However, the algorithm does not complain when the PTs are non-unique, it just fills the pt_table with the first entry and ignores subsequent conflicting entries.

@ibc
Copy link
Contributor Author

ibc commented Oct 6, 2016

I must properly check JSEP Section 7, but it makes sense (something like "all for me" or "first wins" or "the latest wins").

@aboba
Copy link
Contributor

aboba commented Oct 31, 2016

JSEP rules have changed:
https://tools.ietf.org/html/draft-ietf-rtcweb-jsep-17#page-66

PT_table entry added regardless of SSRC values, so still addresses this issue.

aboba added a commit that referenced this issue Jan 9, 2017
Another attempt to fix Issues #368, #546, #547
@aboba
Copy link
Contributor

aboba commented Jan 18, 2017

@ibc @robin-raymond @pthatcherg The JSEP Appendix B RTP matching rules appear to address this issue, correct?
https://tools.ietf.org/html/draft-ietf-rtcweb-jsep-18#appendix-B

The proposed rules will add a pt_table entry regardless of whether encodings[].ssrc, encodings[].rtx.ssrc or encodings[].fec.ssrc is specified, as long as the PTs do not overlap. So "rtx" packets will not be discarded.

aboba added a commit that referenced this issue Apr 18, 2017
Yet another attempt to fix Issues #368, #546, #547
@aboba
Copy link
Contributor

aboba commented Oct 25, 2017

Robin has written down his algorithm, which seems to address this issue:
https://gist.github.com/robin-raymond/cf961ce2a3f16dc28982460ed7f4b2a1

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

3 participants