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

Russell Bryant reviewboard at asterisk.org
Fri Jan 21 15:34:23 CST 2011 


> On 2011-01-21 14:42:57, Tilghman Lesher wrote:
> > This is an architectural review of the changes to the IAX2 stack, only.
> > 
> > The basic idea of the IAX2 protocol is a non-lossy transmit of the internals of one server to another.  This is merely shoehorning the new media architecture into the 64-bit bitfield, which isn't even that old.  I'd like to see the versioned value of the FORMAT2 and CAPABILITY2 fields leveraged, such that we can expect this lossless transmit of codec parameters between Asterisk servers.
> > 
> > One considerable problem with the loss-less transmit is that of the mini-frames, due to the problem of packing this information into a single byte value.  In earlier versions, this was done in a creative way, by using the MSB to encode whether the remaining 7 bits were considered a literal bit-value or the expression of the base2-log of the codec number.  This was sufficient to encode up to 128 + 7 different values, which exceeded even the demands of the 64-bit bitfield conversion.  One way to deal with this would be to use some of the as-yet unused values, along with a full frame, denoting which of the negotiated codecs will be encoded as which byte value (all with the top two bits set for the codec value).  This would be done per-call, on a demand basis.  Most calls proceed with only a single codec anyway, and the probability that there will ever be a practical need for more than the available values is extremely low, other than for pure test cases.
> 
> David Vossel wrote:
>     IAX2 does not have the capability of representing media formats with attributes.  This is a protocol level limitation which is outlined in my Media Architecture Proposal as being something I have no plans to address.  I see no value in expanding the number of media formats IAX2 can represent unless format attributes are associated with them.

It's certainly *feasible* that IAX2 could be heavily modified to include all of the new information that is now internally to Asterisk.  The question is whether we are willing to do it or not.  We've made a huge list of things we'd like to accomplish with this project, and making those changes to IAX2 just seem far less important.  Even though chan_iax2 is operating in a bit of a compatability/conversion layer mode today, does not mean that someone could not come along later and improve IAX2 to support these new features.  We've pretty much already decided it's not going to be a part of the effort Digium puts forth in this effort today.


- Russell


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


