[jitsi-dev] Fwd: [jitsi-videobridge] The way to use WebRtcpDataStream


#1

Hi Li,

I'm forwarding message for our colleagues.

Emil, Lyubomir the subject of "manager" we were discussing at an early
stage of SCTP data channels comes back again.

-----BEGIN PGP SIGNED MESSAGE-----
Hash: SHA1

Hi, Paweł

I tried to use WebRtcDataStream but met some problems. Here are some
suggestions of mine:

First, I find that WebRtcDataStream is inside package
org.jitsi.videobridge. If I want to use WebRtcDataStream, I have to
import videobridge.jar, it's not convenient because Jirecon doesn't
need other things in videobridge.jar. So I think maybe we should move
it into libjitsi?

Next, the construct method of WebRtcDataStream is package visible, so
even if I imported the whole videobridge.jar, I still can't
instantiate it. It seems that I can only copy the code into my
project. But I think that's weird and hard to modify. So I'd suggest
to change the construct method to public visible.

What do you think?

Well the class is public, but the constructor was hidden for purpose.
In SctpConnection there is part of code that handles this WebRTC data
channel protocol from RFC mentioned in our eralier conversations.
SctpConnection allows to open new WebRtcDataStream on demand[1] or to
listen for streams opened by the other side[2].

The problem is that this code is not easily reusable for your purpose.
One idea would be to take this code to some "WebRtcDataStreamManager"
that takes SctpSocket as a parameter and handles this WebRtc data
channel protocol. I feel that it's quite easy to extract out of from
SctpConnection to seprate class(it was like that in my first version).
Then it can be placed in libjitsi.
Unfortunately I am currently busy with some other tasks, so it would
take time. Eventually it would be great if you could try to do this
and make some pull request. What do you think ?

Regards,
Pawel

[1]: Open WebRtcDataStream on request:
https://github.com/jitsi/jitsi-videobridge/blob/master/src/org/jitsi/videobridge/SctpConnection.java#L841

[2]: https://github.com/jitsi/jitsi-videobridge/blob/master/src/org/jitsi/videobridge/SctpConnection.java#L654
https://github.com/jitsi/jitsi-videobridge/blob/master/src/org/jitsi/videobridge/WebRtcDataStreamListener.java

···

On Mon, Aug 11, 2014 at 4:16 PM, Shunyang Li <lishunyang@pku.edu.cn> wrote:


#2

Access visibility (which isn't a problem really because we can modify
it) aside, I don't understand how you could be using WebRtcDataStream
without SctpConnection: they are really interwined together because
WebRTC data channels are implemented as SCTP over DTLS over ICE/UDP,
libjitsi integrates an SCTP library but the DTLS layer for the
purposes of WebRTC data channels and ICE/UDP itself are setup in
SctpConnection. Don't you need not only WebRtcDataStream but a great
part of SctpConnection as well?

···

2014-08-12 12:03 GMT+03:00 Paweł Domas <pawel.domas@jitsi.org>:

On Mon, Aug 11, 2014 at 4:16 PM, Shunyang Li <lishunyang@pku.edu.cn> wrote:

First, I find that WebRtcDataStream is inside package
org.jitsi.videobridge. If I want to use WebRtcDataStream, I have to
import videobridge.jar, it's not convenient because Jirecon doesn't
need other things in videobridge.jar. So I think maybe we should move
it into libjitsi?

Next, the construct method of WebRtcDataStream is package visible, so
even if I imported the whole videobridge.jar, I still can't
instantiate it. It seems that I can only copy the code into my
project. But I think that's weird and hard to modify. So I'd suggest
to change the construct method to public visible.


#3

Hi Li,

Well, the more I think, the more strongly I feel that we should only
add the protocol stuff[1] to WebRtcDataStream, rather than creating a
new "manager" class. Because as the documentation says,
WebRtcDataStream represents WebRTC data channel that runs on top of
DTLS/SCTP connection. It's more natural that WebRtcDataStream handles
the protocol stuff.

There can be many WebRtcDataStreams running on single SctpSocket.
Hence there must be some upper level class that can see and manage all
these streams. It must intercept Sctp packets first to check if there
is no open/close channel request.

At present, WebRtcDataStream is just a simple encapsulation of
SctpSocket. I think it's not very good, because developers need to use
WebRtcDataStream to transfer data(send or receive) while they also
have to manage SctpSocket themselves additionally(create sctp socket,
add listener, add network link). So I think we should strengthen the
WebRtcDataStream, let it hold the SctpSocket. In this way, developers
just need to create WebRtcDataStream and then use it, they don't need
to care about SctpSocket.

Besides, WebRtcDataStream can also support other transport protocol,
not only the DTLS. We can easily do this by giving SctpSocket
different NetworkLink.

In order to do that:
  1. Developer should create their *own* NetworkLink. The network link
is used for receiving or sending packets to remote end, users can
decide how to implement that, no matter DTLS or UDP or other protocol.
  2. Developer provides the WebRtcDataStream with that NetworkLink.
