[Nice] complete example

[Bryce Allen]

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?

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.

-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
> 
> 
-------------- next part --------------
A non-text attachment was scrubbed...
Name: nicetest.c
Type: text/x-csrc
Size: 15497 bytes
Desc: not available
URL: <http://lists.freedesktop.org/archives/nice/attachments/20130204/d66f92c9/attachment.c>
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 490 bytes
Desc: not available
URL: <http://lists.freedesktop.org/archives/nice/attachments/20130204/d66f92c9/attachment.pgp>