Fixing scenario where track is rejected and later un-rejected.
Added `RestartLocalTracks` and `RestartRemoteTracks` methods to
`MediaStreamHandlerContainer` which will redo the track handlers'
initial setup; most importantly, this will re-connect the
renderer/capturer/etc. to a channel which was destroyed and then
re-created.
Also added `AcceptRemoteTracks` method to MediaStreamSignaling, which
does the inverse of `RejectRemoteTracks`. Effectively this will notify
sinks that the track is live again, after previously being set to
`kEnded` when it was rejected.
BUG=webrtc:2136
Review URL: https://codereview.webrtc.org/1231613002
Cr-Commit-Position: refs/heads/master@{#9600}
diff --git a/talk/app/webrtc/mediastreamhandler.cc b/talk/app/webrtc/mediastreamhandler.cc
index f68699d..08be35e 100644
--- a/talk/app/webrtc/mediastreamhandler.cc
+++ b/talk/app/webrtc/mediastreamhandler.cc
@@ -90,7 +90,7 @@
audio_track_(track),
provider_(provider),
sink_adapter_(new LocalAudioSinkAdapter()) {
- OnEnabledChanged();
+ Start();
track->AddSink(sink_adapter_.get());
}
@@ -101,6 +101,10 @@
// TODO(perkj): What should happen when the state change?
}
+void LocalAudioTrackHandler::Start() {
+ OnEnabledChanged();
+}
+
void LocalAudioTrackHandler::Stop() {
audio_track_->RemoveSink(sink_adapter_.get());
cricket::AudioOptions options;
@@ -132,13 +136,19 @@
audio_track_(track),
provider_(provider) {
track->GetSource()->RegisterAudioObserver(this);
- OnEnabledChanged();
+ Start();
}
RemoteAudioTrackHandler::~RemoteAudioTrackHandler() {
audio_track_->GetSource()->UnregisterAudioObserver(this);
}
+void RemoteAudioTrackHandler::Start() {
+ // TODO(deadbeef) - Should we remember the audio playout volume last set,
+ // and set it again in case the audio channel was destroyed and recreated?
+ OnEnabledChanged();
+}
+
void RemoteAudioTrackHandler::Stop() {
provider_->SetAudioPlayout(ssrc(), false, NULL);
}
@@ -166,10 +176,7 @@
: TrackHandler(track, ssrc),
local_video_track_(track),
provider_(provider) {
- VideoSourceInterface* source = local_video_track_->GetSource();
- if (source)
- provider_->SetCaptureDevice(ssrc, source->GetVideoCapturer());
- OnEnabledChanged();
+ Start();
}
LocalVideoTrackHandler::~LocalVideoTrackHandler() {
@@ -178,6 +185,13 @@
void LocalVideoTrackHandler::OnStateChanged() {
}
+void LocalVideoTrackHandler::Start() {
+ VideoSourceInterface* source = local_video_track_->GetSource();
+ if (source)
+ provider_->SetCaptureDevice(ssrc(), source->GetVideoCapturer());
+ OnEnabledChanged();
+}
+
void LocalVideoTrackHandler::Stop() {
provider_->SetCaptureDevice(ssrc(), NULL);
provider_->SetVideoSend(ssrc(), false, NULL);
@@ -199,14 +213,18 @@
: TrackHandler(track, ssrc),
remote_video_track_(track),
provider_(provider) {
- OnEnabledChanged();
- provider_->SetVideoPlayout(ssrc, true,
- remote_video_track_->GetSource()->FrameInput());
+ Start();
}
RemoteVideoTrackHandler::~RemoteVideoTrackHandler() {
}
+void RemoteVideoTrackHandler::Start() {
+ OnEnabledChanged();
+ provider_->SetVideoPlayout(ssrc(), true,
+ remote_video_track_->GetSource()->FrameInput());
+}
+
void RemoteVideoTrackHandler::Stop() {
// Since cricket::VideoRenderer is not reference counted
// we need to remove the renderer before we are deleted.
@@ -301,6 +319,12 @@
track_handlers_.push_back(handler);
}
+void LocalMediaStreamHandler::RestartAllTracks() {
+ for (auto it : track_handlers_) {
+ it->Start();
+ }
+}
+
RemoteMediaStreamHandler::RemoteMediaStreamHandler(
MediaStreamInterface* stream,
AudioProviderInterface* audio_provider,
@@ -327,6 +351,12 @@
track_handlers_.push_back(handler);
}
+void RemoteMediaStreamHandler::RestartAllTracks() {
+ for (auto it : track_handlers_) {
+ it->Start();
+ }
+}
+
MediaStreamHandlerContainer::MediaStreamHandlerContainer(
AudioProviderInterface* audio_provider,
VideoProviderInterface* video_provider)
@@ -396,6 +426,12 @@
handler->RemoveTrack(track);
}
+void MediaStreamHandlerContainer::RestartAllRemoteTracks() {
+ for (auto it : remote_streams_handlers_) {
+ it->RestartAllTracks();
+ }
+}
+
void MediaStreamHandlerContainer::RemoveLocalStream(
MediaStreamInterface* stream) {
DeleteStreamHandler(&local_streams_handlers_, stream);
@@ -438,6 +474,12 @@
handler->RemoveTrack(track);
}
+void MediaStreamHandlerContainer::RestartAllLocalTracks() {
+ for (auto it : local_streams_handlers_) {
+ it->RestartAllTracks();
+ }
+}
+
MediaStreamHandler* MediaStreamHandlerContainer::CreateRemoteStreamHandler(
MediaStreamInterface* stream) {
ASSERT(!FindStreamHandler(remote_streams_handlers_, stream));
diff --git a/talk/app/webrtc/mediastreamhandler.h b/talk/app/webrtc/mediastreamhandler.h
index 801648d..f79eff3 100644
--- a/talk/app/webrtc/mediastreamhandler.h
+++ b/talk/app/webrtc/mediastreamhandler.h
@@ -51,6 +51,8 @@
TrackHandler(MediaStreamTrackInterface* track, uint32 ssrc);
virtual ~TrackHandler();
virtual void OnChanged();
+ // Associate |track_| with |ssrc_|. Can be called multiple times.
+ virtual void Start() = 0;
// Stop using |track_| on this PeerConnection.
virtual void Stop() = 0;
@@ -101,7 +103,7 @@
uint32 ssrc,
AudioProviderInterface* provider);
virtual ~LocalAudioTrackHandler();
-
+ void Start() override;
void Stop() override;
protected:
@@ -127,6 +129,7 @@
uint32 ssrc,
AudioProviderInterface* provider);
virtual ~RemoteAudioTrackHandler();
+ void Start() override;
void Stop() override;
protected:
@@ -150,6 +153,7 @@
uint32 ssrc,
VideoProviderInterface* provider);
virtual ~LocalVideoTrackHandler();
+ void Start() override;
void Stop() override;
protected:
@@ -170,6 +174,7 @@
uint32 ssrc,
VideoProviderInterface* provider);
virtual ~RemoteVideoTrackHandler();
+ void Start() override;
void Stop() override;
protected:
@@ -192,6 +197,7 @@
virtual void AddAudioTrack(AudioTrackInterface* audio_track, uint32 ssrc) = 0;
virtual void AddVideoTrack(VideoTrackInterface* video_track, uint32 ssrc) = 0;
+ virtual void RestartAllTracks() = 0;
virtual void RemoveTrack(MediaStreamTrackInterface* track);
void OnChanged() override;
@@ -214,6 +220,7 @@
void AddAudioTrack(AudioTrackInterface* audio_track, uint32 ssrc) override;
void AddVideoTrack(VideoTrackInterface* video_track, uint32 ssrc) override;
+ void RestartAllTracks() override;
};
class RemoteMediaStreamHandler : public MediaStreamHandler {
@@ -224,6 +231,7 @@
~RemoteMediaStreamHandler();
void AddAudioTrack(AudioTrackInterface* audio_track, uint32 ssrc) override;
void AddVideoTrack(VideoTrackInterface* video_track, uint32 ssrc) override;
+ void RestartAllTracks() override;
};
// Container for MediaStreamHandlers of currently known local and remote
@@ -256,6 +264,10 @@
void RemoveRemoteTrack(MediaStreamInterface* stream,
MediaStreamTrackInterface* track);
+ // Make all remote track handlers reassociate their corresponding tracks
+ // with their corresponding SSRCs.
+ void RestartAllRemoteTracks();
+
// Remove all TrackHandlers for tracks in |stream| and make sure
// the audio_provider and video_provider is notified that the tracks has been
// removed.
@@ -273,6 +285,10 @@
void RemoveLocalTrack(MediaStreamInterface* stream,
MediaStreamTrackInterface* track);
+ // Make all local track handlers reassociate their corresponding tracks
+ // with their corresponding SSRCs.
+ void RestartAllLocalTracks();
+
private:
typedef std::list<MediaStreamHandler*> StreamHandlerList;
MediaStreamHandler* FindStreamHandler(const StreamHandlerList& handlers,
diff --git a/talk/app/webrtc/mediastreamsignaling.cc b/talk/app/webrtc/mediastreamsignaling.cc
index 1f5f14f..d126ac5 100644
--- a/talk/app/webrtc/mediastreamsignaling.cc
+++ b/talk/app/webrtc/mediastreamsignaling.cc
@@ -525,7 +525,13 @@
GetFirstAudioContent(desc->description());
if (audio_content) {
if (audio_content->rejected) {
- RejectRemoteTracks(cricket::MEDIA_TYPE_AUDIO);
+ SetRemoteTracksState(cricket::MEDIA_TYPE_AUDIO,
+ MediaStreamTrackInterface::kEnded);
+ } else {
+ // This is needed in case the local description caused the track to be
+ // rejected, then later accepted, without being destroyed.
+ SetRemoteTracksState(cricket::MEDIA_TYPE_AUDIO,
+ MediaStreamTrackInterface::kLive);
}
const cricket::AudioContentDescription* audio_desc =
static_cast<const cricket::AudioContentDescription*>(
@@ -537,7 +543,13 @@
GetFirstVideoContent(desc->description());
if (video_content) {
if (video_content->rejected) {
- RejectRemoteTracks(cricket::MEDIA_TYPE_VIDEO);
+ SetRemoteTracksState(cricket::MEDIA_TYPE_VIDEO,
+ MediaStreamTrackInterface::kEnded);
+ } else {
+ // This is needed in case the local description caused the track to be
+ // rejected, then later accepted, without being destroyed.
+ SetRemoteTracksState(cricket::MEDIA_TYPE_VIDEO,
+ MediaStreamTrackInterface::kLive);
}
const cricket::VideoContentDescription* video_desc =
static_cast<const cricket::VideoContentDescription*>(
@@ -559,11 +571,13 @@
}
void MediaStreamSignaling::OnAudioChannelClose() {
- RejectRemoteTracks(cricket::MEDIA_TYPE_AUDIO);
+ SetRemoteTracksState(cricket::MEDIA_TYPE_AUDIO,
+ MediaStreamTrackInterface::kEnded);
}
void MediaStreamSignaling::OnVideoChannelClose() {
- RejectRemoteTracks(cricket::MEDIA_TYPE_VIDEO);
+ SetRemoteTracksState(cricket::MEDIA_TYPE_VIDEO,
+ MediaStreamTrackInterface::kEnded);
}
void MediaStreamSignaling::OnDataChannelClose() {
@@ -678,7 +692,9 @@
}
}
-void MediaStreamSignaling::RejectRemoteTracks(cricket::MediaType media_type) {
+void MediaStreamSignaling::SetRemoteTracksState(
+ cricket::MediaType media_type,
+ MediaStreamTrackInterface::TrackState state) {
TrackInfos* current_tracks = GetRemoteTracks(media_type);
for (TrackInfos::iterator track_it = current_tracks->begin();
track_it != current_tracks->end(); ++track_it) {
@@ -689,7 +705,7 @@
// There's no guarantee the track is still available, e.g. the track may
// have been removed from the stream by javascript.
if (track) {
- track->set_state(webrtc::MediaStreamTrackInterface::kEnded);
+ track->set_state(state);
}
}
if (media_type == cricket::MEDIA_TYPE_VIDEO) {
@@ -697,7 +713,7 @@
// There's no guarantee the track is still available, e.g. the track may
// have been removed from the stream by javascript.
if (track) {
- track->set_state(webrtc::MediaStreamTrackInterface::kEnded);
+ track->set_state(state);
}
}
}
diff --git a/talk/app/webrtc/mediastreamsignaling.h b/talk/app/webrtc/mediastreamsignaling.h
index 87eede6..9109d09 100644
--- a/talk/app/webrtc/mediastreamsignaling.h
+++ b/talk/app/webrtc/mediastreamsignaling.h
@@ -323,9 +323,10 @@
const std::string& track_id,
cricket::MediaType media_type);
- // Set the MediaStreamTrackInterface::TrackState to |kEnded| on all remote
+ // Set the MediaStreamTrackInterface::TrackState to |state| on all remote
// tracks of type |media_type|.
- void RejectRemoteTracks(cricket::MediaType media_type);
+ void SetRemoteTracksState(cricket::MediaType media_type,
+ MediaStreamTrackInterface::TrackState state);
// Finds remote MediaStreams without any tracks and removes them from
// |remote_streams_| and notifies the observer that the MediaStream no longer
diff --git a/talk/app/webrtc/peerconnection.cc b/talk/app/webrtc/peerconnection.cc
index dba77da..fa49c77 100644
--- a/talk/app/webrtc/peerconnection.cc
+++ b/talk/app/webrtc/peerconnection.cc
@@ -615,6 +615,14 @@
PostSetSessionDescriptionFailure(observer, error);
return;
}
+
+ // This is necessary because an audio/video channel may have been previously
+ // destroyed by removing it from the remote description, which would NOT
+ // destroy the local handler. So upon receiving a new local description,
+ // we may need to tell that local track handler to connect the capturer
+ // to the now re-created audio/video channel.
+ stream_handler_container_->RestartAllLocalTracks();
+
SetSessionDescriptionMsg* msg = new SetSessionDescriptionMsg(observer);
signaling_thread()->Post(this, MSG_SET_SESSIONDESCRIPTION_SUCCESS, msg);
}
@@ -638,6 +646,14 @@
PostSetSessionDescriptionFailure(observer, error);
return;
}
+
+ // This is necessary because an audio/video channel may have been previously
+ // destroyed by removing it from the local description, which would NOT
+ // destroy the remote handler. So upon receiving a new remote description,
+ // we may need to tell that remote track handler to connect the renderer
+ // to the now re-created audio/video channel.
+ stream_handler_container_->RestartAllRemoteTracks();
+
SetSessionDescriptionMsg* msg = new SetSessionDescriptionMsg(observer);
signaling_thread()->Post(this, MSG_SET_SESSIONDESCRIPTION_SUCCESS, msg);
}
