signalwire / freeswitch

FreeSWITCH is a Software Defined Telecom Stack enabling the digital transformation from proprietary telecom switches to a versatile software implementation that runs on any commodity hardware. From a Raspberry PI to a multi-core server, FreeSWITCH can unlock the telecommunications potential of any device.
https://freeswitch.com/#getting-started
Other
3.48k stars 1.4k forks source link

RTP issue--"Incorrect timestamp" in packet trace (voice quality problems) #1651

Open richard-tkg opened 2 years ago

richard-tkg commented 2 years ago

Summary: large numbers of RTP packets show "Incorrect timestamp" in Wireshark

Details: Freeswitch version 1.10.7 -release-19-883d2cb662 64bit

Call is placed from a Zoiper client to a cellular telephone. Zoiper => Freeswitch => Bandwidth.com trunk/AT&T cellular telephone

The RTP streams are good in all directions except from Freeswitch to the Zoiper client (Stream 0). Stream 1 from Zoiper to Freeswitch shows no issues. A screenshot of the Wireshark RTP Stream Analysis is attached.

The issue occurs on all calls we have analyzed.

I can provide more details as needed. Any suggestions would be much appreciated as this has been a rather vexing issue for us, and our clients will be very happy when the issue is resolved.

image

mjerris commented 2 years ago

did you verify those messages are actually correct? for years i’ve seen that analyzer show stuff like that that was incorrect

mjerris commented 2 years ago

need a full log with debug and sip trace and a full pcap of both legs of the call with media to look at this issue at all

richard-tkg commented 2 years ago

Thanks much. I will check to see if the packets are actually broken and will send the additional debug information.

shaunjstokes commented 2 years ago

This appears to be the same issue we are experiencing. image

It only occurs on missed packets when using record_session or uuid_record. When there is a missed packet on Leg A, FS generates the missing packet on Leg B using a different timestamp. This results in Leg B hearing silence when RTP packets are being passed from Leg A to Leg B.

The issue is resolved in version 1.10.1 and is present from version 1.10.4, I haven't been able to test 1.10.2 or 1.10.3 due to build problems.

shaunjstokes commented 2 years ago

The issue is caused by this commit: https://github.com/signalwire/freeswitch/commit/47c5c8f3e8957c37fad5feeeb791375d05992b93

To resolve the issue, we simply need to re-apply the following line in 'src/switch_rtp.c'. if (!ts) ts = rtp_session->last_write_ts + rtp_session->samples_per_interval;

The above statement is triggered for FS generated RTP packets on an active session, and will result in FS using the next RTP timestamp instead of generating a new timestamp, surely this is correct?

shaunjstokes commented 2 years ago

The change was authored by @anthmFS and committed by @andywolk , can you please explain why this line was removed?

shaunjstokes commented 2 years ago

This is also related to issue #534, and possibly #801.

richard-tkg commented 2 years ago

@shaunjstokes Thanks for your work on this. We upgraded to 1.10.7 -release-19-883d2cb662 a few weeks ago but user reports of audio quality are mixed. Did I understand you to say that the issue is likely present in 1.10.4 and higher? If so I'd be tempted to go back to 1.10.1 until a known good fix is in place.

shaunjstokes commented 2 years ago

@richard-tkg we were able to reproduce the issue consistently once we understood what triggered it. That's correct, the problem does not exist in 1.10.1, but going back that far isn't an option for us as it introduces other issues.

I've attached the patch that we're using. To use the patch, just unzip it into your FS source directory, then use git apply switch_rtp_use_last_write_ts.diff before the make command when building from source.

switch_rtp_use_last_write_ts.zip

I wanted to get some feedback from the FS Devs before I go ahead and create a pull request.

richard-tkg commented 2 years ago

@shaunjstokes On our existing servers, Freeswitch was installed from Debian packages. I have also built FS from source and applied your patch. I'd like to keep the existing package-based installation as-is and modify only as few files as necessary to apply the newly built version of Freeswitch. Have you done this before? Any idea if it's possible and if so, is it advisable?

shaunjstokes commented 2 years ago

@richard-tkg the packaged version is pre-compiled, it would be best to install the source code version to a separate directory, apply your configuration, then update the FS service to use the source code version. Initially we used the packaged version of FreeSWITCH, but we found the source code version to be much more versatile, we've never looked back.

emaktech commented 2 years ago

