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

Draft
wants to merge 3 commits into
base: master
Choose a base branch
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

@zarik5 zarik5 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

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

Successfully merging this pull request may close these issues.

None yet

2 participants