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

Push API for payload #285

Open
wants to merge 4 commits into
base: master
Choose a base branch
from
Open

Push API for payload #285

wants to merge 4 commits into from

Conversation

bgrozev
Copy link
Member

@bgrozev bgrozev commented Jul 18, 2024

  • feat: Add and use a generic Buffer class.
  • fix: Improve isStunPacket check.
  • wip: A new "push" API for the payload.

Return early when the first two bits don't match. This is for:
1. Correctness: A packet that doesn't start with 00 is not STUN even if
   the magic cookie happens to match
2. Performance: Most packets are RTP and will fail the first byte check,
   so no need to check the magic cookie.
When enabled packets are passed directly to the application,

squash: Fix setting initial addresses.
@bgrozev bgrozev marked this pull request as ready for review August 8, 2024 22:43
}
else
{
CandidatePair selectedPair = getSelectedPair();
Copy link
Member

Choose a reason for hiding this comment

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

If you don't yet have a selected pair, but you do have validated pairs, it's valid to send over any validated pair. This will be needed for the project of reducing the startup time ICE/DTLS/etc takes. This doesn't needed to be part of this PR though, necessarily.

localCandidate = l;
}
}
}
Copy link
Member

Choose a reason for hiding this comment

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

Rather than searching all the local candidates every time we send a packet, would it make sense to save a lastLocalCandidate? Or are there locking issues or something?

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.

2 participants