Asterisk
  1. Asterisk
  2. ASTERISK-21976

Set more than one codec in dialplan execution using SIP_CODEC (adapted chan_sip:try_suggested_codec)

    Details

    • Type: Improvement Improvement
    • Status: Closed
    • Severity: Minor Minor
    • Resolution: Fixed
    • Affects Version/s: SVN
    • Target Release Version/s: None
    • Security Level: None
    • Labels:
      None
    • Environment:
      FreeBSD (AMD64) 9.1 using Asterisk provided via ports (at the moment 11.4)

      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).

        Issue Links

        No reviews matched the request. Check your Options in the drop-down menu of this sections header.

          Activity

          Hide
          Matt Jordan added a comment -

          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 problems:

          1. const char *codecList; => should be codec_list. Asterisk doesn't use camelCase or CamelCase.
          2. All if statements should have brackets. Older portions of the codebase didn't always follow this rule, but new code should.

          Minor nitpick:

          1. Reduce indentation here:
            +		ast_getformatbyname(ast_strip(codec), &fmt);
            +		if (fmt.id) {
            

            by using logging a NOTICE and continue if fmt.id is 0.

          Show
          Matt Jordan added a comment - 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 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: + ast_getformatbyname(ast_strip(codec), &fmt); + if (fmt.id) { by using logging a NOTICE and continue if fmt.id is 0.
          Hide
          Dennis Guse added a comment -

          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

          Show
          Dennis Guse added a comment - 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
          Hide
          Dennis Guse added a comment -

          Code adapted to comply with coding style

          Show
          Dennis Guse added a comment - Code adapted to comply with coding style
          Hide
          Rusty Newton added a comment -

          You need to hit "Send back" to change the status once you have provided feedback. Thanks!

          Show
          Rusty Newton added a comment - You need to hit "Send back" to change the status once you have provided feedback. Thanks!
          Hide
          Matt Jordan added a comment -

          Posted to review board at https://reviewboard.asterisk.org/r/2728/

          Show
          Matt Jordan added a comment - Posted to review board at https://reviewboard.asterisk.org/r/2728/

            People

            • Votes:
              0 Vote for this issue
              Watchers:
              3 Start watching this issue

              Dates

              • Created:
                Updated:
                Resolved: