[Nice] Compatibility with MS Office Communicator 2007 R2

[Jakub Adam]

Hi Youness,

thanks for your comments on MSOC compatibility patch and sorry for my late
reply. I'm attaching new version of the patch, reflecting most of the issues
you complained about.

> - Does OC2007 also have the same bug in its CRC32 function as WLM2009 ?

Yes, I made few tests and when the WLM2009 fix is not active, 'bad fingerprint'
errors appear in debug log. This supports your theory that MSOC uses the very
same library as WLM.

> - Why not name the compatibility OC2007? Is that 'R2' meaningful in any way?
> What if there's an R3 release that is also compatible with this (and probably
> will be for interop) ?

I think name OC2007R2 is meaningful, because OC2007 (release 1) uses ICEv6
(IETFDRAFT-ICENAT-06) which is an older version of ICE draft, that, as far as
I know, is not supported by libnice. Please correct me if I'm wrong.

> - Is there some changes required with regards to TURN? Do you know what
> specification is supported? Right now, you seem to let it use draft9.

For now our plugin does not support TURN. MSOC should use Office Communications
Server component called A/V Edge Server that, among other things, implements
TURN server. I was not successful in deploying this component and testing
the compatibility yet, also after looking in the available documentation it
seems we will need to implement some nonstandard authentication mechanism
(in SIPE plugin, not in libnice).

In the code I changed TURN compatibility to STUN_USAGE_TURN_COMPATIBILITY_MSN, as it
is more likely MSOC uses this dialect and chance is higher no other change in libnice
will be required after we implement all needed parts into our plugin.

> - In agent.h, the gtk-doc for nice_agent_set_software might need updating since
> I believe the OC2007R2 compatibility also is a valid one for that API.

Seems it is valid, added.

> - In agent.h, you forgot to add the gtk-doc for the COMPATIBLITY_OC2007R2

Done.

> - In stunmessage.h, you forgot to add the gtk-doc line for
> STUN_ATTRIBUTE_CANDIDATE_IDENTIFIER

added

> - I don't like the fact that you break the API by adding the
> candidate_identifier argument to stun_usage_ice_connecheck_create... I'd like to
> see some other way to do it without breaking the API, although the only solution
> I see for now is to duplicate the function, which I don't really like either...
> This will break the API, but I'm afraid we don't really have a choice. Let me
> know if you have a better idea!

The doc on stun_usage_ice_conncheck_create is saying that there already are
some optional arguments (used only with STUN_USAGE_ICE_COMPATIBILITY_RFC5245), so
adding another one seems logical, of course not ideal solution. With current
implementation it seems API has to be broken anytime some compatibility mode adds
a new argument. Idea that came to my mind is to use a (NULL-terminated) array of GParameter
(http://library.gnome.org/devel/gobject/stable/gobject-The-Base-Object-Type.html#GParameter)
to pass the optional arguments, so the function would change to

size_t
stun_usage_ice_conncheck_create (StunAgent *agent, StunMessage *msg,
     uint8_t *buffer, size_t buffer_len,
     GParameter* params, StunUsageIceCompatibility compatibility);

this solution would allow adding another parameters without more API breakages.

Maybe this should be a separate patch.

Please tell me your opinion, for now I'm leaving the current implementation as it is.

> - In ice.h, you didn't add the gtk-doc for the new
> STUN_USAGE_ICE_COMPATIBILITY_OC2007R2

done

> - In ice.h, you didn't update the gtk-doc for the
> stun_usage_ice_conncheck_create prototype argument change.

done

BR

Jakub
-------------- next part --------------
A non-text attachment was scrubbed...
Name: libnice-Compatibility-with-OC2007-R2.patch
Type: text/x-patch
Size: 8711 bytes
Desc: not available
URL: <http://lists.freedesktop.org/archives/nice/attachments/20100823/3617e535/attachment.bin>