[Home]

Summary:ASTERISK-04602: [patch] + new files Fix ChanSpy
Reporter:Anthony Minessale (anthm)Labels:
Date Opened:2005-07-15 14:26:08Date Closed:2008-01-15 15:42:06.000-0600
Priority:MinorRegression?No
Status:Closed/CompleteComponents:Core/General
Versions:Frequency of
Occurrence
Related
Issues:
Environment:Attachments:( 0) chanspy_rev0.diff
( 1) sfmake.diff
( 2) slinfactory.c
( 3) slinfactory.c_rev1
( 4) slinfactory.h
( 5) slinfactory.h_rev1
Description:There are some issues in chanspy when channels of different codecs are involved.

It uses a new object called slinfactory which is a machine designed to accept frames and store them allowing you to squeeze out an arbitrary number of slin bytes like it was a fd.

DOCUMENTATION

//declare it static
struct ast_slinfactory factory;
//set it up (no malloc or anything just memset)
ast_slinfactory_init(&factory);

//let's pretend get_frames() will give us like 100 gsm frames before returning NULL
while((f = get_frames()) {
//feed the data to the machine
ast_slinfactory_feed(&factory, f);
ast_frfree(f);
}

// suppose you have short buf[1280] and int len handy

len = ast_slinfactory_read(&factory, buf, 640);

//you have now read len bytes into buf and you can keep doing it till
you run out of data or you can continue to fuel the machine and get more

//clear the data if there was any left and kill the translator if it was needed.
ast_slinfactory_destroy(&factory);



****** ADDITIONAL INFORMATION ******

Disclaimer On File
anthmct@yahoo.com

This is not an advertisement for ClueCon, These are not the droids you're looking for, Move Along!
Comments:By: Kevin P. Fleming (kpfleming) 2005-07-15 17:05:55

Code review notes:

- There is no need to initialize variables in functions when you are going to immediately write over them, or to write NULL into a struct member when you have just memset() the entire structure to zeros. Both of these generate extra object code (and data) with no value :-)

- I think you need to clear out sf->trans here:

if (sf->trans && f->subclass != sf->format) {
ast_translator_free_path(sf->trans);
}
if (!sf->trans) {

- Why not if (!f) return -1; at the beginning of ast_slinfactory_feed()?

- Formatting failure :-)

int ast_slinfactory_read(struct ast_slinfactory *sf, short *buf, size_t bytes) {

- Since frame_data is a 'void *', I believe this is non-portable (not every platform will increment by the same amount){same goes for offset}:

frame_data += ineed;

Otherwise this looks really cool, and a very elegant solution :-)
By: Anthony Minessale (anthm) 2005-07-16 05:49:11

ok,

To address some of your points

"- Why not if (!f) return -1; at the beginning of ast_slinfactory_feed()?"

-ok, I'll do it =D I just tend to have a habbit of trying to design my functions to never return in more than 1 place but s'ok it's not a rule in stone or anything.


"- There is no need to initialize variables in functions when you are going to immediately write over them"
-yes, you're probably right, I'm just scarred from the 1 time I don't init something and I am going mad trying to figure out why perfect code seems to not work ;) so out of paranoia I tend to always init everything maybe it'd be a good policy to make optimization passes afterwards and remove any unnecessary inits once you know you won't be touching the code for a while.

"- Since frame_data is a 'void *', I believe this is non-portable (not every platform will increment by the same amount){same goes for offset}:"
-In that case there is an offset pointer in the struct too so I changed them all to short pointers and used sizeof to calculate the increment does that seem sane?



as for the rest of the issues I think I addressed them all unless I still have anymore inits i didn't catch and yes I put the <CR> after the func proto => 1/4 ain't bad?

*hint*
As soon as this slinfactory is adopted I have another cool use for it on deck.
By: Kevin P. Fleming (kpfleming) 2005-07-19 20:46:40

Committed to CVS HEAD with a minor mod to suppress compiler warnings about unused functions. Thanks!
By: Digium Subversion (svnbot) 2008-01-15 15:42:06.000-0600

Repository: asterisk
Revision: 6177

U   trunk/Makefile
U   trunk/apps/app_chanspy.c
A   trunk/include/asterisk/slinfactory.h
A   trunk/slinfactory.c

------------------------------------------------------------------------
r6177 | kpfleming | 2008-01-15 15:42:06 -0600 (Tue, 15 Jan 2008) | 2 lines

add slinfactory object, and change app_chanspy to use it (bug ASTERISK-4602)

------------------------------------------------------------------------

http://svn.digium.com/view/asterisk?view=rev&revision=6177