Summary: | ASTERISK-03746: [patch] Return of ChanSpy | ||
Reporter: | Anthony Minessale (anthm) | Labels: | |
Date Opened: | 2005-03-22 17:54:59.000-0600 | Date Closed: | 2008-01-15 15:28:26.000-0600 |
Priority: | Major | Regression? | No |
Status: | Closed/Complete | Components: | Applications/General |
Versions: | Frequency of Occurrence | ||
Related Issues: | |||
Environment: | Attachments: | ( 0) app_chanspy.c ( 1) chanspy_rc1.diff ( 2) chanspy_sounds.tgz | |
Description: | ChanSpy is back! (and this time it's personal) </cheesy announcer voice> Here it is at long last! show application ChanSpy -= Info about application 'ChanSpy' =- [Synopsis]: Tap into any type of asterisk channel and listen to audio [Description]: Chanspy([<scanspec>][|<options>]) Valid Options: - q: quiet, don't announce channels beep, etc. - b: bridged, only spy on channels involved in a bridged call. - v([-4..4]): adjust the initial volume. (negative is quieter) - g: enforce group. read ${SPYGROUP} and only match calls also in this group. If <scanspec> is specified, only channel names *beginning* with that string will be scanned. ('all' or an empty string are also both valid <scanspec>) While Spying: Dialing # cycles the volume level. Dialing * will stop spying and look for another channel to spy on. Dialing a series of digits followed by # builds a channel name to append to <scanspec> (e.g. run Chanspy(Agent) and dial 1234# while spying to jump to channel Agent/1234) ****** ADDITIONAL INFORMATION ****** Disclamer on file anthmct@yahoo.com <--valid paypal address *wink* | ||
Comments: | By: Anthony Minessale (anthm) 2005-03-22 18:23:28.000-0600 PS Don't let jets use this patch he promised to fund it's development and has since vansihed. By: Kevin P. Fleming (kpfleming) 2005-03-22 19:06:12.000-0600 You don't need to take a lock around setting a single variable in the chanspy structure (chanspy->status = CHANSPY_DONE); setting a variable like that is always atomic. Would it be better to keep a count of the number of frames currently queued in the ast_channel_spy structure so it doesn't have to be determined each time a new frame is queued? You already have locking in the structure, so it would be safe to increment/decrement as frames are queued/dequeued. --- In channel.c and app_chanspy, some functions have ast_ prefixes that really don't need them: static void ast_spy_detach static void ast_queue_spy_frame static struct ast_frame *ast_spyq_shift static int ast_extract_data static void ast_flush_spy_queue static void ast_start_spying static void ast_stop_spying static int ast_spy_on_chan --- "No more frames ending" doesn't seem to be the proper wording... was that supposed to be "No more frames, spy ending"? Without some more details, it will just look like a generic warning message on the console. --- if (chan->spiers) { for(cptr=chan->spiers;cptr && cptr->next;cptr=cptr->next); cptr->next = spy; There are linked-list macros in linkedlists.h that can do this stuff for you. --- memset(inp,0,24); Please use sizeof() so if the declaration gets changed the code will still work. --- memset(&spy,0,sizeof(struct ast_channel_spy)); could be memset(&spy, 0, sizeof(spy)); --- if ((vol = strchr(options, 'v'))) { vol++; if (vol) { volfactor=atoi(vol); } else volfactor=1; Uhh... no. I think you mean 'if (*vol)', since vol will definitely be non-zero here. --- if (volfactor == 6) volfactor = 8; If the user specifies 'v3', they get 'v4'? --- if (! args || ast_strlen_zero(args) || strchr(options, 's')) { args cannot be NULL here. You already checked all the options in the code above this, but not 's'... This loop only runs if the args string is empty or if 's' has been specified. --- if (!spec || ((spec && strlen(spec) < strlen(peer->name) && !strncasecmp(peer->name, spec, strlen(spec))))) { if (peer && (!bronly || (bronly && ast_bridged_channel(peer))) && !ast_check_hangup(peer) && !ast_test_flag(peer, AST_FLAG_SPYING)) { Both of these if statements check a boolean variable for one value and then for its opposite. --- Do we need all these messages with no real content every time ChanSpy is invoked? ast_log(LOG_WARNING, "Done\n"); ast_log(LOG_WARNING, "Stopping Native Bridge\n"); ast_log(LOG_WARNING, "I have nothing to do.\n"); By: Anthony Minessale (anthm) 2005-03-22 19:55:04.000-0600 Updated with some of kpflemming's findings cept: I count the frames cos I have to get to the end of the list anyway so it's not too big a deal esp since most of the time it's gonna be like 1 frame long cos it's virtually a realtime audio transfer. By: Kevin P. Fleming (kpfleming) 2005-03-22 19:57:45.000-0600 Yeah, I figured that would be the case (and the fact that you need the pointer to the end anyway). By: Kevin P. Fleming (kpfleming) 2005-03-22 20:04:04.000-0600 What is the value of ast_extract_data, as opposed to: memcpy(buf, fr->data, fr->datalen * sizeof(short)) OK, I really don't understand chanspy_exec at all... If the user doesn't specify the 's' option, it doesn't seem to do anything at all! I think you are intending to turn the 'scan' flag on if a spec has been provided, but I'm not sure. By: Anthony Minessale (anthm) 2005-03-22 22:09:30.000-0600 Yep spec implies s which was somehow lost in the reshuffle. fixed Esentally ChanSpy(s) or ChanSpy(sb|Agent) lets you spy on bridged agents also allowing you to dial #'s eg dial 1234# and you jump to Agent/1234 (same applies to any channel type like SIP/XXXX) By: Anthony Minessale (anthm) 2005-03-23 10:09:28.000-0600 Updated the patch and the flow of the app again, it's undergone so many changes it started to not make sense in certian syntaxes. also formatted it the best i can to make kram happy By: Kevin P. Fleming (kpfleming) 2005-03-23 10:25:32.000-0600 Not that it matters here, but just for future reference... There is no value in using a 'char' for status in struct ast_channel_spy, since it is surrounded by word-size variables (that require word alignment) it will end up taking 4 bytes anyway. Ideally, you'd do this: enum chanspy_states { CHANSPY_NEW = 0, CHANSPY_RUNNING = 1, CHANSPY_DONE = 2 }; struct ast_channel_spy { ... enum chanspy_states status; ... }; This will still require an int, but now it's obvious that is a limited set of valid values. By: Anthony Minessale (anthm) 2005-03-23 11:00:56.000-0600 I like enum but nothing else uses it so I didnt want to break protocol maybe a patch to make everything into enums should emerge. By: Kevin P. Fleming (kpfleming) 2005-03-23 12:13:33.000-0600 Well, I used them to good effect in the QUEUESTATUS patch, so that's a start :-) By: Mark Spencer (markster) 2005-03-23 19:25:18.000-0600 Added to CVS, thanks! By: Digium Subversion (svnbot) 2008-01-15 15:28:26.000-0600 Repository: asterisk Revision: 5242 U trunk/apps/Makefile A trunk/apps/app_chanspy.c ------------------------------------------------------------------------ r5242 | markster | 2008-01-15 15:28:26 -0600 (Tue, 15 Jan 2008) | 2 lines Add chanspy (bug ASTERISK-3746) ------------------------------------------------------------------------ http://svn.digium.com/view/asterisk?view=rev&revision=5242 |