[ladvoc]

Add support for publishing data tracks.

Architecture and public API closely match Rust and JS clients:

• The user publishes data tracks using the LocalParticipant.PublishDataTrack API.
• A LocalManager manages internal state and transitions for all local data tracks.

Closes BOT-541

[boks1971]

I have not reviewed local.go and local_test.go yet. But, I am stopping here for some comments and pick it up later after your response.

Some notes

1. I think localDataTracks should be moved LocalParticipant fully if possible. It should just follow the media track pattern. At room level, I don't think it should be dealing with tracks. It should be dealing with only participants and tracks should live at participant. We should look at it more closely if that is not possible for some reason (I am going to leave the same comment in the subscribe PR where remote tracks if possible should be managed inside the RemoteParticipant)
2. Because I have not fully ready the above files, I am not sure of the LocalManager (and similarly RemoteManager's roles). But for abstraction and making things private, you can do something like

type LocalManager interface {
     // only publicly needed methods here
}

type localManager struct {
     // the implementation which is package private
}

var _ LocalManager = (*localManager)(nil) // compiler trick to ensure that localManager implements the LocalManager interface.

func NewLocalManager() LocalManager{
}

[ladvoc]

I have not reviewed local.go and local_test.go yet. But, I am stopping here for some comments and pick it up later after your response.

Some notes

1. I think localDataTracks should be moved LocalParticipant fully if possible. It should just follow the media track pattern. At room level, I don't think it should be dealing with tracks. It should be dealing with only participants and tracks should live at participant. We should look at it more closely if that is not possible for some reason (I am going to leave the same comment in the subscribe PR where remote tracks if possible should be managed inside the RemoteParticipant)
2. Because I have not fully ready the above files, I am not sure of the LocalManager (and similarly RemoteManager's roles). But for abstraction and making things private, you can do something like

type LocalManager interface {
     // only publicly needed methods here
}

type localManager struct {
     // the implementation which is package private
}

var _ LocalManager = (*localManager)(nil) // compiler trick to ensure that localManager implements the LocalManager interface.

func NewLocalManager() LocalManager{
}

Some additional context: the data tracks implementations follow a pattern that is gradually being adopted by the other client SDKs as part of the effort to break up the monolithic room and participant objects. The main idea is each independent feature in a room (publishing data tracks, subscribing to data tracks, RPC, etc.) keeps all of its state and business logic in one manager, separate from the the objects that comprise the public API (room, local participant, remote participant, etc.).

One benefit is that each manager can be unit tested in isolation without constructing a room or connecting to an SFU. This also allows test cases to be standardized across implementations and audited to ensure they are equivalent. For reference, here are the local manager test suites for the other implementations:

• JS: https://github.com/livekit/client-sdk-js/blob/main/src/room/data-track/outgoing/OutgoingDataTrackManager.test.ts
• Rust: https://github.com/livekit/rust-sdks/blob/35d7655c580147c2ad05fc8ffa5d885e88c822b9/livekit-datatrack/src/local/manager.rs#L496

The role of each manager:

• datatrack.LocalManager manages all data tracks published by the local participant.
• datatrack.RemoteManager manages all data track subscriptions in the room, not just the tracks of one remote participant.

We could move LocalManager into LocalParticipant. RemoteManager is different though since its state covers the whole room, so there is no single RemoteParticipant it could belong to.

Curious to hear your thoughts given this additional context.

[boks1971]

I have not reviewed local.go and local_test.go yet. But, I am stopping here for some comments and pick it up later after your response.
Some notes

1. I think localDataTracks should be moved LocalParticipant fully if possible. It should just follow the media track pattern. At room level, I don't think it should be dealing with tracks. It should be dealing with only participants and tracks should live at participant. We should look at it more closely if that is not possible for some reason (I am going to leave the same comment in the subscribe PR where remote tracks if possible should be managed inside the RemoteParticipant)
2. Because I have not fully ready the above files, I am not sure of the LocalManager (and similarly RemoteManager's roles). But for abstraction and making things private, you can do something like

type LocalManager interface {
     // only publicly needed methods here
}

type localManager struct {
     // the implementation which is package private
}

var _ LocalManager = (*localManager)(nil) // compiler trick to ensure that localManager implements the LocalManager interface.

func NewLocalManager() LocalManager{
}

Some additional context: the data tracks implementations follow a pattern that is gradually being adopted by the other client SDKs as part of the effort to break up the monolithic room and participant objects. The main idea is each independent feature in a room (publishing data tracks, subscribing to data tracks, RPC, etc.) keeps all of its state and business logic in one manager, separate from the the objects that comprise the public API (room, local participant, remote participant, etc.).

One benefit is that each manager can be unit tested in isolation without constructing a room or connecting to an SFU. This also allows test cases to be standardized across implementations and audited to ensure they are equivalent. For reference, here are the local manager test suites for the other implementations:

• JS: https://github.com/livekit/client-sdk-js/blob/main/src/room/data-track/outgoing/OutgoingDataTrackManager.test.ts
• Rust: https://github.com/livekit/rust-sdks/blob/35d7655c580147c2ad05fc8ffa5d885e88c822b9/livekit-datatrack/src/local/manager.rs#L496

The role of each manager:

• datatrack.LocalManager manages all data tracks published by the local participant.
• datatrack.RemoteManager manages all data track subscriptions in the room, not just the tracks of one remote participant.

We could move LocalManager into LocalParticipant. RemoteManager is different though since its state covers the whole room, so there is no single RemoteParticipant it could belong to.

Curious to hear your thoughts given this additional context.

Sorry about the delay on this. That sounds good to me @ladvoc . I will read through those two managers and drop any feedback.

I am thinking LocalManager in LocalParticipant is still a good one to do rather than at room level.

Ideally, I would do a RemoteParticipantManager for all remote participants and tuck the RemoteManager in that so that there is more separation from Room there.

[devin-ai-integration]

Devin Review found 4 new potential issues.

1 flag not posted on this PR by your GitHub settings — view it in Devin Review. (Configure)