Dev Info: Revised Codecs/ Implementation
I noticed that the various files in codecs/codec_*.c contain a large amount of replicated code in the newpvt, framein, frameout callbacks, buffer definitions and so on.
Additionally there are several bugs in there, from null pointer dereferences (e.g. on malloc failures, the code does check, resets the pointer to NULL and then proceeds to use it as if it were good), to (less severe but terribly confusing) comments that have nothing to do with the code that follows (they refer to the file used as a
template).
I have committed in:
http://svn.digium.com/view/asterisk/
team/rizzo/base/
a revised implementation of the codecs interface, where most of the
common functions are moved to translators.c, so the individual
codecs can just use the generic functions in most cases.
Please read the comments in
include/asterisk/translators.h
that describe the architecture (hopefully it is clear enough; if not, ask).
The results are very interesting - codec_*.c reduced from ~5000 to ~3600 lines, and the code is very consistent now.
One area that can still be improved a lot is the generation of 'sample' frames for each codec. Right now, except one or two cases, those frames are just chunks of silence of various lengths, which is not the best input to evaluate a codec's performance (used when building the translation matrix).
I would suggest to move to a slightly different approach where the input is the same for all - a piece of slin data - and we do a first pass using the slin-to-FOOtranslator to generate a frame in format FOO, and then use these frames as input for the actual evaluation. This would remove the need for a 'sample()' callback from all codecs that can do the slin-to-FOO translation, requiring them only for
those (none at the moment) that do not support direct or indirect translation from slin.
Testing and feedback welcome.
cheers
luigi
Additionally there are several bugs in there, from null pointer dereferences (e.g. on malloc failures, the code does check, resets the pointer to NULL and then proceeds to use it as if it were good), to (less severe but terribly confusing) comments that have nothing to do with the code that follows (they refer to the file used as a
template).
I have committed in:
http://svn.digium.com/view/asterisk/
team/rizzo/base/
a revised implementation of the codecs interface, where most of the
common functions are moved to translators.c, so the individual
codecs can just use the generic functions in most cases.
Please read the comments in
include/asterisk/translators.h
that describe the architecture (hopefully it is clear enough; if not, ask).
The results are very interesting - codec_*.c reduced from ~5000 to ~3600 lines, and the code is very consistent now.
One area that can still be improved a lot is the generation of 'sample' frames for each codec. Right now, except one or two cases, those frames are just chunks of silence of various lengths, which is not the best input to evaluate a codec's performance (used when building the translation matrix).
I would suggest to move to a slightly different approach where the input is the same for all - a piece of slin data - and we do a first pass using the slin-to-FOOtranslator to generate a frame in format FOO, and then use these frames as input for the actual evaluation. This would remove the need for a 'sample()' callback from all codecs that can do the slin-to-FOO translation, requiring them only for
those (none at the moment) that do not support direct or indirect translation from slin.
Testing and feedback welcome.
cheers
luigi
posted by Dal at
11:15 AM
