Skip to content

Commit 57e1622

Browse files
whytolearnduiniuluantanqinossrs-ai
authored
WebRTC: Fix NACK recovered packets not being added to receive queue. v7.0.78 (#4467)
Fixes a bug in WebRTC NACK packet recovery mechanism where recovered packets were being discarded instead of processed. In `SrsRtcRecvTrack::on_nack()`, when a retransmitted packet arrived (found in NACK receiver), the method would: 1. ✅ Remove the packet from NACK receiver (correct) 2. ❌ Return early without adding the packet to RTP queue (BUG) This caused recovered packets to be lost, defeating the purpose of the NACK mechanism and potentially causing media quality issues. Restructured the control flow in `on_nack()` to ensure both new and recovered packets reach the packet insertion logic: - **Before**: Early return for recovered packets → packets discarded - **After**: Conditional NACK management + unified packet processing → all packets queued Closes #3820 --------- Co-authored-by: Haibo Chen <[email protected]> Co-authored-by: OSSRS-AI <[email protected]>
1 parent 6720e96 commit 57e1622

File tree

6 files changed

+541
-15
lines changed

6 files changed

+541
-15
lines changed

trunk/configure

Lines changed: 1 addition & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -50,7 +50,6 @@ if [[ $SRS_GENERATE_OBJS == YES ]]; then exit 0; fi
5050
#####################################################################################
5151
# ubuntu echo in Makefile cannot display color, use bash instead
5252
SRS_BUILD_SUMMARY="_srs_build_summary.sh"
53-
5453
# utest make entry, (cd utest; make)
5554
SrsUtestMakeEntry="@echo -e \"ignore utest for it's disabled\""
5655
if [[ $SRS_UTEST == YES ]]; then SrsUtestMakeEntry="\$(MAKE)\$(JOBS) -C ${SRS_OBJS}/${SRS_PLATFORM}/utest"; fi
@@ -379,7 +378,7 @@ if [[ $SRS_UTEST == YES ]]; then
379378
"srs_utest_mp4" "srs_utest_service" "srs_utest_app" "srs_utest_rtc" "srs_utest_config2"
380379
"srs_utest_protocol" "srs_utest_protocol2" "srs_utest_kernel2" "srs_utest_protocol3"
381380
"srs_utest_st" "srs_utest_rtc2" "srs_utest_rtc3" "srs_utest_fmp4" "srs_utest_source_lock"
382-
"srs_utest_stream_token")
381+
"srs_utest_stream_token" "srs_utest_rtc_recv_track")
383382
# Always include SRT utest
384383
MODULE_FILES+=("srs_utest_srt")
385384
if [[ $SRS_GB28181 == YES ]]; then

trunk/doc/CHANGELOG.md

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -7,6 +7,7 @@ The changelog for SRS.
77
<a name="v7-changes"></a>
88

99
## SRS 7.0 Changelog
10+
* v7.0, 2025-09-04, Merge [#4467](https://github.com/ossrs/srs/pull/4467): WebRTC: Fix NACK recovered packets not being added to receive queue. v7.0.78 (#4467)
1011
* v7.0, 2025-09-03, Merge [#4469](https://github.com/ossrs/srs/pull/4469): Upgrade HTTP parser from http-parser to llhttp. v7.0.77 (#4469)
1112
* v7.0, 2025-09-03, Merge [#4470](https://github.com/ossrs/srs/pull/4470): RTX: Fix race condition for timer. v7.0.76 (#4470)
1213
* v7.0, 2025-09-02, Merge [#4466](https://github.com/ossrs/srs/pull/4466): AI: GB28181: Remove embedded SIP server and enforce external SIP usage. v7.0.75 (#4466)

trunk/src/app/srs_app_rtc_source.cpp

Lines changed: 14 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -3168,24 +3168,26 @@ srs_error_t SrsRtcRecvTrack::on_nack(SrsRtpPacket **ppkt)
31683168
SrsRtpPacket *pkt = *ppkt;
31693169
uint16_t seq = pkt->header.get_sequence();
31703170
SrsRtpNackInfo *nack_info = nack_receiver_->find(seq);
3171+
31713172
if (nack_info) {
31723173
// seq had been received.
31733174
nack_receiver_->remove(seq);
31743175
#ifdef SRS_NACK_DEBUG_DROP_ENABLED
31753176
srs_trace("NACK: recovered seq=%u", seq);
31763177
#endif
3177-
return err;
3178-
}
3179-
3180-
// insert check nack list
3181-
uint16_t nack_first = 0, nack_last = 0;
3182-
if (!rtp_queue_->update(seq, nack_first, nack_last)) {
3183-
srs_warn("NACK: too old seq %u, range [%u, %u]", seq, rtp_queue_->begin, rtp_queue_->end);
3184-
}
3185-
if (srs_rtp_seq_distance(nack_first, nack_last) > 0) {
3186-
srs_trace("NACK: update seq=%u, nack range [%u, %u]", seq, nack_first, nack_last);
3187-
nack_receiver_->insert(nack_first, nack_last);
3188-
nack_receiver_->check_queue_size();
3178+
} else {
3179+
// insert check nack list
3180+
uint16_t nack_first = 0, nack_last = 0;
3181+
if (!rtp_queue_->update(seq, nack_first, nack_last)) {
3182+
srs_warn("NACK: too old seq %u, range [%u, %u]", seq, rtp_queue_->begin,
3183+
rtp_queue_->end);
3184+
}
3185+
if (srs_rtp_seq_distance(nack_first, nack_last) > 0) {
3186+
srs_trace("NACK: update seq=%u, nack range [%u, %u]", seq, nack_first,
3187+
nack_last);
3188+
nack_receiver_->insert(nack_first, nack_last);
3189+
nack_receiver_->check_queue_size();
3190+
}
31893191
}
31903192

31913193
// insert into video_queue and audio_queue

trunk/src/core/srs_core_version7.hpp

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -9,6 +9,6 @@
99

1010
#define VERSION_MAJOR 7
1111
#define VERSION_MINOR 0
12-
#define VERSION_REVISION 77
12+
#define VERSION_REVISION 78
1313

1414
#endif

0 commit comments

Comments
 (0)