-
Notifications
You must be signed in to change notification settings - Fork 851
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
Refax: smaller fixes and added utilities #2504
Refax: smaller fixes and added utilities #2504
Conversation
This comment was marked as off-topic.
This comment was marked as off-topic.
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.
No mercy to the reviewer 😢
This flag has landed in the Internet Draft and in previous versions of SRT, thus it has to be handled somehow. As I see, you removed only the part where it was used to form the handshake. Please move these changes to a separate PR. You actually listed 12 changes unrelated to each other and put all of them in a single commit of a single PR with ~500 lines of changes. No mercy to the reviewer. |
That's why I did not remove the flag itself in the handshake data. Simply this time there's no way how it would be set.
Intended. As it is now already in the protocol draft, I didn't want that it causes any changes there - still, there is no use of this feature anymore.
Will be done.
I understand, but then note that - except for those related to the sync-on-msg flag (still, to be pused into a separate PR) - there are no dependencies between them. Every change entry is kinda on its own, unrelated to the others. The alternative would be to make a separate PR for every single change, or group them into multiple PRs by some weird criteria. |
Changes (LARGELY edited - lots of changes from this list were pushed through separate PRs):
FormatLossArray
function for debug purposes.FormatDuration
now uses fixed floating point format.traceState
to the interface and made generic so that it can be also used with other streams.raise_expection
inFixedArray