----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviewboard.asterisk.org/r/3512/#review11854 -----------------------------------------------------------
team/group/media_formats-reviewed/include/asterisk/codec.h <https://reviewboard.asterisk.org/r/3512/#comment21732> 1) Document that this function appends the names onto the buf rather than just setting the input buf to the codec names. 2) Returning an ast_str ** is awkward and unnecessary. If you wanted to make this return something, a more natural way might be to do something like: struct ast_str *ast_codec_get_names(const struct ast_codec *codec, struct ast_str *buf); And then require that it gets called like: str = ast_codec_get_names(codec, str); As it is, I'm fine with it just returning void (or possibly a pass/fail int). team/group/media_formats-reviewed/main/codec.c <https://reviewboard.asterisk.org/r/3512/#comment21727> This won't compile since buf is undeclared. I can see two ways of fixing this. 1) Switch to using ao2_callback_data() and pass the buf into this callback. 2) Since you're doing a straight pointer comparison, will the same codec ever exist in the container more than once? If not, then instead of performing an ao2_callback to append codec names to an ast_str, change the ao2_callback to find the single matching codec and perform the string append in ast_codec_get_names(). team/group/media_formats-reviewed/main/translate.c <https://reviewboard.asterisk.org/r/3512/#comment21729> Does a comparison between different sample rates of signed linear codecs return AST_FORMAT_CMP_EQUAL? If not, then this functions differently from the old code. - Mark Michelson On May 8, 2014, 5:16 p.m., Kevin Harwell wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > https://reviewboard.asterisk.org/r/3512/ > ----------------------------------------------------------- > > (Updated May 8, 2014, 5:16 p.m.) > > > Review request for Asterisk Developers. > > > Bugs: SWP-6725 > https://issues.asterisk.org/jira/browse/SWP-6725 > > > Repository: Asterisk > > > Description > ------- > > Updated the translation core (translate.c) to use the new media format API. > > > Diffs > ----- > > team/group/media_formats-reviewed/main/translate.c 413539 > team/group/media_formats-reviewed/main/format.c 413539 > team/group/media_formats-reviewed/main/codec.c 413539 > team/group/media_formats-reviewed/include/asterisk/format.h 413539 > team/group/media_formats-reviewed/include/asterisk/codec.h 413539 > > Diff: https://reviewboard.asterisk.org/r/3512/diff/ > > > Testing > ------- > > > Thanks, > > Kevin Harwell > >
-- _____________________________________________________________________ -- Bandwidth and Colocation Provided by http://www.api-digital.com -- asterisk-dev mailing list To UNSUBSCRIBE or update options visit: http://lists.digium.com/mailman/listinfo/asterisk-dev