[sip-comm-dev] [patch] a glance on jingle : making calls with jabber in SC


#1

Hi all,

For those interested in making call from jabber with sc.
The accompanying files will permit, I hope, to do that.

Of course, the actual code, even if usable, is very far away from be 100% OK
(and I think it will never be, perfection isn't for this world :slight_smile: ). I am "actively working" for
getting things better.

The telephony support for jabber is backed by the jingle lib, which handle all
the ugly work, so, there wasn't so much thing to do :slight_smile: . For integration with
SC, I have literally copied the code from the counterpart in sip impl,
adding some lines when needed. Thanks Emil and others :slight_smile:

The most difficult point was the access to media. As the actual media service
api was primarly designed for sc, it was a bitte dificult to use it as is. Thus, I have made
a little door to gain access to the media service as I have said in a previous mail.

This patch includes the previous, related to RtpFlow.

regards.

路路路

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


#2

Sorry, the zip file was missing :o)

Sympho a 锟絚rit :

sc-jingle.zip (24.1 KB)

路路路

Hi all,

For those interested in making call from jabber with sc.
The accompanying files will permit, I hope, to do that.

Of course, the actual code, even if usable, is very far away from be 100% OK
(and I think it will never be, perfection isn't for this world :slight_smile: ). I am "actively working" for
getting things better.

The telephony support for jabber is backed by the jingle lib, which handle all
the ugly work, so, there wasn't so much thing to do :slight_smile: . For integration with
SC, I have literally copied the code from the counterpart in sip impl,
adding some lines when needed. Thanks Emil and others :slight_smile:

The most difficult point was the access to media. As the actual media service
api was primarly designed for sc, it was a bitte dificult to use it as is. Thus, I have made
a little door to gain access to the media service as I have said in a previous mail.

This patch includes the previous, related to RtpFlow.

regards.

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

I had changed some methods name's in the RtpFlow interface
and added publics get.

I also want to notice you that the implementation file isn't the same verbatim compared to the one I provided in the previous patch.

Is it anyone else working on an implementation ?

Regards.

Sympho a 锟絚rit :

RtpFlow.patch (1.89 KB)

路路路

Sorry, the zip file was missing :o)

Sympho a 锟絚rit :

Hi all,

For those interested in making call from jabber with sc.
The accompanying files will permit, I hope, to do that.

Of course, the actual code, even if usable, is very far away from be 100% OK
(and I think it will never be, perfection isn't for this world :slight_smile: ). I am "actively working" for
getting things better.

The telephony support for jabber is backed by the jingle lib, which handle all
the ugly work, so, there wasn't so much thing to do :slight_smile: . For integration with
SC, I have literally copied the code from the counterpart in sip impl,
adding some lines when needed. Thanks Emil and others :slight_smile:

The most difficult point was the access to media. As the actual media service
api was primarly designed for sc, it was a bitte dificult to use it as is. Thus, I have made
a little door to gain access to the media service as I have said in a previous mail.

This patch includes the previous, related to RtpFlow.

regards.

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


#4

Hi Sympho,

Sympho wrote:

Hi Emil,

I had changed some methods name's in the RtpFlow interface
and added publics get.

Cool! Thanks

I also want to notice you that the implementation file isn't the same
verbatim compared to the one I provided in the previous patch.

That's true. The thing is that I've started reviewing your patch and I
am making modifications along the way. I am preparing a mail that would
explain all this but wanted to finish the review and have all
modifications made before sending it.

I have for example renamed the [start|stop]Transmission() methods to
start() and stop() simply because (and this was confirmed by your
comments) they are also starting reception.

Concerning the get methods, could you please also send the corresponding
implementation so that I could also commit the impl part of your
contribution?

Thanks
Emil

路路路

Is it anyone else working on an implementation ?

Regards.

Sympho a 锟絚rit :

Sorry, the zip file was missing :o)

Sympho a 锟絚rit :

Hi all,

For those interested in making call from jabber with sc.
The accompanying files will permit, I hope, to do that.

Of course, the actual code, even if usable, is very far away from be
100% OK
(and I think it will never be, perfection isn't for this world :slight_smile: ). I
am "actively working" for
getting things better.

The telephony support for jabber is backed by the jingle lib, which
handle all
the ugly work, so, there wasn't so much thing to do :slight_smile: . For
integration with
SC, I have literally copied the code from the counterpart in sip impl,
adding some lines when needed. Thanks Emil and others :slight_smile:

The most difficult point was the access to media. As the actual media
service
api was primarly designed for sc, it was a bitte dificult to use it as
is. Thus, I have made
a little door to gain access to the media service as I have said in a
previous mail.

This patch includes the previous, related to RtpFlow.

regards.

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

------------------------------------------------------------------------

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


#5

Hi Emil,

Emil Ivov a 锟絚rit :

Hi Sympho,

Sympho wrote:

Hi Emil,

I had changed some methods name's in the RtpFlow interface
and added publics get.

Cool! Thanks

I also want to notice you that the implementation file isn't the same verbatim compared to the one I provided in the previous patch.

That's true. The thing is that I've started reviewing your patch and I
am making modifications along the way. I am preparing a mail that would
explain all this but wanted to finish the review and have all
modifications made before sending it.

I see. I have just found bizzare to see the interface alone in the repo., (without the implementation). I have thought that you have choosen to make it proprietary because it was too wonderfull, so we may sell it to others and earn a lot of money :o))))) I will quick call the operator to cancel my trip to hawai :slight_smile:

I have for example renamed the [start|stop]Transmission() methods to
start() and stop() simply because (and this was confirmed by your
comments) they are also starting reception.

All right.

Concerning the get methods, could you please also send the corresponding
implementation so that I could also commit the impl part of your
contribution?

See attached.

RtpFlowImpl.java (12.6 KB)

mediaservice.patch (4.64 KB)

mediaimpl.patch (3.27 KB)

路路路

++

Thanks
Emil

Is it anyone else working on an implementation ?

Regards.

Sympho a 锟絚rit :

Sorry, the zip file was missing :o)

Sympho a 锟絚rit :

Hi all,

For those interested in making call from jabber with sc.
The accompanying files will permit, I hope, to do that.

Of course, the actual code, even if usable, is very far away from be 100% OK
(and I think it will never be, perfection isn't for this world :slight_smile: ). I am "actively working" for
getting things better.

The telephony support for jabber is backed by the jingle lib, which handle all
the ugly work, so, there wasn't so much thing to do :slight_smile: . For integration with
SC, I have literally copied the code from the counterpart in sip impl,
adding some lines when needed. Thanks Emil and others :slight_smile:

The most difficult point was the access to media. As the actual media service
api was primarly designed for sc, it was a bitte dificult to use it as is. Thus, I have made
a little door to gain access to the media service as I have said in a previous mail.

This patch includes the previous, related to RtpFlow.

regards.

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

------------------------------------------------------------------------

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


#6

Hi Emil,

This is a new jingle patch which includes all previous (and only jingle related) modifications.

The only client I found to make test with was spark and it worked, we can issue/answer
calls to/from spark.

There are known issues and we have talked of some off list, I will repeat here for record.

- first, there is a little bug in smakx-jingle lib which do not signals when a call has been ended
if the remote user hangup before we answer.

- currently, I do not probe if a remote user support telephony feature before trying to call him. Doing
this, I do not violate the spec wich says we have to respond to discovery request by advertising
features wich we support but, do not impose to query if a feature is supported before trying to use it
with another client. I could add a check in the future as it can bring some "failsafe" capacity
to the implementation.

- the jingle lib provides several TransportManager with some of them able to handle nat traversal
I have used the ICETransportManager whih is not "fully functional" because it is currently created
with an invalid stun server (null).

Well, as always this patch isn't the last and there is still pending work :slight_smile:

regards.

Sympho a 锟絚rit :

sc-jingle.zip (28 KB)

路路路

Hi Emil,

Emil Ivov a 锟絚rit :

Hi Sympho,

Sympho wrote:

Hi Emil,

I had changed some methods name's in the RtpFlow interface
and added publics get.

Cool! Thanks

I also want to notice you that the implementation file isn't the same verbatim compared to the one I provided in the previous patch.

That's true. The thing is that I've started reviewing your patch and I
am making modifications along the way. I am preparing a mail that would
explain all this but wanted to finish the review and have all
modifications made before sending it.

I see. I have just found bizzare to see the interface alone in the repo., (without the implementation). I have thought that you have choosen to make it proprietary because it was too wonderfull, so we may sell it to others and earn a lot of money :o))))) I will quick call the operator to cancel my trip to hawai :slight_smile:

