Add rate control event #421

pthatcher · 2022-09-27T14:24:55Z

Fixes #21 by adding an event to give RateControlFeedback to let the application know the maximum rate it should send at to avoid queuing.

Preview | Diff

index.bs

jan-ivar · 2022-10-11T23:38:26Z

Meeting:

Figure out what data we want and how it will be used before we work out how to deliver it
expected size of congestion window (how much you can send in one roundtrip), remaining/total availability, rtt?, calculations just focused at the next frame, or projections multiple frames ahead? a complex problem, and the information we are able to provide isn't well suited
iframe is 40kb, it's going to take me 4 rtts to send, -> lower resolution etc.
how big a frame can I send?
reporting congstion window leads may lead to odd outcomes, like underutilizing bandwidth.
a leaky abstraction, whereas sites need a lot of context

wilaw · 2022-10-19T17:12:48Z

@pthatcher - while we debate this PR, can you fix the ipr issue signalled above? The system would like you to link your github accounts with your W3C account so that PRs can be automatically accepted.

pthatcher · 2022-11-21T23:41:14Z

@pthatcher - while we debate this PR, can you fix the ipr issue signalled above? The system would like you to link your github accounts with your W3C account so that PRs can be automatically accepted.

It's fixed now.

pthatcher · 2022-11-21T23:53:08Z

Meeting:

Figure out what data we want and how it will be used before we work out how to deliver it

I think we want a bitrate that means "if you write this much or less, you won't cause queueing", so you can go throttle you video encoder, for example.

expected size of congestion window (how much you can send in one roundtrip), remaining/total availability, rtt?, calculations just focused at the next frame, or projections multiple frames ahead? a complex problem, and the information we are able to provide isn't well suited

I don't think we need anything more complex than the bitrate you should send at that will avoid queueing.

iframe is 40kb, it's going to take me 4 rtts to send, -> lower resolution etc.

Does anyone do rate control that way (based on RTT)? It seems strange to me, but I suppose you could calculate it using the RTT.

how big a frame can I send?

I think it's less about an individual frame and more about bitrate over time. If you exceed the bitrate for a brief period (with a key frame), it will queue, so make sure you go lower than the target rate for a while after to a make sure it drains.

reporting congstion window leads may lead to odd outcomes, like underutilizing bandwidth.

a leaky abstraction, whereas sites need a lot of context

I think we should stick with the rate, not the window. In fact, many algorithms, like googcc calculate target rate directly, not a window.

index.bs

pthatcher · 2022-12-07T00:48:17Z

I think this is ready for more review.

martinthomson

Do you have any plans to write some prose to go with this?

index.bs

jan-ivar · 2022-12-07T01:02:02Z

Meeting:

unsigned long long
time range? rtt? coming out of a congestion control algorithm
what about pooling? Is it over multiple sessions?
algorithms to back up webidl

pthatcher · 2022-12-20T14:53:24Z

Do you have any plans to write some prose to go with this?

I have added prose.

pthatcher · 2022-12-20T14:53:39Z

Meeting:

unsigned long long

time range? rtt? coming out of a congestion control algorithm

what about pooling? Is it over multiple sessions?

algorithms to back up webidl

All of these have been added/fixed.

pthatcher · 2023-01-17T16:12:58Z

I've done my best to make WebTransport an EventTarget and define the ratecontrolfeedback event, copying off of the WebRTC and WebCodecs specs. Please let me know what I did wrong :).

jan-ivar

Thanks, this is coming together! Lots of detail to work out here, so pardon all the comments.

index.bs

jan-ivar · 2023-01-25T21:54:09Z

index.bs

+  attribute unsigned long? rateControlFeedbackMinInterval;
+  attribute EventHandler onratecontrolfeedback;
+  readonly attribute WebTransportRateControlFeedback rateControlFeedback;


What's the bikeshed logic for the difference here?

wt.rateControlFeedbackMinInterval wt.rateControlFeedback.sendRate

Isn't the following simpler to remember?

wt.rateControlFeedback.minInterval wt.rateControlFeedback.sendRate

Easier to read, and I don't see why we need to make a difference between input and output.

Lastly, to put in writing what I've advocated in meetings, I'd personally prefer something simpler like:

wt.rateControlMinInterval wt.rateControlSendRate

...as this seems sufficient to satisfy the "capture any state information in the target object." design principle. I really don't see what the extra . dot accomplishes, even if we add more attributes in the future. They're still attributes of the session.

I don't see them as something separate — I.e. the attributes will be readable and update whether one listens for the event or not.

I'd love to hear other opinions on this, to understand the benefit(s) of the . dot.

