[asterisk-dev] [Code Review] Media Project Phase 2: SILK 8khz-24khz, SLINEAR 8khz-192khz, SPEEX 32khz, Custom format creation in codecs.conf

[David Vossel]

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


I'm just pointing out a couple of things I came across while reviewing my code.


/trunk/main/audiohook.c
<https://reviewboard.asterisk.org/r/1104/#comment6593>

    This code works as it is right now for 99% of calls, but I have identified an edge case that invalidates this logic.
    
    The internal rate of the Audiohook should not be determined based on the last read and write rates to the audiohook, but rather the last read and writes to the audiohook's list.  This is a subtle change, but functionally keeping the internal sample rate stored on the list rather than each individual audiohook is the more consistent change.



/trunk/main/channel.c
<https://reviewboard.asterisk.org/r/1104/#comment6592>

    Since I wrote this code, I have made an API call to the Format API that does this exact same thing.  I need to replace this code with a call to that function.


- David


On 2011-02-16 17:41:27, David Vossel wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviewboard.asterisk.org/r/1104/
> -----------------------------------------------------------
> 
> (Updated 2011-02-16 17:41:27)
> 
> 
> Review request for Asterisk Developers.
> 
> 
> Summary
> -------
> 
> -Functional Changes
> Dynamic global format list build by codecs defined in codecs.conf
> SILK 8khz, 12khz, 16khz, and 24khz with custom attributes defined in codecs.conf
> Negotiation of SILK attributes in chan_sip.
> SPEEX 32khz with translation
> SLINEAR 8khz, 12khz, 24khz, 32khz, 44.1khz, 48khz, 96khz, 192khz with translation using codec_resample.c
> Various changes to RTP code required to properly handle the dynamic format list and formats with attributes.
> 
> -Organizational changes
> Global format list is moved from frame.c to format.c
> Various format specific functions moved from frame.c to format.c
> 
> -MIS
> There were several changes that I opted not to do during phase 1 that involved the proper use of formats with attributes.  I felt that the phase 1 patch was too complex to attempt these changes so I have included them here.  The primary change I held off doing the first time around was in the rtp_engine.c Payload and MIME type lists.  These lists were statically defined which made it impossible to set formats with attributes in them.  I have gone back and revised these lists to allow the flexibility required for the new architecture.
> 
> 
> Diffs
> -----
> 
>   /trunk/CHANGES 308199 
>   /trunk/apps/app_chanspy.c 308199 
>   /trunk/apps/app_jack.c 308199 
>   /trunk/apps/app_mixmonitor.c 308199 
>   /trunk/bridges/bridge_multiplexed.c 308199 
>   /trunk/bridges/bridge_softmix.c 308199 
>   /trunk/channels/chan_gtalk.c 308199 
>   /trunk/channels/chan_iax2.c 308199 
>   /trunk/channels/chan_jingle.c 308199 
>   /trunk/channels/chan_sip.c 308199 
>   /trunk/channels/chan_skinny.c 308199 
>   /trunk/channels/iax2.h 308199 
>   /trunk/codecs/Makefile 308199 
>   /trunk/codecs/codec_resample.c 308199 
>   /trunk/codecs/codec_speex.c 308199 
>   /trunk/codecs/speex/arch.h PRE-CREATION 
>   /trunk/codecs/speex/fixed_generic.h PRE-CREATION 
>   /trunk/codecs/speex/resample.c PRE-CREATION 
>   /trunk/codecs/speex/resample_sse.h PRE-CREATION 
>   /trunk/codecs/speex/speex_resampler.h PRE-CREATION 
>   /trunk/codecs/speex/stack_alloc.h PRE-CREATION 
>   /trunk/configs/codecs.conf.sample 308199 
>   /trunk/formats/format_attr_silk.c PRE-CREATION 
>   /trunk/funcs/func_pitchshift.c 308199 
>   /trunk/funcs/func_speex.c 308199 
>   /trunk/funcs/func_volume.c 308199 
>   /trunk/include/asterisk/audiohook.h 308199 
>   /trunk/include/asterisk/format.h 308199 
>   /trunk/include/asterisk/format_cap.h 308199 
>   /trunk/include/asterisk/frame.h 308199 
>   /trunk/include/asterisk/rtp_engine.h 308199 
>   /trunk/include/asterisk/silk.h PRE-CREATION 
>   /trunk/include/asterisk/slinfactory.h 308199 
>   /trunk/include/asterisk/time.h 308199 
>   /trunk/include/asterisk/translate.h 308199 
>   /trunk/main/asterisk.c 308199 
>   /trunk/main/audiohook.c 308199 
>   /trunk/main/channel.c 308199 
>   /trunk/main/data.c 308199 
>   /trunk/main/format.c 308199 
>   /trunk/main/format_cap.c 308199 
>   /trunk/main/format_pref.c 308199 
>   /trunk/main/frame.c 308199 
>   /trunk/main/rtp_engine.c 308199 
>   /trunk/main/slinfactory.c 308199 
>   /trunk/main/translate.c 308199 
>   /trunk/res/res_mutestream.c 308199 
>   /trunk/res/res_rtp_asterisk.c 308199 
> 
> Diff: https://reviewboard.asterisk.org/r/1104/diff
> 
> 
> Testing
> -------
> 
> I used the same testing I did for phase1 with phase2.  This includes load testing with various codec negotiation scenarios.  Speex 32 and its translators were verified using Asterisk back to back.  I verified SILK using a codec translator module I'm working with.  The new slinear resample code was exercised with both Speex32 and SILK 12khz/24khz translation.
> 
> 
> Thanks,
> 
> David
> 
>

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