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

Tilghman Lesher reviewboard at asterisk.org
Fri Jan 21 14:42:57 CST 2011 

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


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.


/trunk/channels/chan_iax2.c
<https://reviewboard.asterisk.org/r/1083/#comment6384>

    What I did with the FORMAT2 field was to apply a version number, knowing that there would be a future need to change how codecs are encoded, in order to prevent using another IE value.  This may be a good opportunity to advance the version number and send an encoded string with the format preferences, instead of a 64-bit bitfield.  Or, depending upon the expressiveness of the codec negotiation, we may need to allocate more IE values, anyway.


- Tilghman


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/a9171ed8/attachment-0001.htm>

More information about the asterisk-dev mailing list