I made #470 and #471 to look at which one we like better (or this one as-is).

index.bs

Co-authored-by: Jan-Ivar Bruaroey <jan-ivar@users.noreply.github.com>

SHA: 6e4b8b7 Reason: push, by pthatcher Co-authored-by: github-actions[bot] <41898282+github-actions[bot]@users.noreply.github.com>

martinthomson · 2023-02-16T00:28:26Z

index.bs

+   <td class="non-normative">A {{long}} indicating the maximum frequency
+   that the {{WebTransport/ratecontrolfeedback}} will be fired.


Isn't this the minimum time between events? Or even the duration over which the event is suppressed after receiving another.

Sure interval is inversely proportional to rate, but only loosely.

martinthomson

This is incomplete, so I object to it having been merged.

martinthomson · 2023-02-16T00:31:08Z

index.bs

+which an application is sending data on another session, one session could learn about the
+the rate at which another application is sending data.  To avoid this, a user agent
+can restrict access to information or modify information provided.  For example, it could leave
+the send rate at an artifical value to make it more difficult to infer anything from it.


This seems pretty loose. This boils down to "maybe the browser could work something out", which is pretty unsatisfying. We should be more concrete, either block access to the attribute (which reveals the presence of another entity, so that needs thought) or define an exemplar strategy for making it available.

martinthomson · 2023-02-16T00:32:11Z

index.bs

+When the user agent changes the rate at which it dequeues bytes for sending on a
+{{WebTransport}} |transport|'s [=WebTransport session=],
+it MUST [=queue a network task=] with |transport| to run these steps:


This requirement is impossible, or nearly so. How does the UA monitor the rate?

martinthomson · 2023-02-16T00:32:26Z

index.bs

+1. Set |transport|.{{WebTransport/rateControlFeedback}}.{{WebTransportRateControlFeedback/[[SendRate]]}}
+    to the new rate for [=WebTransport session=].
+1. Fire an event named {{WebTransport/ratecontrolfeedback}} at |transport|.


Where is the event limiting feature here?

Revert #421.

Peter Thatcher and others added 3 commits September 27, 2022 08:23

Add congestion control event

ebb8988

Add congestion control event

7251fb8

Add interval control and some other changes based on feedback

4cf1042

jan-ivar reviewed Sep 27, 2022

View reviewed changes

index.bs Outdated Show resolved Hide resolved

aboba mentioned this pull request Oct 13, 2022

Transport: Encoder Rate Control and congestion control state w3c/media-pipeline-arch#6

Open

Tweak when the event fires

dab1f13

pthatcher marked this pull request as ready for review November 22, 2022 04:43

jan-ivar reviewed Nov 22, 2022

View reviewed changes

index.bs Outdated Show resolved Hide resolved

wilaw changed the title ~~Add congestion control event~~ Add rate control event Nov 23, 2022

jan-ivar reviewed Nov 30, 2022

View reviewed changes

index.bs Outdated Show resolved Hide resolved

pthatcher added 3 commits December 6, 2022 17:38

Update index.bs

6bfef66

Update index.bs

6221f72

Update index.bs

6a3700c

Add Exposed attribute

299e395

martinthomson reviewed Dec 7, 2022

View reviewed changes

index.bs Outdated Show resolved Hide resolved

martinthomson reviewed Dec 7, 2022

View reviewed changes

index.bs Outdated Show resolved Hide resolved

jan-ivar assigned pthatcher Dec 7, 2022

pthatcher added 4 commits December 20, 2022 06:21

Remove Ms suffix and add congestionControl property

4121eeb

Change to a dictionary and define more clearly.

7bbf7fc

Add more info about congestionControl.

f939533

Specify pooling

cf97c25

Fix name

ace7746

jan-ivar requested changes Jan 25, 2023

View reviewed changes

pthatcher and others added 15 commits February 14, 2023 07:59

Update index.bs

3b45c56

Co-authored-by: Jan-Ivar Bruaroey <jan-ivar@users.noreply.github.com>

Update index.bs

91288e2

Co-authored-by: Jan-Ivar Bruaroey <jan-ivar@users.noreply.github.com>

Update index.bs

e3e7a11

Co-authored-by: Jan-Ivar Bruaroey <jan-ivar@users.noreply.github.com>

Update index.bs

b3728d9

Co-authored-by: Jan-Ivar Bruaroey <jan-ivar@users.noreply.github.com>

Update index.bs

d93882d

Co-authored-by: Jan-Ivar Bruaroey <jan-ivar@users.noreply.github.com>

Update index.bs

ec25dd4

