Suspicious code in rtp_send_frame ( libtrau/rtp_proxy.c )
Hi, At each event since like 27c3 (or maybe even 26c3), we comment out a part of the rtp_send_frame method to solve some audio issues (glitch and sometime progressively increasing delay). This is the piece of code in question: { struct timeval tv, tv_diff; long int usec_diff, frame_diff; gettimeofday(&tv, NULL); tv_difference(&tv_diff, &rs->transmit.last_tv, &tv); rs->transmit.last_tv = tv; usec_diff = tv_diff.tv_sec * 1000000 + tv_diff.tv_usec; frame_diff = (usec_diff / 20000); if (abs(frame_diff) > 1) { long int frame_diff_excess = frame_diff - 1; LOGP(DLMUX, LOGL_NOTICE, "Correcting frame difference of %ld frames\n", frame_diff_excess); rs->transmit.sequence += frame_diff_excess; rs->transmit.timestamp += frame_diff_excess * duration; } } introduced by : commit 392736d38bf9dae29800bde630c9ceee8a3914f3 Author: Harald Welte <laforge@netfilter.org> Date: Sun Dec 20 13:16:14 2009 +0100 RTP Proxy: Fix RTP sequence number and timestamp in case of dropped frames During handover, we will not send RTP frames for quite some time. However, the way the rtp_send code is structured, it will increment the timestamp with a fixed amount every time we send a frame, independent how much wallclock time has actually passed. This code is a hack to update the sequence number and timestamp in case it seems to be wrong. It makes handover much more reliable. Does this speak to anyone ? Cheers, Sylvain
hi sylvain, Sylvain Munaut wrote:
if (abs(frame_diff) > 1) { long int frame_diff_excess = frame_diff - 1;
LOGP(DLMUX, LOGL_NOTICE, "Correcting frame difference of %ld frames\n", frame_diff_excess); rs->transmit.sequence += frame_diff_excess; rs->transmit.timestamp += frame_diff_excess * duration; }
...
This code is a hack to update the sequence number and timestamp in case it seems to be wrong. It makes handover much more reliable.
Does this speak to anyone ?
iirc, it is mandatory to maintain sequence/timestamp over time. some RTP endpoints (like nanoBTS) do not like significant change of these sequence numbers and will drop received RTP frames, because they are too much off the expected time then. they actually adapt only slight changes but no big jumps. maybe it makes sense to increase the number of absolute frame difference. with what setup did you have bad audio? regards, andreas
On Thu, Aug 08, 2013 at 03:19:31PM +0200, Sylvain Munaut wrote:
Hi,
At each event since like 27c3 (or maybe even 26c3), we comment out a part of the rtp_send_frame method to solve some audio issues (glitch and sometime progressively increasing delay).
I think it should just be removed. holger
Hi Holger and Sylvain, On Thu, Aug 08, 2013 at 04:05:42PM +0200, Holger Hans Peter Freyther wrote:
At each event since like 27c3 (or maybe even 26c3), we comment out a part of the rtp_send_frame method to solve some audio issues (glitch and sometime progressively increasing delay).
I think it should just be removed.
As far as I know, if it is removed, then it will break hand-over support for nanoBTS (for reasons that Andreas has just pointed out). So that should be verified (or the code made conditional?) Regards, Harald -- - Harald Welte <laforge@gnumonks.org> http://laforge.gnumonks.org/ ============================================================================ "Privacy in residential applications is a desirable marketing option." (ETSI EN 300 175-7 Ch. A6)
Hi,
As far as I know, if it is removed, then it will break hand-over support for nanoBTS (for reasons that Andreas has just pointed out). So that should be verified (or the code made conditional?)
Well, if it's required for handover, making it optional wouldn't help much. At theses events we'd probably want handover once it's supported/working. I think the main issue is that this code is way too strict. It just shouldn't be triggered unless there is a big discontinuity. During the execution we can see the message that it corrected 1 or 2 frames, but that's probably just because of our delay in processing frames and/or due to small difference in clock rate between the device and the openbsc machine. In both of theses cases, the receiving device should handle that itself. AFAIU a handover would be a massive jump of timestamp. We might also be able to detect that the source of the stream changed and only resync in that case. Is there any handover setup that's supported / working with the HEAD code so that I could try locally ? (I never even tried handover anywhere beforce ...) Cheers, Sylvain
Hi Sylvain, On Wed, Aug 14, 2013 at 11:13:54AM +0200, Sylvain Munaut wrote:
AFAIU a handover would be a massive jump of timestamp. We might also be able to detect that the source of the stream changed and only resync in that case.
agreed.
Is there any handover setup that's supported / working with the HEAD code so that I could try locally ? (I never even tried handover anywhere beforce ...)
it _should_ work with current master/HEAD out-of-the box, just use OsmoNITB with two nanoBTS (if you have them...), activate the rtp proxy by command line switch, establish a voice call and then either manually trigger a handover with the VTY command for it, or use something like a variable/switchable attenuator to trigger the HO algorithm. Regards, Harald -- - Harald Welte <laforge@gnumonks.org> http://laforge.gnumonks.org/ ============================================================================ "Privacy in residential applications is a desirable marketing option." (ETSI EN 300 175-7 Ch. A6)
Harald Welte wrote:
it _should_ work with current master/HEAD out-of-the box, just use OsmoNITB with two nanoBTS (if you have them...), activate the rtp proxy by command line switch, establish a voice call and then either manually trigger a handover with the VTY command for it, or use something like a variable/switchable attenuator to trigger the HO algorithm.
it is actually working out of the box with nanobts. only required is one bts, because handover can be triggered via vty (subscr exten xxxx handover 0). because handover requires a different channel, it will start with a new rtp stream, so sequence numbers should be different. handover works with umtrx and calypso-bts (osmo-bts), but not from master.
Hi Andreas, On Wed, Aug 14, 2013 at 01:19:48PM +0200, Andreas Eversberg wrote:
it is actually working out of the box with nanobts. only required is one bts, because handover can be triggered via vty (subscr exten xxxx handover 0). because handover requires a different channel, it will start with a new rtp stream, so sequence numbers should be different.
That's exactly the reason why I specifically asked for two nanoBTS. The timing behavior of handing over between two channels on the same BTS is not realistic, as both channels are clocked by the same master clock, and thus do not have unsynchronized timing behavior. Yes, the SSRC _might_ jump (technically the RTP specs would not require it in this case), but afther that one jump it would have exactly the same timing as before the HO - unlike the case where you hand-over to another BTS. -- - Harald Welte <laforge@gnumonks.org> http://laforge.gnumonks.org/ ============================================================================ "Privacy in residential applications is a desirable marketing option." (ETSI EN 300 175-7 Ch. A6)
participants (4)
-
Andreas Eversberg -
Harald Welte -
Holger Hans Peter Freyther -
Sylvain Munaut