[Nice] New API and refactor

[Kai.Vehmanen at nokia.com]

Hi,

On 07 May 2008,  Olivier Crête wrote:
>> well, could you elaborate why the code needs major refactoring? 
>
>Youness tried to implement Google Talk compatibility. He has 
>had a very hard time to do it. 

ok, thanks, that makes more sense. I do understand the pain
with GTalk-ICE. As GTalk-ICE is basicly a fork of an old draft of 
the ICE standard, the differences are probably subtle and 
hard to track. Also, Rémi and me didn't know the GTalk-ICE 
variant well enough when writing the fullmode code, so there probably 
are many places around the codebase that need modification
for GTalk-ICE.

But taking a more optimistic view, now we finally have just
two variants (IETF/XMPP-ICE and GTalk), while before there were
many, many variants in more-or-less widespread use. Another
positive aspect is that the two share a common ancestor, so 
some code can be shared.

>compat modes to the agent. Google mode has some subtle 
>differences (remotelocal instead of remote:local). Google Talk 
>fails if "username" is not the only attribute in the message. 

Now as there are only two major variants, would it make sense
implement GTalk-STUN as a separate usage (in a different 
c+h file under nice/stun)...? This won't result in maximal
code reuse, but might in the end result in lesser loss of
mental health for the developers involved. ;)

>So you have to pass the compatibility enum everywhere inside 
>stun, because there is no separation between the stun message 
>bulding/parsing and the behavior and the ICE-specific stuff. 

... yep, and the code becomes hard to follow. I'd vote for
separating them more cleanly.

>The stun part adds fingerprint, ice-controlling, 
>use-candidate, etc.. instead of separating that.

As Rémi already noted, these are part of the ICE-usage, imlemented
by the nice/stun module.

>> Like the current STUN implementation? ;)
>
>I fail to see what the "public" api of the STUN 
>implementation. That makes it hard to see what the layers are. 

Please see nice/docs/design.txt which has sections on 
STUN design (layers, different APIs, design goals).

>Also stuff like
>stun_conncheck_start() is defined in both conncheck.h and 
>stun-ice.h (not what I'd call clean...).

Sorry, that's my fault (merge error) -- Rémi is innocent in
this case. :)

>If you using glib, any allocation that it does will use g_malloc(),
>g_malloc() calls abort() if malloc fails. So really, having a 
>requirement to verify memory allocations for any system that 
>uses glib is bollocks.

I'm painfully aware of this. Note that there are some systems on which glib 
does not abort:
http://www.forum.nokia.com.ngrs.net/document/CDL_Extension_S60_3rd_Ed_FP2/GUID-719955DA-415B-420E-9F9B-F6DB37615EC5/html/s60_openc_using_glib8.html
... and we want to support these systems as well. 

We also want to have a nice libnice GObject API, but in ideal world
the lower layers of the library would be low-memory-friendly, so
the user could choose. Checking the return values is groundwork 
for this (code that assumes memory allocs can fail still works 
on systems on which you never run out of memory, but the reverse
is not true).

br,
-- 
first.surname at nokia.com (Kai Vehmanen)