[Nice] complete example

Bryce Allen ballen at ci.uchicago.edu
Mon Feb 4 15:52:06 PST 2013 

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'.

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.

-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
> 
> 
-------------- 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/1b5c1d08/attachment.pgp>

More information about the nice mailing list