[sip-comm-dev] GSOC 09 : Dtmf progress report


#1

Hi

Here is my week report, Dtmf on RTP is building quickly :

1 - Transformer refactoring :

I refactor TransformConnector as a less specialized class : Connector.
Now a Connector can do transformation or NOT. (Transformer are used in
ZRTP).

A connector is create like this :
connector = new Connector(bindAddress);

If you want this connector to do packetTransformation you just add a
TransformEngine like this :
ZRTPTransformEngine engine = new ZRTPTransformEngine();
connector.setEngine(engine);

With the connector you can inject packet (in progress, not finished)
connector.getDataOutputStream().write(buffer, offset, length);

Here is where i have problems :
When I call this function I have no error, but no DTMF packet are sent.
I did some debug :
int l = connector.getDataOutputStream().write(buffer, offset, length);
System.out.println("send DTMF "+dtmfTone+" len = "+length+" sent : "+l);
System.out.println("dataSocket "+connector.getDataSocket().toString());
System.out.println("localAddress
"+connector.getDataSocket().getLocalAddress());
System.out.println("inetAddress
"+connector.getDataSocket().getInetAddress());

This is what I see :
send DTMF 0 len = 16 sent : 16
dataSocket java.net.DatagramSocket@107c615
localAdress /192.168.142.1
inetAddress null

InetAddress is null. I think this is why nothing is sent.

An other thing, actually I am generating DtmfPacket, and the SequenceNumber
and Timestamp field are 0xFFFF....
I need those values. So I think that I will clone an audio packet, transform
it in a dtmf packet, and inject it using the connector. In so doing I keep
the timestamp and Sequence number.

2 - Configuration :

Now there is an option in the advanced SIP configuration called "Send DTMF
Tone on RTP: RFC4733".
You can see it in the screenshot

Now I need to get the configuration value in the media package. It looks
easy.

3 - Communication between OperationSetDTMF and CallSessionImpl :

OperationSetDTMF know when digits are pressed, CallSessionImpl has a
connector where I can inject packet, so I need communication between this
classes.
In order to do it I added a function in the CallSession interface :
sendDtmf(tone)

And now I am able to call this function in OperationSetDTMF like this :
CallSipImpl call = (CallSipImpl) callParticipant.getCall();
CallSession cs = call.getMediaCallSession();
cs.sendDtmf(tone.getValue());

I don't know if it's the good way to do it. If not tell me, I will correct
it.

4 - OperationSetDtmf like a dispatcher :
Because there is two ways to send Dtmf packets (RTP and SIP INFO),
OperationSetDTMF dispatch the sendDtmf(tone) call to DtmfRtpHandler or to
DtmfSipInfoHandler.

You can find the sources in the zip.

Cheers

Romain

net.zip (554 KB)


#2

Hi Romain,

Thank you for the update!

1 - Transformer refactoring :
2 - Configuration :
3 - Communication between OperationSetDTMF and CallSessionImpl :
4 - OperationSetDtmf like a dispatcher :

As we already talked in a private conversation, I'd like you to break
your modifications in separate patches provided to the dev mailing
list/here. The main reason is that I'd like to incrementally integrate
in the official repository and test your work as you progress.

For example, the refactoring of the TransformConnector seems like a
relatively standalone task which doesn't necessarily involve
DTMF-specific code and since it affects Werner's work on ZRTP, we have
to be careful to not break existing functionality so I want to commit
it as a separate piece of code and test it.

Another example is the UI which only communicates with the
configuration service so it seems to me that committing it early will
allow developers interested in the UI and possibly refining it to work
on their improvements as you focus on the very DTMF over RTP.

Please think about possible ways to break the remaining changes in
standalone patches. (I guess one way is to follow the levels of
abstraction and coupling and divide the modifications along the lines
of the generic interfaces and the weak coupling.)

Best regards,
Lubomir

···

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


#3

Hi
Now DTMF packet injection works and the configuration bouton works too.
I had a problem with ZRTP but now it is fixed. Look at the screenshot :slight_smile:

If you want to test, you will need to apply some patches :

Transformer Patch :
This patch change the TransformConnector behaviour.
Now this is not mandatory to do packet transformation.
If you do NOT want transformation create your Connector like this :
Connector connector = new Connector(bindAddress);
rtpManager.initialize(connector);
this.connectors.put(rtpManager, connector);

If you want transformation, create your Connector just the same, then add a
Transform Engine :
ZRTPTransformEngine engine = new ZRTPTransformEngine();
connector.setEngine(engine);

If you want to test it, here is the patch :
Apply the patch (I tested it with turtoise svn)
And, in the directory Transformer :

mv TransformConnector Connector
mv TransformInputStream InputStream
mv TransformOutputStream OutputStream

I tested it with ZRTP and it is working now. :slight_smile:

Configuration GUI Patch :
Now there is an option in the advanced SIP configuration called "Send DTMF
Tone on RTP: RFC4733".
You can see it in the screenshot

Injection patch :
If you want to see my work in progress you can add this patch.
With this you will be able to inject one DTMF Packet on RTP.

This RTP Packet is not well formated (SeqNum, SSRC incorrect) and is alone
(no respect of the protocol describe in the RFC47333).
I am working on it, this will be done later.

Cheers
Romain

TransformerRefactored.patch (36.6 KB)

Configuration_GUI.patch (8.67 KB)

injection2.patch (3.71 KB)

injection.patch (39.4 KB)

···

2009/7/13 Lubomir Marinov <lubomir.marinov@gmail.com>

Hi Romain,

Thank you for the update!

> 1 - Transformer refactoring :
> 2 - Configuration :
> 3 - Communication between OperationSetDTMF and CallSessionImpl :
> 4 - OperationSetDtmf like a dispatcher :

As we already talked in a private conversation, I'd like you to break
your modifications in separate patches provided to the dev mailing
list/here. The main reason is that I'd like to incrementally integrate
in the official repository and test your work as you progress.

For example, the refactoring of the TransformConnector seems like a
relatively standalone task which doesn't necessarily involve
DTMF-specific code and since it affects Werner's work on ZRTP, we have
to be careful to not break existing functionality so I want to commit
it as a separate piece of code and test it.

Another example is the UI which only communicates with the
configuration service so it seems to me that committing it early will
allow developers interested in the UI and possibly refining it to work
on their improvements as you focus on the very DTMF over RTP.

Please think about possible ways to break the remaining changes in
standalone patches. (I guess one way is to follow the levels of
abstraction and coupling and divide the modifications along the lines
of the generic interfaces and the weak coupling.)

Best regards,
Lubomir

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


#4

Hello Romain,

I'd like to focus on the refactoring of TransformConnector and its
associated streams here.

Transformer Patch :
This patch change the TransformConnector behaviour.
Now this is not mandatory to do packet transformation.

I clearly understand the idea to repurpose TransformConnector to not
only do packet transformations but also enable general packet
injection. Consequently, I find the refactoring which always creates a
connector and only sometimes attaches a transformation engine to it
appropriate.

mv TransformConnector Connector
mv TransformInputStream InputStream
mv TransformOutputStream OutputStream

The renaming though seems rather cosmetic and I personally don't find
it appropriate.

For starters and with the least importance, InputStream and
OutputStream are well-known names from the java.io package. I expect
the first expectation of a developer looking at the source code
directory hierarchy to these files/classes will be that you've created
your own implementations of the java.io classes.

Then and again purely on the side of naming, I'm not comfortable with
the idea that Connector extends RTPConnector, InputStream extends
PushSourceStream and OutputStream extends OutputDataStream because the
names of the extenders sound more generic than the supers. In other
words, these names would make much more sense if the class hierarchy
was reversed.

Finally on naming alone, the classes are left in the .transform package.

Given that these three classes still do transformations, I'd
personally vote to not perform the renaming you've specified.

I think this refactoring coupled with the renaming would've made much
more sense if you had introduced two layers of abstractions. First and
what you need for your task, a layer which ensures that the RTPManager
always has an RTPConnector attached and that you can access it for the
purpose of acquiring its OutputDataStream. This would take most of
what's currently in TransformConnector, TransformInputStream and
TransformOutputStream and would put it in actual generic classes
residing in a package more generic than ,transform. Then, you would've
refactored the current TransformXXX classes to extend the generic
layer introduced in the first step and they would've been left as
rather lightweight classes. Otherwise, you get a connector which
states by its name that it's generic but it does both transformations
and injections, the injections don't have anything to do with
transformations and one has to understand both if she wants to just
look at one of them.

In a summary, I'd advise on leaving Werner's naming as it is and just
making sure that the RTPManager is always initialized with a
TransformConnector which can have a transform engine set or unset at a
later time. This will not only lead to more appropriate naming that
the one you've proposed but will also significantly shorten your patch
and make it more easy for review.

Regards,
Lubomir

···

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


#5

Hello again Romain,

Transformer Patch

Now on the subject of the implementation approach for transmitting
DTMF over RTP, I feel compelled to ask why you've chosen the injection
of RTP packets in the fashion of ZRTP.

As Emil is more often available online than you, I turned to him and
got the answer that it was him who made the decision. His idea is (and
I'm paraphrasing here so take my statements with a large grain of
salt) to eventually have a generic way of injecting RTP packets. If
this idea can be implemented and it works (in a sane non-hacky) for
DTMF over RTP, I welcome it.

In the light of your difficulty to get the timestamp and, most
importantly, the sequence number of the injected packet to the right
values though, I think it may be worth it to look at the task at hand
from a different angle.

ZRTP does inject packets (thank you, Emil, for pointing it to me!). So
I can see how injecting DTMF packets in the same way does make a great
deal of sense.

However, ZRTP has its own sequence numbers which do not depend on the
audio packet sequence numbers while for DTMF over RTP we have in the
RFC that:

   Named telephone events are carried as part of the audio stream and
   MUST use the same sequence number and timestamp base as the regular
   audio channel to simplify the generation of audio waveforms at a
   gateway. The named telephone-event payload type can be considered to
   be a very highly-compressed audio codec and is treated the same as
   other codecs.

Even if you manage to get the sequence numbers right at the layer
which transmits (and receives, for that matter) RTP packets, you're
implementation will be in a way diverging from the expectations of the
RFC reader because of the sentence "The named telephone-event payload
type can be considered to be a very highly-compressed audio codec and
is treated the same as other codecs." At least when I read it, I
thought your implementation would deal with the javax.media/JMF
classes such as Format, Codec, DataSource because these are on the
level of abstraction of codecs, RTP packet generation, they are not in
the layer which transmits RTP packets.

Anyway, Emil and I thought that if you were able to get the sequence
numbers right using your approach, we'd go with it because it is in
effect more generic with respect to RTP packet injection (and on my
side at least because you've already started working on it).

But if it turns out to be a no go, we may need to explore the other
approach of having DTMF at the codec level.

Regards,
Lubomir

···

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


#6

Hello Lubomir
Thank you for your advice but I found an other way to do DTMF
injection without Transformer.
I don't know if the Transformer Refactoring could be of some help for
other projects.
But if it is, I would correct my classes following your advices.

Hello Romain,

I'd like to focus on the refactoring of TransformConnector and its
associated streams here.

> Transformer Patch :
> This patch change the TransformConnector behaviour.
> Now this is not mandatory to do packet transformation.

I clearly understand the idea to repurpose TransformConnector to not
only do packet transformations but also enable general packet
injection. Consequently, I find the refactoring which always creates a
connector and only sometimes attaches a transformation engine to it
appropriate.

> mv TransformConnector Connector
> mv TransformInputStream InputStream
> mv TransformOutputStream OutputStream

The renaming though seems rather cosmetic and I personally don't find
it appropriate.

I agree with you. Naming is very important and my names was too generic.

For starters and with the least importance, InputStream and
OutputStream are well-known names from the java.io package. I expect
the first expectation of a developer looking at the source code
directory hierarchy to these files/classes will be that you've created
your own implementations of the java.io classes.

Then and again purely on the side of naming, I'm not comfortable with
the idea that Connector extends RTPConnector, InputStream extends
PushSourceStream and OutputStream extends OutputDataStream because the
names of the extenders sound more generic than the supers. In other
words, these names would make much more sense if the class hierarchy
was reversed.

Finally on naming alone, the classes are left in the .transform package.

True, indeed.

Given that these three classes still do transformations, I'd
personally vote to not perform the renaming you've specified.

I think this refactoring coupled with the renaming would've made much
more sense if you had introduced two layers of abstractions. First and
what you need for your task, a layer which ensures that the RTPManager
always has an RTPConnector attached and that you can access it for the
purpose of acquiring its OutputDataStream. This would take most of
what's currently in TransformConnector, TransformInputStream and
TransformOutputStream and would put it in actual generic classes
residing in a package more generic than ,transform. Then, you would've
refactored the current TransformXXX classes to extend the generic
layer introduced in the first step and they would've been left as
rather lightweight classes. Otherwise, you get a connector which
states by its name that it's generic but it does both transformations
and injections, the injections don't have anything to do with
transformations and one has to understand both if she wants to just
look at one of them.

Yes, your right.
The idea of two packages, with one extending the other, is very good.

In a summary, I'd advise on leaving Werner's naming as it is and just
making sure that the RTPManager is always initialized with a
TransformConnector which can have a transform engine set or unset at a
later time. This will not only lead to more appropriate naming that
the one you've proposed but will also significantly shorten your patch
and make it more easy for review.

Thank you for your advices.

Regards,
Lubomir

Romain

···

2009/7/18 Lubomir Marinov <lubomir.marinov@gmail.com>

---------------------------------------------------------------------
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


#7

Hello Lubomir

You are totally right about the fact that DTMF should be seen as a
DataSource with DataStream and Format. And this is what I did.

I send it now on a new thread because the conception is totally different.

I hope that the previous conception with RTPConnector could be use in
an other project.

···

2009/7/18 Lubomir Marinov <lubomir.marinov@gmail.com>:

Hello again Romain,

Transformer Patch

Now on the subject of the implementation approach for transmitting
DTMF over RTP, I feel compelled to ask why you've chosen the injection
of RTP packets in the fashion of ZRTP.

As Emil is more often available online than you, I turned to him and
got the answer that it was him who made the decision. His idea is (and
I'm paraphrasing here so take my statements with a large grain of
salt) to eventually have a generic way of injecting RTP packets. If
this idea can be implemented and it works (in a sane non-hacky) for
DTMF over RTP, I welcome it.

In the light of your difficulty to get the timestamp and, most
importantly, the sequence number of the injected packet to the right
values though, I think it may be worth it to look at the task at hand
from a different angle.

ZRTP does inject packets (thank you, Emil, for pointing it to me!). So
I can see how injecting DTMF packets in the same way does make a great
deal of sense.

However, ZRTP has its own sequence numbers which do not depend on the
audio packet sequence numbers while for DTMF over RTP we have in the
RFC that:

Named telephone events are carried as part of the audio stream and
MUST use the same sequence number and timestamp base as the regular
audio channel to simplify the generation of audio waveforms at a
gateway. The named telephone-event payload type can be considered to
be a very highly-compressed audio codec and is treated the same as
other codecs.

Even if you manage to get the sequence numbers right at the layer
which transmits (and receives, for that matter) RTP packets, you're
implementation will be in a way diverging from the expectations of the
RFC reader because of the sentence "The named telephone-event payload
type can be considered to be a very highly-compressed audio codec and
is treated the same as other codecs." At least when I read it, I
thought your implementation would deal with the javax.media/JMF
classes such as Format, Codec, DataSource because these are on the
level of abstraction of codecs, RTP packet generation, they are not in
the layer which transmits RTP packets.

Anyway, Emil and I thought that if you were able to get the sequence
numbers right using your approach, we'd go with it because it is in
effect more generic with respect to RTP packet injection (and on my
side at least because you've already started working on it).

But if it turns out to be a no go, we may need to explore the other
approach of having DTMF at the codec level.

Regards,
Lubomir

---------------------------------------------------------------------
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