Revert of Fixing scenario where track is rejected and later un-rejected. (patchset #5 id:80001 of https://codereview.webrtc.org/1231613002/)
Reason for revert:
I think this causes WebRtcBrowserTest.CallAndModifyStream to fail on Android. See https://code.google.com/p/webrtc/issues/detail?id=4857 for more info.
Original issue's description:
> 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
>
> Committed: https://crrev.com/be37888b6d5d269dbd5385569dba15c0d70594f2
> Cr-Commit-Position: refs/heads/master@{#9600}
TBR=pthatcher@webrtc.org,juberti@webrtc.org,deadbeef@webrtc.org
NOPRESUBMIT=true
NOTREECHECKS=true
NOTRY=true
BUG=webrtc:2136
Review URL: https://codereview.webrtc.org/1247443005
Cr-Commit-Position: refs/heads/master@{#9622}
diff --git a/talk/app/webrtc/mediastreamhandler.cc b/talk/app/webrtc/mediastreamhandler.cc
index 08be35e..f68699d 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()) {
- Start();
+ OnEnabledChanged();
track->AddSink(sink_adapter_.get());
}
@@ -101,10 +101,6 @@
// 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;
@@ -136,19 +132,13 @@
audio_track_(track),
provider_(provider) {
track->GetSource()->RegisterAudioObserver(this);
- Start();
+ OnEnabledChanged();
}
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);
}
@@ -176,7 +166,10 @@
: TrackHandler(track, ssrc),
local_video_track_(track),
provider_(provider) {
- Start();
+ VideoSourceInterface* source = local_video_track_->GetSource();
+ if (source)
+ provider_->SetCaptureDevice(ssrc, source->GetVideoCapturer());
+ OnEnabledChanged();
}
LocalVideoTrackHandler::~LocalVideoTrackHandler() {
@@ -185,13 +178,6 @@
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);
@@ -213,18 +199,14 @@
: TrackHandler(track, ssrc),
remote_video_track_(track),
provider_(provider) {
- Start();
+ OnEnabledChanged();
+ provider_->SetVideoPlayout(ssrc, true,
+ remote_video_track_->GetSource()->FrameInput());
}
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.
@@ -319,12 +301,6 @@
track_handlers_.push_back(handler);
}
-void LocalMediaStreamHandler::RestartAllTracks() {
- for (auto it : track_handlers_) {
- it->Start();
- }
-}
-
RemoteMediaStreamHandler::RemoteMediaStreamHandler(
MediaStreamInterface* stream,
AudioProviderInterface* audio_provider,
@@ -351,12 +327,6 @@
track_handlers_.push_back(handler);
}
-void RemoteMediaStreamHandler::RestartAllTracks() {
- for (auto it : track_handlers_) {
- it->Start();
- }
-}
-
MediaStreamHandlerContainer::MediaStreamHandlerContainer(
AudioProviderInterface* audio_provider,
VideoProviderInterface* video_provider)
@@ -426,12 +396,6 @@
handler->RemoveTrack(track);
}
-void MediaStreamHandlerContainer::RestartAllRemoteTracks() {
- for (auto it : remote_streams_handlers_) {
- it->RestartAllTracks();
- }
-}
-
void MediaStreamHandlerContainer::RemoveLocalStream(
MediaStreamInterface* stream) {
DeleteStreamHandler(&local_streams_handlers_, stream);
@@ -474,12 +438,6 @@
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 f79eff3..801648d 100644
--- a/talk/app/webrtc/mediastreamhandler.h
+++ b/talk/app/webrtc/mediastreamhandler.h
@@ -51,8 +51,6 @@
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;
@@ -103,7 +101,7 @@
uint32 ssrc,
AudioProviderInterface* provider);
virtual ~LocalAudioTrackHandler();
- void Start() override;
+
void Stop() override;
protected:
@@ -129,7 +127,6 @@
uint32 ssrc,
AudioProviderInterface* provider);
virtual ~RemoteAudioTrackHandler();
- void Start() override;
void Stop() override;
protected:
@@ -153,7 +150,6 @@
uint32 ssrc,
VideoProviderInterface* provider);
virtual ~LocalVideoTrackHandler();
- void Start() override;
void Stop() override;
protected:
@@ -174,7 +170,6 @@
uint32 ssrc,
VideoProviderInterface* provider);
virtual ~RemoteVideoTrackHandler();
- void Start() override;
void Stop() override;
protected:
@@ -197,7 +192,6 @@
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;
@@ -220,7 +214,6 @@
void AddAudioTrack(AudioTrackInterface* audio_track, uint32 ssrc) override;
void AddVideoTrack(VideoTrackInterface* video_track, uint32 ssrc) override;
- void RestartAllTracks() override;
};
class RemoteMediaStreamHandler : public MediaStreamHandler {
@@ -231,7 +224,6 @@
~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
@@ -264,10 +256,6 @@
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.
@@ -285,10 +273,6 @@
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 d126ac5..1f5f14f 100644
--- a/talk/app/webrtc/mediastreamsignaling.cc
+++ b/talk/app/webrtc/mediastreamsignaling.cc
@@ -525,13 +525,7 @@
GetFirstAudioContent(desc->description());
if (audio_content) {
if (audio_content->rejected) {
- 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);
+ RejectRemoteTracks(cricket::MEDIA_TYPE_AUDIO);
}
const cricket::AudioContentDescription* audio_desc =
static_cast<const cricket::AudioContentDescription*>(
@@ -543,13 +537,7 @@
GetFirstVideoContent(desc->description());
if (video_content) {
if (video_content->rejected) {
- 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);
+ RejectRemoteTracks(cricket::MEDIA_TYPE_VIDEO);
}
const cricket::VideoContentDescription* video_desc =
static_cast<const cricket::VideoContentDescription*>(
@@ -571,13 +559,11 @@
}
void MediaStreamSignaling::OnAudioChannelClose() {
- SetRemoteTracksState(cricket::MEDIA_TYPE_AUDIO,
- MediaStreamTrackInterface::kEnded);
+ RejectRemoteTracks(cricket::MEDIA_TYPE_AUDIO);
}
void MediaStreamSignaling::OnVideoChannelClose() {
- SetRemoteTracksState(cricket::MEDIA_TYPE_VIDEO,
- MediaStreamTrackInterface::kEnded);
+ RejectRemoteTracks(cricket::MEDIA_TYPE_VIDEO);
}
void MediaStreamSignaling::OnDataChannelClose() {
@@ -692,9 +678,7 @@
}
}
-void MediaStreamSignaling::SetRemoteTracksState(
- cricket::MediaType media_type,
- MediaStreamTrackInterface::TrackState state) {
+void MediaStreamSignaling::RejectRemoteTracks(cricket::MediaType media_type) {
TrackInfos* current_tracks = GetRemoteTracks(media_type);
for (TrackInfos::iterator track_it = current_tracks->begin();
track_it != current_tracks->end(); ++track_it) {
@@ -705,7 +689,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(state);
+ track->set_state(webrtc::MediaStreamTrackInterface::kEnded);
}
}
if (media_type == cricket::MEDIA_TYPE_VIDEO) {
@@ -713,7 +697,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(state);
+ track->set_state(webrtc::MediaStreamTrackInterface::kEnded);
}
}
}
diff --git a/talk/app/webrtc/mediastreamsignaling.h b/talk/app/webrtc/mediastreamsignaling.h
index 9109d09..87eede6 100644
--- a/talk/app/webrtc/mediastreamsignaling.h
+++ b/talk/app/webrtc/mediastreamsignaling.h
@@ -323,10 +323,9 @@
const std::string& track_id,
cricket::MediaType media_type);
- // Set the MediaStreamTrackInterface::TrackState to |state| on all remote
+ // Set the MediaStreamTrackInterface::TrackState to |kEnded| on all remote
// tracks of type |media_type|.
- void SetRemoteTracksState(cricket::MediaType media_type,
- MediaStreamTrackInterface::TrackState state);
+ void RejectRemoteTracks(cricket::MediaType media_type);
// 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 fa49c77..dba77da 100644
--- a/talk/app/webrtc/peerconnection.cc
+++ b/talk/app/webrtc/peerconnection.cc
@@ -615,14 +615,6 @@
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);
}
@@ -646,14 +638,6 @@
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);
}
