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

Added API to get instantaneous stats instead of moving averages for a… #288

Merged
merged 3 commits into from
Feb 28, 2018

Conversation

jeandube
Copy link
Collaborator

…pps requiring faster reaction to network condition changes.

Removed extraneous handshakeDone() call preventing connection to peers running older SRT versions.

…pps requiring faster reaction to network condition changes.

Removed extraneous handshakeDone() call preventing connection to peers running older SRT versions.
srtcore/core.cpp Outdated
#ifdef SRT_ENABLE_SNDBUFSZ_MAVG
perf->pktSndBuf = m_pSndBuffer->getAvgBufSize(Ref(perf->byteSndBuf), Ref(perf->msSndBuf));
if (instantaneous
|| ((perf->pktSndBuf == 0x5AE) && (perf->msSndBuf ==0x5AE))){ //5AE: SRT Adapted Encoding: Preserve historical API to get instantaneous stats
Copy link
Collaborator

Choose a reason for hiding this comment

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

Could you please add more explanation in form of code comments on 0x5AE magic?
Also is this endian-safe?

Copy link
Collaborator Author

@jeandube jeandube Feb 27, 2018

Choose a reason for hiding this comment

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

Before the API was changed to add the instantaneous param, and because of lazy programming, the perf structure was used as and IN and OUT param to enable the "instantaneous" behavior. The value 0x5Ae is just a signature set in 2 fields to lower the probability of accidental stack junk setting. I hope this comment will stay for history as much as a comment would be and save me from redoing another PR.

Copy link
Collaborator

Choose a reason for hiding this comment

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

If there's a chance for a singular value ("stack junk") in this structure, then maybe we need to somehow initialize it?

Copy link
Collaborator

Choose a reason for hiding this comment

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

I would vote for removing the magic if it is already deprecated. @jeandube let me know if you agree and I will push the change.

@rndi rndi merged commit 1476472 into Haivision:dev Feb 28, 2018
Gummilion added a commit to Gummilion/srt that referenced this pull request May 10, 2018
* dev: (21 commits)
  Fix SRT file transmit app to work in non-blocking mode (Haivision#334)
  Ensure stdout silence in console pipe out mode plus a few minor fixups (Haivision#275)
  Replaced hardcoded installdirs with GNUInstallDirs. Fixed some status messages. (Haivision#323)
  Breaking connection when recv buffer inflation caused sequence discrepancy (Haivision#300)
  Fixed problems with encryption decision and status report on encryption failure. (Haivision#318)
  Fixed invalid symbol names in doc (Haivision#311)
  Dev add version to winpackages (Haivision#328)
  Android build for dev (Haivision#326)
  Change License to MPLv2.0 (Haivision#327)
  Used constants for input rate. Fixed after-start rate sampling period to 1s (Haivision#315)
  Made SockaddrToString use only numeric string by default (Haivision#312)
  Ported change in Haivision#307 PR to dev
  Fixed SockaddrToString to format as 4dotIP if unknown (Haivision#299)
  Build with debug information for lldb on iOS platform (Haivision#302)
  Fix for sudden stop sending data on macOS/iOS (Haivision#303)
  Fix broken build when testing apps enabled (Haivision#296)
  Removed all code introduced for CBR (Haivision#293)
  Added API to get instantaneous stats instead of moving averages for a… (Haivision#288)
  Added toolchain file and build instruction for iOS (Haivision#286)
  Fix windows build (Haivision#290)
  ...
@jeandube jeandube deleted the upstream-dev branch July 4, 2019 15:32
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.

3 participants