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

[core] Fixed checkTimers interval (100ms -> 10 ms) #913

Merged
merged 1 commit into from
Oct 28, 2019

Conversation

maxsharabayko
Copy link
Collaborator

@maxsharabayko maxsharabayko commented Oct 21, 2019

There are two triggers to check timers on a receiving socket.
The first one is to check timers on every packet received. It is done in CRcvQueue::worker_ProcessAddressedPacket(…).

The second place to check the timers is by a timeout in CRcvQueue::worker(…).
The condition was to check the timers if the time of the last check for that socket was at least 100 ms in the past.
However, the timer needs to be checked every 10 ms, in accordance to the communication synchronization interval.

Impact of the bug

Having a 100 ms check does not impact live transmission mode, because the receiver has a constant stream of incoming packets, that trigger timers check.

On the contrary, in file mode the sender first sends 16 packets and waits for an ACK packet from the receiver. And the waiting time might be 100 ms, that slows down the start of the transmission.

@maxsharabayko maxsharabayko added Priority: Medium Type: Bug Indicates an unexpected problem or unintended behavior [core] Area: Changes in SRT library core labels Oct 21, 2019
@maxsharabayko maxsharabayko added this to the v1.4.1 milestone Oct 21, 2019
@rndi rndi merged commit 3a5ead0 into Haivision:master Oct 28, 2019
@maxsharabayko maxsharabayko deleted the hotfix/check-timers-interval branch October 29, 2019 12:25
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
[core] Area: Changes in SRT library core Priority: Medium Type: Bug Indicates an unexpected problem or unintended behavior
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants