[Nice] complete example

Youness Alaoui youness.alaoui at collabora.co.uk
Mon Feb 4 17:18:53 PST 2013 

On 02/04/2013 06:52 PM, Bryce Allen wrote:
> I'm glad we could make a useful contribution! Libnice is saving us tons
> of effort trying to implement ICE ourselves.

That's why I wrote it (well, that, and because I was told to :p)!
And yeah, ICE is a beast, not so easy to implement correctly.

> Found another doc issue: Ctrl-D no longer works to quit when in
> send/recv mode. Could just change the text to 'Ctrl-C to quit'.
Even better, I made it quit when Ctrl-D is pressed :)

> 
> Neither version shows how to detect disconnects - if you close one end,
> the other end continues to accept input and try to send. Would be nice
> to add that to the examples, if it's within scope of the ICE layer.
Humm.. technically, there is no mechanism for that. But I was sure it would work
anyways because there's a keepalive every 30 seconds and if we don't get a
response for it (and there was no data received in those 30 seconds), we
consider the connection to have died... it doesn't actually work.. I checked the
code and I realized that only happens when you use GOOGLE compatibility mode.
When you use RFC mode, the keepailve is a binding indication, not a binding
request, so there is no response to be received.
Anyways, I added to the example of simple way of doing that, when one exits
(with Ctrl-D), it will send a "\0" before quitting, and when we receive "\0", we
quit.
http://cgit.collabora.com/git/user/kakaroto/libnice.git/commit/?id=818427d0b9e6d34d6ceb05cb73c1bf41446c9050


> 
> -Bryce
> 
> On Mon, 04 Feb 2013 15:32:28 -0500
> Youness Alaoui <youness.alaoui at collabora.co.uk> wrote:
>> On 02/04/2013 03:10 PM, Bryce Allen wrote:
>>> I'm all for simplifying the licensing and I have permission, so
>>> here is a copy of nicetest.c licensed under MPL/LGPL just like the
>>> rest of libnice. I think that is enough so you can change the
>>> license on your derived version?
>> Yes, I believe that's enough. Thanks a lot.
>>
>>>
>>> The async version looks good, the only thing I noticed was the
>>> doc comment needs to be changed with the new source/file and program
>>> name. I'm in the process of testing it now on different hosts, I'll
>>> let you know if I run into any issues.
>> Good point, I forgot I renamed the file! I will change that as well
>> as the license and commit the new version.
>> Let me know if you find any bugs in my modified version.
>>
>>>
>>> -Bryce
>>>
>>>
>>> On Fri, 01 Feb 2013 19:35:53 -0500
>>> Youness Alaoui <youness.alaoui at collabora.co.uk> wrote:
>>>> Thanks!
>>>> I've finished cleaning up the code and integrated it into libnice.
>>>> I've pushed the changes here :
>>>> http://cgit.collabora.com/git/user/kakaroto/libnice.git/log/
>>>> First of all, I added a nice_agent_get_selected_pair because I
>>>> think it's much more efficient and logical to have that than being
>>>> forced to listen to the signal and keep track of the foundations.
>>>> I think I will also add a way to get the state without storing it
>>>> from the state-changed signal handler.
>>>> I think it would be a good idea to make the license match the rest
>>>> of the code from libnice. If you don't want to, then it's not a
>>>> big deal. So.. here's the changelog :
>>>> - Better to use '-4 -t A' options to 'host' command because it
>>>> seems to freeze for a long time otherwise. Added that to the
>>>> comment.
>>>> - include <agent.h> instead of <nice/agent.h>, since
>>>> $includedir/nice is added to CFLAGS with pkg-config --cflags. (and
>>>> it caused it to take the agent.h from my installation rather than
>>>> the local version)
>>>> - I used a table for transforming the candidate_type and state into
>>>> strings, much easier, although less robust, but since the data
>>>> comes from libnice, it's expected to be correct.
>>>> - Reordered the functions to be more linear/sequential.. the main
>>>> function at the top, then followed by whichever function gets
>>>> executed next.
>>>> - Removed the thread, and used the mainloop directly and make the
>>>> code work asynchronously in the callbacks.
>>>> - stun address is now optional, and I check validity of the
>>>> controlling argument.
>>>> - I use a g_io_channel for stdin and listen asynchronously to data
>>>> written on stdin.
>>>> - I use nice_agent_get_selected_pair to print the ip/port used, and
>>>> do it on the state-changed signal.
>>>> - I exit the app in case there's a failure to connect
>>>> - I use g_strsplit_set instead of parse_args (which I removed).
>>>> - I use g_strsplit instead of sscanf for parsing the candidate
>>>> - I loop through the candidate_type_name instead of doing an
>>>> if/else of each possibility (when parsing the candidate)
>>>> - I prefered using if (!function) rather than 'ok = function(); if
>>>> (!ok)'.. it's a personal choice though.
>>>> - Other small coding style changes to better match libnice's coding
>>>> style (or what it should be), like indentation, \n{ in a function
>>>> declaration, etc..
>>>>
>>>> If you could review the code I wrote and let me know if there's
>>>> anything you think needs fixing, let me know.
>>>>
>>>> Thanks!
>>>> Youness.
>>>>
>>>>
>>>>
>>>> On 02/01/2013 04:28 PM, Bryce Allen wrote:
>>>>> Patch attached. Found a bug in print_local_data when there is more
>>>>> than one local candidate (wasn't printing spaces in between). Also
>>>>> replaced the busy waiting with condition variables.
>>>>>
>>>>> -Bryce
>>>>>
>>>>> On Fri, 1 Feb 2013 11:24:32 -0600
>>>>> Bryce Allen <ballen at ci.uchicago.edu> wrote:
>>>>>> Hi Rohan,
>>>>>>
>>>>>> Thanks for testing it out and for the feedback!
>>>>>>
>>>>>> Re parse_args, I think it is correct as is. The line returned by
>>>>>> fgets may not contains '\n' (if it's very long), so it's not safe
>>>>>> to only check for '\n' as end of string, and the !isspace(*p)
>>>>>> already handles the '\n' as well as other whitespace to make sure
>>>>>> it won't be included in the arg:
>>>>>>
>>>>>>         // find end of current arg
>>>>>>         while (*p != '\0' && !isspace(*p)) { p++; }
>>>>>>         if (*p == '\0')
>>>>>>             break;
>>>>>>         *p = '\0';
>>>>>>
>>>>>> Were you getting a crash or a hang? It works for me when I copy
>>>>>> just the single line, but if I include extra empty lines it just
>>>>>> hangs. Also if your terminal wraps the input line, it will fail.
>>>>>> It would be nice to have a less fragile format - I think that is
>>>>>> what Youness was suggesting with the SDP format. I've attached a
>>>>>> new version that handles empty lines and re-prompts if the parse
>>>>>> fails, and fixes a memory leak in parse_remote_data.
>>>>>>
>>>>>> Re removing the thread for mainloop, I'm not sure the STUN
>>>>>> binding/keep alive requests will continue if the thread is quit
>>>>>> after negotiation is complete (during the send/receive phase). It
>>>>>> seems nice to manually drive the loop for tests and avoid the
>>>>>> complexity of threads, but I'm not sure it's what we want for an
>>>>>> example.
>>>>>>
>>>>>> -Bryce
>>>>>>
>>>>>>
>>>>>> On Fri, 1 Feb 2013 12:37:43 +0530
>>>>>> Rohan Garg <rohan16garg at gmail.com> wrote:
>>>>>>> Hey everyone :)
>>>>>>> I went over the program a bit, and I'm attaching a patch that
>>>>>>> fixes a couple of issues, just minor stuff regarding the
>>>>>>> thread_mainloop and making the code a wee bit more async.
>>>>>>>
>>>>>>> There also seems to be a bug on line 241 of the original code
>>>>>>> where it only stops at \0 , I believe that should be \n. When
>>>>>>> using \0 the program simply crashes at " remote_candidates =
>>>>>>> g_slist_reverse(remote_candidates); " in parse_remote_data.
>>>>>>>
>>>>>>>
>>>>>>> On Fri, Feb 1, 2013 at 6:05 AM, Youness Alaoui
>>>>>>>> I have a plan of adding a new API call for transforming the
>>>>>>>> candidates into SDP and to parse candidates from SDP. I think
>>>>>>>> it will help people, and I think it would be a good idea to
>>>>>>>> port your code to it. I'll let you know tomorrow what I find!
>>>>>>>>
>>>>>>> Could you explain this a bit on the list in another thread? :)
>>>>>>>
>>>>>>> Regards
>>>>>>> Rohan Garg
>>>>>>>
>>>>>>>> Thanks,
>>>>>>>> Youness.
>>>>>>>>
>>>>>>>>
>>>>>>>> On 01/31/2013 07:30 PM, Bryce Allen wrote:
>>>>>>>>> I expanded my nicetest.c into a complete example, using a
>>>>>>>>> simple serialization format and copying/pasting the data
>>>>>>>>> between clients. It's inspired by icedemo from pjnath, but I
>>>>>>>>> didn't find having separate commands for each phase useful
>>>>>>>>> for testing and it made reading the code more difficult, so I
>>>>>>>>> just have three phases:
>>>>>>>>>
>>>>>>>>> 1) gather candidates, print serialized local data
>>>>>>>>> 2) prompt user for remote data, parse and set when entered
>>>>>>>>> 3) if successful, prompt for data to send to remote end
>>>>>>>>>
>>>>>>>>> I'm probably missing some failure cases and some error
>>>>>>>>> checking, but it seems like it could be a useful addition to
>>>>>>>>> the main distribution after a fixup. I can re-license under
>>>>>>>>> LGPL/MPL if needed.
>>>>>>>>>
>>>>>>>>> Also please let me know if I'm doing anything fundamentally
>>>>>>>>> wrong here. I'm using pretty much the same flow for a real
>>>>>>>>> application, with condition variables to synchronize instead
>>>>>>>>> busy waiting.
>>>>>>>>>
>>>>>>>>> Thanks,
>>>>>>>>> Bryce
>>>>>>>>>
>>>>>>>>>
>>>>>>>>>
>>>>>>>>> _______________________________________________
>>>>>>>>> nice mailing list
>>>>>>>>> nice at lists.freedesktop.org
>>>>>>>>> http://lists.freedesktop.org/mailman/listinfo/nice
>>>>>>>>>
>>>>>>>>
>>>>>>>>
>>>>>>>>
>>>>>>>> _______________________________________________
>>>>>>>> nice mailing list
>>>>>>>> nice at lists.freedesktop.org
>>>>>>>> http://lists.freedesktop.org/mailman/listinfo/nice
>>>>>>>>
>>>>>>>>
>>>>>>>>
>>>>>>>> _______________________________________________
>>>>>>>> nice mailing list
>>>>>>>> nice at lists.freedesktop.org
>>>>>>>> http://lists.freedesktop.org/mailman/listinfo/nice
>>>>
>>>>
>>>>
>>>>
>>>> _______________________________________________
>>>> nice mailing list
>>>> nice at lists.freedesktop.org
>>>> http://lists.freedesktop.org/mailman/listinfo/nice
>>
>>
>>
>>
>> _______________________________________________
>> 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: 263 bytes
Desc: OpenPGP digital signature
URL: <http://lists.freedesktop.org/archives/nice/attachments/20130204/dd75b2c6/attachment.pgp>

More information about the nice mailing list