Implements adaptive track context switching with some other fixes. #752
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
gpolitis
changed the title
bbaldino
reviewed
Feb 5, 2019
| /** | ||
| * The number of transmitted bytes so far. | ||
| */ | ||
| public final long transmittedBytes; |
There was a problem hiding this comment.
what do we use this and transmittedPackets for?
There was a problem hiding this comment.
It's used in RTCP SRs, we replace the values from the endpoint with the values that the bridge computes.
There was a problem hiding this comment.
It's the responsibility of the adaptive track context to update these values in the SRs, so you will find these updates in GenericAdaptiveTrackProjectionContext and VP8AdaptiveTrackProjectionContext, along with timestamp updates in order for the receiving endpoint to be able to correctly map RTP timestamps to NTP timestamps.
bbaldino
approved these changes
Feb 7, 2019
bgrozev
added a commit
to bgrozev/jitsi-videobridge
that referenced
this pull request
Mar 19, 2019
…merge-master Brings in 'feat: Implements adaptive track projection context switches.' (jitsi#752).
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
In theory the media engine can switch from sending simulcast -> non-simulcast on the fly and without additional signaling (either intentionally, or unintentionally due to a bug for example). The JS code can also activate/de-activate simulcast on the fly without signaling that back to the bridge (that would definitely be due to a bug). We recently observed for some scrambling video issues with the latest bridge and suspected that a bug in the JS code was causing the media engine to switch from doing simulcast -> not simulcast without signaling that to the bridge. This PR implements adaptive track projection context switches and treats the change from simulcast -> non-simulcast stream as a codec switch. Further analysis showed that there this wasn't an issue in the client and that there's no such simulcast -> non-simulcast switch, but it's good to be able to cope with such switches on the fly, so I'm opening a PR for that functionality.
Also this PR includes fixes for a few minor issues and it also brings back logging of the COLIBRI messages received/sent by the bridge.