admin/webrtc_m130
1
0
Fork 0
Code Issues Pull Requests Actions Packages Projects Releases Wiki Activity

Fixing a segfault that can occur when changing the track of an RtpSender.

Browse Source 
The reference to the old track needs to be kept alive until SetAudioSend/
SetSource is called, because otherwise it could be deleted while the audio/
video engine is still trying to use the track.

BUG=webrtc:5796

Review-Url: https://codereview.webrtc.org/1894283002
Cr-Commit-Position: refs/heads/master@{#12598}
This commit is contained in:
deadbeef 2016-05-02 16:20:01 -07:00 committed by Commit bot
parent acf143128f
commit 5dd42fd849
2 changed files with 30 additions and 5 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

6
webrtc/api/rtpsender.cc
View File

@ -122,6 +122,9 @@ bool AudioRtpSender::SetTrack(MediaStreamTrackInterface* track) {
// Attach to new track.
bool prev_can_send_track = can_send_track();
// Keep a reference to the old track to keep it alive until we call
// SetAudioSend.
rtc::scoped_refptr<AudioTrackInterface> old_track = track_;
track_ = audio_track;
if (track_) {
cached_track_enabled_ = track_->enabled();
@ -276,6 +279,9 @@ bool VideoRtpSender::SetTrack(MediaStreamTrackInterface* track) {
// Attach to new track.
bool prev_can_send_track = can_send_track();
// Keep a reference to the old track to keep it alive until we call
// SetSource.
rtc::scoped_refptr<VideoTrackInterface> old_track = track_;
track_ = video_track;
if (track_) {
cached_track_enabled_ = track_->enabled();

29
webrtc/api/rtpsenderreceiver_unittest.cc
View File

@ -28,6 +28,7 @@
using ::testing::_;
using ::testing::Exactly;
using ::testing::InvokeWithoutArgs;
using ::testing::Return;
static const char kStreamLabel1[] = "local_stream_1";
@ -420,7 +421,14 @@ TEST_F(RtpSenderReceiverTest, AudioSenderTrackSetToNull) {
new AudioRtpSender(track, kStreamLabel1, &audio_provider_, nullptr);
sender->SetSsrc(kAudioSsrc);
EXPECT_CALL(audio_provider_, SetAudioSend(kAudioSsrc, false, _, _)).Times(1);
// Expect that SetAudioSend will be called before the reference to the track
// is released.
EXPECT_CALL(audio_provider_, SetAudioSend(kAudioSsrc, false, _, nullptr))
.Times(1)
.WillOnce(InvokeWithoutArgs([&track] {
EXPECT_LT(2, track->AddRef());
track->Release();
}));
EXPECT_TRUE(sender->SetTrack(nullptr));
// Make sure it's SetTrack that called methods on the provider, and not the
@ -429,14 +437,25 @@ TEST_F(RtpSenderReceiverTest, AudioSenderTrackSetToNull) {
}
TEST_F(RtpSenderReceiverTest, VideoSenderTrackSetToNull) {
AddVideoTrack();
EXPECT_CALL(video_provider_, SetSource(kVideoSsrc, video_track_.get()));
rtc::scoped_refptr<VideoTrackSourceInterface> source(
FakeVideoTrackSource::Create());
rtc::scoped_refptr<VideoTrackInterface> track =
VideoTrack::Create(kVideoTrackId, source);
EXPECT_CALL(video_provider_, SetSource(kVideoSsrc, track.get()));
EXPECT_CALL(video_provider_, SetVideoSend(kVideoSsrc, true, _));
rtc::scoped_refptr<VideoRtpSender> sender =
new VideoRtpSender(video_track_, kStreamLabel1, &video_provider_);
new VideoRtpSender(track, kStreamLabel1, &video_provider_);
sender->SetSsrc(kVideoSsrc);
EXPECT_CALL(video_provider_, SetSource(kVideoSsrc, nullptr)).Times(1);
// Expect that SetSource will be called before the reference to the track
// is released.
EXPECT_CALL(video_provider_, SetSource(kVideoSsrc, nullptr))
.Times(1)
.WillOnce(InvokeWithoutArgs([&track] {
EXPECT_LT(2, track->AddRef());
track->Release();
return true;
}));
EXPECT_CALL(video_provider_, SetVideoSend(kVideoSsrc, false, _)).Times(1);
EXPECT_TRUE(sender->SetTrack(nullptr));