getroot
commented
1 year ago Thanks for the great report. I will look into this soon.
Closed the-real-rusty-shackleford closed 1 year ago
getroot
commented
1 year ago Thanks for the great report. I will look into this soon.
the-real-rusty-shackleford
commented
1 year ago I could be very wrong here, but could it be that the lip sync clock needs to keep track of the number of times the SR's RTP timestamp has wrapped around? Something like the below patch?
diff --git a/src/projects/base/ovlibrary/lip_sync_clock.cpp b/src/projects/base/ovlibrary/lip_sync_clock.cpp
index e07d94f2..b121eeb1 100644
--- a/src/projects/base/ovlibrary/lip_sync_clock.cpp
+++ b/src/projects/base/ovlibrary/lip_sync_clock.cpp
@@ -77,7 +77,7 @@ std::optional<uint64_t> LipSyncClock::CalcPTS(uint32_t id, uint32_t rtp_timestam
std::shared_lock<std::shared_mutex> lock(clock->_clock_lock);
// The timestamp difference can be negative.
- auto pts = clock->_pts + ((int64_t)clock->_extended_rtp_timestamp - (int64_t)clock->_rtcp_timestamp);
+ auto pts = clock->_pts + ((int64_t)clock->_extended_rtp_timestamp - (int64_t)clock->_rtcp_timestamp - clock->_wraparound_count * std::numeric_limits<uint32_t>::max());
// This is to make pts start at zero.
if (_first_pts == true)
@@ -112,7 +112,19 @@ bool LipSyncClock::UpdateSenderReportTime(uint32_t id, uint32_t ntp_msw, uint32_
std::lock_guard<std::shared_mutex> lock(clock->_clock_lock);
clock->_updated = true;
+
+ if (rtcp_timestamp < clock->_rtcp_timestamp)
+ {
+ uint32_t delta = clock->_rtcp_timestamp - rtcp_timestamp;
+ if (delta > 0x80000000)
+ {
+ // wrap around
+ clock->_wraparound_count++;
+ }
+ }
+
clock->_rtcp_timestamp = rtcp_timestamp;
+
clock->_pts = ov::Converter::NtpTsToSeconds(ntp_msw, ntp_lsw) / clock->_timebase;
logtd("Update SR : id(%u) NTP(%u/%u) pts(%lld) timestamp(%u)", id, ntp_msw, ntp_lsw, clock->_pts, clock->_rtcp_timestamp);
diff --git a/src/projects/base/ovlibrary/lip_sync_clock.h b/src/projects/base/ovlibrary/lip_sync_clock.h
index 2b229cef..721e734d 100644
--- a/src/projects/base/ovlibrary/lip_sync_clock.h
+++ b/src/projects/base/ovlibrary/lip_sync_clock.h
@@ -24,6 +24,7 @@ private:
uint32_t _last_rtp_timestamp = 0;
uint64_t _extended_rtp_timestamp = 0;
uint64_t _pts = 0; // converted NTP timestamp to timebase timestamp
+ int64_t _wraparound_count = 0;
};
// Id, Clock
Counting the wrap around count is also a good approach. But I modified it and committed it by updating delta for consistency with other code. Could you please confirm if this commit solves your problem? thank you
https://github.com/AirenSoft/OvenMediaEngine/commit/a1ba0f66e81a93b0a46bf5bc9b339295cef48358
the-real-rusty-shackleford
commented
1 year ago Thank you, I will run my tests and let you know.
the-real-rusty-shackleford
commented
1 year ago Looks good! Thank you for fixing this.
[10-02 19:04:39.409] D [StreamMotor:52] LipSyncClock | lip_sync_clock.cpp:92 | Calc PTS : id(0) pts(351473604526817) final_pts(1211412884) last_rtp_timestamp(4294956813) rtp_timestamp(4294956813) delta(12000) extended_rtp_timestamp(4294956813)
[10-02 19:04:39.544] D [StreamMotor:52] LipSyncClock | lip_sync_clock.cpp:92 | Calc PTS : id(0) pts(351473604538817) final_pts(1211424884) last_rtp_timestamp(1517) rtp_timestamp(1517) delta(12000) extended_rtp_timestamp(4294968813)
Describe the bug When an RTCP sender report is used to adjust timestamps from an RTSP stream, the lip sync clock's pts does not appear to be calculated correctly. When the RTP timestamp, which is an unsigned 32-bit integer, rolls over, the PTS jumps by close to UINT32MAX.
The relevant code is here: https://github.com/AirenSoft/OvenMediaEngine/blob/master/src/projects/base/ovlibrary/lip_sync_clock.cpp#L80
When
clock->_rtcp_timestamprolls over to 0, nothing is being subtracted, which results in a large jump.This ultimately caused an issue when using the HLS dump feature. The PTS jumps by nearly UINT32MAX, which is 13.25 hours when the time base is 1/90000. This causes the HLS chunklist to become corrupted with a large jump in the timestamps. See the snippit below.
From
chunklist_0_video_bad_llhls.m3u8:I have seen the issue with three different brands of IP cameras (AXIS, Digital Watchdog, and Hanwa). I am not sure whether OME is handling this roll over incorrectly, or whether the cameras are not sending correct SR reports.
To Reproduce See code example below. If necessary I can provide more details on how to reproduce in OME. The basic idea is to connect to an IP camera's RTSP stream using RTPSPull, then start an HLS dump and let it run for 13.25 hours.
Expected behavior The calculated PTS should not jump.
Logs The jump occurs here.
The more complete logs are below, which go all the way back to the previous SR. You can see that the SR's RTP timestamp has rolled over from
4294855205to343909, and with that, the calculated pts jumps from1938055453to6233028895Server (please complete the following information):
Additional context The following C program demonstrates the issue.
Output: