[sip-comm-dev] Proposal to re-factor CallSession and CallSessionImpl


#1

All,

triggered by some discussions around the implementation of Jingle call
support I had a look into the overall RTP implementation at SC and how
to reuse it for Jingle. It turned out that it is a fairly monolithic
implementation that mixes several aspects into one big class. The
CallSessionImpl class contains SDP handling, RTP handling, Player
handling, and some sort of listener/callback handling for various
purposes. This approach makes it nearly impossible to reuse methods
for the Jingle implementation.

Thus I propose to re-factor this implementation and I include a first
draft of the proposal in this mail.

The refactoring would affect several interfaces and classes:

- Split the current CallSessionImpl into two classes (and Interfaces
  according to SC conventions). One class contains the SDP relevant
  methods and other class the RTP relevant methods and data.

- rename the current CallSession interface to SipCallSession and
  implement the according SipCallSessionImpl.

- the new SipCallSession and associated SipCallSessionImpl contains
  the same set of methods as the current CallSession interface. In
  fact SipCallSessionImpl acts as a facade and controls access to the
  underlying SDP/RTP media classes. The current CallSession interface
  that the SIP call handling uses consists of 31 methods, 7 or 8 of
  them deal with SDP handling, some others are support methods (set
  mute, start/stop streaming, etc.) and some others are required for
  GUI handling. Given the complexity of the overall handling this is a
  fair size.

This facade approach decouples the SIP call session handling from the
underlying low layer implementations and would allow for further
refactoring without affecting the upper layer SIP call session
implementation. We could use the same technique to implement the
Jingle support: create a facade that contains all required Jingle
functions (could be similar to the SIP call session facade). This
facade coordinates access to the underlying low layer
implementation.

The figure shows a Jingle session class: this class would contain the
Jingle specific media signaling methods. Jingle media signaling is
different from SDP signaling.

Thoughts, ideas, comments?

Regards,
Werner

Figure: High level overview of classes after refactoring (Jingle
specific classes will be added later)

···

+---------+
         SipCallSession | |
            +-----+ uses | Sdp |
            > +----------------+ Session |
            > > > >
            > > uses | |
            > +----------+ | |
            +-----+ | +---------+
                             >
                             > +---------+
                             > > >
            +-----+ +-----+ RTP |
            > > uses | Media |
            > +----------------+ Session |
            > > uses | |
            > +----------+ | |
            +-----+ | | |
       JingleCallSession | +---------+
                             >
                             > +---------+
                             > > >
                             +-----+ Jingle |
                                   > Session |
                                   > >
                                   > >
                                   > >
                                   +---------+

---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@sip-communicator.dev.java.net
For additional commands, e-mail: dev-help@sip-communicator.dev.java.net


#2

Hi Werner, that looks quite reasonable to me.

···

On 4/13/09 6:16 AM, Werner Dittmann wrote:

All,

triggered by some discussions around the implementation of Jingle call
support I had a look into the overall RTP implementation at SC and how
to reuse it for Jingle. It turned out that it is a fairly monolithic
implementation that mixes several aspects into one big class. The
CallSessionImpl class contains SDP handling, RTP handling, Player
handling, and some sort of listener/callback handling for various
purposes. This approach makes it nearly impossible to reuse methods
for the Jingle implementation.

Thus I propose to re-factor this implementation and I include a first
draft of the proposal in this mail.

The refactoring would affect several interfaces and classes:

- Split the current CallSessionImpl into two classes (and Interfaces
  according to SC conventions). One class contains the SDP relevant
  methods and other class the RTP relevant methods and data.

- rename the current CallSession interface to SipCallSession and
  implement the according SipCallSessionImpl.

- the new SipCallSession and associated SipCallSessionImpl contains
  the same set of methods as the current CallSession interface. In
  fact SipCallSessionImpl acts as a facade and controls access to the
  underlying SDP/RTP media classes. The current CallSession interface
  that the SIP call handling uses consists of 31 methods, 7 or 8 of
  them deal with SDP handling, some others are support methods (set
  mute, start/stop streaming, etc.) and some others are required for
  GUI handling. Given the complexity of the overall handling this is a
  fair size.

This facade approach decouples the SIP call session handling from the
underlying low layer implementations and would allow for further
refactoring without affecting the upper layer SIP call session
implementation. We could use the same technique to implement the
Jingle support: create a facade that contains all required Jingle
functions (could be similar to the SIP call session facade). This
facade coordinates access to the underlying low layer
implementation.

The figure shows a Jingle session class: this class would contain the
Jingle specific media signaling methods. Jingle media signaling is
different from SDP signaling.

