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

RTPReceiver encoding parameters are useless #445

Closed
murillo128 opened this Issue Mar 31, 2016 · 4 comments

Comments

Projects
None yet
4 participants
@murillo128

murillo128 commented Mar 31, 2016

Correct me if I am wrong.

On chapter 8.3 RTP matching rules the rtp packets are routed to the correct RTPReceiver based on the following parameters:

  • parameters.muxId
  • parameters.encodings[i].ssrc
  • parameters.encodings[i].fec.ssrc
  • parameters.encodings[i].fec.ssrc
  • parameters.codecs[j].payloadType
  • parameters.encodings[i].rtx.payloadType

First weird case, we check the payloadType on the codecs, but the rtx payloadType on the encodings (???). Luckily this will be solved by #444

We only use encodings for looking for the ssrc, but we then don't do anything with it (is it relevant at all that a packet is received by a particular encoding? i don't think so, we know the ssrc and the codec already).

So my question is what are encodings really used for in receving side? We just need a sequence of <ssrc,fecssrc> at most.

Moreover, as discussed in #439, if we just allow one incoming stream for simulcast, we just need one ssrc and one fecssrc, don't we?

So if we do #440 we could simplify to:

dictionary RTCRtpReceivingParameters {
             DOMString                                 muxId = "";
             sequence<RTCRtpCodecParameters>           codecs;
             sequence<RTCRtpHeaderExtensionParameters> headerExtensions;
             unsigned long                             ssrc;
             unsigned long                             fecSsrc;
             RTCRtcpParameters                         rtcp;
};

As usual, am I missing anything?

@ibc

This comment has been minimized.

Show comment
Hide comment
@ibc

ibc Mar 31, 2016

Contributor

We only use encodings for looking for the ssrc, but we then don't do anything with it (is it relevant at all that a packet is received by a particular encoding? i don't think so, we know the ssrc and the codec already).

The "RTP matching rules" section is a bit special in the sense that it is not intended for API users, but for ORTC implementors (browser vendors). It helps anyhow but it does not provide complete guidelines for the implementor.

if we just allow one incoming stream for simulcast, we just need one ssrc and one fecssrc, don't we?

Agreed. Also note that the SSRC for received RTX payloads should also be unique (not sure why, but in the real world it is unique). So I would add a rtxSsrc to that dictionary.

Contributor

ibc commented Mar 31, 2016

We only use encodings for looking for the ssrc, but we then don't do anything with it (is it relevant at all that a packet is received by a particular encoding? i don't think so, we know the ssrc and the codec already).

The "RTP matching rules" section is a bit special in the sense that it is not intended for API users, but for ORTC implementors (browser vendors). It helps anyhow but it does not provide complete guidelines for the implementor.

if we just allow one incoming stream for simulcast, we just need one ssrc and one fecssrc, don't we?

Agreed. Also note that the SSRC for received RTX payloads should also be unique (not sure why, but in the real world it is unique). So I would add a rtxSsrc to that dictionary.

@murillo128

This comment has been minimized.

Show comment
Hide comment
@murillo128

murillo128 Apr 1, 2016

@ibc matching rules actually defines what input parameters are required in order to work. We should not request the API user to fill more parameters that the ones strictly required.

murillo128 commented Apr 1, 2016

@ibc matching rules actually defines what input parameters are required in order to work. We should not request the API user to fill more parameters that the ones strictly required.

aboba added a commit that referenced this issue Apr 1, 2016

Table of RTCRtpEncodingParameters
Explanation related to Issue #445

@aboba aboba added PR exists and removed publication-blocker labels Apr 1, 2016

@aboba

This comment has been minimized.

Show comment
Hide comment
@aboba

aboba Apr 6, 2016

Contributor

A table has been added, indicating which attributes within RTCRtpEncodingParameters relate to the RtpSender, and which relate to the RtpReceiver. Is this sufficient?

Contributor

aboba commented Apr 6, 2016

A table has been added, indicating which attributes within RTCRtpEncodingParameters relate to the RtpSender, and which relate to the RtpReceiver. Is this sufficient?

@robin-raymond

This comment has been minimized.

Show comment
Hide comment
@robin-raymond

robin-raymond Apr 7, 2016

Contributor

@aboba I believe it is what was needed. Anything further requires a proposed redesign which is beyond the scope of this bug and the original statement about encodings being useless in the title of the bug is inaccurate.

I filed a separate bug to clarify when encodings are not specified to describe exactly the behaviour expected. As far as I'm concerned, this bug is closed.

Contributor

robin-raymond commented Apr 7, 2016

@aboba I believe it is what was needed. Anything further requires a proposed redesign which is beyond the scope of this bug and the original statement about encodings being useless in the title of the bug is inaccurate.

I filed a separate bug to clarify when encodings are not specified to describe exactly the behaviour expected. As far as I'm concerned, this bug is closed.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment