Skip to content
Sean DuBois edited this page Sep 30, 2018 · 17 revisions

Issues with Current API

Concurrency/Race issues -- Golang provides an excellent API for sharing data between threads (channels) and we aren't taking advantage of them at all. There is a big mental burden (and chance of error) by locking/unlocking in the networkManager.

Maintainability -- people find the current design confusing. Would prefer more clear boundaries between components. This would also look like the ECMAScript WebRTC API. However, they have zero relation. This is internal to pion-WebRTC and has no effect on a user.

Callback heavy -- Everytime we want to add new features we add a new callback, and it is making things cumbersome

Current API

NetworkManager

Manages all network traffic. To send a packet call a method on the networkManager.

Responding to an inbound packet is done via callbacks. When you create a networkManager we have multiple distinct callbacks for events (RTP, SCTP, ICE etc..)

ICE

Handles ICE status. NetworkManager pushes packets on it, and uses callbacks when it wants to send traffic.

DTLS

Handles encryption for SCTP packets, and DTLS handshake. NetworkManager pushes packets on it, and uses callbacks when it wants to send traffic.

SCTP

Protocol used by DataChannels. NetworkManager pushes packets on it, and uses callbacks when it wants to send traffic.

SRTP

Used for Encrypting/Decrypting audio/video data. This is used by the NetworkManager directly, it isn't async so provides simple Encrypt/Decrypt blocking APIs

Proposed API: Sean

The new API is designed to take advantage of Go channels, and tries copy the pattern of 'Transport Chaining' see in WebRTC documentation.

A new package will be created internal/transport, this package will contain chainable transports that all inherit from a generic interface. Transports that that implement it will also add their own generic APIs, but MUST be MT safe.

type Transport interface {
  // Stop tears down the transport, cleaning up any state.
  Stop()

  // Start the transport, passing any linked Transports
  Start(Transport...) error

  // Send pushes data to the transport and takes ownership
  Send(interface{}) error

  // Recv pulls data from the transport, puller takes ownership
  Recv() (interface{}, error)
}

With the following instances

  • internal/Transport.ICE
  • internal/Transport.DTLS
  • internal/Transport.SCTP
  • internal/Transport.SRTP

Example usage

package webrtc

type RTCPeerConnection struct {
    iceTransport  *Transport.ICE
    dtlsTransport *Transport.DTLS
    sctpTransport *Transport.SCTP
    srtpTransport *Transport.SRTP
}

