[asterisk-dev] [Code Review] 3889: format: Remove incorrectly assigned format compatibility bits for Opus and VP8.

[rmudgett]

> On Aug. 5, 2014, 8:03 a.m., Joshua Colp wrote:
> > Can you elaborate on what actually happens here between 11<->12 and 12<->12 that makes it so we need to remove the newly added ones?

The format compatibility bits need to remain frozen to new codecs because chan_iax2 leaks internal implementation values and doesn't know how to negotiate any codec options.  Opus has options can be negotiated.  I don't know about VP8 but as its a newer codec it likely has negotiable options.

The IAX_IE_CODEC_PREFS body is a sequence of 8 bit values.  In v1.8 and earlier the preference values are the index+1 into the frame.c:AST_FORMAT_LIST[].  In v10-v11 the preference values are the order that formats are put into the format_list ao2 list conatiner in format.c:format_list_init().  The format_list is then converted to the format_list_array object and indexed like AST_FORMAT_LIST[].  For backwards compatibility, the first 28 match the order in v1.8's AST_FORMAT_LIST[] and have format compatibility bits assigned.  There is a comment saying the order of the first 28 must not change.  The comment does not state why.  However, that order cannot change because those formats are in the historical AST_FORMAT_LIST[], those formats have compatibility bits, and the index values are sent over the wire by IAX2.  After the fixed formats there is a comment saying the order may change.  The comment does not state why.  However, the order may change because none of the following formats have a format combatibility bit defined.  In v12 the Opus and VP8 formats were added to the format_list eight formats after the end of the fixed formats and also assigned format compatibility bits.

Interoperation between v12 and earlier Asterisk version should not be a problem because earlier versions doesn't know about Opus and VP8 and thus won't pick those codecs.

Between v12 instances, Opus and VP8 could be picked.  However, because of their order in the format_list there will be a gap of eight between G719 and Opus that will be difficult to maintain without dire comments that won't be followed.  Also as stated, IAX2 doesn't know how to negotiate any codec options.


- rmudgett


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


On Aug. 4, 2014, 5:47 p.m., rmudgett wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviewboard.asterisk.org/r/3889/
> -----------------------------------------------------------
> 
> (Updated Aug. 4, 2014, 5:47 p.m.)
> 
> 
> Review request for Asterisk Developers.
> 
> 
> Repository: Asterisk
> 
> 
> Description
> -------
> 
> The format compatibility bits need to remain frozen to new codecs.  Otherwise, older channel drivers like chan_iax2 could attempt to negotiate them when they have no support for negotiating any of the format's other potential parameters.  In addition, the chan_iax2 format preference order values sent over the wire have no defined fixed value to represent Opus and VP8.
> 
> 
> Diffs
> -----
> 
>   /branches/12/main/format.c 420025 
> 
> Diff: https://reviewboard.asterisk.org/r/3889/diff/
> 
> 
> Testing
> -------
> 
> Compiles and code inspeciton.
> 
> See format.c:format_list_init() and format_pref.c:ast_codec_pref_convert().
> The format_list container is used to generate the format_list_array which is then is used by chan_iax2 to determine the format index sent over the wire in IAX_IE_CODEC_PREFS.
> 
> 
> Thanks,
> 
> rmudgett
> 
>

-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.digium.com/pipermail/asterisk-dev/attachments/20140805/2a535444/attachment.html>