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

Fixed bitrate calculation #2018

Closed
wants to merge 3 commits into from

Conversation

fernandomaura97
Copy link

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.

Comment on lines +289 to +290
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
Copy link
Member

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;
Copy link
Member

@zmerp zmerp Mar 6, 2024

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) {
Copy link
Member

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,
Copy link
Member

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,
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Same as before

@zmerp zmerp marked this pull request as draft March 8, 2024 11:18
@zmerp zmerp force-pushed the master branch 2 times, most recently from d01478e to 005c4c7 Compare March 22, 2024 09:18
@github-actions github-actions bot added the stale label Jul 24, 2024
@zmerp zmerp closed this Jul 27, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants