[Nice] New API and refactor

[Youness Alaoui]

Rémi Denis-Courmont wrote:
> Le Tuesday 24 June 2008 00:09:39 ext Youness Alaoui, vous avez écrit :
>   
>>> The STUN-ICE usage is a corner case. It is of course ICE-specific. We put
>>> it to the STUN part because it had a lot of common code with the binding
>>> discovery, and because we did not want to expose (as far as possible) the
>>> ICE agent to the STUN messages.
>>>       
>> yeah, it's ICE specific and should be in libnice, at least, that's what
>> I think.
>> To summarize what I just said :
>> - the usages are just "how to use libstun" and should not be part of
>> libstun itself.
>>     
>
> No. Binding discovery is part of STUN. So are the timers. You could run ICE 
> with any other binding discovery scheme (TURN, Teredo server, UPnP IGD, 
> NAT-PMP, SOCKS...). This is part of STUN. Both in the spec, and in the 
> architecture. The timers are part of STUN. They simply are.
>   
Yes, that's true, but those timers only apply for UDP, not for TCP or TLS.
I wouldn't mind having those timers implementation as utility functions 
for the library.
> ICE has its own timers, for connectivity probing. They are _different_ from 
> the STUN ones.
>
> I'm totally fine splitting the low-layer (message format) and mid-layer 
> (usages) of STUN into different libraries. But I really think the mid-layer 
> does not belong in ICE. Besides, the ICE agent code is already complicated 
> enough. And if we implement another STUN usage, for instance TURN, we would 
> end up having to copy all this STUN transaction code. Bad idea.
>   
I would like to separate them as two libraries.. stun-format and 
stun-usage, or something like that. Anyways, we're not implementing the 
whole STUN specs as far as I know (I don't see the TCP or TLS 
implementation, or the DNS discovery). Maybe it will be better once 
refactored, but the problem now is that it's not very extensible and 
does too much stuff that it shouldn't be doing, so if we want to keep 
those timers working even for TLS sockets, it needs to be refactored.
>   
>> - whoever wants to do binding requests, would only need a few lines of
>> code, so no need for the bind usage
>>     
>
> That's not true. You would need to reimplement the timers (tens of lines of 
> code..), the legacy STUN rollover, the error cases, etc. All in all, you end 
> up with a few hundreds of lines.
>
> Compare the amount of STUN code in the command line STUN client, with the 
> amount of STUN code in what you want to put out of libstun...
>   
Yes, adding timers and error cases, etc.. will add code, but the purpose 
of the library is to make it easy to do STUN, and not "everything in one 
function". It would be nice if glib had a one function "g_do_voip"... no 
it doesn't, but it gives a lot of small functions that makes it easier 
to build something to do that. Or a more realistic example, farsight 
doesn't have a 'farsight_start_audio(ip, port)' function... That's just 
the way I see it.

As I said, I understand your point of view, and I agree with most of 
what you said. The best solution would probably be somewhere in the 
middle. I will refactor the bind usage into something less 'obscure', 
and I'll make the timers as a utility function, so anyone can use them 
for any usage they want (it's not bind usage specific, it's 'sending 
over UDP' specific). I'll also try to make a simple API out of the bind 
usage, something like :

stun_usage_bind_create(agent, msg, buffer, buffer_len);
stun_usage_bind_parse(agent, msg, &addr);

Right now, the bind usage has its own agent in there, it does some 
mallocs, creates sockets, sends data, etc.. We need to keep it simple, 
no malloc, no socket mess, no agent created by the usage itself (I have 
one agent in ice, and another one in the bind usage, and they don't 
share the same list of sent IDs, and one can't validate a response to a 
request sent from the other, so it's confusing).

Is that all ok with you ?

Thanks for helping,
Youness.