lerouxrgd / datachannel-rs

Rust wrappers for libdatachannel
Mozilla Public License 2.0
135 stars 26 forks source link

Add on_track support for peer connection #24

Closed dtzxporter closed 2 years ago

dtzxporter commented 2 years ago

Adds on_track callback support for peer connections

lerouxrgd commented 2 years ago

Actually I think that having the same logic as on_data_channel for on_track is better because it lets you access &mut self on the struct that implements PeerConnectionHandler.

I implemented it in this commit.

Does it solve your use-case ?

dtzxporter commented 2 years ago

Nope! Then you are limited to one type of track per peer connection and most peer connections will support multiple different track types, with this you are limited to one...

dtzxporter commented 2 years ago

For example, a track that packetizes audio, and one that packetizes video, etc

lerouxrgd commented 2 years ago

There is only one struct RtcTrack so I don't see what types you are mentioning. Could you provide a code example ?

SE2Dev commented 2 years ago

In essence, a single peer connection can have multiple tracks which can in turn serve multiple roles; e.g. one track for video and one track for audio. In situations like this, especially ones where there's completely different types of processing being done for these tracks, you would want to have the events for each of these tracks to be handled differently, and would likely have different structs that implement TrackHandler to accomplish this. With the existing code model, you're restricted to a single type of TrackHandler for all tracks on a given peer connection and therefore can't achieve the desired behavior.

With the proposed code changes, you'd be able to define custom types that could be used similarly to a track interface and be passed around to other sections of code, which becomes increasingly relevant when using something like async-datachannel.

struct MyVideoTrack {};

/// MyVideoTrack doubles as a TrackHandler, and can provide video specific handling logic
impl TrackHandler for MyVideoTrack {
    ...
}

impl MyVideoTrack {
    /// Used to create a `MyVideoTrack` struct from a provided `TrackState` and the
    /// user provided track handler.
    pub fn from_state<T: RtcTrack + Send + 'static>(
        state: TrackState,
    ) -> Result<Self, MyError> {
        let track_handler = MyVideoTrack { ... };
        match state.into_track(track_handler.clone()) {
            Ok(track) => {
                ...
            }
            Err(e) => {
                ...
            },
        }
    }
}
impl RtcPeerConnection for MyConnection {
    ...

    async fn on_track(
        &mut self,
        _pc: &mut RtcPeerConnection<ConnInternal>,
        state: TrackState,
    ) {
        let track = MyVideoTrack::from_state(state);
        ...
    }
}
lerouxrgd commented 2 years ago

But in your example how do you know whether track is let track = MyVideoTrack::from_state(state); or let track = MyAudioTrack::from_state(state); ?

My point is that I don't see how the approach in this PR can differentiate it neither.

SE2Dev commented 2 years ago

I suspect that this behavior would be determined by those consuming the library by checking various track attributes like MID, and the track description - in which case, these would need to be exposed via TrackState as well.