-
-
Notifications
You must be signed in to change notification settings - Fork 520
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
Fixed bitrate calculation #2018
Conversation
if dt_throughput != Duration::ZERO { | ||
// TODO: Assuming UDP for 42.0, for TCP it would be 54.0 instead. This loses a bit of accuracy for TCP (it's still a good estimate) but I could need to import session settings in the future |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I would like you to make this correct for TCP from the start
@@ -271,6 +271,7 @@ fn connection_pipeline(capabilities: ClientCapabilities) -> ConResult { | |||
stream_socket.subscribe_to_stream::<Haptics>(HAPTICS, MAX_UNREAD_PACKETS); | |||
let statistics_sender = stream_socket.request_stream(STATISTICS); | |||
|
|||
let mut actual_throughput_inseconds: f32 = 30_000_000.0; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Rename to actual_throughput_bps
@@ -94,6 +94,15 @@ impl StatisticsManager { | |||
} | |||
} | |||
|
|||
pub fn report_throughput_client(&mut self, target_timestamp: Duration, throughput_client: f32) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
rename to throughput_client_bps
@@ -314,6 +314,7 @@ pub struct ClientStatistics { | |||
pub rendering: Duration, | |||
pub vsync_queue: Duration, | |||
pub total_pipeline_latency: Duration, | |||
pub throughput_client: f32, |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Same as before
@@ -99,6 +99,7 @@ impl BitrateManager { | |||
timestamp: Duration, | |||
network_latency: Duration, | |||
decoder_latency: Duration, | |||
throughput_reported_client: f32, |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Same as before
d01478e
to
005c4c7
Compare
Made some changes I mentioned on discord (elpotas there) for a more accurate bitrate at client stimation, then used for Statistics/plots and an improved adaptive bitrate. This fixes some issues with adaptive too, that saw huge drops in bitrate if network latency increased for any reason.