I have for example renamed the [start|stop]Transmission() methods to
start() and stop() simply because (and this was confirmed by your
comments) they are also starting reception.

All right.

Concerning the get methods, could you please also send the corresponding
implementation so that I could also commit the impl part of your
contribution?

See attached.

++

Thanks
Emil

Is it anyone else working on an implementation ?

Regards.

Sympho a 锟絚rit :

Sorry, the zip file was missing :o)

Sympho a 锟絚rit :

Hi all,

For those interested in making call from jabber with sc.
The accompanying files will permit, I hope, to do that.

Of course, the actual code, even if usable, is very far away from be 100% OK
(and I think it will never be, perfection isn't for this world :slight_smile: ). I am "actively working" for
getting things better.

The telephony support for jabber is backed by the jingle lib, which handle all
the ugly work, so, there wasn't so much thing to do :slight_smile: . For integration with
SC, I have literally copied the code from the counterpart in sip impl,
adding some lines when needed. Thanks Emil and others :slight_smile:

The most difficult point was the access to media. As the actual media service
api was primarly designed for sc, it was a bitte dificult to use it as is. Thus, I have made
a little door to gain access to the media service as I have said in a previous mail.

This patch includes the previous, related to RtpFlow.

regards.

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

------------------------------------------------------------------------

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

------------------------------------------------------------------------

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

I have finally found some time to review and commit your Jingle patch.
Sorry for all the lingering and thanks for your patience!

I really like your code! I've had to make very few modifications and
most of them were minor. Besides, Jingle support is really a great
feature that is far from being trivial to implement. Great work!

Here are the things that I've changed:

* Removed NightlyBuildID.java from the patch (guess it must have slipped
there unintentionally).

* I've been adding you as an author to classes that you have modified.
You should consider doing this yourself when you make substantial
modifications (e.g. add new methods) to existing classes.

* Make sure all javadocs start with a capital letter

* I've seen that you have added comments to existing methods and fields
in classes that you have modified. This is awesome! :slight_smile:

* MediaService.java

-- You should generally avoid using the Hashtable class as a type
specifier, and go for Map instead. This is especially useful in places
like the service package that are supposed to be very implementation
friendly.

* RtpFlowImpl.java

-- You were catching the media exception and re-throwing it as an
IllegalStateException (any particular reason?). I removed the catch
since the method is declared to throw a MediaException. And besides the
IllegalArgumentException() constructor that takes an exception parameter
only exists in J1.5 and we're not there yet.

-- Set default values (null for objects, -1 for ints) on class fields.
This is more by convention rather than anything else.

-- Right now you don't seem to be doing anything to handle incoming
video flows. Yet it would be an easy hack and you could mostly reuse
code from CallSessionImpl. Care to give it a try?

* ProtocolProviderServiceJabberImpl.java

-- I've seen that you have left some of the feature declarations as
comments unsure of whether or not they had a corresponding XEP. It might
be a good idea to start a discussion on this mailing list in case anyone
would know.

-- Replaced calls to String.contains() (@since 1.5) with
String.indexOf()!= -1

-- Remove class import DiscoveryInfo.* (@since 1.5) and replace Identity
usages with DiscoveryInfo.Identity

* CallParticipantJabberImpl.java

-- In the [set|get]DisplayName() methods you don't really seem to be
using Jabber display names. Could you please fix this ?

That's it.

Thanks again, Sympho!

Emil

Sympho wrote:

路路路

Hi Emil,

This is a new jingle patch which includes all previous (and only jingle
related) modifications.

The only client I found to make test with was spark and it worked, we
can issue/answer
calls to/from spark.

There are known issues and we have talked of some off list, I will
repeat here for record.

- first, there is a little bug in smakx-jingle lib which do not signals
when a call has been ended
if the remote user hangup before we answer.

- currently, I do not probe if a remote user support telephony feature
before trying to call him. Doing
this, I do not violate the spec wich says we have to respond to
discovery request by advertising
features wich we support but, do not impose to query if a feature is
supported before trying to use it
with another client. I could add a check in the future as it can bring
some "failsafe" capacity
to the implementation.

- the jingle lib provides several TransportManager with some of them
able to handle nat traversal
I have used the ICETransportManager whih is not "fully functional"
because it is currently created
with an invalid stun server (null).

Well, as always this patch isn't the last and there is still pending work :slight_smile:

regards.

Sympho a 锟絚rit :

Hi Emil,

Emil Ivov a 锟絚rit :

Hi Sympho,

Sympho wrote:

Hi Emil,

I had changed some methods name's in the RtpFlow interface
and added publics get.

Cool! Thanks

I also want to notice you that the implementation file isn't the
same verbatim compared to the one I provided in the previous patch.

That's true. The thing is that I've started reviewing your patch and I
am making modifications along the way. I am preparing a mail that would
explain all this but wanted to finish the review and have all
modifications made before sending it.

I see. I have just found bizzare to see the interface alone in the
repo., (without the implementation). I have thought that you have
choosen to make it proprietary because it was too wonderfull, so we
may sell it to others and earn a lot of money :o))))) I will quick
call the operator to cancel my trip to hawai :slight_smile:

