w3c / ortc

ORTC Community Group specification repository (see W3C WebRTC for official standards track)
http://www.w3.org/community/ortc/
122 stars 42 forks source link

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

Open ibc opened 8 years ago

ibc commented 8 years ago

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 commented 8 years ago

@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 commented 8 years ago

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 commented 8 years ago

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:

This works fine so far.

aboba commented 7 years ago

Routing Rules PR in JSEP: https://github.com/rtcweb-wg/jsep/pull/320

aboba commented 7 years ago

@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 commented 7 years ago

@ibc @robin-raymond - Can you check whether this issue is resolved by PR https://github.com/w3c/ortc/pull/602 (which is based on JSEP Section 7)?

aboba commented 7 years ago

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 commented 7 years ago

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

aboba commented 7 years ago

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 commented 7 years ago

@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 commented 6 years ago

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