From eb0954248d77bf9a97af27f5905a119c9b8e147a Mon Sep 17 00:00:00 2001 From: "pthatcher@webrtc.org" Date: Wed, 3 Dec 2014 00:34:10 +0000 Subject: [PATCH] Don't reset sequence number for a stream on deactivate/reactivate. BUG=chromium:431908 R=pbos@webrtc.org, sprang@webrtc.org Review URL: https://webrtc-codereview.appspot.com/28119004 git-svn-id: http://webrtc.googlecode.com/svn/trunk@7788 4adac7df-926f-26a2-2b94-8c16560cd09d --- talk/media/webrtc/webrtcvideoengine.cc | 10 ++++-- .../webrtc/webrtcvideoengine_unittest.cc | 31 ++++++++++++++++++- 2 files changed, 38 insertions(+), 3 deletions(-) diff --git a/talk/media/webrtc/webrtcvideoengine.cc b/talk/media/webrtc/webrtcvideoengine.cc index a081bad447..32c80b09f4 100644 --- a/talk/media/webrtc/webrtcvideoengine.cc +++ b/talk/media/webrtc/webrtcvideoengine.cc @@ -3943,8 +3943,14 @@ bool WebRtcVideoMediaChannel::SetSendParams( } engine()->vie()->rtp()->SetTransmissionSmoothingStatus(channel_id, true); - if (!SetSendSsrcs(channel_id, send_params.stream, codec)) { - return false; + // If the set of SSRCs isn't populated, then don't apply them. If we + // do, we'll cause a bug where adding a stream, then removing a + // stream, then re-adding a stream with the same primary SSRC will + // cause the sequence numbers to change and confuse the sender. + if (send_params.stream.first_ssrc() != 0) { + if (!SetSendSsrcs(channel_id, send_params.stream, codec)) { + return false; + } } // NOTE: SetRtxSendPayloadType must be called after all SSRCs are diff --git a/talk/media/webrtc/webrtcvideoengine_unittest.cc b/talk/media/webrtc/webrtcvideoengine_unittest.cc index 102d718112..aaf959501d 100644 --- a/talk/media/webrtc/webrtcvideoengine_unittest.cc +++ b/talk/media/webrtc/webrtcvideoengine_unittest.cc @@ -41,7 +41,6 @@ #include "talk/media/webrtc/webrtcvoiceengine.h" #include "talk/session/media/mediasession.h" #include "webrtc/system_wrappers/interface/trace.h" - // Tests for the WebRtcVideoEngine/VideoChannel code. using cricket::kRtpTimestampOffsetHeaderExtension; @@ -2583,3 +2582,33 @@ TEST_F(WebRtcVideoMediaChannelTest, 640, 400, 30, 0)); } + +// Test that sequence number are not reset if stopping and then +// resuming a stream. +TEST_F(WebRtcVideoMediaChannelTest, DontResetSequenceNumbers) { + cricket::VideoCodec codec = DefaultCodec(); + EXPECT_TRUE(SetOneCodec(codec)); + + uint16_t seq_before = + engine_.vie() + ->rtp() + ->GetRtpStateForSsrc(channel_->GetDefaultChannelId(), kSsrc) + .sequence_number; + + // Deactive. + EXPECT_TRUE(channel_->RemoveSendStream(kSsrc)); + EXPECT_TRUE(SetOneCodec(codec)); + + // Reactivate. + EXPECT_TRUE(channel_->AddSendStream(DefaultSendStreamParams())); + EXPECT_TRUE(SetOneCodec(codec)); + + // Sequence number should now have changed. + uint16_t seq_after = + engine_.vie() + ->rtp() + ->GetRtpStateForSsrc(channel_->GetDefaultChannelId(), kSsrc) + .sequence_number; + + EXPECT_EQ(seq_before, seq_after); +}