I have for example renamed the [start|stop]Transmission() methods to
start() and stop() simply because (and this was confirmed by your
comments) they are also starting reception.

All right.

Concerning the get methods, could you please also send the corresponding
implementation so that I could also commit the impl part of your
contribution?

See attached.

++

Thanks
Emil

Is it anyone else working on an implementation ?

Regards.

Sympho a 锟絚rit :

Sorry, the zip file was missing :o)

Sympho a 锟絚rit :

Hi all,

For those interested in making call from jabber with sc.
The accompanying files will permit, I hope, to do that.

Of course, the actual code, even if usable, is very far away from
be 100% OK
(and I think it will never be, perfection isn't for this world :slight_smile:
). I am "actively working" for
getting things better.

The telephony support for jabber is backed by the jingle lib,
which handle all
the ugly work, so, there wasn't so much thing to do :slight_smile: . For
integration with
SC, I have literally copied the code from the counterpart in sip
impl,
adding some lines when needed. Thanks Emil and others :slight_smile:

The most difficult point was the access to media. As the actual
media service
api was primarly designed for sc, it was a bitte dificult to use
it as is. Thus, I have made
a little door to gain access to the media service as I have said
in a previous mail.

This patch includes the previous, related to RtpFlow.

regards.

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

------------------------------------------------------------------------

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

------------------------------------------------------------------------

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

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


#8

Hi,

Emil Ivov <emcho@emcho.com> a 茅crit :

Hello Sympho,

I have finally found some time to review and commit your Jingle patch.
Sorry for all the lingering and thanks for your patience!

:slight_smile:

I really like your code! I've had to make very few modifications and
most of them were minor. Besides, Jingle support is really a great
feature that is far from being trivial to implement. Great work!

Here are the things that I've changed:

* Removed NightlyBuildID.java from the patch (guess it must have slipped
there unintentionally).

You are right.

* I've been adding you as an author to classes that you have modified.
You should consider doing this yourself when you make substantial
modifications (e.g. add new methods) to existing classes.

* Make sure all javadocs start with a capital letter

recordeD. Be warned that you could end up flooded with tonnes of patch wich such directives :o)

* I've seen that you have added comments to existing methods and fields
in classes that you have modified. This is awesome! :slight_smile:

* MediaService.java

-- You should generally avoid using the Hashtable class as a type
specifier, and go for Map instead. This is especially useful in places
like the service package that are supposed to be very implementation
friendly.

* RtpFlowImpl.java

-- You were catching the media exception and re-throwing it as an
IllegalStateException (any particular reason?). I removed the catch
since the method is declared to throw a MediaException. And besides the
IllegalArgumentException() constructor that takes an exception parameter
only exists in J1.5 and we're not there yet.

It was a mistake.

-- Set default values (null for objects, -1 for ints) on class fields.
This is more by convention rather than anything else.

-- Right now you don't seem to be doing anything to handle incoming
video flows. Yet it would be an easy hack and you could mostly reuse
code from CallSessionImpl. Care to give it a try?

Will look it closer.

* ProtocolProviderServiceJabberImpl.java

-- I've seen that you have left some of the feature declarations as
comments unsure of whether or not they had a corresponding XEP. It might
be a good idea to start a discussion on this mailing list in case anyone
would know.

In fact the xmpp.org and jabber.org sites gone down when I was checking xep(s) last week. They are online again, I will open a new thread about this.

-- Replaced calls to String.contains() (@since 1.5) with
String.indexOf()!= -1

