[Home]

Summary:ASTERISK-24274: [patch]Codec Format Is Not Included in the SDP Media Attributes When SLIN48 Codec Is Used
Reporter:Frankie Chin (fchin)Labels:
Date Opened:2014-08-27 18:08:29Date Closed:2014-12-01 12:53:01.000-0600
Priority:MajorRegression?
Status:Closed/CompleteComponents:Core/RTP
Versions:11.12.0 12.4.0 Frequency of
Occurrence		Constant
Related
Issues:
must be completed before resolvingASTERISK-24314 ConfBridge Doesn't Deliver 48 kHz Audio with Local channels
Environment:Attachments:( 0) 24274_frame_patch.diff
( 1) 24274_rtp_patch.diff
( 2) sip_server_A.conf
( 3) sip_server_B.conf
Description:I first submitted this question in the forum and was later asked to submit it as an issue: http://forums.digium.com/viewtopic.php?f=13&t=91190&start=0&hilit=Confbridge&sid=9ee675f5b376c518edec9952fb0de9b5

I described the issue background and my findings in the forum. So I think I don't need to repeat it here. In summary:
1) When using SLIN16, the media attributes are: "m=audio 19942 RTP/AVP 118 101"
2) When using SLIN48, the media attributes are: "m=audio 17868 RTP/AVP 101"

Please find the attached sip.conf of Server A and B.

Another thing I noticed in Server A's debug log is that the Joint capabilities are (nothing). This is the same when using "slin16" as well.
[Aug 27 02:30:30] DEBUG[6124] chan_sip.c: *** Our native formats are (slin48)
[Aug 27 02:30:30] DEBUG[6124] chan_sip.c: *** Joint capabilities are (nothing)
[Aug 27 02:30:30] DEBUG[6124] chan_sip.c: *** Our capabilities are (slin48)
[Aug 27 02:30:30] DEBUG[6124] chan_sip.c: *** AST_CODEC_CHOOSE formats are slin48
[Aug 27 02:30:30] DEBUG[6124] chan_sip.c: *** Our preferred formats from the incoming channel are (slin)
[Aug 27 02:30:30] DEBUG[6124] chan_sip.c: This channel will not be able to handle video.

What other settings I need to configure in order to get some valid joint capabilities?
Comments:By: Frankie Chin (fchin) 2014-08-27 18:36:08.261-0500

I modified the "rtp_engine.c" in the "ast_rtp_engine_init()" to add in the following two lines...

set_next_mime_type(ast_format_set(&tmpfmt, AST_FORMAT_SLINEAR48, 0), 0, "audio", "L16", 48000);
add_static_payload(120, ast_format_set(&tmpfmt, AST_FORMAT_SLINEAR48, 0), 0);

I also modified "frame.c" in the "ast_codec_get_samples()" to add another switch case for slin48:
       
       case AST_FORMAT_SLINEAR:
       case AST_FORMAT_SLINEAR16:
       case AST_FORMAT_SLINEAR48:      
               samples = f->datalen / 2;
               break;

Now I'm able to get both servers talking using "slin48", which I verified using "sip show channels".
By: Frankie Chin (fchin) 2014-08-28 18:56:12.918-0500

Since I'm new to this JIRA forum, I'd like to mention that I wasn't trying to provide a patch for this issue. I think I just got lucky that I managed to get it working for me as a temporary solution. I'm still very puzzled by the log that said "Joint capabilities are (nothing)". Looking forward to hearing from you.
By: Rusty Newton (rnewton) 2014-09-02 18:26:09.789-0500

