improving initial video quality by setting x-google-start-bitrate for svc codecs #820
Conversation
|
Maybe we should add a simple unit test for |
| pub type OnOfferCreated = Box<dyn FnMut(SessionDescription) + Send + Sync>; | ||
|
|
||
| #[cfg(any(target_os = "windows", target_os = "macos", target_os = "linux"))] | ||
| const DEFAULT_VP9_START_BITRATE_KBPS: u32 = 2500; |
There was a problem hiding this comment.
I think we should use the currently negotiated bitrate? instead of a fixed amount? the JS logic uses 80% of ultimate bitrate. I think it's ok to go to 100% as well
There was a problem hiding this comment.
I will address the comment
There was a problem hiding this comment.
Do we have negotiated birate in SDP ? any one can point me to the relevant code or give me some hint ?
There was a problem hiding this comment.
In Flutter, the preset maxBitrate * 0.7 is used as the startBitrate here.
There was a problem hiding this comment.
There was a problem hiding this comment.
thanks, I addressed the comments.
There was a problem hiding this comment.
why are we using 70 or 80? is there a good reason to? I'd like to avoid magic numbers if possible.
| Ok(answer) | ||
| } | ||
|
|
||
| fn munge_x_google_start_bitrate(sdp: &str, start_bitrate_kbps: u32) -> String { |
There was a problem hiding this comment.
should we use a proper sdp parser/rewriter? instead of string changes? https://crates.io/crates/sdp
in case we'd need to do more munging for other things
There was a problem hiding this comment.
@ladvoc @cloudwebrtc , what do you think here ?
If this is the only place that we will need to parse sdp, I would prefer keeping this handmade parser rather than introducing a new dep
There was a problem hiding this comment.
JS munges SDP for several purposes, including enabling stereo publishing, and a few other things
There was a problem hiding this comment.
I think brining in the SDP crate you have linked to do munging is ok. The crate is lightweight (minimal transitive dependencies) and it comes from the webrtc-rs project.
xianshijing-lk
force-pushed
the
sxian/CLT-2374/implement_x-google-start-bitrate_for_vp9
branch
from
c88e62d to
b33149c
Compare
xianshijing-lk
deleted the
sxian/CLT-2374/implement_x-google-start-bitrate_for_vp9
branch
davidzhao
changed the title
And we should do it for Desktop only