-- Remove class import DiscoveryInfo.* (@since 1.5) and replace Identity
usages with DiscoveryInfo.Identity

* CallParticipantJabberImpl.java

-- In the [set|get]DisplayName() methods you don't really seem to be
using Jabber display names. Could you please fix this ?

Will be done by the end of the week.

That's it.

Thanks again, Sympho!

I have a lot fun working on sc. Thanks to you and all others.

路路路

++

Emil

Sympho wrote:

Hi Emil,

This is a new jingle patch which includes all previous (and only jingle
related) modifications.

The only client I found to make test with was spark and it worked, we
can issue/answer
calls to/from spark.

There are known issues and we have talked of some off list, I will
repeat here for record.

- first, there is a little bug in smakx-jingle lib which do not signals
when a call has been ended
if the remote user hangup before we answer.

- currently, I do not probe if a remote user support telephony feature
before trying to call him. Doing
this, I do not violate the spec wich says we have to respond to
discovery request by advertising
features wich we support but, do not impose to query if a feature is
supported before trying to use it
with another client. I could add a check in the future as it can bring
some "failsafe" capacity
to the implementation.

- the jingle lib provides several TransportManager with some of them
able to handle nat traversal
I have used the ICETransportManager whih is not "fully functional"
because it is currently created
with an invalid stun server (null).

Well, as always this patch isn't the last and there is still pending work :slight_smile:

regards.

Sympho a 茅crit :

Hi Emil,

Emil Ivov a 茅crit :

Hi Sympho,

Sympho wrote:

Hi Emil,

I had changed some methods name's in the RtpFlow interface
and added publics get.

Cool! Thanks

I also want to notice you that the implementation file isn't the
same verbatim compared to the one I provided in the previous patch.

That's true. The thing is that I've started reviewing your patch and I
am making modifications along the way. I am preparing a mail that would
explain all this but wanted to finish the review and have all
modifications made before sending it.

I see. I have just found bizzare to see the interface alone in the
repo., (without the implementation). I have thought that you have
choosen to make it proprietary because it was too wonderfull, so we
may sell it to others and earn a lot of money :o))))) I will quick
call the operator to cancel my trip to hawai :slight_smile:

I have for example renamed the [start|stop]Transmission() methods to
start() and stop() simply because (and this was confirmed by your
comments) they are also starting reception.

All right.

Concerning the get methods, could you please also send the corresponding
implementation so that I could also commit the impl part of your
contribution?

See attached.

++

Thanks
Emil

Is it anyone else working on an implementation ?

Regards.

Sympho a 茅crit :

Sorry, the zip file was missing :o)

Sympho a 茅crit :

Hi all,

For those interested in making call from jabber with sc.
The accompanying files will permit, I hope, to do that.

Of course, the actual code, even if usable, is very far away from
be 100% OK
(and I think it will never be, perfection isn't for this world :slight_smile:
). I am "actively working" for
getting things better.

The telephony support for jabber is backed by the jingle lib,
which handle all
the ugly work, so, there wasn't so much thing to do :slight_smile: . For
integration with
SC, I have literally copied the code from the counterpart in sip
impl,
adding some lines when needed. Thanks Emil and others :slight_smile:

The most difficult point was the access to media. As the actual
media service
api was primarly designed for sc, it was a bitte dificult to use
it as is. Thus, I have made
a little door to gain access to the media service as I have said
in a previous mail.

This patch includes the previous, related to RtpFlow.

regards.

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

------------------------------------------------------------------------

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

------------------------------------------------------------------------

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

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


#9

Hey,

* Make sure all javadocs start with a capital letter

recordeD. Be warned that you could end up flooded with tonnes of patch
wich such directives :o)

I think you'd probably soon be able to fix them yourself :slight_smile:

Emil

路路路

symphorien.wanko-tchuente@ulp.u-strasbg.fr wrote:

* I've seen that you have added comments to existing methods and fields
in classes that you have modified. This is awesome! :slight_smile:

* MediaService.java

-- You should generally avoid using the Hashtable class as a type
specifier, and go for Map instead. This is especially useful in places
like the service package that are supposed to be very implementation
friendly.

* RtpFlowImpl.java

-- You were catching the media exception and re-throwing it as an
IllegalStateException (any particular reason?). I removed the catch
since the method is declared to throw a MediaException. And besides the
IllegalArgumentException() constructor that takes an exception parameter
only exists in J1.5 and we're not there yet.

