Review Board 1.7.16


rtp timestamp to timeval calculation fix

Review Request #468 - Created Jan. 20, 2010 and submitted

David Vossel
Reviewers
asterisk-dev
Asterisk
------Problem
The rtp timestamp field, from best I understand, starts at a marked value and increments by the number of samples contained in each packet.  When we receive a rtp timestamp we attempt to convert that value into a timeval to keep up with the actual time the timestamp represents.  This value is very important because it ends up representing the ast_frame's delivery timeval.  The delivery timeval is used during ast_translate to predict time the next frame should arrive.  If rtp incorrectly calculates the frame's delivery timeval, ast_translate will attempt to compensate for the new delivery time by adding the difference between what it expects and what it received to the translated frame's delivery time.

So, in ast_translate if the incoming frame's delivery time is 20ms above our expected next incoming time, the next outgoing delivery time has 20ms added to it.

Right now the conversion between the rtp timestamp to timeval appears to only be accurate for 8000khz audio.  The problem is primarily in how we calculate the tv_usec portion of the timeval.

-----Current method of timestamp to timeval calculation
Ideally the sec and usec result from all of these calculations should be the same since they represent the same length of audio.

    --8000 kHz Example
        timestamp = 1422080 (8888 packets, 160 samples, 20ms ulaw)
        rate = 8000;

        sec = timestamp / rate
        usec = (timestamp % rate) * 125
        result is sec: 177 usec: 760000

    --16000 kHz example
        timestamp = 2844160 (8888 packets, 320 samples, 20ms siren7)
        rate = 16000;

        sec = timestamp / rate
        usec = (timestamp % rate) * 125
        result is sec: 177 usec: 1520000

    --32000 kHz example
        timestamp = 5688320 (8888 packets, 640 samples, 20ms siren14)
        rate = 16000;

        sec = timestamp / rate
        usec = (timestamp % rate) * 125
        result is sec: 177 usec: 3040000

The usec value is incorrect for both the 16000kHz and 32000kHz calculation.  This results in 16000kHz delivery looking like it contains twice as much audio than it does, and 32000kHz containing 4 times as much.  In fact, the usec values for 16000kHz and 32000kHz are even above the max usec value allowed.  We attempt to account for this error by using the sanitize_tv function.

-----New method of timestamp to timeval calculation

    -- Method 1: modifying current method.
        sec = timestamp / rate
        usec = (((timestamp % rate) / (rate / 8000))) * 125

    -- Method 2: use time.h api.
        timeval = ast_samp2tv(timestamp, rate);


Method 2 is the one this patch currently implements.  Unless the rtp timestamp does not always reflect the number of samples incremented, I don't see any reason not to use this method.  Both methods have been tested.
siren7 to siren14 translation now works.  siren7 to ulaw works, siren14 to ulaw works.
Ship it!
Posted (Jan. 20, 2010, 8:50 a.m.)
This looks like exactly the right fix. Nice work.
Posted (Jan. 20, 2010, 9:03 a.m.)
Lets assume everywhere I refer to 8000kHz audio or 16000kHz audio I really mean 8kHz audio or 16kHz audio :)
Posted (Jan. 21, 2010, 5:29 a.m.)
I would like to see it tested with G.722 before it is shipped as I think the problem with RFC1890 may apply to the new method

RFC 3551 Section 4.5.2 states:
   Even though the actual sampling rate for G.722 audio is 16,000 Hz,
   the RTP clock rate for the G722 payload format is 8,000 Hz because
   that value was erroneously assigned in RFC 1890 and must remain
   unchanged for backward compatibility.  The octet rate or sample-pair
   rate is 8,000 Hz.


https://reviewboard.asterisk.org/ runs on a server provided by Digium, Inc. and uses bandwidth donated to the open source Asterisk community by API Digital Communications in Huntsville, AL USA.
Please report problems with this site to asteriskteam@digium.com.