// Not a real API call, just an example starting transports for an RTCPeerConnection
func (r *RTCPeerConnection) Start() {
    dtlsTransport.Start(iceTransport, srtpTransport, sctpTransport)
    iceTransport.Start(dtlsTransport, srtpTransport)
    srtpTransport.Start(iceTransport)
    sctpTransport.Start(dtlsTransport)

    go func() {
      rtpPacket, err := srtpTransport.Recv()
      // Distribute packet to user, we will also do the same for SCTP (distributing message to proper DataChannel
    }()


// Not a real API call, just an example of sending a packet
func (r *RTCPeerConnection) SendRTPPacket(p *rtp.Packet) {
    r.srtpTransport.Send(p)
}

Notes

This design isn't concerned with someday exporting parts of pion-WebRTC. I don't see much value in that, one of the mistakes of the TURN server was exporting STUN. It would have been better that we exported it after we started the WebRTC project, then we would have known the API we wanted.

Instead we designed it for a theoretical user, which ended up being much different then we expected.

Feedback

Backkem:

  • Transports knowing about each other could lead to circularity issues.
  • Do transports need each others data structs? Or do you also mean to pass events using Send/Recv?
  • Lots of interface{} creeps me out :/

Proposed API: Backkem

This proposal is inspired by work done in libp2p/go-libp2p-transport, perlin-network/noise and xtaci/kcp-go. In addition, it mirrors some basic interfaces in the io and net package. Note: Some details may be missing but the overall approach seems feasible.

// Transport represents a layer-able transport
type Transport interface {
  Dial(Conn) (Conn, error)
  Listen(Conn) (net.Listener, error)
}

// Conn is a minimal clone of the net.Conn interface to reduce implementation overhead. Could be completed down the line.
type Conn interface {
  // Read reads data from the connection.
  Read(b []byte) (n int, err error)

  // Write writes data to the connection.
  Write(b []byte) (n int, err error)

  // LocalAddr returns the local network address.
  LocalAddr() net.Addr

  // RemoteAddr returns the remote network address.
  RemoteAddr() net.Addr

  // Close shuts down the transport and the underlying transports
  // Any blocked Accept operations will be unblocked and return errors.
  Close()
}

The net.Listener interface for reference:

type Listener interface {
    // Accept waits for and returns the next connection to the listener.
    Accept() (Conn, error)

    // Close closes the listener.
    // Any blocked Accept operations will be unblocked and return errors.
    Close() error

    // Addr returns the listener's network address.
    Addr() Addr
}

With the following instances:

  • internal/Transport.ICE
    • Dial & Listen return once the first candidate valid pair binding succeeds. At that point upper lagers may start talking.
    • Read/Write calls Read/Write on the current best valid candidate pair.
  • internal/Transport.DTLS
    • Dial & Listen return once the handshake succeeds. At that point upper lagers may start talking.
    • Read/Write encrypts the data and calls Read/Write on the underlying conn.
  • internal/Transport.SRTP
    • Read/Write calls Read/Write on the underlying conn.
  • internal/Transport.SCTP
    • Dial & Listen return once SCTP Init succeeds. At that point upper lagers may start talking.
    • Read/Write do the needed queuing and calls Read/Write on the underlying conn.
  • internal/Transport.DataChannel: In charge data channel connections (Seems best to split Binary/Text in the WebRTC layer)
    • Dial & Listen return once the data channel is open. At that point upper layers may start talking.
    • Read/Write wraps the data and calls Read/Write on the underlying conn.

Each instance has a specialized constructor for passing options. In addition, they expose their own specific methods for introspection by the upper WebRTC layer. Some of the protocol RFCs actually contain recommendations for those. Any values used by these methods will be translated into their RTP* alternatives by the WebRTC layer this entails some code duplication but the internal state will be safe.

It seems advisable to create a TransportBuilder that can dynamically build the correct transport based on the SDP info. This is also where protocol re-starts would be handled. And we could do things like avoid spinning up SCTP when no data channels are used.

Example usage

The following show an example of API usage. The examples hides some edge cases and error handling for clarity.

// # Initialization

iceTransport := NewICETransport(specificOptions...)
dtlsTransport := NewDTLSTransport(specificOptions...)

srtpTransport := NewSRTPTransport(specificOptions...)

sctpTransport := NewSCTPTransport(specificOptions...)
dcTransport := NewDCTransport(specificOptions...)

// # Dial

// Note: This example will probably need some muxers for (de-)multiplexing the different protocols, E.g.: NewABMux(a Conn, b Conn) Conn
//       I see this as a positive since now this logic is kinda hidden in port-send and port-receive

iceConn, _ := iceTransport.Dial(nil)
dtlsConn, _ := dtlsTransport.Dial(iceConn)

srtpConn, _ := srtpTransport.Dial(dtlsConn)

sctpConn, _ := sctpTransport.Dial(dtlsConn)
dcConn, _ := dcTransport.Dial(sctpConn)

// Listen
iceListener, _ := iceTransport.Listen(nil)
iceConn, _ := iceListener.Accept()

dtlsListener, _ := dtlsTransport.Listen(iceConn)
dtlsConn, _ := dtlsListener.Accept()

// etc...

// # Sending data

// Option 1: Marshal and call write on the conn:
var p *rtp.Packet
b, _ := p.Marshal()
_, _ := srtpConn.Write(b)


// Option 2: Pass the io.Writer:
var p *rtp.Packet
p.WriteTo(srtpConn)

// # Writing data

// Option 1: Read on the conn and unmarshal:
b := make([]byte, 1024)
_, _ = srtpConn.Read(b)
var p *rtp.Packet
_ = p.Unmarshal(b, p)

// Option 2: Pass the io.Reader:
var p *rtp.Packet
p.ReadFrom(srtpConn)

Notes

  • The API is familiar to anyone that has worked with a io.ReadWriteCloser or the net package.
  • The API is blocking, reducing the need for 'OnConnected' or 'OnClose' events. Instead, the caller just waits for Dial or Accept to finish.
  • For SRTP we could provide a io.ReadWriter all the way into the RTCTrack object. Intensive DataChannels apps may want a similar option, can be investigated in the future.
  • Hardcore separation of concerns (Hopefully not to much).
  • If we want to add buffering we can create an instance of Conn around a bytes.Buffer and inject them into the transport stack.
  • There is an argument for making STUN/TURN a transport as well. I kept it out of the above discussion for clarity, but:
    • The STUN transport can keep its binding alive (becomes even more important in the TCP case).
    • The TURN transport can keep its association/channel alive.
    • Making them separate transports could keep this additional complexity out of the ICE package.
  • We could take a shortcut and put Accept directly on Transport but this seems like a minor sacrifice for sticking to the standard interfaces.
  • I'm not suggesting we do this now but if/when we want to expose protocols in their own package at some point in the future we only need to add basic Dial(address string) and Accept(address string) methods. (I do expect there will be a high demand for DTLS.)

Feedback

Sean Backkem's API is 100% the way we should go. Reading through it now all seems so obvious! Thanks for taking the time to explain/formalize this!

Async Events between Transports

The tight coupling of Transports now allows them to talk to each other. One scenario is that DTLS must send a message to SRTP when a certificate is ready, this can't be done via Dial. SRTP doesn't use DTLS, it just gets a certificate from it.

How would you implement this? Can we have a generic API for 'events'?

parsing multiple time

I don't think this will be large performance concern, but is it an issue we need to parse multiple times? If Transports can only send/recv []byte will we be Marshaling/Unmarshaling multiple times? How would you implement DataChannel sending an SCTP message?

1.) DataChannel Transport constructs a SCTP struct, and populates the values 2.) DataChannel does an unmarshal to send via SCTP 3.) SCTP Marshals because it needs data about the SCTP packet we want to send

Trying to satisfy interfaces that don't make sense

I think this is only an issue for SRTP/DataChannel/ICE but what will LocalAddr/RemoteAddr return (just be nil?) I actually don't even think this an issue just asserting! But that is really cool, people can use ICE and hook it up to anything that wants to write generic data over the internet.