It was a mistake.

-- Set default values (null for objects, -1 for ints) on class fields.
This is more by convention rather than anything else.

-- Right now you don't seem to be doing anything to handle incoming
video flows. Yet it would be an easy hack and you could mostly reuse
code from CallSessionImpl. Care to give it a try?

Will look it closer.

* ProtocolProviderServiceJabberImpl.java

-- I've seen that you have left some of the feature declarations as
comments unsure of whether or not they had a corresponding XEP. It might
be a good idea to start a discussion on this mailing list in case anyone
would know.

In fact the xmpp.org and jabber.org sites gone down when I was
checking xep(s) last week. They are online again, I will open a new
thread about this.

-- Replaced calls to String.contains() (@since 1.5) with
String.indexOf()!= -1

-- Remove class import DiscoveryInfo.* (@since 1.5) and replace Identity
usages with DiscoveryInfo.Identity

* CallParticipantJabberImpl.java

-- In the [set|get]DisplayName() methods you don't really seem to be
using Jabber display names. Could you please fix this ?

Will be done by the end of the week.

That's it.

Thanks again, Sympho!

I have a lot fun working on sc. Thanks to you and all others.

++

Emil

Sympho wrote:

Hi Emil,

This is a new jingle patch which includes all previous (and only jingle
related) modifications.

The only client I found to make test with was spark and it worked, we
can issue/answer
calls to/from spark.

There are known issues and we have talked of some off list, I will
repeat here for record.

- first, there is a little bug in smakx-jingle lib which do not signals
when a call has been ended
if the remote user hangup before we answer.

- currently, I do not probe if a remote user support telephony feature
before trying to call him. Doing
this, I do not violate the spec wich says we have to respond to
discovery request by advertising
features wich we support but, do not impose to query if a feature is
supported before trying to use it
with another client. I could add a check in the future as it can bring
some "failsafe" capacity
to the implementation.

- the jingle lib provides several TransportManager with some of them
able to handle nat traversal
I have used the ICETransportManager whih is not "fully functional"
because it is currently created
with an invalid stun server (null).

Well, as always this patch isn't the last and there is still pending work :slight_smile:

regards.

Sympho a 锟絚rit :

Hi Emil,

Emil Ivov a 锟絚rit :

Hi Sympho,

Sympho wrote:

Hi Emil,

I had changed some methods name's in the RtpFlow interface
and added publics get.

Cool! Thanks

I also want to notice you that the implementation file isn't the
same verbatim compared to the one I provided in the previous patch.

That's true. The thing is that I've started reviewing your patch and I
am making modifications along the way. I am preparing a mail that would
explain all this but wanted to finish the review and have all
modifications made before sending it.

I see. I have just found bizzare to see the interface alone in the
repo., (without the implementation). I have thought that you have
choosen to make it proprietary because it was too wonderfull, so we
may sell it to others and earn a lot of money :o))))) I will quick
call the operator to cancel my trip to hawai :slight_smile:

I have for example renamed the [start|stop]Transmission() methods to
start() and stop() simply because (and this was confirmed by your
comments) they are also starting reception.

All right.

Concerning the get methods, could you please also send the corresponding
implementation so that I could also commit the impl part of your
contribution?

See attached.

++

Thanks
Emil

Is it anyone else working on an implementation ?

Regards.

Sympho a 锟絚rit :

Sorry, the zip file was missing :o)

Sympho a 锟絚rit :

Hi all,

For those interested in making call from jabber with sc.
The accompanying files will permit, I hope, to do that.

Of course, the actual code, even if usable, is very far away from
be 100% OK
(and I think it will never be, perfection isn't for this world :slight_smile:
). I am "actively working" for
getting things better.

The telephony support for jabber is backed by the jingle lib,
which handle all
the ugly work, so, there wasn't so much thing to do :slight_smile: . For
integration with
SC, I have literally copied the code from the counterpart in sip
impl,
adding some lines when needed. Thanks Emil and others :slight_smile:

The most difficult point was the access to media. As the actual
media service
api was primarly designed for sc, it was a bitte dificult to use
it as is. Thus, I have made
a little door to gain access to the media service as I have said
in a previous mail.

This patch includes the previous, related to RtpFlow.

regards.

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

------------------------------------------------------------------------

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

------------------------------------------------------------------------

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

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

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