From 52cea6a2dcf29785df4dcbbedcd74fe6241ff6ac Mon Sep 17 00:00:00 2001 From: Pierre Lespagnol <pierre.lespagnol@savoirfairelinux.com> Date: Mon, 27 Apr 2020 17:17:17 -0400 Subject: [PATCH] srtp: fix decrypt errors Before this patch when the the sender is restarted, we get the last sequence number from media_encoder that is refreshed on each frame only. But the sequence number continue to be incremented on each RTP packet sent, this result in rollover counter (ROC) desynchronisation in srtp context because the largest sequence number sent is now superior to sequence number that is supposed to occur only when sequence number overflow 65535. With the patch when the sender is restarted, we keep the last sequence number from socket_pair (RTP level) Change-Id: I531e3d0a073c251c78bbf9f0ffc702aafaf6ccc8 --- src/media/media_encoder.cpp | 2 +- src/media/socket_pair.cpp | 16 ++++++++++++++-- src/media/socket_pair.h | 4 +++- src/media/video/video_rtp_session.cpp | 7 ++++--- 4 files changed, 22 insertions(+), 7 deletions(-) diff --git a/src/media/media_encoder.cpp b/src/media/media_encoder.cpp index 09edf5365b..0c47e8618a 100644 --- a/src/media/media_encoder.cpp +++ b/src/media/media_encoder.cpp @@ -962,7 +962,7 @@ MediaEncoder::stopEncoder() av_free(encoderCtx); } -bool +bool MediaEncoder::isDynBitrateSupported(AVCodecID codecid) { if (accel_) { diff --git a/src/media/socket_pair.cpp b/src/media/socket_pair.cpp index 4d5f581bfc..0eab384c55 100644 --- a/src/media/socket_pair.cpp +++ b/src/media/socket_pair.cpp @@ -520,9 +520,9 @@ SocketPair::readCallback(uint8_t* buf, int buf_size) rtpDelayCallback_(gradient, deltaT); auto err = ff_srtp_decrypt(&srtpContext_->srtp_in, buf, &len); - if(packetLossCallback_ and (buf[2] << 8 | buf[3]) != lastSeqNum_+1) + if(packetLossCallback_ and (buf[2] << 8 | buf[3]) != lastSeqNumIn_+1) packetLossCallback_(); - lastSeqNum_ = buf[2] << 8 | buf[3]; + lastSeqNumIn_ = buf[2] << 8 | buf[3]; if (err < 0) JAMI_WARN("decrypt error %d", err); } @@ -574,6 +574,9 @@ SocketPair::writeData(uint8_t* buf, int buf_size) int SocketPair::writeCallback(uint8_t* buf, int buf_size) { + if (noWrite_) + return 0; + int ret; bool isRTCP = RTP_PT_IS_RTCP(buf[1]); unsigned int ts_LSB, ts_MSB; @@ -701,4 +704,13 @@ SocketPair::parse_RTP_ext(uint8_t* buf, float* abs) return true; } +uint16_t +SocketPair::lastSeqValOut() +{ + if (srtpContext_) + return srtpContext_->srtp_out.seq_largest; + JAMI_ERR("SRTP context not found."); + return 0; +} + } // namespace jami diff --git a/src/media/socket_pair.h b/src/media/socket_pair.h index 010f24b228..9644b45f1d 100644 --- a/src/media/socket_pair.h +++ b/src/media/socket_pair.h @@ -175,6 +175,8 @@ class SocketPair { int writeData(uint8_t* buf, int buf_size); + uint16_t lastSeqValOut(); + private: NON_COPYABLE(SocketPair); using clock = std::chrono::steady_clock; @@ -222,7 +224,7 @@ class SocketPair { std::list<double> histoLatency_; time_point lastRR_time; - uint16_t lastSeqNum_ {0}; + uint16_t lastSeqNumIn_ {0}; float lastSendTS_ {0.0f}; time_point lastReceiveTS_ {}; time_point arrival_TS {}; diff --git a/src/media/video/video_rtp_session.cpp b/src/media/video/video_rtp_session.cpp index 8456fb5153..f242d9f093 100644 --- a/src/media/video/video_rtp_session.cpp +++ b/src/media/video/video_rtp_session.cpp @@ -133,13 +133,14 @@ void VideoRtpSession::startSender() send_.linkableHW = conference_ == nullptr; send_.bitrate = videoBitrateInfo_.videoBitrateCurrent; - if (sender_) - initSeqVal_ = sender_->getLastSeqValue() + 10; // Skip a few sequences to make nvenc happy on a sender restart + if (socketPair_) + initSeqVal_ = socketPair_->lastSeqValOut(); + try { sender_.reset(); socketPair_->stopSendOp(false); sender_.reset(new VideoSender(getRemoteRtpUri(), localVideoParams_, - send_, *socketPair_, initSeqVal_, mtu_)); + send_, *socketPair_, initSeqVal_+1, mtu_)); if (changeOrientationCallback_) sender_->setChangeOrientationCallback(changeOrientationCallback_); if (socketPair_) -- GitLab