On 2011-01-21 13:52:50, David Vossel wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviewboard.asterisk.org/r/1083/
> -----------------------------------------------------------
> 
> (Updated 2011-01-21 13:52:50)
> 
> 
> 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
> -----
> 
>   /configure UNKNOWN 
>   /trunk/CHANGES 303283 
>   /trunk/addons/chan_mobile.c 303283 
>   /trunk/addons/chan_ooh323.h 303283 
>   /trunk/addons/chan_ooh323.c 303283 
>   /trunk/addons/format_mp3.c 303283 
>   /trunk/addons/ooh323cDriver.h 303283 
>   /trunk/addons/ooh323cDriver.c 303283 
>   /trunk/apps/app_alarmreceiver.c 303283 
>   /trunk/apps/app_amd.c 303283 
>   /trunk/apps/app_chanspy.c 303283 
>   /trunk/apps/app_confbridge.c 303283 
>   /trunk/apps/app_dahdibarge.c 303283 
>   /trunk/apps/app_dial.c 303283 
>   /trunk/apps/app_dictate.c 303283 
>   /trunk/apps/app_dumpchan.c 303283 
>   /trunk/apps/app_echo.c 303283 
>   /trunk/apps/app_fax.c 303283 
>   /trunk/apps/app_festival.c 303283 
>   /trunk/apps/app_followme.c 303283 
>   /trunk/apps/app_ices.c 303283 
>   /trunk/apps/app_jack.c 303283 
>   /trunk/apps/app_meetme.c 303283 
>   /trunk/apps/app_milliwatt.c 303283 
>   /trunk/apps/app_mixmonitor.c 303283 
>   /trunk/apps/app_mp3.c 303283 
>   /trunk/apps/app_nbscat.c 303283 
>   /trunk/apps/app_originate.c 303283 
>   /trunk/apps/app_parkandannounce.c 303283 
>   /trunk/apps/app_privacy.c 303283 
>   /trunk/apps/app_queue.c 303283 
>   /trunk/apps/app_record.c 303283 
>   /trunk/apps/app_rpt.c 303283 
>   /trunk/apps/app_sms.c 303283 
>   /trunk/apps/app_speech_utils.c 303283 
>   /trunk/apps/app_talkdetect.c 303283 
>   /trunk/apps/app_test.c 303283 
>   /trunk/apps/app_voicemail.c 303283 
>   /trunk/apps/app_waitforsilence.c 303283 
>   /trunk/bridges/bridge_multiplexed.c 303283 
>   /trunk/bridges/bridge_simple.c 303283 
>   /trunk/bridges/bridge_softmix.c 303283 
>   /trunk/channels/chan_agent.c 303283 
>   /trunk/channels/chan_alsa.c 303283 
>   /trunk/channels/chan_bridge.c 303283 
>   /trunk/channels/chan_console.c 303283 
>   /trunk/channels/chan_dahdi.c 303283 
>   /trunk/channels/chan_gtalk.c 303283 
>   /trunk/channels/chan_h323.c 303283 
>   /trunk/channels/chan_iax2.c 303283 
>   /trunk/channels/chan_jingle.c 303283 
>   /trunk/channels/chan_local.c 303283 
>   /trunk/channels/chan_mgcp.c 303283 
>   /trunk/channels/chan_misdn.c 303283 
>   /trunk/channels/chan_multicast_rtp.c 303283 
>   /trunk/channels/chan_nbs.c 303283 
>   /trunk/channels/chan_oss.c 303283 
>   /trunk/channels/chan_phone.c 303283 
>   /trunk/channels/chan_sip.c 303283 
>   /trunk/channels/chan_skinny.c 303283 
>   /trunk/channels/chan_unistim.c 303283 
>   /trunk/channels/chan_usbradio.c 303283 
>   /trunk/channels/chan_vpb.cc 303283 
>   /trunk/channels/h323/ast_h323.cxx 303283 
>   /trunk/channels/h323/chan_h323.h 303283 
>   /trunk/channels/iax2-parser.h 303283 
>   /trunk/channels/iax2-parser.c 303283 
>   /trunk/channels/iax2-provision.c 303283 
>   /trunk/channels/iax2.h 303283 
>   /trunk/channels/sip/include/globals.h 303283 
>   /trunk/channels/sip/include/sip.h 303283 
>   /trunk/codecs/codec_a_mu.c 303283 
>   /trunk/codecs/codec_adpcm.c 303283 
>   /trunk/codecs/codec_alaw.c 303283 
>   /trunk/codecs/codec_dahdi.c 303283 
>   /trunk/codecs/codec_g722.c 303283 
>   /trunk/codecs/codec_g726.c 303283 
>   /trunk/codecs/codec_gsm.c 303283 
>   /trunk/codecs/codec_ilbc.c 303283 
>   /trunk/codecs/codec_lpc10.c 303283 
>   /trunk/codecs/codec_resample.c 303283 
>   /trunk/codecs/codec_speex.c 303283 
>   /trunk/codecs/codec_ulaw.c 303283 
>   /trunk/codecs/ex_adpcm.h 303283 
>   /trunk/codecs/ex_alaw.h 303283 
>   /trunk/codecs/ex_g722.h 303283 
>   /trunk/codecs/ex_g726.h 303283 
>   /trunk/codecs/ex_gsm.h 303283 
>   /trunk/codecs/ex_lpc10.h 303283 
>   /trunk/codecs/ex_speex.h 303283 
>   /trunk/codecs/ex_ulaw.h 303283 
>   /trunk/configs/extensions.conf.sample 303283 
>   /trunk/configs/queues.conf.sample 303283 
>   /trunk/configure.ac 303283 
>   /trunk/contrib/scripts/install_prereq 303283 
>   /trunk/formats/format_g719.c 303283 
>   /trunk/formats/format_g723.c 303283 
>   /trunk/formats/format_g726.c 303283 
>   /trunk/formats/format_g729.c 303283 
>   /trunk/formats/format_gsm.c 303283 
>   /trunk/formats/format_h263.c 303283 
>   /trunk/formats/format_h264.c 303283 
>   /trunk/formats/format_ilbc.c 303283 
>   /trunk/formats/format_jpeg.c 303283 
>   /trunk/formats/format_ogg_vorbis.c 303283 
>   /trunk/formats/format_pcm.c 303283 
>   /trunk/formats/format_siren14.c 303283 
>   /trunk/formats/format_siren7.c 303283 
>   /trunk/formats/format_sln.c 303283 
>   /trunk/formats/format_sln16.c 303283 
>   /trunk/formats/format_vox.c 303283 
>   /trunk/formats/format_wav.c 303283 
>   /trunk/formats/format_wav_gsm.c 303283 
>   /trunk/funcs/func_channel.c 303283 
>   /trunk/funcs/func_db.c 303283 
>   /trunk/funcs/func_frame_trace.c 303283 
>   /trunk/funcs/func_pitchshift.c 303283 
>   /trunk/include/asterisk/abstract_jb.h 303283 
>   /trunk/include/asterisk/astdb.h 303283 
>   /trunk/include/asterisk/astobj2.h 303283 
>   /trunk/include/asterisk/audiohook.h 303283 
>   /trunk/include/asterisk/bridging.h 303283 
>   /trunk/include/asterisk/bridging_technology.h 303283 
>   /trunk/include/asterisk/callerid.h 303283 
>   /trunk/include/asterisk/channel.h 303283 
>   /trunk/include/asterisk/data.h 303283 
>   /trunk/include/asterisk/file.h 303283 
>   /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 303283 
>   /trunk/include/asterisk/frame_defs.h 303283 
>   /trunk/include/asterisk/image.h 303283 
>   /trunk/include/asterisk/mod_format.h 303283 
>   /trunk/include/asterisk/pbx.h 303283 
>   /trunk/include/asterisk/rtp_engine.h 303283 
>   /trunk/include/asterisk/slin.h 303283 
>   /trunk/include/asterisk/slinfactory.h 303283 
>   /trunk/include/asterisk/speech.h 303283 
>   /trunk/include/asterisk/translate.h 303283 
>   /trunk/main/abstract_jb.c 303283 
>   /trunk/main/app.c 303283 
>   /trunk/main/asterisk.c 303283 
>   /trunk/main/astobj2.c 303283 
>   /trunk/main/audiohook.c 303283 
>   /trunk/main/bridging.c 303283 
>   /trunk/main/callerid.c 303283 
>   /trunk/main/ccss.c 303283 
>   /trunk/main/channel.c 303283 
>   /trunk/main/cli.c 303283 
>   /trunk/main/config.c 303283 
>   /trunk/main/data.c 303283 
>   /trunk/main/dial.c 303283 
>   /trunk/main/dsp.c 303283 
>   /trunk/main/features.c 303283 
>   /trunk/main/file.c 303283 
>   /trunk/main/format.c PRE-CREATION 
>   /trunk/main/format_cap.c PRE-CREATION 
>   /trunk/main/format_pref.c PRE-CREATION 
>   /trunk/main/frame.c 303283 
>   /trunk/main/image.c 303283 
>   /trunk/main/indications.c 303283 
>   /trunk/main/manager.c 303283 
>   /trunk/main/pbx.c 303283 
>   /trunk/main/rtp_engine.c 303283 
>   /trunk/main/slinfactory.c 303283 
>   /trunk/main/translate.c 303283 
>   /trunk/main/udptl.c 303283 
>   /trunk/main/utils.c 303283 
>   /trunk/pbx/pbx_spool.c 303283 
>   /trunk/res/res_adsi.c 303283 
>   /trunk/res/res_agi.c 303283 
>   /trunk/res/res_calendar.c 303283 
>   /trunk/res/res_clioriginate.c 303283 
>   /trunk/res/res_fax.c 303283 
>   /trunk/res/res_fax_spandsp.c 303283 
>   /trunk/res/res_musiconhold.c 303283 
>   /trunk/res/res_rtp_asterisk.c 303283 
>   /trunk/res/res_rtp_multicast.c 303283 
>   /trunk/res/res_speech.c 303283 
>   /trunk/res/res_timing_timerfd.c 303283 
>   /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/20110121/91dfeb58/attachment-0001.htm>

More information about the asterisk-dev mailing list