Thoughts, ideas, comments?

Regards,
Werner

Figure: High level overview of classes after refactoring (Jingle
specific classes will be added later)

                                   +---------+
         SipCallSession | |
            +-----+ uses | Sdp |
            > +----------------+ Session |
            > > > >
            > > uses | |
            > +----------+ | |
            +-----+ | +---------+
                             >
                             > +---------+
                             > > >
            +-----+ +-----+ RTP |
            > > uses | Media |
            > +----------------+ Session |
            > > uses | |
            > +----------+ | |
            +-----+ | | |
       JingleCallSession | +---------+
                             >
                             > +---------+
                             > > >
                             +-----+ Jingle |
                                   > Session |
                                   > >
                                   > >
                                   > >
                                   +---------+


#3

Hey Werner,

Thanks for bringing this up.

--inline

Werner Dittmann wrote:

All,

triggered by some discussions around the implementation of Jingle call
support I had a look into the overall RTP implementation at SC and how
to reuse it for Jingle. It turned out that it is a fairly monolithic
implementation that mixes several aspects into one big class. The
CallSessionImpl class contains SDP handling, RTP handling, Player
handling, and some sort of listener/callback handling for various
purposes. This approach makes it nearly impossible to reuse methods
for the Jingle implementation.

Thus I propose to re-factor this implementation and I include a first
draft of the proposal in this mail.

The refactoring would affect several interfaces and classes:

- Split the current CallSessionImpl into two classes (and Interfaces
  according to SC conventions). One class contains the SDP relevant
  methods and other class the RTP relevant methods and data.

- rename the current CallSession interface to SipCallSession and
  implement the according SipCallSessionImpl.

- the new SipCallSession and associated SipCallSessionImpl contains
  the same set of methods as the current CallSession interface. In
  fact SipCallSessionImpl acts as a facade and controls access to the
  underlying SDP/RTP media classes. The current CallSession interface
  that the SIP call handling uses consists of 31 methods, 7 or 8 of
  them deal with SDP handling, some others are support methods (set
  mute, start/stop streaming, etc.) and some others are required for
  GUI handling. Given the complexity of the overall handling this is a
  fair size.

This facade approach decouples the SIP call session handling from the
underlying low layer implementations and would allow for further
refactoring without affecting the upper layer SIP call session
implementation. We could use the same technique to implement the
Jingle support: create a facade that contains all required Jingle
functions (could be similar to the SIP call session facade). This
facade coordinates access to the underlying low layer
implementation.

The figure shows a Jingle session class: this class would contain the
Jingle specific media signaling methods. Jingle media signaling is
different from SDP signaling.

Thoughts, ideas, comments?

Yup, as I had already mentioned to you off list, refactoring
CallSessionImpl and moving the SDP processing out of it is something
I've been planning to do for a while now.

Your diagram is more or less in line with what I've had in mind. We'll
probably place its left side (i.e. the SDP processors) into the protocol
implementations as this is how things seem to be working in the real
world today. That is, despite the protocol agnostic design of SDP it is
almost exclusively employed by SIP and the other major telephony
solutions like XMPP use different mechanisms. In other words it is safe
to consider session descriptions as something specific to the signaling
protocol.

Once SDP has been processed, it will be translated into an independent
format. Part of it (the one describing connection addresses) would also
be used to feed and retrieve candidate pairs to our ICE implementation
(which we need to get done before we get busy with jingle). I'll
concentrate on that as soon as we are done with the GSoC selection
process. We'd also need to wait for the java.net update so that we could
spin our 1.0 branch off trunk before diving into such low level changes.

Cheers
Emil

···

Regards,
Werner

Figure: High level overview of classes after refactoring (Jingle
specific classes will be added later)

                                   +---------+
         SipCallSession | |
            +-----+ uses | Sdp |
            > +----------------+ Session |
            > > > >
            > > uses | |
            > +----------+ | |
            +-----+ | +---------+
                             >
                             > +---------+
                             > > >
            +-----+ +-----+ RTP |
            > > uses | Media |
            > +----------------+ Session |
            > > uses | |
            > +----------+ | |
            +-----+ | | |
       JingleCallSession | +---------+
                             >
                             > +---------+
                             > > >
                             +-----+ Jingle |
                                   > Session |
                                   > >
                                   > >
                                   > >
                                   +---------+

---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@sip-communicator.dev.java.net
For additional commands, e-mail: dev-help@sip-communicator.dev.java.net

---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@sip-communicator.dev.java.net
For additional commands, e-mail: dev-help@sip-communicator.dev.java.net