[asterisk-dev] [Code Review] 4609: chan_pjsip/res_pjsip/bridge_softmix/core: Improve translation path choices.

[Matt Jordan]

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


Reviewed: I didn't have any findings.

- Matt Jordan


On April 9, 2015, 3:50 p.m., rmudgett wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviewboard.asterisk.org/r/4609/
> -----------------------------------------------------------
> 
> (Updated April 9, 2015, 3:50 p.m.)
> 
> 
> Review request for Asterisk Developers.
> 
> 
> Bugs: ASTERISK-24841
>     https://issues.asterisk.org/jira/browse/ASTERISK-24841
> 
> 
> Repository: Asterisk
> 
> 
> Description
> -------
> 
> With this patch, chan_pjsip/res_pjsip now sets the native formats to the
> codecs negotiated by a call.
> 
> * The changes in chan_pjsip.c and res_pjsip_sdp_rtp.c set the native
> formats to include all the negotiated audio codecs instead of only the
> initial preferred audio codec and later the currently received audio
> codec.
> 
> * The audio frame handling in channel.c:ast_read() is more streamlined and
> will automatically adjust to changes in received frame formats.  The new
> policy is to remove translation and pass the new frame format to the
> receiver except if the translation was to a signed linear format.  A more
> long winded version is commented in ast_read() along with some caveats.
> 
> * The audio frame handling in channel.c:ast_write() is more streamlined
> and will automatically adjust any needed translation to changes in the
> frame formats sent.  Frame formats sent can change for many reasons such
> as a recording is being played back or the bridged peer changed the format
> it sends.  Since it is a normal expectation that sent formats can change,
> the codec mismatch warning message is demoted to a debug message.
> 
> * Removed the short circuit check in
> channel.c:ast_channel_make_compatible_helper().  Two party bridges need to
> make channels compatible with each other.  However, transfers and moving
> channels among bridges can result in otherwise compatible channels having
> sub-optimal translation paths if the make compatible check is short
> circuited.  A result of forcing the reevaluation of channel compatibility
> is that the asterisk.conf:transcode_via_slin and codecs.conf:genericplc
> options take effect consistently now.  It is unfortunate that these two
> options are enabled by default and negate some of the benefits to the
> changes in channel.c:ast_read() by forcing translation through signed
> linear on a two party bridge.
> 
> * Improved the softmix bridge technology to better control the translation
> of frames to the bridge.  All of the incoming translation is now normally
> handled by ast_read() instead of splitting any translation steps between
> ast_read() and the slin factory.  If any frame comes in with an unexpected
> format then the translation path in ast_read() is updated for the next
> frame and the slin factory handles the current frame translation.
> 
> This is the final patch in a series of patches aimed at improving
> translation path choices.  The other patches are on the following reviews:
> https://reviewboard.asterisk.org/r/4600/
> https://reviewboard.asterisk.org/r/4605/
> 
> 
> Diffs
> -----
> 
>   /branches/13/res/res_pjsip_sdp_rtp.c 434526 
>   /branches/13/main/channel.c 434526 
>   /branches/13/include/asterisk/channel.h 434526 
>   /branches/13/channels/chan_pjsip.c 434526 
>   /branches/13/bridges/bridge_softmix.c 434526 
> 
> Diff: https://reviewboard.asterisk.org/r/4609/diff/
> 
> 
> Testing
> -------
> 
> * The testsuite still passes as well as it ever has.
> 
> * Manual SIP and DTMF attended transfers still function.  With all patches
> in the series applied, if a low speed party transfers a higher speed party
> to another high speed party then when the transfer completes the resulting
> call works at the higher speed.  Without the patch the resulting call may
> go through a sub-optimal translation path with reduced audio quality.
> 
> * ConfBridge bridges are able to change mixing rates as different speed
> participants enter and leave the bridge.  Sound files played back to
> individual participants may go out with a different codec than the
> participant sends to the conference.  If the conference bridge is mixing
> at a lower rate than a participant then the conference media may go out
> with a different codec than the participant sends to the conference.
> 
> * Used app_originate to setup a call through a non-optimizing local
> channel.  The resulting call used the same codecs as before the patch even
> between parties with different speeds.
> 
> 
> Thanks,
> 
> rmudgett
> 
>

-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.digium.com/pipermail/asterisk-dev/attachments/20150410/1fc4cea4/attachment.html>