[Speex-dev] New jitter.c, bug in speex_jitter_get?

[SteveK]

On May 3, 2006, at 7:40 PM, Jean-Marc Valin wrote:

>
>> Yes.  Jean-Marc has made the API more similar.
>>
>> Jean-Marc:  Have you looked at the API we have for the
>> asterisk/iaxclient jitterbuffer?
>
> Just did.
>
>> It's pretty close to what you have now -- the major difference is  
>> that
>> your jb still assumes it can "own" the data passed in -- it copies  
>> it,
>> and it destroys it at will.  With the API I put together, the
>> jitterbuffer only keeps a poiunter to the data, and considers it
>> opaque -- if it wants to destroy the data, it needs to return the
>> pointer with a flag, and then the caller is responsible for whatever
>> cleanup is needed.
>
> Yeah, I wasn't sure about that one. The problem is what to do when you
> need to discard. You seem to say you return it and ask the user to
> destroy it (how?), but this seemed a bit error prone to me.

We just return a frame with the return value JB_DROP, which tells the  
caller to drop this frame, and call jb_get again.

When the caller is done with the jitterbuffer, it calls jb_getall()  
repeatedly, until it's empty, and then it can discard all the frames.


> And it's not
> like copying data is that expensive compared to decoding it.

Perhaps it's not expensive, but it's unnecessary..  It also means  
that the jitterbuffer's pointers can point to structures, or other  
types of data, and the jitterbuffer doesn't need to understand them.   
In particular, it's designed to be able to buffer and reorder frames  
(things) which aren't audio -- like video and control frames.


> Another
> comment is that I think it's not a good idea to expose the jb  
> structs in
> the header file.

Yeah, that's just laziness.   struct jb_frame needs to be defined, as  
does the struct which passes in the settings jb_conf, IIRC.  The rest  
should/could be made private in jitterbuf.c.


>
>> Here's that API:
>> http://svn.sourceforge.net/viewcvs.cgi/iaxclient/trunk/iaxclient/ 
>> lib/jitterbuf.h?view=markup&rev=470
>
> A couple things I don't understand. Why do you need both the local  
> clock
> and the remote clock and how do you define those anyway?

There's the "timestamp", which the remote side puts on frames, and  
the local time, which is used for jb_put and jb_get.  They're defined  
in milliseconds.


> BTW, does your
> jitter buffer consider that there can be overlaps or holes between
> frames, especially if using PCM?

There shouldn't be overlaps, but I think it currently deals with some  
types of messy timestamps that are, for example, +- one frame length  
from precise.  (i.e. sequences of 20 ms frames with timestamps like  
0, 18, 42, 55, 82, instead of 0, 20, 40, 60, 80)..

>
>> It would be cool if the speex JB could use a compatible API, then
>> people could always mix/match these things, and see which one works
>> best for different situations.
>
> You could always write a sort of abstraction. I'm not sure if we could
> use *exactly* the same API -- unless you wish to use the Speex one :-)
> That being said, I'm open to changes, especially in cases where what I
> have would be too limiting for a particular application. Any
> suggestions?

For reference, the API I've shown is already used in asterisk and  
iaxclient.  There are two other jitterbuffers that I know of which  
are using basically the same API:  A non-adaptive jitterbuffer, and  
another one which more closely follows the research models I was  
looking at when I wrote mine, by Jesse Kaijen.  The former is in one  
of asterisk's SVN branches and the bugtracker, and I think that  
Jesse's is at speakup.nl.

My jitterbuffer is by no means perfect -- mainly it seems to still  
have situations where there's garbage input which confuses it in a  
way that I'd hope it could recover from.  (i.e. nonsensical  
timestamps that it gets sent).

-SteveK