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}
This commit is contained in:
@ -90,7 +90,7 @@ LocalAudioTrackHandler::LocalAudioTrackHandler(
|
||||
audio_track_(track),
|
||||
provider_(provider),
|
||||
sink_adapter_(new LocalAudioSinkAdapter()) {
|
||||
Start();
|
||||
OnEnabledChanged();
|
||||
track->AddSink(sink_adapter_.get());
|
||||
}
|
||||
|
||||
@ -101,10 +101,6 @@ void LocalAudioTrackHandler::OnStateChanged() {
|
||||
// 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 @@ RemoteAudioTrackHandler::RemoteAudioTrackHandler(
|
||||
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 @@ LocalVideoTrackHandler::LocalVideoTrackHandler(
|
||||
: 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 @@ LocalVideoTrackHandler::~LocalVideoTrackHandler() {
|
||||
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 @@ RemoteVideoTrackHandler::RemoteVideoTrackHandler(
|
||||
: 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 @@ void LocalMediaStreamHandler::AddVideoTrack(VideoTrackInterface* video_track,
|
||||
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 @@ void RemoteMediaStreamHandler::AddVideoTrack(VideoTrackInterface* video_track,
|
||||
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 @@ void MediaStreamHandlerContainer::RemoveRemoteTrack(
|
||||
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 @@ void MediaStreamHandlerContainer::RemoveLocalTrack(
|
||||
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));
|
||||
|
||||
@ -51,8 +51,6 @@ class TrackHandler : public ObserverInterface {
|
||||
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 @@ class LocalAudioTrackHandler : public TrackHandler {
|
||||
uint32 ssrc,
|
||||
AudioProviderInterface* provider);
|
||||
virtual ~LocalAudioTrackHandler();
|
||||
void Start() override;
|
||||
|
||||
void Stop() override;
|
||||
|
||||
protected:
|
||||
@ -129,7 +127,6 @@ class RemoteAudioTrackHandler : public AudioSourceInterface::AudioObserver,
|
||||
uint32 ssrc,
|
||||
AudioProviderInterface* provider);
|
||||
virtual ~RemoteAudioTrackHandler();
|
||||
void Start() override;
|
||||
void Stop() override;
|
||||
|
||||
protected:
|
||||
@ -153,7 +150,6 @@ class LocalVideoTrackHandler : public TrackHandler {
|
||||
uint32 ssrc,
|
||||
VideoProviderInterface* provider);
|
||||
virtual ~LocalVideoTrackHandler();
|
||||
void Start() override;
|
||||
void Stop() override;
|
||||
|
||||
protected:
|
||||
@ -174,7 +170,6 @@ class RemoteVideoTrackHandler : public TrackHandler {
|
||||
uint32 ssrc,
|
||||
VideoProviderInterface* provider);
|
||||
virtual ~RemoteVideoTrackHandler();
|
||||
void Start() override;
|
||||
void Stop() override;
|
||||
|
||||
protected:
|
||||
@ -197,7 +192,6 @@ class MediaStreamHandler : public ObserverInterface {
|
||||
|
||||
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 @@ class LocalMediaStreamHandler : public MediaStreamHandler {
|
||||
|
||||
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 @@ class RemoteMediaStreamHandler : public MediaStreamHandler {
|
||||
~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 @@ class MediaStreamHandlerContainer {
|
||||
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 @@ class MediaStreamHandlerContainer {
|
||||
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,
|
||||
|
||||
@ -525,13 +525,7 @@ void MediaStreamSignaling::OnLocalDescriptionChanged(
|
||||
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 @@ void MediaStreamSignaling::OnLocalDescriptionChanged(
|
||||
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::OnLocalDescriptionChanged(
|
||||
}
|
||||
|
||||
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::OnRemoteTrackRemoved(
|
||||
}
|
||||
}
|
||||
|
||||
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 @@ void MediaStreamSignaling::SetRemoteTracksState(
|
||||
// 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 @@ void MediaStreamSignaling::SetRemoteTracksState(
|
||||
// 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);
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
@ -323,10 +323,9 @@ class MediaStreamSignaling : public sigslot::has_slots<> {
|
||||
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
|
||||
|
||||
@ -615,14 +615,6 @@ void PeerConnection::SetLocalDescription(
|
||||
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 @@ void PeerConnection::SetRemoteDescription(
|
||||
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);
|
||||
}
|
||||
|
||||
Reference in New Issue
Block a user