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

wcc with trendline filter / delay-base controller #7284

Draft
wants to merge 127 commits into
base: dev
Choose a base branch
from
Draft

Conversation

wangyu-
Copy link
Contributor

@wangyu- wangyu- commented Sep 20, 2022

backgroud

See gist: https://gist.github.com/wangyu-/e6208baf47945d56595872d9af5c6542

Description

New version of WCC with delay-based controller from GCC correctly implemented,

Addresses issues of #7380

Implementation

Documentation & Tests Added

Testing Instructions

the version avaliable for test is cf244a2, which has been rebased to the lasted dev.

If you run with --wcc_v2 --always-saturate on client side, then wcc_v2 is running. (at the moment, bandwitdh saving logic for wcc_v2 is not implement, always-saturate is needed)

If you run with no additional parameter, then the original wcc is running.

you can also run with --always-saturate, then it's original wcc with bandwith saving logic off. So that you can compare wcc and wcc_v2 without the affected of decoder undershooting.

No matter wcc or wcc_v2, the plot data will be automatically generated at /tmp/plot.json, if you encounter anything weird, sent the plot file to me.

wcc and wcc_v2 can share the same server, you don't need to switch between wcc and wcc_v2 on server side. (but make sure your server is also on cf244a2)

What to expected

On a very good link (with very large bandwith, no competing traffic)

there shouldn't be much different with the old wcc, since for such a good link congestion control is not really taking into account.

On a constrained link (either the link's bandwith is small, or there are competing traffic, or the channel itself is noisy)

  1. less freeze time on first connection. i.e. you might encountered a lot, after you start whist client, the video got freezed for a few seconds. This will get reduced. (in future the "prober" idea from webrtc can hopefully solve this completely. )

  2. less latency spikes. which in further means, less audio pops, less video freezes (tiny or big), less response time for click or play online game. If the difference is too subtle to tell, you can see the difference from short_term_latency from the plot.

  3. less weird loss of connection.

drawback:

  1. a little less clearness/sharpness of text sometime. Since anyway congestion void is achieved by reduce birate. But make sure this doesn't happen too often or too much.

Testing suggestions

  1. pay more attention to test on real world network (for example, office wifi, office wired network), instead of simulated ideal link.

Since I have tested on simulated environments like a thousand times, there shouldn't be big problems. The real world networks are the parts my test can't could cover.

  1. pay more attention to the clearness/sharpenss of video, instead of trying to feel the difference of latency.

the clearness/sharpenss on real world network is what I am worrying most. In term of latency, wcc_v2 should be almost always better (or not worse). The worry is, to achieve better latency, if video quailty is compromised (too much).

  1. when comparing wcc and wcc_v2, especially for this version, it's prefered to test on moving content (like video, html5 animation , online game) than mostly static content.

Since on static content, for original wcc (with --always-saturate off), the encoder is in serious undershoot most of the time, the congestion control is not taking too much account.

Others

If you want to test on simulated links, here is some scripts I use for simulated links.

PR Checklist**

  • Did the PR author fully test this PR end-to-end?
  • Did one PR reviewer fully test this PR end-to-end?
  • Did one PR reviewer conduct a thorough code design review?

@wangyu- wangyu- marked this pull request as draft September 20, 2022 16:16
@github-actions github-actions bot added the 📁 Repo: protocol This PR/Issue modifies /protocol code label Sep 20, 2022
@wangyu-
Copy link
Contributor Author

wangyu- commented Oct 20, 2022

UPDATE Oct 20th

the 2nd version for test is f078a54. Main changes:

  1. the over sensitivity issue reported by ming should have been addressed.
  2. the PR is rebased on top of StreamPlotter PR, so that plot file will generated automatically for chrome. (/tmp/plot.json)

@wangyu-
Copy link
Contributor Author

wangyu- commented Oct 21, 2022

UPDATE Oct 21th:

In the last two versions, wcc_v2 need to be enabled with --wcc_v2 --always-saturate (mentioned in the Test Instruction), otherwise the algorithm is the old wcc.

This has caused confusion in testing. Now I released the third version 8c5bb7f.

In this version, wcc_v2 is enabled by default. No parameter passing is needed. In addtion you can confirm WCC_V2 is running from the log, which prints roughly every 5seconds:

image

If you want to run the old wcc and get plots for old wcc, use --no-wcc_v2.

@github-actions
Copy link

Protocol End-to-End Streaming Test Results

Experiments Summary

Expand Summary

Experiment 1 - unknown. Download logs:

aws s3 cp s3://whist-e2e-protocol-test-logs/yancey/gcc/2022_10_23@10-17-09/ 2022_10_23@10-17-09/ --recursive

Experiment 2 - unknown. Logs not available.
Experiment 3 - unknown. Logs not available.
Experiment 4 - unknown. Logs not available.

Full Results: link here

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
📁 Repo: protocol This PR/Issue modifies /protocol code
Projects
None yet
Development

Successfully merging this pull request may close these issues.

1 participant