Co-authored-by: Jan-Ivar Bruaroey <jan-ivar@users.noreply.github.com>

Update index.bs

13586bf

Co-authored-by: Jan-Ivar Bruaroey <jan-ivar@users.noreply.github.com>

Update index.bs

fbd45ad

Co-authored-by: Jan-Ivar Bruaroey <jan-ivar@users.noreply.github.com>

Update index.bs

d606678

Co-authored-by: Jan-Ivar Bruaroey <jan-ivar@users.noreply.github.com>

Update index.bs

9c50c6b

Co-authored-by: Jan-Ivar Bruaroey <jan-ivar@users.noreply.github.com>

Remove need for bidi include

a5b0c8f

Update index.bs

c17693b

Co-authored-by: Jan-Ivar Bruaroey <jan-ivar@users.noreply.github.com>

Remove spec:infra

3239355

Make default min interval readable

c7ef1f7

Add some internal slots

2fbd1be

This was referenced Feb 14, 2023

Congestion control event with everything in WebTranspsort.rateControlFeedback. #470

Closed

Congestion control event with no sub-object pthatcher/webtransport#1

Closed

Congestion control event with no sub-object #471

Closed

Fix bikeshed

adacfb9

pthatcher merged commit 6e4b8b7 into w3c:main Feb 14, 2023

github-actions bot added a commit that referenced this pull request Feb 14, 2023

Merge pull request #421 from pthatcher/congestion-control-event

605c096

SHA: 6e4b8b7 Reason: push, by pthatcher Co-authored-by: github-actions[bot] <41898282+github-actions[bot]@users.noreply.github.com>

martinthomson reviewed Feb 16, 2023

View reviewed changes

This was referenced Feb 22, 2023

rateControlFeedbackMinInterval is unused #475

Closed

Final API shape of rate control feedback #481

Closed

jan-ivar added a commit to jan-ivar/webtransport that referenced this pull request Mar 3, 2023

Revert w3c#421.

bd094b0

jan-ivar added a commit that referenced this pull request Mar 3, 2023

Merge pull request #488 from jan-ivar/reverted

2081df6

Revert #421.

jan-ivar mentioned this pull request Apr 7, 2023

Add estimatedSendRate to stats. #494

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Add rate control event #421

Add rate control event #421

pthatcher commented Sep 27, 2022 •

edited by pr-preview bot

Loading

jan-ivar commented Oct 11, 2022

wilaw commented Oct 19, 2022

pthatcher commented Nov 21, 2022

pthatcher commented Nov 21, 2022

pthatcher commented Dec 7, 2022

martinthomson left a comment

jan-ivar commented Dec 7, 2022 •

edited

Loading

pthatcher commented Dec 20, 2022

pthatcher commented Dec 20, 2022

pthatcher commented Jan 17, 2023

jan-ivar left a comment

jan-ivar Jan 25, 2023

jan-ivar Jan 25, 2023

pthatcher Feb 14, 2023

martinthomson Feb 16, 2023

martinthomson left a comment

martinthomson Feb 16, 2023

martinthomson Feb 16, 2023

martinthomson Feb 16, 2023

		<td class="non-normative">A {{long}} indicating the maximum frequency
		that the {{WebTransport/ratecontrolfeedback}} will be fired.

Add rate control event #421

Add rate control event #421

Conversation

pthatcher commented Sep 27, 2022 • edited by pr-preview bot Loading

jan-ivar commented Oct 11, 2022

wilaw commented Oct 19, 2022

pthatcher commented Nov 21, 2022

pthatcher commented Nov 21, 2022

pthatcher commented Dec 7, 2022

martinthomson left a comment

Choose a reason for hiding this comment

jan-ivar commented Dec 7, 2022 • edited Loading

pthatcher commented Dec 20, 2022

pthatcher commented Dec 20, 2022

pthatcher commented Jan 17, 2023

jan-ivar left a comment

Choose a reason for hiding this comment

jan-ivar Jan 25, 2023

Choose a reason for hiding this comment

jan-ivar Jan 25, 2023

Choose a reason for hiding this comment

pthatcher Feb 14, 2023

Choose a reason for hiding this comment

martinthomson Feb 16, 2023

Choose a reason for hiding this comment

martinthomson left a comment

Choose a reason for hiding this comment

martinthomson Feb 16, 2023

Choose a reason for hiding this comment

martinthomson Feb 16, 2023

Choose a reason for hiding this comment

martinthomson Feb 16, 2023

Choose a reason for hiding this comment

pthatcher commented Sep 27, 2022 •

edited by pr-preview bot

Loading

jan-ivar commented Dec 7, 2022 •

edited

Loading