Summary: | ASTERISK-21976: Set more than one codec in dialplan execution using SIP_CODEC (adapted chan_sip:try_suggested_codec) | ||
Reporter: | Dennis Guse (dennis.guse) | Labels: | |
Date Opened: | 2013-06-28 08:03:17 | Date Closed: | 2013-08-21 08:47:06 |
Priority: | Minor | Regression? | No |
Status: | Closed/Complete | Components: | Channels/chan_sip/CodecHandling |
Versions: | SVN | Frequency of Occurrence | |
Related Issues: | |||
Environment: | FreeBSD (AMD64) 9.1 using Asterisk provided via ports (at the moment 11.4) | Attachments: | ( 0) patch-channels-chan__sip.c-393919 |
Description: | For video calls, we would like to set the codecs in the dialplan using
SIP_CODEC. However, if SIP_CODEC is set, all codecs except the ONE set are disallowed and thus either audio or video is available. Attached is a patch for 11.4 that allows SIP_CODEC to contain a list of codecs , e.g. "gsm,h264". Patch against: 11.4: https://docs.google.com/file/d/0ByFooYVveHXdNng4WThoNV8zLVk/edit?usp=sharing Do you have any feedback? Authors: Dennis Guse (dennis.guse@qu.tu-berlin.de) und Frank Haase (fra.haase@googlemail.com). | ||
Comments: | By: Matt Jordan (mjordan) 2013-06-28 08:52:11.587-0500 A few comments here: # Patches cannot be supplied inline or in comments on the issue. They must be attached in unified diff format to this issue after signing a license contributor agreeement. # Improvements and new features are only allowed against trunk, and will not be applied to release branches (particularly LTS releases). Please write the patch against Asterisk trunk. I've removed the code in the comment section on the issue and put this into Waiting for Feedback - thanks! By: Dennis Guse (dennis.guse) 2013-06-28 10:08:40.854-0500 Patch against trunk (393126) https://docs.google.com/file/d/0ByFooYVveHXdV05WUkVuMzVnLUk/edit?usp=sharing and contributor agreement submitted. By: Dennis Guse (dennis.guse) 2013-07-02 02:40:34.912-0500 Contributor assignment acknowledged. Patch ported to TRUNK. By: Richard Mudgett (rmudgett) 2013-07-02 10:50:58.415-0500 Please attach the patch to the issue itself instead of pasting a link to an external entity. Posting patches to external entities most likely will result in the external entity deleting the post after so many days. Attach the patch in "diff -u" format by selecting More Actions -> Attach files in the menu above. By: Dennis Guse (dennis.guse) 2013-07-02 12:45:17.716-0500 Patch is now attached! By: Dennis Guse (dennis.guse) 2013-07-04 09:33:45.044-0500 Patch attached By: Rusty Newton (rnewton) 2013-07-09 09:02:37.708-0500 Thanks. Here is the guidelines for future reference. It covers patch submission: https://wiki.asterisk.org/wiki/display/AST/Asterisk+Issue+Guidelines By: Matt Jordan (mjordan) 2013-07-09 09:04:16.955-0500 While it's fine to have a patch on the issue for a released branch - which can be useful for users of Asterisk - since this is an improvement/new feature, it must be written against Asterisk trunk to be included in the next version. Please make sure your patch applies cleanly to trunk and attach it to the issue. Some [coding guideline|https://wiki.asterisk.org/wiki/display/AST/Coding+Guidelines] problems: # {{const char *codecList;}} => should be {{codec_list}}. Asterisk doesn't use camelCase or CamelCase. # All {{if}} statements should have brackets. Older portions of the codebase didn't always follow this rule, but new code should. Minor nitpick: # Reduce indentation here: {noformat} + ast_getformatbyname(ast_strip(codec), &fmt); + if (fmt.id) { {noformat} by using logging a NOTICE and {{continue}} if {{fmt.id}} is 0. By: Dennis Guse (dennis.guse) 2013-07-10 02:50:57.733-0500 Fixed code to comply with coding style guidelines (variables names, if {}). + made ast_strip use more clear (Sorry, didnt saw your comment). + use continue, if fmt.id == 0 By: Dennis Guse (dennis.guse) 2013-07-10 02:52:45.399-0500 Code adapted to comply with coding style By: Rusty Newton (rnewton) 2013-07-16 14:12:32.062-0500 You need to hit "Send back" to change the status once you have provided feedback. Thanks! By: Matt Jordan (mjordan) 2013-07-31 19:43:16.142-0500 Posted to review board at https://reviewboard.asterisk.org/r/2728/ |