[Home]

Summary:ASTERISK-00987: [patch] - design suggestion: Codec names unification
Reporter:malzina (malzina)Labels:
Date Opened:2004-02-04 08:51:12.000-0600Date Closed:2011-06-07 14:05:01
Priority:MajorRegression?No
Status:Closed/CompleteComponents:Core/General
Versions:Frequency of
Occurrence
Related
Issues:
Environment:Attachments:( 0) codecs_naming.html
( 1) patch_codec.diff
Description:This patch begin to unifies the codec names around * to the IANA RTP codec names as defined in http://www.iana.org/assignments/rtp-parameters
The modifications made in this patch are done in a compatible way, so that it is not required to update config files or anything else right now.

****** ADDITIONAL INFORMATION ******

Haven't you noticed that in * we use various codec names to designate the same codec. For instance you have:
- "G.711 u-law" from the "show codecs" console command
- "ULAW" from "ast_getformatname()"
- "ulaw" from "ast_getformatbyname()"
- "PCMU" from sip/sdp (defined by the IANA)

In order to clarify things for the user as well as for the developper, I suggest we use the IANA codec names everywhere.
Moreover not having to use codec name constants at different places increases the maintainability of the code.
Comments:By: Olle Johansson (oej) 2004-02-05 15:22:29.000-0600

I like this. But if we start cleaning up, we need to make a general standard including everything. A list consisting of:

Codec no in RTP, Codec name, synonyms, media type, file name extension

* Codec names are used in allow/deny statements for codecs in device configs
* Codec file name extensions is used in voicemail.conf and various applications, like record()
* Media types could be used in web interfaces and for e-mail  attachments (voicemail)

edited on: 02-05-04 15:23

By: malzina (malzina) 2004-02-06 06:21:07.000-0600

Ok, I uploaded "codecs_naming.html" as a start for that.
By the way, I hate to walk away for the standard, but instead of using the IANA RTP names, I suggest to use the same names but in lowercase, since it would be quite un-graceful to have uppercases everywhere... If there is a generally agreed opinion on this, I could remake the patch for that.

I do not know what you mean by "codec not in RTP"...

By: Olle Johansson (oej) 2004-02-25 09:16:11.000-0600

I meant codec number...

By: James Golovich (jamesgolovich) 2004-02-25 13:14:19.000-0600

I think this should be a text document included with *, not an html document

By: Mark Spencer (markster) 2004-04-15 13:11:53

This would entirely break backwards compatibility, right?

By: Olle Johansson (oej) 2004-04-15 14:38:04

That price is too high. Let's document all the names and add to documentation to make life simpler for coders and users. Maybe also start a long-term process to see if we can standardize in future releases without totally breaking backwards compatibility.

By: Brian West (bkw918) 2004-04-18 01:28:33

oej can you document this or post maybe a README.codecs?

bkw

By: sbingner (sbingner) 2004-04-29 05:48:39

Dupe bug 1516 was opened, reopening this one so discussion can continue and closing out dupe.

By: malzina (malzina) 2004-04-29 05:53:20

You guys assumed that unifying codec names would break backwards compatibility of config files, but I do not see why.

We could accept several codec names (the old one and the new one) to designate the same codec. When the old one is used, a warning message could be generated to encourage the user to use the new syntax, and we accept it anyway, that is it.

By: sbingner (sbingner) 2004-04-29 06:07:35

Why don't you try to come up with a patch that will do as you say, and submit that... it would stand a much better chance of being accepted.  You also need to make sure it doesn't break backwards compatibility on channels, filenames, etc...

By: malzina (malzina) 2004-04-29 08:45:59

There seems to be a little misunderstanding on what this patch does and does not.

I suggest string constants reprensenting a codec name must be declared once for all. Do not duplucate ways to print or parse a codec name, it will only increase the probability to make a typo or to accidentally use a variant of the name.

1) To enforce that, here are the tools this patches gives:

The idea is to use the mimeTypes[] array (in rtp.c) as the reference for codec names.

ast_rtp_lookup_mime_subtype() (rtp.c) is modified so that is can be used to return a codec name (providing isAstFormat=-1) since mimeTypes[] is static to rtp.c

ast_getformatname() (frame.c) is rewritten to use ast_rtp_lookup_mime_subtype() to produce names.

ast_codec2str() is removed (use ast_getformatname() instead)

added ast_rtp_get_mime_subtype_code() (rtp.c) to get a codec id (AST_FORMAT_xxx) from a string, based on mimeTypes[].subtype

modified ast_getformatbyname() to recognize new format names, as well as OLD FORMAT NAMES.

2) As a begining towards a proper use of codec names

show_codecs() (rtp.c) is changed to use ast_getformatname() instead of ast_codec2str().

load_module() (chan_phone.c) is modified to parse codec names using ast_getformatbyname() instead of a localy defined string comparisions

and ... it is up to us to use ast_getformatname() and ast_getformatbyname() in future developments and updates.

I hope that this helps.

By: Mark Spencer (markster) 2004-05-25 11:38:04

The formats used generally in configuration files are not the mime-type style declarations though.

By: Olle Johansson (oej) 2004-06-09 14:45:22

No consensus, no action.