search

fishjam-dev / membrane_ice_plugin

Membrane plugin for ICE protocol
Apache License 2.0
4 stars 2 forks source link

ICE.Endpoint implementation #1

Closed FelonEkonom closed 2 years ago

mat-hek commented 2 years ago

@mickel8

I would say that we should reference ICE in the name of this repo so .e.g membrane_ice2_plugin cc @mat-hek?

I'd avoid using numbers in names :P I'd rather rename membrane_ice_plugin to membrane_libnice_plugin and name this membrane_ice_plugin. Or maybe membrane_ice_turn_plugin?

Therefore I would hardcode DTLS and allow to turn it off and I would assume that only one dynamic pad can be connected cc @mat-hek

Ok, but if there will be always one pad, maybe it can be static?

I think we should describe in details architecture of our solution as it is not so obvious.

Not sure this is the place for that. Maybe just link to webrtc_plugin as a state-of-the-art usage example?

mickel8 commented 2 years ago

@mat-hek

I would say that we should reference ICE in the name of this repo so .e.g membrane_ice2_plugin cc @mat-hek?

I'd avoid using numbers in names :P I'd rather rename membrane_ice_plugin to membrane_libnice_plugin and name this membrane_ice_plugin. Or maybe membrane_ice_turn_plugin?

Both sound good

Therefore I would hardcode DTLS and allow to turn it off and I would assume that only one dynamic pad can be connected cc @mat-hek

Ok, but if there will be always one pad, maybe it can be static?

I think that RTP stream can be linked after spawning TURN.Endpoint

I think we should describe in details architecture of our solution as it is not so obvious.

Not sure this is the place for that. Maybe just link to webrtc_plugin as a state-of-the-art usage example?

I meant only part related to ICE and TURNs excluding RTP and other stuff but we can also enhance docs. Anyway, something that will tell that we are creating two TURN instances, we are communicating with them using Erlang messages, why we are using fake address as ICE candidate and stuff like this

mat-hek commented 2 years ago

@mickel8

I think that RTP stream can be linked after spawning TURN.Endpoint

Can't we link it immediately? Not having dynamic pads simplifies things

I meant only part related to ICE and TURNs excluding RTP and other stuff but we can also enhance docs

That's a good point, though I wouldn't describe here how the RTC engine works

mickel8 commented 2 years ago

@mat-hek

Can't we link it immediately? Not having dynamic pads simplifies things

Probably no, as there can be a peer that only sends data without receiving anything. In such a case input pad would remain unlinked. If we want to we can create Fake.Source that would ignore incoming demand to solve this problem

mat-hek commented 2 years ago

@mickel8 ok then, let them be dynamic ;)