[~fchin] would you feel comfortable following the [Code Review|https://wiki.asterisk.org/wiki/display/AST/Code+Review] process to help get the patch into Asterisk? If so, please follow that guide and get the patch on Reviewboard. Then others can help you along.

We can't accept inline patches, we also need you to add the file using "More > Attach Files" and select code contribution. Note you'll only be able to do this after you sign the submission agreement (see link at the very top of JIRA).




By: Frankie Chin (fchin) 2014-09-02 19:52:52.667-0500

Hi Rusty, yes I can do that. I'll get myself familiar with the process first.
By: Matt Jordan (mjordan) 2014-09-03 09:25:39.077-0500

A few comments:

# It's rather odd to be offering signed linear in the first place. Is there a reason why you're doing that?
# This problem would most likely exist in Asterisk 11 as well. Luckily, formats are represented the same in both 11 and 12.
# You most likely wouldn't want to use the same MIME sub-type as 16 khz SLIN ("L16"). That would probably cause conflicts (or other badness) if Asterisk or another SIP endpoint offered L16.
# There's obviously a slew of other signed linear formats in addition to this one. If you do decide to write and attach a more formal patch (code in comments *cannot* be accepted), you'll want to take a look at handling all of those.
By: Frankie Chin (fchin) 2014-09-09 19:46:08.064-0500

Matt, referring to your previous comment #1, we want to be able to deliver 48 kHz high quality audio from one server to another. So I chose to use SLIN48 codec for the prototype I'm working on now. I'll take note of your other comments when creating the patch.

Rusty, this is what I plan to do to create the patch:
- Download the source from http://svn.asterisk.org/svn/asterisk/tags/12.4.0/
- Make the changes in rtp_engine.c and frame.c and create the patch.
- Sign the License Agreement and follow the Code Review process to submit the patch.

Does it sound right? Thanks.

By: Rusty Newton (rnewton) 2014-09-29 09:54:23.701-0500

{quote}
Rusty, this is what I plan to do to create the patch:

* Download the source from http://svn.asterisk.org/svn/asterisk/tags/12.4.0/
* Make the changes in rtp_engine.c and frame.c and create the patch.
* Sign the License Agreement and follow the Code Review process to submit the patch.

Does it sound right? Thanks.
{quote}

Yes, except you probably want to grab the latest head of SVN for the 12 and 11 branch. Once you have a patch working in one, you'll want to validate it works in the other branch, as well as Trunk.

The Code Review process will have you put it on Reviewboard at that point for others to review and test. Thanks.
By: Rusty Newton (rnewton) 2014-09-29 18:32:51.311-0500

[~fchin] thanks again for following through with the contribution process. As I mentioned in E-mail, you'll need to sign the [Digium License Agreement|https://wiki.asterisk.org/wiki/display/AST/Digium+License+Agreement] and then you'll be able to attach the patch to this issue (More Actions> Attach Files) and use Reviewboard.

Thanks a ton!
By: Frankie Chin (fchin) 2014-10-14 18:02:50.029-0500

Uploaded patches for code review.
By: Rusty Newton (rnewton) 2014-10-14 19:33:13.862-0500

Thanks. Once you get your code on Reviewboard you can edit this issue and add the Reviewboard URL in the Reviewboard field. That helps us keep track of things.
By: Joshua C. Colp (jcolp) 2014-12-01 12:53:01.357-0600

I have placed this change into Asterisk trunk. You may continue to use your patch but if you would like it in earlier versions tests can be provided which confirm that negotiation and functionality works.
By: Friendly Automation (friendly-automation) 2016-10-31 12:25:58.664-0500

Change 3680 had a related patch set uploaded by Alexander Traud:
rtp_engine: Allow more than 32 dynamic payload types.

[https://gerrit.asterisk.org/3680|https://gerrit.asterisk.org/3680]
By: Friendly Automation (friendly-automation) 2016-10-31 12:30:38.021-0500

Change 3680 had a related patch set uploaded by Alexander Traud:
rtp_engine: Allow more than 32 dynamic payload types.

[https://gerrit.asterisk.org/3680|https://gerrit.asterisk.org/3680]
By: Friendly Automation (friendly-automation) 2016-10-31 12:46:53.866-0500

Change 3681 had a related patch set uploaded by Alexander Traud:
rtp_engine: Allow more than 32 dynamic payload types.

[https://gerrit.asterisk.org/3681|https://gerrit.asterisk.org/3681]
By: Friendly Automation (friendly-automation) 2016-10-31 12:53:54.080-0500

Change 3682 had a related patch set uploaded by Alexander Traud:
rtp_engine: Allow more than 32 dynamic payload types.

[https://gerrit.asterisk.org/3682|https://gerrit.asterisk.org/3682]
By: Friendly Automation (friendly-automation) 2016-11-02 08:17:46.777-0500

Change 3680 had a related patch set uploaded by Alexander Traud:
rtp_engine: Allow more than 32 dynamic payload types.

[https://gerrit.asterisk.org/3680|https://gerrit.asterisk.org/3680]
By: Friendly Automation (friendly-automation) 2016-11-02 08:21:42.665-0500

Change 3680 had a related patch set uploaded by Alexander Traud:
rtp_engine: Allow more than 32 dynamic payload types.

[https://gerrit.asterisk.org/3680|https://gerrit.asterisk.org/3680]
By: Friendly Automation (friendly-automation) 2016-11-02 08:40:57.516-0500

Change 3681 had a related patch set uploaded by George Joseph:
rtp_engine: Allow more than 32 dynamic payload types.

[https://gerrit.asterisk.org/3681|https://gerrit.asterisk.org/3681]
By: Friendly Automation (friendly-automation) 2016-11-02 08:43:01.315-0500

Change 3682 had a related patch set uploaded by George Joseph:
rtp_engine: Allow more than 32 dynamic payload types.

[https://gerrit.asterisk.org/3682|https://gerrit.asterisk.org/3682]
By: Friendly Automation (friendly-automation) 2016-11-02 08:44:51.342-0500

Change 3680 had a related patch set uploaded by George Joseph:
rtp_engine: Allow more than 32 dynamic payload types.

[https://gerrit.asterisk.org/3680|https://gerrit.asterisk.org/3680]
By: Friendly Automation (friendly-automation) 2016-11-07 06:49:08.472-0600

Change 3680 merged by zuul:
rtp_engine: Allow more than 32 dynamic payload types.

[https://gerrit.asterisk.org/3680|https://gerrit.asterisk.org/3680]
By: Friendly Automation (friendly-automation) 2016-11-07 07:07:33.496-0600

Change 3682 merged by Joshua Colp:
rtp_engine: Allow more than 32 dynamic payload types.

[https://gerrit.asterisk.org/3682|https://gerrit.asterisk.org/3682]