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

Simulcast behavior #1996

Closed
wants to merge 6 commits into from
Closed

Simulcast behavior #1996

wants to merge 6 commits into from

Conversation

aboba
Copy link
Contributor

@aboba aboba commented Sep 28, 2018

Partial fix for Issue #1964


Preview | Diff

Work-in-progress, do not merge. 

Fix for Issue #1964
@aboba aboba added the Simulcast Issue relating to Simulcast label Sep 28, 2018
@aboba aboba self-assigned this Sep 28, 2018
@aboba aboba requested a review from alvestrand October 4, 2018 05:18
@aboba aboba requested a review from jan-ivar October 4, 2018 05:24
@aboba
Copy link
Contributor Author

aboba commented Oct 4, 2018

@fippo Any thoughts?

@aboba aboba requested review from youennf and fluffy October 4, 2018 05:29
webrtc.html Outdated
can be utilized to set the maximum bitrate of a simulcast stream. However,
<code>maxBitrate</code> does not influence the user agent's response to congestion,
including allocation of bandwidth between simulcast streams or the decision to
stop sending one or more streams.</p>
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Does it make sense to have a maxBitrate per-layer and not per-sender? Does the application want/need this amount of control?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Having maxBitrate per-layer is useful in scenarios where the application wants to control rates of streams (e.g. to make sure a thumbnail doesn't use too much bandwidth).

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

this is also what firefox supports today also.

webrtc.html Outdated
<section id="maxbitrateinsimulcast">
<h3>maxBitrate</h3>
<p><code><a>RTCRtpEncodingParameters</a></code> dictionary member <code>maxBitrate</code>
can be utilized to set the maximum bitrate of a simulcast stream. However,
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Bitrate can be measured for any interval of time, and packets are likely measured discretely, so depending on how you measure things a UA may or may not at times exceed the bitrate (e.g. if you measure the last second, you have a "budget" of up to 1 second to work with, but could also work with 10 seconds or 10 ms). Should this say "target maximum bitrate" (or bikeshed: clarify how bitrate is measured)? Same comment for framerate.

Copy link
Contributor Author

@aboba aboba Oct 4, 2018

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The TIAS definition is referenced in the maxBitrate member definition.

between simulcast streams sent by an <code><a>RTCRtpSender</a></code> object.</p>
</section>
<section id="maxbitrateinsimulcast">
<h3>maxBitrate</h3>
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

What if unreasonably low bitrates are used? What if 0 is used? Same comment for framerate.

I assume there is some softness to these restrictions, where the UA only has to "try" to obey?

webrtc.html Outdated
<h3>maxFramerate</h3>
<p><code><a>RTCRtpEncodingParameters</a></code> dictionary member <code>maxFramerate</code>
can be utilized to set the maximum framerate of a simulcast stream. However,
<code>maxFramerate</code> does not influence the user agent's response to congestion,
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Here and above: This only sets a "max", not a "min"; would response to congestion control ever lead to an increase of bits/frames?

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

When congestion control says "plenty of bandwidth available", it will lead to an increase in bitrate, but shouldn't go over maxBitrate anyway.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

the use-case for this is to support things like "give me the low-resolution layer only at 10fps, I am only going to show thumbnails anyway", right? 👍

<code>maxFramerate</code> does not influence the user agent's response to congestion,
including allocation of bandwidth between simulcast streams or the decision to
stop sending one or more streams.</p>
</section>
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

May setting different settings on different layers in any way affect the encoder's/decoder's performance? Are there inabilities to adhere to the limits?

If an encoder is not able to produce different framerates on different layers, should it take the lowest, highest or average framerate based on layers?

Should there be guidance about what reasonable values are? Should the UA be allowed to ignore application hints?

@fippo
Copy link
Contributor

fippo commented Oct 4, 2018

one thing I noticed while discussing https://bugzilla.mozilla.org/show_bug.cgi?id=1474661 with folks (:wave: @docfaraday) that there is no way to set a target bitrate per layer. So I may (today) request max bitrates of 500, 800 and 2300 and would only get... 150, 500 and 2300 or something like that which is hardcoded deep down in webrtc.org

@alvestrand
Copy link
Contributor

@aboba will make updates according to comments.

webrtc.html Outdated
<code>maxFramerate</code> does not influence the user agent's response to congestion,
including allocation of bandwidth between simulcast streams or the decision to
stop sending one or more streams.</p>
the user agent is not required to take <code>maxFramerate</code> into account when
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested text: "The user agent is free to allocate bandwidth between the encodings, as long as the maxFramerate is not exceeded". And similar above.

webrtc.html Outdated
responding to congestion, such as modifying the allocation of bandwidth between
simulcast streams or deciding when to stop sending one or more streams.</p>
can be utilized to set the maximum bitrate of a simulcast stream. However, the user
agent may not take <code>maxBitrate</code> into account in responding to congestion,
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Still think "may not" is a bad echo of the RFC 2119 MAY NOT (equivalent of MAY); suggest "may choose to not" to break it up.

@henbos
Copy link
Contributor

henbos commented Dec 6, 2018

PTAL @henbos

@alvestrand
Copy link
Contributor

Should be ready to review.

@aboba aboba closed this Jan 21, 2019
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

Successfully merging this pull request may close these issues.

4 participants