[Nice] Compatibility with MS Office Communicator 2007 R2

[Youness Alaoui]

Hi Jakub,

Thanks for using the mailing list, I wouldn't have seen your patch otherwise.
I've quickly reviewed your patch and I have a few comments.
First, thanks for sending the patch, it's appreciated and I'm happy to see even
more compatibility modes added into libnice.

Now the thing is that, in theory, WLM 2009 is supposed to be using the same
library as MS Office Communicator, so it should technically be the same
compatibiilty mode. However, I don't think it had the CANDIDATE-IDENTIFIER
attribute added to the STUN messages. So maybe there is a slight variation, but
I will assume that everything else is basically the same.

So, here are my comments :
- Does OC2007 also have the same bug in its CRC32 function as WLM2009 ?
- 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) ?
- 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.
- 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.
- In agent.h, you forgot to add the gtk-doc for the COMPATIBLITY_OC2007R2
- In stunmessage.h, you forgot to add the gtk-doc line for
STUN_ATTRIBUTE_CANDIDATE_IDENTIFIER
- 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!
- In ice.h, you didn't add the gtk-doc for the new
STUN_USAGE_ICE_COMPATIBILITY_OC2007R2
- In ice.h, you didn't update the gtk-doc for the
stun_usage_ice_conncheck_create prototype argument change.

That's about it. Good job!
Let me know when you get a new patch for me to review!

Thanks again for your contribution,
Youness.


On 08/13/2010 05:03 PM, Jakub Adam wrote:
> Hi,
> 
> some time ago I submitted a patch adding support for MS Office
> Communicator 2007 to libnice
> https://bugs.freedesktop.org/show_bug.cgi?id=28215 .
> 
> As we are planning a new release of SIPE libpurple plugin
> (http://sipe.sourceforge.net) with voice and
> video support, we would like to ask you to review the patch and give
> feedback if there is possibility
> to add this code into next release of libnice, or how we should improve
> it to be acceptable for you.
> 
> This patch is mandatory to make ICE work with MSOC clients, details are
> provided in the bug report.
> 
> Thanks,
> Jakub
> 
> _______________________________________________
> Nice mailing list
> Nice at lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/nice


-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 262 bytes
Desc: OpenPGP digital signature
URL: <http://lists.freedesktop.org/archives/nice/attachments/20100816/0e3565c7/attachment.pgp>