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

Order of RTCRtpSendParameters.encodings is not described #1896

Closed
alvestrand opened this issue Jun 15, 2018 · 5 comments
Closed

Order of RTCRtpSendParameters.encodings is not described #1896

alvestrand opened this issue Jun 15, 2018 · 5 comments
Assignees
Labels
Simulcast Issue relating to Simulcast

Comments

@alvestrand
Copy link
Contributor

alvestrand commented Jun 15, 2018

In the RTCRTPSendParameters.encodings, the description is:

"A sequence containing parameters for RTP encodings of media"

In "getParameters", it says "encodings is populated based on the RIDs present in the current remote description."

Example 15 says:

pc.addTransceiver(stream.getVideoTracks()[0], {
direction: 'sendonly',
sendEncodings: [
{rid: 'f'},
{rid: 'h', scaleResolutionDownBy: 2.0},
{rid: 'q', scaleResolutionDownBy: 4.0}
]

This seems to indicate that the spec-writers were thinking in terms of "highest resolution first, lowest resolution last". But this is never written down as a rule anywhere. Neither is the idea that order stays consistent between calls.

The issue came up because one lower level implementation had a built-in assumption that lowest resolution would go first and highest resolution would go last - the opposite of this example.

Possible resoutions:
a - Declare that order is not fixed; the implementation can reorder encodings whenever it wants
b - Declare that order is fixed, but resolutions can occur in any order
c - Declare that order is fixed, and that scaleResolutionDownBy has to be greater or equal to previous encoding (ascending scaleResolutionDownBy) (or the other way round)

a would seem to be the alternative that requires minimal changes to implementations.
tagging @henbos @Orphis @taylor-b

@taylor-b
Copy link
Contributor

This seems to indicate that the spec-writers were thinking in terms of "highest resolution first, lowest resolution last". But this is never written down as a rule anywhere. Neither is the idea that order stays consistent between calls.

I think we can start by saying "the order stays consistent", since JSEP says that for subsequent offers, "any "a=simulcast" line MUST stay the same."

And "highest resolution first" is written down as a rule... sort of. draft-ietf-mmusic-sdp-simulcast also does "highest resolution first" in its examples, and says:

The order of the listed simulcast streams in the "send" direction suggests a proposed order of preference, in decreasing order: the rid-id listed first is the most preferred and subsequent streams have progressively lower preference.

Later it also talks about congestion control, saying:

What simulcast streams to prioritize when allocating available bitrate among the simulcast streams in such adaptation SHOULD be taken from the simulcast stream order on the "a=simulcast" line

Which actually seems backwards, since we want to drop the high quality encoding first... Unless this is meant to imply "prioritize the last stream"? I'm going to ask the authors if they can clarify.

Regardless, the point is that the order in the simulcast attribute does matter.

However, a wrinkle is that JSEP doesn't explicitly say "the order of the RIDS in sendEncodings MUST match the order of the RIDs in the simulcast attribute"; it just says "the m= section for the RtpSender will include an "a=simulcast" attribute ... with a "send" simulcast stream description that lists each desired encoding"

@juberti: Can you clarify whether or not JSEP was intended to preserve the ordering? For example, if I use the reverse of the order in the example:

pc.addTransceiver(track, {
  direction: 'sendonly',
  sendEncodings: [
    {rid: 'q', scaleResolutionDownBy: 4.0},
    {rid: 'h', scaleResolutionDownBy: 2.0},
    {rid: 'f'}
  ]});

Should the generated SDP be "a=simulcast:send q;h;f", and should the implementation actually pause the low-resolution encoding first when congestion occurs, contrary to how you'd normally want things to work?

@taylor-b
Copy link
Contributor

I've been talking with the draft-ietf-mmusic-sdp-simulcast authors, and have learned there are situations where you'd want to pause the low-resolution encoding first. When dealing with a mixer that does transcoding, as opposed to an SFU, the mixer can transcode the high-resolution stream into a low-resolution stream for downlink receivers that don't have enough bandwidth.

Also, it sounds like I've been misinterpreting "proposed order of preference", and this is just referring to the "priority" used for congestion control (as described above).

So in conclusion, I think implementations do need to respect the order if we intend to support different usages of simulcast.

@aboba
Copy link
Contributor

aboba commented Aug 14, 2018

In the current specification, order matters in several aspects:

  1. When more simulcast layers are requested in sendEncodings than can be supported, layers are dropped "from the tail".

  2. setParameters does not currently permit reordering of encodings, so that ordering is fixed once addTransceiver is called.

My advice would be to indicate that no particular ordering is required, but to include a note in setParameters explaining the effect of ordering.

@amithilbuch
Copy link
Contributor

Adding a couple cents.
I would say that the order matters, and in the SDP it should correspond to the order of the encodings in setParameters on the transceiver.
However, i do not think that the actual contents of the layers should be restricted to having any particular order.
it should be fine to define hi,med,lo or lo,med,hi or even a jumbled hi,lo,med. This would be based on your preference for congestion control etc. as suggested in the comments above.
Signaling the interpretation of each layer can be done using the rid restrictions (ex. max-width and max-height).

@henbos
Copy link
Contributor

henbos commented Jan 15, 2019

What things are signaled about the encoding envelope? Do we need to say that the order of the SDP generated is the same as the order of [[SendEncodings]]?

amithilbuch pushed a commit to amithilbuch/webrtc-pc that referenced this issue Feb 1, 2019
daemory pushed a commit to dangxiwang/webrtc that referenced this issue Apr 19, 2024
Patch Set 13:

> Patch Set 13:
> 
> > Patch Set 13: Code-Review+1
> > 
> > > Patch Set 13:
> > > 
> > > For the options, 1) might require a lot of changes and I'm not sure if it should be part of this task. For 2) and 3), you probably have a better idea which would be the best option.
> > 
> > In that case I'd lean towards just using the "increasing quality" order, since using different orders in different places would be pretty confusing and bug-prone. In that case, this LGTM, though I'd recommend documenting how simulcast config works somewhere (above RtpSenderInterface::SetParmaeters?).
> 
> Chrome might have to reverse the order. I'll raise a spec bug pointing out that the order is not clear.

Filed w3c/webrtc-pc#1896

Patch-set: 13
daemory pushed a commit to dangxiwang/webrtc that referenced this issue Apr 19, 2024
Patch Set 15:

> Patch Set 13:
> 
> > Patch Set 13:
> > 
> > > Patch Set 13: Code-Review+1
> > > 
> > > > Patch Set 13:
> > > > 
> > > > For the options, 1) might require a lot of changes and I'm not sure if it should be part of this task. For 2) and 3), you probably have a better idea which would be the best option.
> > > 
> > > In that case I'd lean towards just using the "increasing quality" order, since using different orders in different places would be pretty confusing and bug-prone. In that case, this LGTM, though I'd recommend documenting how simulcast config works somewhere (above RtpSenderInterface::SetParmaeters?).
> > 
> > Chrome might have to reverse the order. I'll raise a spec bug pointing out that the order is not clear.
> 
> Filed w3c/webrtc-pc#1896

Is it ok to land this CL?

Patch-set: 15
Reviewer: Gerrit User 5492 <5492@58829da1-049c-39fb-b951-ebdcd0984767>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Simulcast Issue relating to Simulcast
Projects
None yet
Development

No branches or pull requests

5 participants