> On June 25, 2014, 6:45 a.m., Joshua Colp wrote: > > /team/group/media_formats-reviewed-trunk/main/format.c, lines 200-202 > > <https://reviewboard.asterisk.org/r/3671/diff/1/?file=60620#file60620line200> > > > > Just curious - does this happen? > > Corey Farrell wrote: > This could happen if someone configures a blank format_cap (disallow=all) > - the first format would be NULL (ast_best_codec). > > Maybe an assert should go here until we get a better handle on things? > If NULL's to this procedure are possible we probably want to check this > second (if both are NULL then they are equal). > > Joshua Colp wrote: > My question becomes: Should this accept NULLs or should the caller see > that something they expected to be non-NULL is NULL and thus should print out > an error/warning/something.
The reason why it is useful to accept NULLs here is explicitly because of res_rtp_asterisk and its silliness. res_rtp_asterisk has to deal both with 'rtp-isms' - things like Cisco specific DTMF - as well Asterisk formats. It stores both of these in the same static array. As such, there are times when it has a format pointer that can be NULL (because the 'format' is Cisco DTMF or something equally silly) but it is running through checks to see if the format is T.140, or G722, or other such things. Having this function gracefully handle NULLs keeps some code in res_rtp_asterisk slightly more sane. Since this is a heavily used function, it's probably good for it to be as 'safe' as possible. The NULL check here is unlikely to be a performance bottleneck. - Matt ----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviewboard.asterisk.org/r/3671/#review12307 ----------------------------------------------------------- On June 25, 2014, 7:14 a.m., Corey Farrell wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > https://reviewboard.asterisk.org/r/3671/ > ----------------------------------------------------------- > > (Updated June 25, 2014, 7:14 a.m.) > > > Review request for Asterisk Developers, Joshua Colp and Matt Jordan. > > > Repository: Asterisk > > > Description > ------- > > This update gives media_formats the ability to receive a call using chan_sip. > Possibly other channel drivers might work, I haven't tried them. > > * ast_format_cap_is_compatible_format needs to be checked against > AST_FORMAT_CMP_NOT_EQUAL, not zero/non-zero. All calls to > ast_format_cap_is_compatible_format were fixed. > * res_rtp_asterisk was updated by Matt Jordan, along with related changes to > codec.c, codec.h, format.c, format.c and codec_builtin.c. > * Switch ast_format_copy from function to macro to ao2_bump. This allows > REF_DEBUG to give better results. > > > Diffs > ----- > > /team/group/media_formats-reviewed-trunk/res/res_speech.c 417190 > /team/group/media_formats-reviewed-trunk/res/res_rtp_asterisk.c 417190 > /team/group/media_formats-reviewed-trunk/main/translate.c 417190 > /team/group/media_formats-reviewed-trunk/main/frame.c 417190 > /team/group/media_formats-reviewed-trunk/main/format.c 417190 > /team/group/media_formats-reviewed-trunk/main/codec_builtin.c 417190 > /team/group/media_formats-reviewed-trunk/main/codec.c 417190 > /team/group/media_formats-reviewed-trunk/main/channel.c 417190 > /team/group/media_formats-reviewed-trunk/main/bridge.c 417190 > /team/group/media_formats-reviewed-trunk/include/asterisk/format.h 417190 > /team/group/media_formats-reviewed-trunk/include/asterisk/codec.h 417190 > /team/group/media_formats-reviewed-trunk/channels/chan_unistim.c 417190 > /team/group/media_formats-reviewed-trunk/channels/chan_skinny.c 417190 > /team/group/media_formats-reviewed-trunk/channels/chan_sip.c 417190 > /team/group/media_formats-reviewed-trunk/channels/chan_pjsip.c 417190 > /team/group/media_formats-reviewed-trunk/channels/chan_oss.c 417190 > /team/group/media_formats-reviewed-trunk/channels/chan_nbs.c 417190 > /team/group/media_formats-reviewed-trunk/channels/chan_motif.c 417190 > /team/group/media_formats-reviewed-trunk/channels/chan_mgcp.c 417190 > /team/group/media_formats-reviewed-trunk/channels/chan_jingle.c 417190 > /team/group/media_formats-reviewed-trunk/channels/chan_h323.c 417190 > /team/group/media_formats-reviewed-trunk/channels/chan_gtalk.c 417190 > /team/group/media_formats-reviewed-trunk/channels/chan_alsa.c 417190 > /team/group/media_formats-reviewed-trunk/addons/chan_ooh323.c 417190 > /team/group/media_formats-reviewed-trunk/addons/chan_mobile.c 417190 > > Diff: https://reviewboard.asterisk.org/r/3671/diff/ > > > Testing > ------- > > Called from Asterisk 11 to a test server with this code, I was able to hear > the 'invalid' message, everything seemed during the call. I received TONS of > ao2 frack's when stopping Asterisk. The sip.conf peer on both Asterisk > servers was setup for disallow=all / allow=ulaw. > > > Thanks, > > Corey Farrell > >
-- _____________________________________________________________________ -- 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
