[Home]

Summary:ASTERISK-09776: add support for wideband speex (Openwengo variant)
Reporter:Adam Gundy (adamgundy)Labels:
Date Opened:2007-08-21 17:08:28Date Closed:2011-06-07 14:02:35
Priority:MajorRegression?No
Status:Closed/CompleteComponents:Codecs/codec_speex
Versions:Frequency of
Occurrence
Related
Issues:
Environment:Attachments:( 0) 20071106__format_conservation.diff.txt
( 1) astwb.pat
( 2) astwb2.pat
Description:the attached patch adds support for wideband speex to asterisk 1.4.10.1

it relies on the fact that the speex codec (in asterisk or the client) will transparently handle conversion between wide and narrow band speex, so there are no changes to the codec, we just register a new type. asterisk will either (a) allow wideband packets to pass through client<->client (sounds great!), or when actually talking to asterisk (voicemail, prompts etc), it will send or decode as narrowband speex, so quality while talking to asterisk itself is only narrowband (but of course this means minimal changes since asterisk still deals with 8khz audio). this is all transparent to the caller, apart from the lower quality while talking to asterisk itself.

the only complex part of the patch is about teaching rtp.c and frame.c about 16khz codecs, so that the RTP timestamps are set correctly.

openwengo, for some odd reason, insist on sending speex wideband as 'G726-64wb', with a different RTP type. this patch adds support for that. it should be trivial to add support for 'speex/16000', for eg ekiga, but that's another patch... and asterisk will have to 'transcode' between them, because the RTP types don't match :-(
Comments:By: Ronald Chan (loloski) 2007-08-22 06:47:55

openwengo, for some odd reason, insist on sending speex wideband as 'G726-64wb', with a different RTP type. this patch adds support for that. it should be trivial to add support for 'speex/16000', for eg ekiga, but that's another patch... and asterisk will have to 'transcode' between them, because the RTP types don't match :-(

You forgot to attach the patch :)

By: Adam Gundy (adamgundy) 2007-08-22 09:25:27

depending on the comments on the initial patch (code style, the concept of handling a wideband codec as narrowband internally, and codec name I guess) I might take a stab at adding 'proper' speex/16000 for ekiga and others.

first, I'd have to persuade ekiga to actually work properly on my machines..

as a side note, we're getting pretty close to running out of bits for new codecs.. what's the plan for that?

By: Adam Gundy (adamgundy) 2007-08-22 21:18:50

second version of the patch uploaded, I removed references to G726_64_WB everywhere but the MIME type used in the SDP. there's no need to propagate OpenWengo's broken idea of speex naming anywhere but the SIP SDP. the codec name is now 'speexwb-wengo', eg 'allow=speexwb-wengo'

By: Jason Parker (jparker) 2007-09-04 18:02:00

For the most part, this looks okay (except that ast_codec_get_rate returns 8000 for g722 :).

If you could figure out how to do this without taking up a new codec number, that would be pretty awesome.

By: Adam Gundy (adamgundy) 2007-09-05 09:54:50

I return 8000 for g722 because that is how asterisk currently behaves (ie without the patch), since everything uses hard coded 8kHz. since I can't test g722, I don't want to break it with this patch...  and it obviously works currently because people are using it. now you could be correct and asterisk might be generating garbage timestamps for g722 when playing back recorded audio, someone with a g722 phone could test that I guess.

unfortunately we need a new codec number, because we have to spot the difference between speex/8000 and wengo's speex at 16kHz, otherwise rtp.c puts incorrect timestamps in the packets, and the audio sounds *very* odd. we'll also need another codec number for true speex/16000 since we need to know which RTP type to put in the packets (it's not the same number as wengo uses, apparently they use 'cirpack'? servers which won't pass unknown types) :-(

the fact that we're using the same codec code for all three doesn't help much, since it's the RTP code which needs to know the difference, for the type number, and the rate.

By: Tilghman Lesher (tilghman) 2007-11-06 16:58:30.000-0600

Okay, two things:

1.  This patch is against 1.4.  That version is frozen for new features, so your patch needs to be against trunk.

2.  We have now added native slin16 support to trunk, so your codec translation should provide a native slin16 translator, so that speex16 to g722 can be natively translated.

By: Dmitry Andrianov (dimas) 2007-11-06 17:14:07.000-0600

To me it looks like significant part of your patch is not related to new codec at all - I'm talking about introduction of AST_FORMAT_AUDIO_MASK and use of it instead of AST_FORMAT_MAX_AUDIO. While I agree it is good thing to do in general, I'm not sure it is a good idea to mix such a "code cleanup" changes with actually new features like addition of new codec in the same svn commit...

By: Tilghman Lesher (tilghman) 2007-11-06 17:57:13.000-0600

dimas:  ignore my patch.  It was a cleanup patch, inspired by this one.

By: Tilghman Lesher (tilghman) 2007-12-07 14:34:29.000-0600

No response from reporter.