MapPortHandler Callback for container support #831
Conversation
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@ ## master #831 +/- ## ========================================== - Coverage 87.27% 87.01% -0.27% ========================================== Files 43 43 Lines 4819 4836 +17 ========================================== + Hits 4206 4208 +2 - Misses 438 453 +15 Partials 175 175
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
JoeTurki left a comment
JoeTurki left a comment There was a problem hiding this comment.
Thank you so much, I like this direction let me know when it's ready for test and review.
agent_config.go Outdated
| EnableUseCandidateCheckPriority bool | ||
| | ||
| // MapPortHanlder is the handler used to compute mapped port for host candidate. | ||
| MapPortHanlder func(candidate Candidate) int |
There was a problem hiding this comment.
We've stopped adding new configurations to the AgentConfig struct since it's planned for deprecation in favor of a more Pion-aligned API. this feature should instead be exposed as an option, WithMapPortHandler, and used with the new NewAgentWithConfig API. Users who want to use this should migrate to the new API.
we did the same thing recently with the renomination API: #822 (comment)
candidate.go Outdated
| Port() int | ||
| | ||
| // Port mapping support for containers | ||
| MappedPort() int |
There was a problem hiding this comment.
question, do we have to make this public?
There was a problem hiding this comment.
Should we? Will webrtc care about this? I was thinking for design of separted ICE (maybe the future ion) this will be somehow used. What do you think?
There was a problem hiding this comment.
I think we should keep it private for now, unless we need it, or someone requests that we expose it later.
| @xinze-zheng I'm sorry about the internal API change, we had to fix the options so we can upgrade webrtc to use them. |
| I think it would be better if we can change the test to use the new Public api instead, |
Description
Expose a handler
func(candidate Candidate) inthandler that will be called uponaddCandidate. The handler will set themappedPortprivate field. During unmarshal, ifmappedPortis not 0, it will replace the port in ICE candidate string.Reference issue
pion/webrtc#3155