[Home]

Summary:ASTERISK-02263: [patch] Unable to call X-lite using SpeeX
Reporter:pfn (pfn)Labels:
Date Opened:2004-08-24 01:18:30Date Closed:2008-01-15 15:05:49.000-0600
Priority:MinorRegression?No
Status:Closed/CompleteComponents:Core/CodecInterface
Versions:Frequency of
Occurrence
Related
Issues:
Environment:Attachments:( 0) rtp.c.txt
( 1) rtpdiff.txt
( 2) rtpdiff[1].txt
( 3) rtpdiff[1].txt
( 4) speex.txt
Description:Anytime I call X-lite, I get the following notice repeated 50 times per second (20ms) and the sound from X-lite to me is garbled and *barely* intelligible.

Aug 23 22:51:37 WARNING[20494]: codec_speex.c:155 speextolin_framein: Out of buffer space

(Ignore the line number)


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

It appears that there should not be an outer loop around the speex_decode call in codec_speex.c.  Attached is a patch that fixes this behavior.

I have tested this patch against NuFone's Echo server, and to one of my own * boxes across the 'net with this patch installed.  I was not able to do the Echo test on my box (for some reason nothing echo'd back) but I was able to complete a call in each direction using SpeeX.  If someone else could test this across 2 Asterisk boxes, I'd feel much more comfortable about this being 100%.  For me, it is working 100%.
Comments:By: pfn (pfn) 2004-08-25 03:44:45

Waitaminute, there's more to this bug than I thought.  Asterisk RTP behavior is incorrect when it comes to payload type.  When receiving RTP, the payload type lookup must be from the local PT table, not the remote's.  When sending RTP, the payload type must match the remote's value.

The problem here is, we have a dynamic payload type mismatch between X-lite and Asterisk, there is a .reg file in CVS to "fix" this, but that is really just a kludge, Asterisk should be able to handle the differing payload types internally.  I will submit a patch for this shortly.

----------------

I have uploaded the change to rtp.c; the speex diff still applies, although, I don't know if it's still necessary?  It still works correctly regardless.

With the RTP change, the registry files for X-lite and X-pro are no longer necessary as the payload type handling in * will be correct.

edited on: 08-25-04 03:54

edited on: 08-25-04 03:55
By: zoa (zoa) 2004-08-25 03:55:53

coooooooool!
By: pfn (pfn) 2004-08-25 04:05:13

Oh, one more note, this diff may break SpeeX 'tween * to *.  The IETF/RFC draft for SpeeX specifies that speex be lowercase, not SPEEX in the rtpmap.

-----

Bleh, tired, can't sleep, quote from the speex rfc draft:

5. SDP usage of Speex

  When conveying information by SDP [4], the encoding name MUST be
  set to "speex".  An example of the media representation in SDP for
  offering a single channel of Speex at 8000 samples per second might
  be:

m=audio 8088 RTP/AVP 97
a=rtpmap:97 speex/8000

  Note that the RTP payload type code of 97 is defined in this media
  definition to be 'mapped' to the speex codec at an 8kHz sampling
  frequency using the 'a=rtpmap' line.  Any number from 96 to 127
  could have been chosen (the allowed range for dynamic types).

edited on: 08-25-04 04:07
By: Mark Spencer (markster) 2004-08-25 08:30:15

The patch isn't quite accurate.  Originally I thought, like you, that we should use our own static RTP types for the response, but this, as logical as it sounds and as clean as it would have been to implementors, turns out to be discouraged as per RFC 3264 section 6.1:

  In the case of RTP, if a particular codec was referenced with a
  specific payload type number in the offer, that same payload type
  number SHOULD be used for that codec in the answer.  Even if the same
  payload type number is used, the answer MUST contain rtpmap
  attributes to define the payload type mappings for dynamic payload
  types, and SHOULD contain mappings for static payload types.  The
  media formats in the "m=" line MUST be listed in order of preference,
  with the first format listed being preferred.  In this case,
  preferred means that the offerer SHOULD use the format with the
  highest preference from the answer.

So, we're faced with a choice...  The ideal thing is to always use *our* numbers in our answer, but clearly the RFC suggests we don't, and the only reason this was changed in the first place was because of incompatibilities with other programs (including xlite if I remember correctly).  A quick search of the bug tracker should help you find the bug numbers that made us change it in the first place.
By: pfn (pfn) 2004-08-25 10:13:00

I don't know?  I'm taking a look at RFC 3264 right now and my interpretation is saying that we use our own static PT list?  It is saying that the answering payload type must match that of the offer.  So if the remote end offers iLBC as 105, we must send to the remote end as payload type 105 and vice versa:

  In the case of RTP, if a particular codec was referenced with a
  specific payload type number in the offer, that same payload type
  number SHOULD be used for that codec in the answer.

This is vague to me, though, as to whether they're saying the SDP payload must match, or if the RTP must match.  Preferably the latter case.

The only other bug # I could find relating to payload types was 1780, and the reporter wasn't very clear in that case... It seems they are also complaining about this same issue?  I see that rtp.c r1.21 was changed to match the behavior I'm describing, but I can't tell when it was changed to the current.

As for incompatibility with other programs, incompatibility with X-lite is how I stumbled across this.  My experience with SIP is pretty limited so I haven't had a chance to run into other problems with dynamic payload types.

edited on: 08-25-04 10:14
By: pfn (pfn) 2004-08-25 14:30:48

Erm, this patch breaks X-lite calling into *

I am looking into this.

--

Because when X-lite calls, * advertises the same payload type that X-lite has.  When * receives the call, the dynamic payload isn't being looked up anymore.

edited on: 08-25-04 14:36
By: pfn (pfn) 2004-08-25 15:45:48

Ok, uploaded another diff that fixes the inbound issue.  Needed to touch chan_mgcp and chan_sip because of this change.  Added another function to specifically look up the payload type out of the static PT table, use this function when advertising our capabilities.
By: Mark Spencer (markster) 2004-08-25 16:12:43

It says the payload type *number* must match.  In other words, you can't reuse the same numbers for a different codec, so, alas, no dice.  Thank the IETF for that bit of genius.
By: pfn (pfn) 2004-08-25 16:23:58

Mark,

I don't see where it says payload numbers *must* match on both sides in SDP?  Offer is the SDP, answer is the RTP packet, yes?  So, the numbers *do* match.

*SIGH*  I guess they do mean the SDP answer.  Silly IETF.

But the end of 6.1:
 
  <snipped>
  answer.  In the case of RTP, it MUST use the payload type numbers
  from the offer, even if they differ from those in the answer.

edited on: 08-25-04 16:39
By: pfn (pfn) 2004-08-25 16:52:52

Ok, one more note on this subject, from section 5.1 of the same rfc, specifying Unicast Offers:

  the offerer is planning to send for that codec.  For sendrecv RTP
  streams, the payload type numbers indicate the value of the payload
  type field the offerer expects to receive, and would prefer to send.
  However, for sendonly and sendrecv streams, the answer might indicate
  different payload type numbers for the same codecs, in which case,
  the offerer MUST send with the payload type numbers from the answer.

     Different payload type numbers may be needed in each direction
     because of interoperability concerns with H.323.

  As per RFC 2327, fmtp parameters MAY be present to provide additional
  parameters of the media format.

So, having the same PT is recommended but not necessary.  It wouldn't be going against RFC to do so...
By: Mark Spencer (markster) 2004-08-25 17:05:57

The offer in this case is the SDP of the "INVITE" and the answer is the SDP in the "200 OK"

It would not go against the RFC however we have found practical issues related to this.  Again search the bug tracker and you'll see what we're talking about.

It could potentially be done as a global option in rtp.conf
By: pfn (pfn) 2004-08-25 17:28:23

Ugh, the notes from me never end!

In any case, there is an error in handling the RTP payload type on *.  Whenever there is a payload mismatch, we must send using the remote end's PT and receive using our own PT values.  When the values match up, then we must send and receive expecting the same value.  The first case is not currently true in cvs.

We send an offer to X-lite, 97 for ILBC, X-lite comes back with 97 for speex and 98 for ILBC.  X-lite sends to us using 97, we send to X-lite 97.  We parse 97 as speex, where we should really be parsing it as ILBC.

Now lets take the reverse:

X-lite sends an offer to us 98 for ILBC and 97 for speex, we answer with 98 for ILBC.  X-lite sends to us using 98 and we send to X-lite using 98.  We parse 98 correctly as ILBC.

To be fully compliant with rfc3264, we need to be able to support the former case.  My patch supports only the first and totally ignores the latter case...
By: pfn (pfn) 2004-08-25 23:39:12

Ok, I've made one last patch, and this one appears to satisfy rfc3264.

When we are offering, always do the local PT lookup out of the static table.  If there is a PT mismatch, we must always send using the remote PT and receive using our local PT.  If the payload types match, we have to use our static payload type anyway (static would be the same as negotiated value).

For every other situation, we can safely use the negotiated payload type.

I have been able to successfully test this on my system between X-lite and my 7960.  Speex and iLBC both work without the registry patch.  Please apply both speex.txt and rtpdiff.txt
By: Mark Spencer (markster) 2004-08-25 23:59:51

I've applied the rtp portion of this but not the speex portion because Asterisk has to be able to combine frames together into a single unit.  As a result, it will be important to, within RTP, find a way to apply the termination to the bit stream.
By: Mark Spencer (markster) 2004-08-26 00:00:53

I'm going to go ahead and close this one out.  If you want to prepare a proper fix for speex, i'll certainly be interested.  Nice work on the RTP.
By: Mark Spencer (markster) 2005-12-20 10:53:23.000-0600

I just got rid of the old local RTP stuff because I'm now using the incoming / outgoing SDP's properly.  Please confirm I've not broken anything for you (re bug ASTERISK-2263) and send me a private e-mail at markster@digium.com, thanks!
By: Digium Subversion (svnbot) 2008-01-15 15:05:49.000-0600

Repository: asterisk
Revision: 3658

U   trunk/channels/chan_mgcp.c
U   trunk/channels/chan_sip.c
U   trunk/include/asterisk/frame.h
U   trunk/include/asterisk/rtp.h
U   trunk/rtp.c

------------------------------------------------------------------------
r3658 | markster | 2008-01-15 15:05:49 -0600 (Tue, 15 Jan 2008) | 2 lines

Repair offer/answer model (bug ASTERISK-2263), initial CNG work for new frametype

------------------------------------------------------------------------

http://svn.digium.com/view/asterisk?view=rev&revision=3658