So it seems that this line was removed after this discussion: https://github.com/signalwire/freeswitch/issues/146

Can anybody confirm adding this line back fixes the issue described here? Because we have this too and it's a huge issue whenever there's dropped packets or jitter on the A leg. e.g. Mark bits and packet timestamps jumping all over the place to huge numbers causing audio to cut out on B leg.

EDIT: Did a build from source with this change and ran some tests and definitely seems to fix this issue! Will have to do a lot more testing to confirm though. Thanks @richard-tkg for submitting and @shaunjstokes for the suggested fix.

shaunjstokes commented 2 years ago

Well spotted @emaktech, so the final patch needs to determine "when files are being played back" as mentioned by @htrendev in #146. I'm not yet sure how we can achieve that, perhaps there's someone here that can help?

emaktech commented 2 years ago

Indeed, it seems that at present we need to pick between timestamp issues on phrase playback or timestamp issues with lost/dropped packets :(

FYI I found the original commit by @mjerris here. Obviously this issue had been reported and patched in the past, but unfortunately caused https://github.com/signalwire/freeswitch/issues/146

Somebody please send help!

emaktech commented 2 years ago

So we have made some progress on this. During FreeSWITCH office hours today, Brian West pointed me toward some undocumented dialplan variables that can be set. Or rather, they are documented, but only in the source code here:

https://github.com/signalwire/freeswitch/blob/9df85738e10d80a36c4f84807fbc30c8e831ee11/src/include/switch_types.h#L969

You can try setting rtp_manual_rtp_bugs to a couple of different values and see if it helps:

Try RTP_BUG_SEND_LINEAR_TIMESTAMPS and RTP_BUG_GEN_ONE_GEN_ALL (and maybe others, read through the list starting at line 856 and see what you find).

Basically do a set (or export) rtp_manual_rtp_bugs=RTP_BUG_GEN_ONE_GEN_ALL or any of the others above and see if it helps!

shaunjstokes commented 1 year ago

@emaktech some of those variables might fix the issue, but they also go further than what's required, we did testing with various RTP bug variables before coming here.

The problem is very specific, when there are missing RTP packets on Leg A, we still want the RTP packets generated by FreeSWITCH on Leg B to use the next timestamp which is logical. Otherwise, FreeSWITCH should pass on the RTP timestamp from Leg A to Leg B, so the patch is still the best option for us.

Kobyhud commented 9 months ago

We can confirm this issue. Building from source and making the change recommended by shaunjstokes does seem to solve the issue that we experience. In our case it is manifested only on uuid_recorded outbound channels. Certain endpoints really cannot seem to deal with the timestamp re-sequences very well at all. In our case the problematic endpoint is Counterpath's Bria. Based on our testing shaunjstokes patch doesn't have any effect on phrases. We see RTP timestamp issues both before and after this patch.

olofssonanton commented 6 months ago

I can confirm that recorded sessions still suffer from this issue on v1.10.11, and reverting https://github.com/signalwire/freeswitch/commit/47c5c8f3e8957c37fad5feeeb791375d05992b93 as suggested by @shaunjstokes resolves the issue.

olofssonanton commented 6 months ago

Although the real question is why are there packages being generated at all?

Inserting packages into the audio stream as soon as there are seemingly missing packets will negate the effect of any jitter buffer on the receiving end, while simultaneously distorting the audio.

I assume it is done for the sake of the recording, but this should be possible without affecting the packets written to the receiver. Perhaps even without introducing any distortion into the recording either, by re-ordering packets in a jitter buffer before writing to the file?

zusrut commented 1 month ago

when do uuid_record with RECORD_READ_ONLY=true for per leg recording - on any jitter in recording freeswitch inputs silence and then when packet appears it append it after this extra silence. so leg recordings have different duration and after some duration starts overlapping. here example call with constant beep playback - with this stats:

in_jitter_packet_count:0
in_dtmf_packet_count:0
in_cng_packet_count:0
in_flush_packet_count:27
in_largest_jb_size:0
in_jitter_min_variance:110.50
in_jitter_max_variance:474.53
in_jitter_loss_rate:0.00
in_jitter_burst_rate:1.00
in_mean_interval:20.50
in_flaw_total:1123

and got such picture:

image

for better jitter the picture is better but bad anyway . increase of param auto-jitterbuffer-msec helps but with own cons. issue still can appear on worse quality calls