web: trim deprecated options and defaults #1685
Conversation
|
|
||
| config.enableRemb = {{ $ENABLE_REMB }}; | ||
| config.enableTcc = {{ $ENABLE_TCC }}; | ||
| {{ if not $ENABLE_REMB -}} |
There was a problem hiding this comment.
Why go the verbose route? Setting the value always since it has a sane default is a good idea IMHO.
There was a problem hiding this comment.
@jallamsetty1 and I discussed, and since the code itself has sane defaults, it was felt that an additional default wasn't needed here. Open to reverting that pieces as I don't feel strongly on it.
There was a problem hiding this comment.
Right now most settings are set by taking defaults in the template itself.
Generally speaking we have avoided changing defaults drastically, because it leads to problems.
There was a problem hiding this comment.
Yes, the current practice is to set the defaults in config.js but for settings that affect the core media functionality, I don't think we should make them configurable anymore. I do not see a reason why we should let users disable REMB, TCC, RTX or simulcast for that matter. IMHO not adding the defaults to the template is a first step in that direction.
There was a problem hiding this comment.
Why do we have a setting outside of the testing section for them then?
Not arguing, just curious.
There was a problem hiding this comment.
I guess they were added when the features were implemented but we didn't bother to remove them after the default behavior was implemented in code. That happens with a lot of features that we implement. We conditionally enable them, then enable them by default and don't remove the config.js flag.
No description provided.