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

If multiple encodings are given, ssrcs are needed #568

Closed
ibc opened this issue Jun 20, 2016 · 15 comments
Closed

If multiple encodings are given, ssrcs are needed #568

ibc opened this issue Jun 20, 2016 · 15 comments

Comments

@ibc
Copy link
Contributor

ibc commented Jun 20, 2016

Once capabilities are negotiated and so on, a browser sends the following to a SFU (simulcast with two audio encodings using different codecs):

rtpSender.send({
    codecs : [
        {
            name        : 'audio/opus',
            payloadType : 101
        },
        {
            name        : 'audio/pcma',
            payloadType : 102
        },
        {
            name        : 'audio/CN',
            payloadType : 96
        }
    ],
    encodings : [
        {
            codecPayloadType : 101
        },
        {
            codecPayloadType : 102
        }
    ]
});

Note that SSRCs are not signaled.

Then, the first packet the browser sends is a CN with ssrc '12345678'. How does the receiver (the SFU) know which encoding it belongs to?

This applies to other use cases (different encodings with same codecPayloadType and unset ssrc, etc).

@pthatcherg
Copy link

That's what the encodingId is for.

@ibc
Copy link
Contributor Author

ibc commented Jun 20, 2016

encodingId is not placed within CN RTP packets, so that does not fix the issue.

@pthatcherg
Copy link

Why wouldn't CN packets use the encodingId's header extension?

@ibc
Copy link
Contributor Author

ibc commented Jun 20, 2016

True, sorry.

So we end with a single RtpSender sending simulcast (multiple encodings). But the MID header is not enough since it just indicates the RtpSender, and hence we end also adding the RID header. Too cool, but it could work.

However, does the spec states somewhere that, in case of simulcast, the sender should indicate the encodingId for each encoding and must place the RID header into RTP packets?

@robin-raymond
Copy link
Contributor

Sure, spec needs to say to add RID / MID header extension if supported. Are they mandatory to support?

@aboba
Copy link
Contributor

aboba commented Jun 22, 2016

Currently a base URN (urn:ietf:params:rtp-hdrext:sdes) is allocated for the "RTP Header Extension for RTCP Source Description Items" (https://tools.ietf.org/html/draft-ietf-avtext-sdes-hdr-ext). However, it is not clear to me whether this would actually be included in RTCRtpCapabilities.headerExtensions, or only URNs for individual elements (e.g. MID, RID, etc.) which are not yet allocated. Jonathan's recent post to AVTEXT WG mailing list should help clear this up: https://www.ietf.org/mail-archive/web/avtext/current/msg01244.html

@aboba
Copy link
Contributor

aboba commented Jun 22, 2016

@pthatcherg @ibc Section 9.8.1 says "An RTCRtpSender places the value of encodingId into the RID header extension [RID].". Is that enough?

One thing that isn't in the current spec is a way to specify the RRID (but as I understand it, no browser vendor currently intends to implement the RRID).

@ibc
Copy link
Contributor Author

ibc commented Jun 23, 2016

However, it is not clear to me whether this would actually be included in RTCRtpCapabilities.headerExtensions, or only URNs for individual elements (e.g. MID, RID, etc.) which are not yet allocated.

BUNDLE draft defines urn:ietf:params:rtp-hdrext:sdes:mid in section 15.2.

@ibc
Copy link
Contributor Author

ibc commented Jun 23, 2016

Sure, spec needs to say to add RID / MID header extension if supported. Are they mandatory to support?

My point is that, in case of simulcast, RID MUST be signaled, so if there are more than a single encoding, all the entries in encodings MUST have encodingId.

@aboba
Copy link
Contributor

aboba commented Jun 23, 2016

@ibc

Yes, encodingId needs to be present. RID draft needs to allocate a URN, most likely urn:ietf:params:rtp-hdrext:sdes:rid

@robin-raymond
Copy link
Contributor

On sender for simulcast, encodingId is required. On receiver, if encodings[] are specified then the encodingId is required (or it is basically dynamically assigned based on best match).

aboba added a commit that referenced this issue Jan 11, 2017
Fix for Issue #568
@aboba aboba self-assigned this Jan 11, 2017
@robin-raymond
Copy link
Contributor

FYI - CN is not ambiguous in the case because it's simulcast and each stream will have it's own SSRC thus the CN is always in the context of the SSRC of 101 or 102 thus there's no issue here. This is related to routing rules.

@aboba
Copy link
Contributor

aboba commented Jan 11, 2017

@robin-raymond @pthatcherg Right - that is a consequence of PT latching (which is supported in both ORTC Lib and in the text in JSEP-17 Section 6). So encodingId is not required for this PT-based use case, but it does avoid rtpunhandled events that can occur before arrival of an Answer when SSRC signaling is used. I've been assuming that the AVTEXT and MMUSIC RID drafts say everything that needs to be said about this, but if there is something that needs to be in the ORTC API spec, let us know.

@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 an SSRC to the ssrc_table after a PT match occurs. So if the audio switches from G.711 to CN, RTP packets will be routed to the same receiver as long as the SSRC does not change.

aboba added a commit that referenced this issue Sep 28, 2017
@aboba
Copy link
Contributor

aboba commented Oct 18, 2017

It appears that this issue is resolved in the latest draft.

@aboba aboba closed this as completed Oct 18, 2017
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

4 participants