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

Improve pluggability and testability across pion modules though Interfaces #2694

Open
enobufs opened this issue Mar 3, 2024 · 1 comment

Comments

@enobufs
Copy link
Member

enobufs commented Mar 3, 2024

Summary

This is a pion/webrtc v4 item. We'd like to use interfaces between pion modules as much as possible to provide users with true pluggability. To achieve this, we'd also need to introduce customizable factories.

Motivation

Pion WebRTC is comprised of many modules (a poly-repo solution) with pluggability in mind. This has allowed us to manage each WebRTC sub-components with independent versions. It is, however, not flexible enough to achieve intended pluggability because a module depends on another via direct struct pointers rather than via interfaces.

By converting struct pointer to an interface, we can achieve the following:

  • Testing a module much easier by mocking the dependencies
  • It allows us to dynamically swap multiple versions of a module on the fly (e.g. A/B testing in benching marking and/or in production environment)
  • It allows us to use custom implementation of a module along with stock pion modules.

Proposal: New Interfaces and CustomFactories

classDiagram
    `webrtc.API` --> `webrtc.CustomFactories` : newXxx()
    `webrtc.API` --> `webrtc.CustomFactories` : Replace factory
    `webrtc.CustomFactories` --> `ice.Agent` : &lt&ltcreate&gt&gt
    `ice.Agent` --> `ice.Conn` : &lt&ltcreate&gt&gt
    `webrtc.CustomFactories` --> `dtls.StreamRTP` : &lt&ltcreate&gt&gt
    `dtls.StreamRTP` --> `dtls.ReadStreamRTP` : &lt&ltcreate&gt&gt
    `dtls.StreamRTP` --> `dtls.WriteStreamRTP` : &lt&ltcreate&gt&gt
    `dtls.StreamRTCP` --> `dtls.ReadStreamRTCP` : &lt&ltcreate&gt&gt
    `dtls.StreamRTCP` --> `dtls.WriteStreamRTCP` : &lt&ltcreate&gt&gt
    `webrtc.CustomFactories` --> `dtls.StreamRTCP` : &lt&ltcreate&gt&gt
    `webrtc.CustomFactories` --> `sctp.Association` : &lt&ltcreate&gt&gt
    `sctp.Association` --> `sctp.Stream` : &lt&ltcreate&gt&gt
    `webrtc.CustomFactories` --> `datachannel.DataChannel` : &lt&ltcreate&gt&gt
    <<interface>>`ice.Agent`
    <<interface>>`ice.Conn`
    <<interface>>`dtls.StreamRTP`
    <<interface>>`dtls.ReadStreamRTP`
    <<interface>>`dtls.WriteStreamRTP`
    <<interface>>`dtls.StreamRTCP`
    <<interface>>`dtls.ReadStreamRTCP`
    <<interface>>`dtls.WriteStreamRTCP`
    <<interface>>`sctp.Association`
    <<interface>>`sctp.Stream`
    <<interface>>`datachannel.DataChannel`
Loading

Plan (high level)

  • Use this issue as an umbrella for individual issues/pull-requests in different modules related to this work. Detailed plan will follow.
  • Create a quick migration guide (e.g. *sctp.Assocation => sctp.Association)
@enobufs
Copy link
Member Author

enobufs commented Mar 3, 2024

These are current candidates for converting to an interface:
https://docs.google.com/spreadsheets/d/1uXNQQ0Qn3NfBXBgsXFSVgXmCpsYwtZrdQOGCQoAl-bA
I have once locally tested based on this earlier.
Please feel free to leave any comments.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Development

No branches or pull requests

2 participants