[asterisk-dev] [Code Review] Asterisk media architecture conversion - no more format bitfields

David Vossel reviewboard at asterisk.org
Thu Feb 3 09:33:31 CST 2011 


> On 2011-02-02 16:37:08, Terry Wilson wrote:
> > /trunk/include/asterisk/translate.h, line 70
> > <https://reviewboard.asterisk.org/r/1083/diff/3/?file=15408#file15408line70>
> >
> >     What is the reasoning for the specific numbers chosen? Is the number order all that is important or is arithmetic done on them at some point?

For the most part it is the number order that matters, but there is one rule.  Two translation costs in the table can never equal less than the most expensive single cost in the table.  That is why the first two costs are 400000 and 600000 which equals 1000000.  The highest single cost is 985000.  This guarantees that a single translation path will always beat a multi-step translation path no matter what.  Other than that, the exact numbers mean nothing other than one type of translation is preferred over another.


> On 2011-02-02 16:37:08, Terry Wilson wrote:
> > /trunk/include/asterisk/rtp_engine.h, line 1075
> > <https://reviewboard.asterisk.org/r/1083/diff/3/?file=15404#file15404line1075>
> >
> >     s/F/f/

fixed


> On 2011-02-02 16:37:08, Terry Wilson wrote:
> > /trunk/include/asterisk/rtp_engine.h, line 1089
> > <https://reviewboard.asterisk.org/r/1083/diff/3/?file=15404#file15404line1089>
> >
> >     s/asterisk/Asterisk/

fixed


> On 2011-02-02 16:37:08, Terry Wilson wrote:
> > /trunk/include/asterisk/translate.h, line 38
> > <https://reviewboard.asterisk.org/r/1083/diff/3/?file=15408#file15408line38>
> >
> >     Not yours, but C++-style comment

fixed


> On 2011-02-02 16:37:08, Terry Wilson wrote:
> > /trunk/include/asterisk/translate.h, lines 56-61
> > <https://reviewboard.asterisk.org/r/1083/diff/3/?file=15408#file15408line56>
> >
> >

re-worded this so it doesn't sound weird


> On 2011-02-02 16:37:08, Terry Wilson wrote:
> > /trunk/main/app.c, line 746
> > <https://reviewboard.asterisk.org/r/1083/diff/3/?file=15410#file15410line746>
> >
> >     ast_format_clear

fixed


> On 2011-02-02 16:37:08, Terry Wilson wrote:
> > /trunk/main/channel.c, lines 5563-5566
> > <https://reviewboard.asterisk.org/r/1083/diff/3/?file=15417#file15417line5563>
> >
> >     I may be misunderstanding, but it seems like we should send everything capable of being translated.
> >     
> >     What if we determine the best codec to be a codec that isn't available in the destination channel due to a peer config? Say the requestor supports ulaw and g729 and we determine that ulaw is the best. We then send the call to a SIP device that is configured in sip.conf to only allow g729. In this case, wouldn't we do an unnecessary transcode since jointcaps for the peer/pvt in chan_sip would be empty when it might not have been if we had sent everything?

This appears to be true to me as well.  I put this comment in while I was doing the project.  My plan is to re-evaluate this when I work on improving how we handle initial call setup across Asterisk.


- David


-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviewboard.asterisk.org/r/1083/#review3162
-----------------------------------------------------------


On 2011-01-24 15:25:45, David Vossel wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviewboard.asterisk.org/r/1083/
> -----------------------------------------------------------
> 
> (Updated 2011-01-24 15:25:45)
> 
> 
> Review request for Asterisk Developers.
> 
> 
> Summary
> -------
> 
> This patch is the foundation of an entire new way of looking at media in Asterisk.  The code present in this review is everything required to complete phase1 of my Media Architecture proposal.
> 
> For more information about this project visit the link below.
> https://wiki.asterisk.org/wiki/display/AST/Media+Architecture+Proposal
> 
> 
> Diffs
> -----
> 
>   /trunk/addons/chan_mobile.c 303557 
>   /trunk/addons/chan_ooh323.h 303557 
>   /trunk/addons/chan_ooh323.c 303557 
>   /trunk/addons/format_mp3.c 303557 
>   /trunk/addons/ooh323cDriver.h 303557 
>   /trunk/addons/ooh323cDriver.c 303557 
>   /trunk/apps/app_alarmreceiver.c 303557 
>   /trunk/apps/app_amd.c 303557 
>   /trunk/apps/app_chanspy.c 303557 
>   /trunk/apps/app_confbridge.c 303557 
>   /trunk/apps/app_dahdibarge.c 303557 
>   /trunk/apps/app_dictate.c 303557 
>   /trunk/apps/app_dumpchan.c 303557 
>   /trunk/apps/app_echo.c 303557 
>   /trunk/apps/app_fax.c 303557 
>   /trunk/apps/app_festival.c 303557 
>   /trunk/apps/app_followme.c 303557 
>   /trunk/apps/app_ices.c 303557 
>   /trunk/apps/app_jack.c 303557 
>   /trunk/apps/app_meetme.c 303557 
>   /trunk/apps/app_milliwatt.c 303557 
>   /trunk/apps/app_mixmonitor.c 303557 
>   /trunk/apps/app_mp3.c 303557 
>   /trunk/apps/app_nbscat.c 303557 
>   /trunk/apps/app_originate.c 303557 
>   /trunk/apps/app_parkandannounce.c 303557 
>   /trunk/apps/app_record.c 303557 
>   /trunk/apps/app_rpt.c 303557 
>   /trunk/apps/app_sms.c 303557 
>   /trunk/apps/app_speech_utils.c 303557 
>   /trunk/apps/app_talkdetect.c 303557 
>   /trunk/apps/app_test.c 303557 
>   /trunk/apps/app_voicemail.c 303557 
>   /trunk/apps/app_waitforsilence.c 303557 
>   /trunk/bridges/bridge_multiplexed.c 303557 
>   /trunk/bridges/bridge_simple.c 303557 
>   /trunk/bridges/bridge_softmix.c 303557 
>   /trunk/channels/chan_agent.c 303557 
>   /trunk/channels/chan_alsa.c 303557 
>   /trunk/channels/chan_bridge.c 303557 
>   /trunk/channels/chan_console.c 303557 
>   /trunk/channels/chan_dahdi.c 303557 
>   /trunk/channels/chan_gtalk.c 303557 
>   /trunk/channels/chan_h323.c 303557 
>   /trunk/channels/chan_iax2.c 303557 
>   /trunk/channels/chan_jingle.c 303557 
>   /trunk/channels/chan_local.c 303557 
>   /trunk/channels/chan_mgcp.c 303557 
>   /trunk/channels/chan_misdn.c 303557 
>   /trunk/channels/chan_multicast_rtp.c 303557 
>   /trunk/channels/chan_nbs.c 303557 
>   /trunk/channels/chan_oss.c 303557 
>   /trunk/channels/chan_phone.c 303557 
>   /trunk/channels/chan_sip.c 303557 
>   /trunk/channels/chan_skinny.c 303557 
>   /trunk/channels/chan_unistim.c 303557 
>   /trunk/channels/chan_usbradio.c 303557 
>   /trunk/channels/chan_vpb.cc 303557 
>   /trunk/channels/h323/ast_h323.cxx 303557 
>   /trunk/channels/h323/chan_h323.h 303557 
>   /trunk/channels/iax2-parser.h 303557 
>   /trunk/channels/iax2-parser.c 303557 
>   /trunk/channels/iax2-provision.c 303557 
>   /trunk/channels/iax2.h 303557 
>   /trunk/channels/sip/include/globals.h 303557 
>   /trunk/channels/sip/include/sip.h 303557 
>   /trunk/codecs/codec_a_mu.c 303557 
>   /trunk/codecs/codec_adpcm.c 303557 
>   /trunk/codecs/codec_alaw.c 303557 
>   /trunk/codecs/codec_dahdi.c 303557 
>   /trunk/codecs/codec_g722.c 303557 
>   /trunk/codecs/codec_g726.c 303557 
>   /trunk/codecs/codec_gsm.c 303557 
>   /trunk/codecs/codec_ilbc.c 303557 
>   /trunk/codecs/codec_lpc10.c 303557 
>   /trunk/codecs/codec_resample.c 303557 
>   /trunk/codecs/codec_speex.c 303557 
>   /trunk/codecs/codec_ulaw.c 303557 
>   /trunk/codecs/ex_adpcm.h 303557 
>   /trunk/codecs/ex_alaw.h 303557 
>   /trunk/codecs/ex_g722.h 303557 
>   /trunk/codecs/ex_g726.h 303557 
>   /trunk/codecs/ex_gsm.h 303557 
>   /trunk/codecs/ex_lpc10.h 303557 
>   /trunk/codecs/ex_speex.h 303557 
>   /trunk/codecs/ex_ulaw.h 303557 
>   /trunk/formats/format_g719.c 303557 
>   /trunk/formats/format_g723.c 303557 
>   /trunk/formats/format_g726.c 303557 
>   /trunk/formats/format_g729.c 303557 
>   /trunk/formats/format_gsm.c 303557 
>   /trunk/formats/format_h263.c 303557 
>   /trunk/formats/format_h264.c 303557 
>   /trunk/formats/format_ilbc.c 303557 
>   /trunk/formats/format_jpeg.c 303557 
>   /trunk/formats/format_ogg_vorbis.c 303557 
>   /trunk/formats/format_pcm.c 303557 
>   /trunk/formats/format_siren14.c 303557 
>   /trunk/formats/format_siren7.c 303557 
>   /trunk/formats/format_sln.c 303557 
>   /trunk/formats/format_sln16.c 303557 
>   /trunk/formats/format_vox.c 303557 
>   /trunk/formats/format_wav.c 303557 
>   /trunk/formats/format_wav_gsm.c 303557 
>   /trunk/funcs/func_channel.c 303557 
>   /trunk/funcs/func_frame_trace.c 303557 
>   /trunk/funcs/func_pitchshift.c 303557 
>   /trunk/include/asterisk/abstract_jb.h 303557 
>   /trunk/include/asterisk/astobj2.h 303557 
>   /trunk/include/asterisk/audiohook.h 303557 
>   /trunk/include/asterisk/bridging.h 303557 
>   /trunk/include/asterisk/bridging_technology.h 303557 
>   /trunk/include/asterisk/callerid.h 303557 
>   /trunk/include/asterisk/channel.h 303557 
>   /trunk/include/asterisk/data.h 303557 
>   /trunk/include/asterisk/file.h 303557 
>   /trunk/include/asterisk/format.h PRE-CREATION 
>   /trunk/include/asterisk/format_cap.h PRE-CREATION 
>   /trunk/include/asterisk/format_pref.h PRE-CREATION 
>   /trunk/include/asterisk/frame.h 303557 
>   /trunk/include/asterisk/frame_defs.h 303557 
>   /trunk/include/asterisk/image.h 303557 
>   /trunk/include/asterisk/mod_format.h 303557 
>   /trunk/include/asterisk/pbx.h 303557 
>   /trunk/include/asterisk/rtp_engine.h 303557 
>   /trunk/include/asterisk/slin.h 303557 
>   /trunk/include/asterisk/slinfactory.h 303557 
>   /trunk/include/asterisk/speech.h 303557 
>   /trunk/include/asterisk/translate.h 303557 
>   /trunk/main/abstract_jb.c 303557 
>   /trunk/main/app.c 303557 
>   /trunk/main/asterisk.c 303557 
>   /trunk/main/astobj2.c 303557 
>   /trunk/main/audiohook.c 303557 
>   /trunk/main/bridging.c 303557 
>   /trunk/main/callerid.c 303557 
>   /trunk/main/ccss.c 303557 
>   /trunk/main/channel.c 303557 
>   /trunk/main/cli.c 303557 
>   /trunk/main/data.c 303557 
>   /trunk/main/dial.c 303557 
>   /trunk/main/dsp.c 303557 
>   /trunk/main/features.c 303557 
>   /trunk/main/file.c 303557 
>   /trunk/main/format.c PRE-CREATION 
>   /trunk/main/format_cap.c PRE-CREATION 
>   /trunk/main/format_pref.c PRE-CREATION 
>   /trunk/main/frame.c 303557 
>   /trunk/main/image.c 303557 
>   /trunk/main/indications.c 303557 
>   /trunk/main/manager.c 303557 
>   /trunk/main/pbx.c 303557 
>   /trunk/main/rtp_engine.c 303557 
>   /trunk/main/slinfactory.c 303557 
>   /trunk/main/translate.c 303557 
>   /trunk/main/udptl.c 303557 
>   /trunk/pbx/pbx_spool.c 303557 
>   /trunk/res/res_adsi.c 303557 
>   /trunk/res/res_agi.c 303557 
>   /trunk/res/res_calendar.c 303557 
>   /trunk/res/res_clioriginate.c 303557 
>   /trunk/res/res_fax.c 303557 
>   /trunk/res/res_fax_spandsp.c 303557 
>   /trunk/res/res_musiconhold.c 303557 
>   /trunk/res/res_rtp_asterisk.c 303557 
>   /trunk/res/res_rtp_multicast.c 303557 
>   /trunk/res/res_speech.c 303557 
>   /trunk/tests/test_format_api.c PRE-CREATION 
> 
> Diff: https://reviewboard.asterisk.org/r/1083/diff
> 
> 
> Testing
> -------
> 
> Below are the major areas I tested during development.  I will continue testing as this patch is being reviewed.
> -Local Channel + IAX2 channel load testing
> -SIP Calls with and without video
> -IAX2 Calls
> -AudioHooks and apps using audiohooks
> -Masquerades
> -DTMF Attended Transfers
> -SIP Transfers
> -Gtalk
> 
> 
> Thanks,
> 
> David
> 
>

-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.digium.com/pipermail/asterisk-dev/attachments/20110203/26092b51/attachment-0001.htm>

More information about the asterisk-dev mailing list