And then WebRtcDataStream will create SctpSocket automatically and do
some initialize job.
  3. Developer registers himself into WebRtcDataStream as listeners.
If WebRtcDataStream got any data, it will notify him.
  4. Developer can only use WebRtcDataStream to send or receive data,
because SctpSocket is transparent to him.

As for WebRtcDataStreamManager, I don't think we need this. Of course,
we can treat it as a service used for initializing WebRtcDataStream
instance or do some other static jobs.

I agree with that part about NetworkLink, but it should end up in
WebRtcDataStreamManager and not in the stream itself, because of the
reason mentioned above.

By the way, I notice that class SCTP has some static methods. I'm not
clear whether it is OK to call "init" method many times. So that's
should be a problem.

Not really, check against multiple intialization is at Sctp
implementation level[1].

Regards,
Pawel

[1]: https://github.com/jitsi/libjitsi/blob/master/src/org/jitsi/sctp4j/Sctp.java#L154

···

On Wed, Aug 13, 2014 at 2:37 PM, Shunyang Li <lishunyang@pku.edu.cn> wrote:


#4

Hi Li,

(please use "reaply-to-all" so that your replies will be visible on dev list).

-----BEGIN PGP SIGNED MESSAGE-----
Hash: SHA1

Hi, Paweł.

Thank your for your answer, sorry for my mistakes, but I have some
questions now :slight_smile:

Hi Li,

Well, the more I think, the more strongly I feel that we should
only add the protocol stuff[1] to WebRtcDataStream, rather than
creating a new "manager" class. Because as the documentation
says, WebRtcDataStream represents WebRTC data channel that runs
on top of DTLS/SCTP connection. It's more natural that
WebRtcDataStream handles the protocol stuff.

There can be many WebRtcDataStreams running on single SctpSocket.

Sorry, I didn't know that, I originally thought each WebRtcDataStream
has a SctpSocket. But why should we let them share one SctpSocket? I'm
not clear about that, can you give me some example situation?

Hence there must be some upper level class that can see and manage
all these streams. It must intercept Sctp packets first to check if
there is no open/close channel request.

Is that means that one SctpSocket can contain more than one data
channel? And each of them is distinguished by "sid" and "label"?

Yes, WebRtcDataStream is identified by "sid", which is SCTP stream
identifier. SCTP can handle many streams at one time, track their
retransmissons, send/receive queues independently. It identifies each
stream by this SID value. Each WebRtcDataStream uses only the SID for
which it was created(you can see that by looking at send methods
code). Also currently SctpConnection is doing de-multiplexing of
received data by this SID value.

We currently use only default stream on SID 0, but we probably should
use different one for each use case(one for active speaker, second for
video lastN etc.).

Regards,
Pawel

···

On Wed, Aug 13, 2014 at 3:14 PM, Shunyang Li <lishunyang@pku.edu.cn> wrote:

On 08/13/2014 08:56 PM, Paweł Domas wrote:

On Wed, Aug 13, 2014 at 2:37 PM, Shunyang Li >> <lishunyang@pku.edu.cn> wrote:


#5

Yes, WebRtcDataStream is highly associated with SctpConnection. So
actually I don't use WebRtcDataStream now, I just use SctpSocket
directly(same as SctpConnection but didn't use WebRtcDataStream).

Well, Pawel and I were disgusting about writing a
WebRtcDataStreamManager. Mainly move the code in SctpConnection into
this Manager so that we can reuse it easily.

- -Li

- --
Shunyang Li
School of Electronics Engineering & Computer Science
Peking University, China
Email: lishunyang@pku.edu.cn

···

On 08/13/2014 05:06 PM, Lyubomir Marinov wrote:

2014-08-12 12:03 GMT+03:00 Paweł Domas <pawel.domas@jitsi.org>:

On Mon, Aug 11, 2014 at 4:16 PM, Shunyang Li >> <lishunyang@pku.edu.cn> wrote:

First, I find that WebRtcDataStream is inside package
org.jitsi.videobridge. If I want to use WebRtcDataStream, I
have to import videobridge.jar, it's not convenient because
Jirecon doesn't need other things in videobridge.jar. So I
think maybe we should move it into libjitsi?

Next, the construct method of WebRtcDataStream is package
visible, so even if I imported the whole videobridge.jar, I
still can't instantiate it. It seems that I can only copy the
code into my project. But I think that's weird and hard to
modify. So I'd suggest to change the construct method to public
visible.

Access visibility (which isn't a problem really because we can
modify it) aside, I don't understand how you could be using
WebRtcDataStream without SctpConnection: they are really interwined
together because WebRTC data channels are implemented as SCTP over
DTLS over ICE/UDP, libjitsi integrates an SCTP library but the DTLS
layer for the purposes of WebRTC data channels and ICE/UDP itself
are setup in SctpConnection. Don't you need not only
WebRtcDataStream but a great part of SctpConnection as well?