diff --git a/talk/media/webrtc/webrtcvideoengine.cc b/talk/media/webrtc/webrtcvideoengine.cc index 70264171ab..ec8445e975 100644 --- a/talk/media/webrtc/webrtcvideoengine.cc +++ b/talk/media/webrtc/webrtcvideoengine.cc @@ -2243,6 +2243,17 @@ bool WebRtcVideoMediaChannel::RemoveRecvStream(uint32 ssrc) { } bool WebRtcVideoMediaChannel::RemoveRecvStreamInternal(uint32 ssrc) { + // First remove the RTX SSRC mapping, to include this step even if exiting in + // the default receive channel logic below. + SsrcMap::iterator rtx_it = rtx_to_primary_ssrc_.begin(); + while (rtx_it != rtx_to_primary_ssrc_.end()) { + if (rtx_it->second == ssrc) { + rtx_to_primary_ssrc_.erase(rtx_it++); + } else { + ++rtx_it; + } + } + WebRtcVideoChannelRecvInfo* recv_channel = GetRecvChannelBySsrc(ssrc); if (!recv_channel) { // TODO(perkj): Remove this once BWE works properly across different send @@ -2263,16 +2274,6 @@ bool WebRtcVideoMediaChannel::RemoveRecvStreamInternal(uint32 ssrc) { return false; } - // Remove any RTX SSRC mappings to this stream. - SsrcMap::iterator rtx_it = rtx_to_primary_ssrc_.begin(); - while (rtx_it != rtx_to_primary_ssrc_.end()) { - if (rtx_it->second == ssrc) { - rtx_to_primary_ssrc_.erase(rtx_it++); - } else { - ++rtx_it; - } - } - int channel_id = recv_channel->channel_id(); if (engine()->vie()->render()->RemoveRenderer(channel_id) != 0) { LOG_RTCERR1(RemoveRenderer, channel_id); diff --git a/talk/media/webrtc/webrtcvideoengine_unittest.cc b/talk/media/webrtc/webrtcvideoengine_unittest.cc index 24e9af2835..e98d7799ab 100644 --- a/talk/media/webrtc/webrtcvideoengine_unittest.cc +++ b/talk/media/webrtc/webrtcvideoengine_unittest.cc @@ -793,6 +793,46 @@ TEST_F(WebRtcVideoEngineTestFake, TestReceiveRtxOneStream) { EXPECT_EQ(rtx_codec.id, vie_.GetLastRecvdPayloadType(channel_num)); } +// Verify we don't crash when inserting packets after removing the default +// receive channel. +TEST_F(WebRtcVideoEngineTestFake, TestReceiveRtxWithRemovedDefaultChannel) { + EXPECT_TRUE(SetupEngine()); + + // Setup one channel with an associated RTX stream. + cricket::StreamParams params = + cricket::StreamParams::CreateLegacy(kSsrcs1[0]); + params.AddFidSsrc(kSsrcs1[0], kRtxSsrcs1[0]); + EXPECT_TRUE(channel_->AddRecvStream(params)); + int channel_num = vie_.GetLastChannel(); + EXPECT_EQ(static_cast(kRtxSsrcs1[0]), + vie_.GetRemoteRtxSsrc(channel_num)); + + // Register codecs. + std::vector codec_list; + codec_list.push_back(kVP8Codec720p); + cricket::VideoCodec rtx_codec(96, "rtx", 0, 0, 0, 0); + rtx_codec.SetParam("apt", kVP8Codec.id); + codec_list.push_back(rtx_codec); + EXPECT_TRUE(channel_->SetRecvCodecs(codec_list)); + + // Construct a fake RTX packet and verify that it is passed to the + // right WebRTC channel. + const size_t kDataLength = 12; + uint8_t data[kDataLength]; + memset(data, 0, sizeof(data)); + data[0] = 0x80; + data[1] = rtx_codec.id; + rtc::SetBE32(&data[8], kRtxSsrcs1[0]); + rtc::Buffer packet(data, kDataLength); + rtc::PacketTime packet_time; + channel_->OnPacketReceived(&packet, packet_time); + EXPECT_EQ(rtx_codec.id, vie_.GetLastRecvdPayloadType(channel_num)); + + // Remove the default channel and insert one more packet. + EXPECT_TRUE(channel_->RemoveRecvStream(kSsrcs1[0])); + channel_->OnPacketReceived(&packet, packet_time); +} + // Test that RTX packets are routed to the correct video channel. TEST_F(WebRtcVideoEngineTestFake, TestReceiveRtxThreeStreams) { EXPECT_TRUE(SetupEngine());