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

[rmudgett]

> On April 10, 2015, 5:15 p.m., Mark Michelson wrote:
> > /branches/13/main/channel.c, lines 4146-4149
> > <https://reviewboard.asterisk.org/r/4609/diff/1/?file=73842#file73842line4146>
> >
> >     I figure I should point out why translate_via_slin is on by default. The big reason is for audiohooks or any other core audio manipulation features. Let's say that you do not have translate_via_slin enabled. Alice speaks g.722 and calls Bob who speaks ulaw. Asterisk might set up a translation path like so:
> >     
> >     Alice read path: g.722->slin->ulaw
> >     Alice write path: ulaw->slin->g.722
> >     Bob read path: ulaw
> >     Bob write path: ulaw
> >     
> >     Every frame that comes in from Alice or Bob ends up getting extra translations to slin if audiohooks are in use. Meanwhile, if you do have translate_via_slin enabled, you'd be guaranteed to have these translation paths:
> >     
> >     Alice read path: g.722->slin
> >     Alice write path: slin->g.722
> >     Bob read path: ulaw->slin
> >     Bob write path: slin->ulaw
> >     
> >     By having slin as the read and write formats of both channels, frames that come into the core don't have to go through any extra translations in order to be used in audiohooks.

Maybe that is what should happen but it does not.  Audiohooks on the read path examine the incoming frame before it is translated for return by ast_read().  Audiohooks on the write path examine frames after it is translated to go out on the wire.


- rmudgett


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


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/2ed48ee7/attachment-0001.html>