diff --git a/talk/app/webrtc/mediastreamsignaling.cc b/talk/app/webrtc/mediastreamsignaling.cc index 89129acaa2..610b3f8e71 100644 --- a/talk/app/webrtc/mediastreamsignaling.cc +++ b/talk/app/webrtc/mediastreamsignaling.cc @@ -295,24 +295,24 @@ bool MediaStreamSignaling::AddLocalStream(MediaStreamInterface* local_stream) { AudioTrackVector audio_tracks = local_stream->GetAudioTracks(); for (AudioTrackVector::const_iterator it = audio_tracks.begin(); it != audio_tracks.end(); ++it) { - TrackInfos::const_iterator track_info_it = - local_audio_tracks_.find((*it)->id()); - if (track_info_it != local_audio_tracks_.end()) { - const TrackInfo& info = track_info_it->second; - OnLocalTrackSeen(info.stream_label, info.track_id, info.ssrc, - cricket::MEDIA_TYPE_AUDIO); + const TrackInfo* track_info = FindTrackInfo(local_audio_tracks_, + local_stream->label(), + (*it)->id()); + if (track_info) { + OnLocalTrackSeen(track_info->stream_label, track_info->track_id, + track_info->ssrc, cricket::MEDIA_TYPE_AUDIO); } } VideoTrackVector video_tracks = local_stream->GetVideoTracks(); for (VideoTrackVector::const_iterator it = video_tracks.begin(); it != video_tracks.end(); ++it) { - TrackInfos::const_iterator track_info_it = - local_video_tracks_.find((*it)->id()); - if (track_info_it != local_video_tracks_.end()) { - const TrackInfo& info = track_info_it->second; - OnLocalTrackSeen(info.stream_label, info.track_id, info.ssrc, - cricket::MEDIA_TYPE_VIDEO); + const TrackInfo* track_info = FindTrackInfo(local_video_tracks_, + local_stream->label(), + (*it)->id()); + if (track_info) { + OnLocalTrackSeen(track_info->stream_label, track_info->track_id, + track_info->ssrc, cricket::MEDIA_TYPE_VIDEO); } } return true; @@ -321,6 +321,7 @@ bool MediaStreamSignaling::AddLocalStream(MediaStreamInterface* local_stream) { void MediaStreamSignaling::RemoveLocalStream( MediaStreamInterface* local_stream) { local_streams_->RemoveStream(local_stream); + stream_observer_->OnRemoveLocalStream(local_stream); } @@ -474,28 +475,6 @@ void MediaStreamSignaling::OnDataChannelClose() { } } -bool MediaStreamSignaling::GetRemoteAudioTrackSsrc( - const std::string& track_id, uint32* ssrc) const { - TrackInfos::const_iterator it = remote_audio_tracks_.find(track_id); - if (it == remote_audio_tracks_.end()) { - return false; - } - - *ssrc = it->second.ssrc; - return true; -} - -bool MediaStreamSignaling::GetRemoteVideoTrackSsrc( - const std::string& track_id, uint32* ssrc) const { - TrackInfos::const_iterator it = remote_video_tracks_.find(track_id); - if (it == remote_video_tracks_.end()) { - return false; - } - - *ssrc = it->second.ssrc; - return true; -} - void MediaStreamSignaling::UpdateSessionOptions() { options_.streams.clear(); if (local_streams_ != NULL) { @@ -554,12 +533,12 @@ void MediaStreamSignaling::UpdateRemoteStreamsList( // new StreamParam. TrackInfos::iterator track_it = current_tracks->begin(); while (track_it != current_tracks->end()) { - TrackInfo info = track_it->second; + const TrackInfo& info = *track_it; cricket::StreamParams params; if (!cricket::GetStreamBySsrc(streams, info.ssrc, ¶ms) || params.id != info.track_id) { OnRemoteTrackRemoved(info.stream_label, info.track_id, media_type); - current_tracks->erase(track_it++); + track_it = current_tracks->erase(track_it); } else { ++track_it; } @@ -583,10 +562,10 @@ void MediaStreamSignaling::UpdateRemoteStreamsList( new_streams->AddStream(stream); } - TrackInfos::iterator track_it = current_tracks->find(track_id); - if (track_it == current_tracks->end()) { - (*current_tracks)[track_id] = - TrackInfo(stream_label, track_id, ssrc); + const TrackInfo* track_info = FindTrackInfo(*current_tracks, stream_label, + track_id); + if (!track_info) { + current_tracks->push_back(TrackInfo(stream_label, track_id, ssrc)); OnRemoteTrackSeen(stream_label, track_id, it->first_ssrc(), media_type); } } @@ -642,7 +621,7 @@ 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) { - TrackInfo info = track_it->second; + const TrackInfo& info = *track_it; MediaStreamInterface* stream = remote_streams_->find(info.stream_label); if (media_type == cricket::MEDIA_TYPE_AUDIO) { AudioTrackInterface* track = stream->FindAudioTrack(info.track_id); @@ -695,15 +674,16 @@ void MediaStreamSignaling::MaybeCreateDefaultStream() { } if (remote_info_.default_audio_track_needed && default_remote_stream->GetAudioTracks().size() == 0) { - remote_audio_tracks_[kDefaultAudioTrackLabel] = - TrackInfo(kDefaultStreamLabel, kDefaultAudioTrackLabel, 0); + remote_audio_tracks_.push_back(TrackInfo(kDefaultStreamLabel, + kDefaultAudioTrackLabel, 0)); + OnRemoteTrackSeen(kDefaultStreamLabel, kDefaultAudioTrackLabel, 0, cricket::MEDIA_TYPE_AUDIO); } if (remote_info_.default_video_track_needed && default_remote_stream->GetVideoTracks().size() == 0) { - remote_video_tracks_[kDefaultVideoTrackLabel] = - TrackInfo(kDefaultStreamLabel, kDefaultVideoTrackLabel, 0); + remote_video_tracks_.push_back(TrackInfo(kDefaultStreamLabel, + kDefaultVideoTrackLabel, 0)); OnRemoteTrackSeen(kDefaultStreamLabel, kDefaultVideoTrackLabel, 0, cricket::MEDIA_TYPE_VIDEO); } @@ -736,16 +716,16 @@ void MediaStreamSignaling::UpdateLocalTracks( cricket::MediaType media_type) { TrackInfos* current_tracks = GetLocalTracks(media_type); - // Find removed tracks. Ie tracks where the track id or ssrc don't match the - // new StreamParam. + // Find removed tracks. Ie tracks where the track id, stream label or ssrc + // don't match the new StreamParam. TrackInfos::iterator track_it = current_tracks->begin(); while (track_it != current_tracks->end()) { - TrackInfo info = track_it->second; + const TrackInfo& info = *track_it; cricket::StreamParams params; if (!cricket::GetStreamBySsrc(streams, info.ssrc, ¶ms) || - params.id != info.track_id) { + params.id != info.track_id || params.sync_label != info.stream_label) { OnLocalTrackRemoved(info.stream_label, info.track_id, media_type); - current_tracks->erase(track_it++); + track_it = current_tracks->erase(track_it); } else { ++track_it; } @@ -759,10 +739,11 @@ void MediaStreamSignaling::UpdateLocalTracks( const std::string& stream_label = it->sync_label; const std::string& track_id = it->id; uint32 ssrc = it->first_ssrc(); - TrackInfos::iterator track_it = current_tracks->find(track_id); - if (track_it == current_tracks->end()) { - (*current_tracks)[track_id] = - TrackInfo(stream_label, track_id, ssrc); + const TrackInfo* track_info = FindTrackInfo(*current_tracks, + stream_label, + track_id); + if (!track_info) { + current_tracks->push_back(TrackInfo(stream_label, track_id, ssrc)); OnLocalTrackSeen(stream_label, track_id, it->first_ssrc(), media_type); } @@ -948,4 +929,18 @@ void MediaStreamSignaling::OnDtlsRoleReadyForSctp(talk_base::SSLRole role) { } } +const MediaStreamSignaling::TrackInfo* +MediaStreamSignaling::FindTrackInfo( + const MediaStreamSignaling::TrackInfos& infos, + const std::string& stream_label, + const std::string track_id) const { + + for (TrackInfos::const_iterator it = infos.begin(); + it != infos.end(); ++it) { + if (it->stream_label == stream_label && it->track_id == track_id) + return &*it; + } + return NULL; +} + } // namespace webrtc diff --git a/talk/app/webrtc/mediastreamsignaling.h b/talk/app/webrtc/mediastreamsignaling.h index 6c54f409fa..c730b468d6 100644 --- a/talk/app/webrtc/mediastreamsignaling.h +++ b/talk/app/webrtc/mediastreamsignaling.h @@ -238,10 +238,6 @@ class MediaStreamSignaling { // Called when the data channel closes. void OnDataChannelClose(); - // Returns the SSRC for a given track. - bool GetRemoteAudioTrackSsrc(const std::string& track_id, uint32* ssrc) const; - bool GetRemoteVideoTrackSsrc(const std::string& track_id, uint32* ssrc) const; - // Returns all current known local MediaStreams. StreamCollectionInterface* local_streams() const { return local_streams_;} @@ -287,7 +283,7 @@ class MediaStreamSignaling { std::string track_id; uint32 ssrc; }; - typedef std::map TrackInfos; + typedef std::vector TrackInfos; void UpdateSessionOptions(); @@ -366,6 +362,10 @@ class MediaStreamSignaling { const std::vector& active_channels, bool is_local_update); void CreateRemoteDataChannel(const std::string& label, uint32 remote_ssrc); + const TrackInfo* FindTrackInfo(const TrackInfos& infos, + const std::string& stream_label, + const std::string track_id) const; + RemotePeerInfo remote_info_; talk_base::Thread* signaling_thread_; DataChannelFactory* data_channel_factory_; diff --git a/talk/app/webrtc/mediastreamsignaling_unittest.cc b/talk/app/webrtc/mediastreamsignaling_unittest.cc index 6debcfde4f..49625ef526 100644 --- a/talk/app/webrtc/mediastreamsignaling_unittest.cc +++ b/talk/app/webrtc/mediastreamsignaling_unittest.cc @@ -26,6 +26,7 @@ */ #include +#include #include "talk/app/webrtc/audiotrack.h" #include "talk/app/webrtc/mediastream.h" @@ -383,31 +384,48 @@ class MockSignalingObserver : public webrtc::MediaStreamSignalingObserver { std::string track_id; uint32 ssrc; }; - typedef std::map TrackInfos; + typedef std::vector TrackInfos; void AddTrack(TrackInfos* track_infos, MediaStreamInterface* stream, MediaStreamTrackInterface* track, uint32 ssrc) { - (*track_infos)[track->id()] = TrackInfo(stream->label(), track->id(), - ssrc); + (*track_infos).push_back(TrackInfo(stream->label(), track->id(), + ssrc)); } void RemoveTrack(TrackInfos* track_infos, MediaStreamInterface* stream, MediaStreamTrackInterface* track) { - TrackInfos::iterator it = track_infos->find(track->id()); - ASSERT_TRUE(it != track_infos->end()); - ASSERT_EQ(it->second.stream_label, stream->label()); - track_infos->erase(it); + for (TrackInfos::iterator it = track_infos->begin(); + it != track_infos->end(); ++it) { + if (it->stream_label == stream->label() && it->track_id == track->id()) { + track_infos->erase(it); + return; + } + } + ADD_FAILURE(); } + const TrackInfo* FindTrackInfo(const TrackInfos& infos, + const std::string& stream_label, + const std::string track_id) const { + for (TrackInfos::const_iterator it = infos.begin(); + it != infos.end(); ++it) { + if (it->stream_label == stream_label && it->track_id == track_id) + return &*it; + } + return NULL; + } + + void VerifyTrack(const TrackInfos& track_infos, const std::string& stream_label, const std::string& track_id, uint32 ssrc) { - TrackInfos::const_iterator it = track_infos.find(track_id); - ASSERT_TRUE(it != track_infos.end()); - EXPECT_EQ(stream_label, it->second.stream_label); - EXPECT_EQ(ssrc, it->second.ssrc); + const TrackInfo* track_info = FindTrackInfo(track_infos, + stream_label, + track_id); + ASSERT_TRUE(track_info != NULL); + EXPECT_EQ(ssrc, track_info->ssrc); } TrackInfos remote_audio_tracks_; @@ -1051,6 +1069,47 @@ TEST_F(MediaStreamSignalingTest, ChangeSsrcOnTrackInLocalSessionDescription) { observer_->VerifyLocalVideoTrack(kStreams[0], kVideoTracks[0], 98); } +// This test that the correct MediaStreamSignalingObserver methods are called +// if a new session description is set with the same tracks but they are now +// sent on a another MediaStream. +TEST_F(MediaStreamSignalingTest, SignalSameTracksInSeparateMediaStream) { + talk_base::scoped_ptr desc; + CreateSessionDescriptionAndReference(1, 1, desc.use()); + + signaling_->AddLocalStream(reference_collection_->at(0)); + signaling_->OnLocalDescriptionChanged(desc.get()); + EXPECT_EQ(1u, observer_->NumberOfLocalAudioTracks()); + EXPECT_EQ(1u, observer_->NumberOfLocalVideoTracks()); + + std::string stream_label_0 = kStreams[0]; + observer_->VerifyLocalAudioTrack(stream_label_0, kAudioTracks[0], 1); + observer_->VerifyLocalVideoTrack(stream_label_0, kVideoTracks[0], 2); + + // Add a new MediaStream but with the same tracks as in the first stream. + std::string stream_label_1 = kStreams[1]; + talk_base::scoped_refptr stream_1( + webrtc::MediaStream::Create(kStreams[1])); + stream_1->AddTrack(reference_collection_->at(0)->GetVideoTracks()[0]); + stream_1->AddTrack(reference_collection_->at(0)->GetAudioTracks()[0]); + signaling_->AddLocalStream(stream_1); + + // Replace msid in the original SDP. + std::string sdp; + desc->ToString(&sdp); + talk_base::replace_substrs( + kStreams[0], strlen(kStreams[0]), kStreams[1], strlen(kStreams[1]), &sdp); + + talk_base::scoped_ptr updated_desc( + webrtc::CreateSessionDescription(SessionDescriptionInterface::kOffer, + sdp, NULL)); + + signaling_->OnLocalDescriptionChanged(updated_desc.get()); + observer_->VerifyLocalAudioTrack(kStreams[1], kAudioTracks[0], 1); + observer_->VerifyLocalVideoTrack(kStreams[1], kVideoTracks[0], 2); + EXPECT_EQ(1u, observer_->NumberOfLocalAudioTracks()); + EXPECT_EQ(1u, observer_->NumberOfLocalVideoTracks()); +} + // Verifies that an even SCTP id is allocated for SSL_CLIENT and an odd id for // SSL_SERVER. TEST_F(MediaStreamSignalingTest, SctpIdAllocationBasedOnRole) { diff --git a/talk/app/webrtc/peerconnectionfactory.cc b/talk/app/webrtc/peerconnectionfactory.cc index ee15b5d012..dc14bfb567 100644 --- a/talk/app/webrtc/peerconnectionfactory.cc +++ b/talk/app/webrtc/peerconnectionfactory.cc @@ -106,10 +106,10 @@ struct CreateVideoSourceParams : public talk_base::MessageData { }; struct StartAecDumpParams : public talk_base::MessageData { - explicit StartAecDumpParams(FILE* aec_dump_file) + explicit StartAecDumpParams(talk_base::PlatformFile aec_dump_file) : aec_dump_file(aec_dump_file) { } - FILE* aec_dump_file; + talk_base::PlatformFile aec_dump_file; bool result; }; @@ -289,7 +289,7 @@ PeerConnectionFactory::CreateVideoSource_s( return VideoSourceProxy::Create(signaling_thread_, source); } -bool PeerConnectionFactory::StartAecDump_s(FILE* file) { +bool PeerConnectionFactory::StartAecDump_s(talk_base::PlatformFile file) { return channel_manager_->StartAecDump(file); } @@ -380,7 +380,7 @@ scoped_refptr PeerConnectionFactory::CreateAudioTrack( return AudioTrackProxy::Create(signaling_thread_, track); } -bool PeerConnectionFactory::StartAecDump(FILE* file) { +bool PeerConnectionFactory::StartAecDump(talk_base::PlatformFile file) { StartAecDumpParams params(file); signaling_thread_->Send(this, MSG_START_AEC_DUMP, ¶ms); return params.result; diff --git a/talk/app/webrtc/peerconnectionfactory.h b/talk/app/webrtc/peerconnectionfactory.h index 63d37f03d6..46b095fd87 100644 --- a/talk/app/webrtc/peerconnectionfactory.h +++ b/talk/app/webrtc/peerconnectionfactory.h @@ -78,7 +78,7 @@ class PeerConnectionFactory : public PeerConnectionFactoryInterface, CreateAudioTrack(const std::string& id, AudioSourceInterface* audio_source); - virtual bool StartAecDump(FILE* file); + virtual bool StartAecDump(talk_base::PlatformFile file); virtual cricket::ChannelManager* channel_manager(); virtual talk_base::Thread* signaling_thread(); @@ -109,7 +109,7 @@ class PeerConnectionFactory : public PeerConnectionFactoryInterface, PortAllocatorFactoryInterface* allocator_factory, DTLSIdentityServiceInterface* dtls_identity_service, PeerConnectionObserver* observer); - bool StartAecDump_s(FILE* file); + bool StartAecDump_s(talk_base::PlatformFile file); // Implements talk_base::MessageHandler. void OnMessage(talk_base::Message* msg); diff --git a/talk/app/webrtc/peerconnectioninterface.h b/talk/app/webrtc/peerconnectioninterface.h index d24c9a9ecc..667774e270 100644 --- a/talk/app/webrtc/peerconnectioninterface.h +++ b/talk/app/webrtc/peerconnectioninterface.h @@ -76,6 +76,7 @@ #include "talk/app/webrtc/jsep.h" #include "talk/app/webrtc/mediastreaminterface.h" #include "talk/app/webrtc/statstypes.h" +#include "talk/base/fileutils.h" #include "talk/base/socketaddress.h" namespace talk_base { @@ -442,9 +443,10 @@ class PeerConnectionFactoryInterface : public talk_base::RefCountInterface { // Starts AEC dump using existing file. Takes ownership of |file| and passes // it on to VoiceEngine (via other objects) immediately, which will take - // the ownerhip. + // the ownerhip. If the operation fails, the file will be closed. // TODO(grunell): Remove when Chromium has started to use AEC in each source. - virtual bool StartAecDump(FILE* file) = 0; + // http://crbug.com/264611. + virtual bool StartAecDump(talk_base::PlatformFile file) = 0; protected: // Dtor and ctor protected as objects shouldn't be created or deleted via diff --git a/talk/base/fileutils.cc b/talk/base/fileutils.cc index ff34147db7..d73997afe7 100644 --- a/talk/base/fileutils.cc +++ b/talk/base/fileutils.cc @@ -28,6 +28,9 @@ #include #ifdef WIN32 +// TODO(grunell): Remove io.h includes when Chromium has started +// to use AEC in each source. http://crbug.com/264611. +#include #include "talk/base/win32.h" #endif @@ -294,4 +297,28 @@ bool CreateUniqueFile(Pathname& path, bool create_empty) { return true; } +// Taken from Chromium's base/platform_file_*.cc. +// TODO(grunell): Remove when Chromium has started to use AEC in each source. +// http://crbug.com/264611. +FILE* FdopenPlatformFileForWriting(PlatformFile file) { +#if defined(WIN32) + if (file == kInvalidPlatformFileValue) + return NULL; + int fd = _open_osfhandle(reinterpret_cast(file), 0); + if (fd < 0) + return NULL; + return _fdopen(fd, "w"); +#else + return fdopen(file, "w"); +#endif +} + +bool ClosePlatformFile(PlatformFile file) { +#if defined(WIN32) + return CloseHandle(file) != 0; +#else + return close(file); +#endif +} + } // namespace talk_base diff --git a/talk/base/fileutils.h b/talk/base/fileutils.h index 186c963322..fba0d000b0 100644 --- a/talk/base/fileutils.h +++ b/talk/base/fileutils.h @@ -452,6 +452,21 @@ class FilesystemScope{ // process). bool CreateUniqueFile(Pathname& path, bool create_empty); +// Taken from Chromium's base/platform_file.h. +// Don't use ClosePlatformFile to close a file opened with FdopenPlatformFile. +// Use fclose instead. +// TODO(grunell): Remove when Chromium has started to use AEC in each source. +// http://crbug.com/264611. +#if defined(WIN32) +typedef HANDLE PlatformFile; +const PlatformFile kInvalidPlatformFileValue = INVALID_HANDLE_VALUE; +#elif defined(POSIX) +typedef int PlatformFile; +const PlatformFile kInvalidPlatformFileValue = -1; +#endif +FILE* FdopenPlatformFileForWriting(PlatformFile file); +bool ClosePlatformFile(PlatformFile file); + } // namespace talk_base #endif // TALK_BASE_FILEUTILS_H_ diff --git a/talk/libjingle.gyp b/talk/libjingle.gyp index e77e48a3e3..38a165be80 100755 --- a/talk/libjingle.gyp +++ b/talk/libjingle.gyp @@ -509,6 +509,8 @@ 'xmpp/pubsub_task.h', 'xmpp/pubsubclient.cc', 'xmpp/pubsubclient.h', + 'xmpp/pubsubstateclient.cc', + 'xmpp/pubsubstateclient.h', 'xmpp/pubsubtasks.cc', 'xmpp/pubsubtasks.h', 'xmpp/receivetask.cc', diff --git a/talk/libjingle.scons b/talk/libjingle.scons index bab18b2016..d879cc277a 100644 --- a/talk/libjingle.scons +++ b/talk/libjingle.scons @@ -486,7 +486,7 @@ talk.App(env, name = "relayserver", "jingle", ], srcs = [ - "p2p/base/relayserver_main.cc", + "examples/relayserver/relayserver_main.cc", ], ) talk.App(env, name = "stunserver", @@ -494,7 +494,7 @@ talk.App(env, name = "stunserver", "jingle", ], srcs = [ - "p2p/base/stunserver_main.cc", + "examples/stunserver/stunserver_main.cc", ], ) talk.App(env, name = "turnserver", @@ -503,7 +503,7 @@ talk.App(env, name = "turnserver", "ssl", ], srcs = [ - "p2p/base/turnserver_main.cc", + "examples/turnserver/turnserver_main.cc", ], libs = [ "jingle", diff --git a/talk/media/base/fakemediaengine.h b/talk/media/base/fakemediaengine.h index 9ffe07453b..28facca0f5 100644 --- a/talk/media/base/fakemediaengine.h +++ b/talk/media/base/fakemediaengine.h @@ -790,7 +790,7 @@ class FakeVoiceEngine : public FakeBaseEngine { bool SetLocalMonitor(bool enable) { return true; } - bool StartAecDump(FILE* file) { return false; } + bool StartAecDump(talk_base::PlatformFile file) { return false; } bool RegisterProcessor(uint32 ssrc, VoiceProcessor* voice_processor, MediaProcessorDirection direction) { diff --git a/talk/media/base/filemediaengine.h b/talk/media/base/filemediaengine.h index e7956ecfac..be196ae80f 100644 --- a/talk/media/base/filemediaengine.h +++ b/talk/media/base/filemediaengine.h @@ -133,7 +133,7 @@ class FileMediaEngine : public MediaEngineInterface { virtual bool FindVideoCodec(const VideoCodec& codec) { return true; } virtual void SetVoiceLogging(int min_sev, const char* filter) {} virtual void SetVideoLogging(int min_sev, const char* filter) {} - virtual bool StartAecDump(FILE* file) { return false; } + virtual bool StartAecDump(talk_base::PlatformFile) { return false; } virtual bool RegisterVideoProcessor(VideoProcessor* processor) { return true; diff --git a/talk/media/base/mediaengine.h b/talk/media/base/mediaengine.h index 6e071ec2a7..93586bb5b2 100644 --- a/talk/media/base/mediaengine.h +++ b/talk/media/base/mediaengine.h @@ -36,6 +36,7 @@ #include #include +#include "talk/base/fileutils.h" #include "talk/base/sigslotrepeater.h" #include "talk/media/base/codec.h" #include "talk/media/base/mediachannel.h" @@ -136,7 +137,7 @@ class MediaEngineInterface { virtual void SetVideoLogging(int min_sev, const char* filter) = 0; // Starts AEC dump using existing file. - virtual bool StartAecDump(FILE* file) = 0; + virtual bool StartAecDump(talk_base::PlatformFile file) = 0; // Voice processors for effects. virtual bool RegisterVoiceProcessor(uint32 ssrc, @@ -256,7 +257,7 @@ class CompositeMediaEngine : public MediaEngineInterface { video_.SetLogging(min_sev, filter); } - virtual bool StartAecDump(FILE* file) { + virtual bool StartAecDump(talk_base::PlatformFile file) { return voice_.StartAecDump(file); } @@ -316,7 +317,7 @@ class NullVoiceEngine { return rtp_header_extensions_; } void SetLogging(int min_sev, const char* filter) {} - bool StartAecDump(FILE* file) { return false; } + bool StartAecDump(talk_base::PlatformFile file) { return false; } bool RegisterProcessor(uint32 ssrc, VoiceProcessor* voice_processor, MediaProcessorDirection direction) { return true; } diff --git a/talk/media/webrtc/fakewebrtcvideoengine.h b/talk/media/webrtc/fakewebrtcvideoengine.h index bb75c2a7fe..0b07925ae3 100644 --- a/talk/media/webrtc/fakewebrtcvideoengine.h +++ b/talk/media/webrtc/fakewebrtcvideoengine.h @@ -640,9 +640,7 @@ class FakeWebRtcVideoEngine } WEBRTC_STUB(RegisterCpuOveruseObserver, (int channel, webrtc::CpuOveruseObserver* observer)); -#ifdef USE_WEBRTC_DEV_BRANCH WEBRTC_STUB(CpuOveruseMeasures, (int, int*, int*, int*, int*)); -#endif WEBRTC_STUB(ConnectAudioChannel, (const int, const int)); WEBRTC_STUB(DisconnectAudioChannel, (const int)); WEBRTC_FUNC(StartSend, (const int channel)) { @@ -827,12 +825,8 @@ class FakeWebRtcVideoEngine } WEBRTC_STUB(RegisterSendTransport, (const int, webrtc::Transport&)); WEBRTC_STUB(DeregisterSendTransport, (const int)); -#ifdef USE_WEBRTC_DEV_BRANCH WEBRTC_STUB(ReceivedRTPPacket, (const int, const void*, const int, const webrtc::PacketTime&)); -#else - WEBRTC_STUB(ReceivedRTPPacket, (const int, const void*, const int)); -#endif WEBRTC_STUB(ReceivedRTCPPacket, (const int, const void*, const int)); // Not using WEBRTC_STUB due to bool return value virtual bool IsIPv6Enabled(int channel) { return true; } @@ -1040,9 +1034,7 @@ class FakeWebRtcVideoEngine channels_[channel]->rtp_absolute_send_time_receive_id_ = (enable) ? id : 0; return 0; } -#ifdef USE_WEBRTC_DEV_BRANCH WEBRTC_STUB(SetRtcpXrRrtrStatus, (int, bool)); -#endif WEBRTC_FUNC(SetTransmissionSmoothingStatus, (int channel, bool enable)) { WEBRTC_CHECK_CHANNEL(channel); channels_[channel]->transmission_smoothing_ = enable; diff --git a/talk/media/webrtc/fakewebrtcvoiceengine.h b/talk/media/webrtc/fakewebrtcvoiceengine.h index a68d65ef75..0eb880b692 100644 --- a/talk/media/webrtc/fakewebrtcvoiceengine.h +++ b/talk/media/webrtc/fakewebrtcvoiceengine.h @@ -631,13 +631,11 @@ class FakeWebRtcVoiceEngine // webrtc::VoENetEqStats WEBRTC_STUB(GetNetworkStatistics, (int, webrtc::NetworkStatistics&)); -#ifdef USE_WEBRTC_DEV_BRANCH WEBRTC_FUNC_CONST(GetDecodingCallStatistics, (int channel, webrtc::AudioDecodingCallStats*)) { WEBRTC_CHECK_CHANNEL(channel); return 0; } -#endif // webrtc::VoENetwork WEBRTC_FUNC(RegisterExternalTransport, (int channel, @@ -923,9 +921,7 @@ class FakeWebRtcVoiceEngine WEBRTC_STUB(GetEcDelayMetrics, (int& delay_median, int& delay_std)); WEBRTC_STUB(StartDebugRecording, (const char* fileNameUTF8)); -#ifdef USE_WEBRTC_DEV_BRANCH WEBRTC_STUB(StartDebugRecording, (FILE* handle)); -#endif WEBRTC_STUB(StopDebugRecording, ()); WEBRTC_FUNC(SetTypingDetectionStatus, (bool enable)) { diff --git a/talk/media/webrtc/webrtcmediaengine.h b/talk/media/webrtc/webrtcmediaengine.h index 82abefa32b..84319a11fd 100644 --- a/talk/media/webrtc/webrtcmediaengine.h +++ b/talk/media/webrtc/webrtcmediaengine.h @@ -145,7 +145,7 @@ class WebRtcMediaEngine : public cricket::MediaEngineInterface { virtual void SetVideoLogging(int min_sev, const char* filter) OVERRIDE { delegate_->SetVideoLogging(min_sev, filter); } - virtual bool StartAecDump(FILE* file) OVERRIDE { + virtual bool StartAecDump(talk_base::PlatformFile file) OVERRIDE { return delegate_->StartAecDump(file); } virtual bool RegisterVoiceProcessor( diff --git a/talk/media/webrtc/webrtcvideoengine.cc b/talk/media/webrtc/webrtcvideoengine.cc index 63c9effda4..ca0ed414c8 100644 --- a/talk/media/webrtc/webrtcvideoengine.cc +++ b/talk/media/webrtc/webrtcvideoengine.cc @@ -2300,7 +2300,6 @@ bool WebRtcVideoMediaChannel::GetStats(VideoMediaInfo* info) { sinfo.encode_usage_percent = -1; sinfo.capture_queue_delay_ms_per_s = -1; -#ifdef USE_WEBRTC_DEV_BRANCH int capture_jitter_ms = 0; int avg_encode_time_ms = 0; int encode_usage_percent = 0; @@ -2316,7 +2315,6 @@ bool WebRtcVideoMediaChannel::GetStats(VideoMediaInfo* info) { sinfo.encode_usage_percent = encode_usage_percent; sinfo.capture_queue_delay_ms_per_s = capture_queue_delay_ms_per_s; } -#endif // Get received RTCP statistics for the sender (reported by the remote // client in a RTCP packet), if available. @@ -2465,9 +2463,7 @@ bool WebRtcVideoMediaChannel::SetCapturer(uint32 ssrc, MaybeDisconnectCapturer(old_capturer); send_channel->set_video_capturer(capturer); - capturer->SignalVideoFrame.connect( - this, - &WebRtcVideoMediaChannel::SendFrame); + MaybeConnectCapturer(capturer); if (!capturer->IsScreencast() && ratio_w_ != 0 && ratio_h_ != 0) { capturer->UpdateAspectRatio(ratio_w_, ratio_h_); } @@ -2500,12 +2496,8 @@ void WebRtcVideoMediaChannel::OnPacketReceived( engine()->vie()->network()->ReceivedRTPPacket( which_channel, packet->data(), -#ifdef USE_WEBRTC_DEV_BRANCH static_cast(packet->length()), webrtc::PacketTime(packet_time.timestamp, packet_time.not_before)); -#else - static_cast(packet->length())); -#endif } void WebRtcVideoMediaChannel::OnRtcpReceived( diff --git a/talk/media/webrtc/webrtcvideoengine_unittest.cc b/talk/media/webrtc/webrtcvideoengine_unittest.cc index 24eae46944..e331188b59 100644 --- a/talk/media/webrtc/webrtcvideoengine_unittest.cc +++ b/talk/media/webrtc/webrtcvideoengine_unittest.cc @@ -1274,11 +1274,7 @@ TEST_F(WebRtcVideoEngineTestFake, SetOptionsWithDenoising) { EXPECT_FALSE(vie_.GetCaptureDenoising(capture_id)); } -// Test disabled because it drops frames when adapt-before-effects is turned -// off (turned off because it was exposing a crash - see bug 12250150). This is -// safe for now because this test exercises an unused feature. -// TODO(tpsiaki) reenable once adapt-before-effects is turned back on. -TEST_F(WebRtcVideoEngineTestFake, DISABLED_MultipleSendStreamsWithOneCapturer) { +TEST_F(WebRtcVideoEngineTestFake, MultipleSendStreamsWithOneCapturer) { EXPECT_TRUE(SetupEngine()); // Start the capturer diff --git a/talk/media/webrtc/webrtcvoiceengine.cc b/talk/media/webrtc/webrtcvoiceengine.cc index 4d1705736e..b4b96d1800 100644 --- a/talk/media/webrtc/webrtcvoiceengine.cc +++ b/talk/media/webrtc/webrtcvoiceengine.cc @@ -1433,20 +1433,23 @@ bool WebRtcVoiceEngine::SetAudioDeviceModule(webrtc::AudioDeviceModule* adm, return true; } -bool WebRtcVoiceEngine::StartAecDump(FILE* file) { -#ifdef USE_WEBRTC_DEV_BRANCH +bool WebRtcVoiceEngine::StartAecDump(talk_base::PlatformFile file) { + FILE* aec_dump_file_stream = talk_base::FdopenPlatformFileForWriting(file); + if (!aec_dump_file_stream) { + LOG(LS_ERROR) << "Could not open AEC dump file stream."; + if (!talk_base::ClosePlatformFile(file)) + LOG(LS_WARNING) << "Could not close file."; + return false; + } StopAecDump(); - if (voe_wrapper_->processing()->StartDebugRecording(file) != + if (voe_wrapper_->processing()->StartDebugRecording(aec_dump_file_stream) != webrtc::AudioProcessing::kNoError) { - LOG_RTCERR1(StartDebugRecording, "FILE*"); - fclose(file); + LOG_RTCERR0(StartDebugRecording); + fclose(aec_dump_file_stream); return false; } is_dumping_aec_ = true; return true; -#else - return false; -#endif } bool WebRtcVoiceEngine::RegisterProcessor( diff --git a/talk/media/webrtc/webrtcvoiceengine.h b/talk/media/webrtc/webrtcvoiceengine.h index e50bb3ccf5..4b31656236 100644 --- a/talk/media/webrtc/webrtcvoiceengine.h +++ b/talk/media/webrtc/webrtcvoiceengine.h @@ -175,7 +175,7 @@ class WebRtcVoiceEngine webrtc::AudioDeviceModule* adm_sc); // Starts AEC dump using existing file. - bool StartAecDump(FILE* file); + bool StartAecDump(talk_base::PlatformFile file); // Check whether the supplied trace should be ignored. bool ShouldIgnoreTrace(const std::string& trace); diff --git a/talk/p2p/base/turnport_unittest.cc b/talk/p2p/base/turnport_unittest.cc index d559894ac5..0284f51ccf 100644 --- a/talk/p2p/base/turnport_unittest.cc +++ b/talk/p2p/base/turnport_unittest.cc @@ -24,6 +24,9 @@ * OTHERWISE) ARISING IN ANY WAY OUT OF THE USE OF THIS SOFTWARE, EVEN IF * ADVISED OF THE POSSIBILITY OF SUCH DAMAGE. */ +#if defined(POSIX) +#include +#endif #include "talk/base/asynctcpsocket.h" #include "talk/base/buffer.h" @@ -71,7 +74,7 @@ static const char kIcePwd1[] = "TESTICEPWD00000000000001"; static const char kIcePwd2[] = "TESTICEPWD00000000000002"; static const char kTurnUsername[] = "test"; static const char kTurnPassword[] = "test"; -static const int kTimeout = 1000; +static const unsigned int kTimeout = 1000; static const cricket::ProtocolAddress kTurnUdpProtoAddr( kTurnUdpIntAddr, cricket::PROTO_UDP); @@ -80,8 +83,26 @@ static const cricket::ProtocolAddress kTurnTcpProtoAddr( static const cricket::ProtocolAddress kTurnUdpIPv6ProtoAddr( kTurnUdpIPv6IntAddr, cricket::PROTO_UDP); +static const unsigned int MSG_TESTFINISH = 0; + +#if defined(LINUX) +static int GetFDCount() { + struct dirent *dp; + int fd_count = 0; + DIR *dir = opendir("/proc/self/fd/"); + while ((dp = readdir(dir)) != NULL) { + if (dp->d_name[0] == '.') + continue; + ++fd_count; + } + closedir(dir); + return fd_count; +} +#endif + class TurnPortTest : public testing::Test, - public sigslot::has_slots<> { + public sigslot::has_slots<>, + public talk_base::MessageHandler { public: TurnPortTest() : main_(talk_base::Thread::Current()), @@ -95,10 +116,17 @@ class TurnPortTest : public testing::Test, turn_error_(false), turn_unknown_address_(false), turn_create_permission_success_(false), - udp_ready_(false) { + udp_ready_(false), + test_finish_(false) { network_.AddIP(talk_base::IPAddress(INADDR_ANY)); } + virtual void OnMessage(talk_base::Message* msg) { + ASSERT(msg->message_id == MSG_TESTFINISH); + if (msg->message_id == MSG_TESTFINISH) + test_finish_ = true; + } + void OnTurnPortComplete(Port* port) { turn_ready_ = true; } @@ -264,6 +292,7 @@ class TurnPortTest : public testing::Test, bool turn_unknown_address_; bool turn_create_permission_success_; bool udp_ready_; + bool test_finish_; std::vector turn_packets_; std::vector udp_packets_; }; @@ -378,3 +407,24 @@ TEST_F(TurnPortTest, TestTurnLocalIPv6AddressServerIPv6ExtenalIPv4) { EXPECT_NE(0, turn_port_->Candidates()[0].address().port()); } +// This test verifies any FD's are not leaked after TurnPort is destroyed. +// https://code.google.com/p/webrtc/issues/detail?id=2651 +#if defined(LINUX) +TEST_F(TurnPortTest, TestResolverShutdown) { + turn_server_.AddInternalSocket(kTurnUdpIPv6IntAddr, cricket::PROTO_UDP); + int last_fd_count = GetFDCount(); + // Need to supply unresolved address to kick off resolver. + CreateTurnPort(kLocalIPv6Addr, kTurnUsername, kTurnPassword, + cricket::ProtocolAddress(talk_base::SocketAddress( + "stun.l.google.com", 3478), cricket::PROTO_UDP)); + turn_port_->PrepareAddress(); + ASSERT_TRUE_WAIT(turn_error_, kTimeout); + EXPECT_TRUE(turn_port_->Candidates().empty()); + turn_port_.reset(); + talk_base::Thread::Current()->Post(this, MSG_TESTFINISH); + // Waiting for above message to be processed. + ASSERT_TRUE_WAIT(test_finish_, kTimeout); + EXPECT_EQ(last_fd_count, GetFDCount()); +} +#endif + diff --git a/talk/session/media/channelmanager.cc b/talk/session/media/channelmanager.cc index 4d5d8fc7fe..ccc3527707 100644 --- a/talk/session/media/channelmanager.cc +++ b/talk/session/media/channelmanager.cc @@ -947,7 +947,7 @@ bool ChannelManager::SetAudioOptions(const AudioOptions& options) { return true; } -bool ChannelManager::StartAecDump(FILE* file) { +bool ChannelManager::StartAecDump(talk_base::PlatformFile file) { return worker_thread_->Invoke( Bind(&MediaEngineInterface::StartAecDump, media_engine_.get(), file)); } diff --git a/talk/session/media/channelmanager.h b/talk/session/media/channelmanager.h index f19d3d0824..deb7b9ebda 100644 --- a/talk/session/media/channelmanager.h +++ b/talk/session/media/channelmanager.h @@ -32,6 +32,7 @@ #include #include "talk/base/criticalsection.h" +#include "talk/base/fileutils.h" #include "talk/base/sigslotrepeater.h" #include "talk/base/thread.h" #include "talk/media/base/capturemanager.h" @@ -215,7 +216,7 @@ class ChannelManager : public talk_base::MessageHandler, const VideoFormat& max_format); // Starts AEC dump using existing file. - bool StartAecDump(FILE* file); + bool StartAecDump(talk_base::PlatformFile file); sigslot::repeater0<> SignalDevicesChange; sigslot::signal2 SignalVideoCaptureStateChange; diff --git a/talk/xmpp/hangoutpubsubclient.cc b/talk/xmpp/hangoutpubsubclient.cc index b6669a1086..8e92a7b2d3 100644 --- a/talk/xmpp/hangoutpubsubclient.cc +++ b/talk/xmpp/hangoutpubsubclient.cc @@ -44,54 +44,8 @@ const char kPresenting[] = "s"; const char kNotPresenting[] = "o"; const char kEmpty[] = ""; -const std::string GetPublisherNickFromPubSubItem(const XmlElement* item_elem) { - if (item_elem == NULL) { - return ""; - } - - return Jid(item_elem->Attr(QN_ATTR_PUBLISHER)).resource(); -} - } // namespace - -// Knows how to handle specific states and XML. -template -class PubSubStateSerializer { - public: - virtual ~PubSubStateSerializer() {} - virtual XmlElement* Write(const QName& state_name, const C& state) = 0; - virtual C Parse(const XmlElement* state_elem) = 0; -}; - -// Knows how to create "keys" for states, which determines their -// uniqueness. Most states are per-nick, but block is -// per-blocker-and-blockee. This is independent of itemid, especially -// in the case of presenter state. -class PubSubStateKeySerializer { - public: - virtual ~PubSubStateKeySerializer() {} - virtual std::string GetKey(const std::string& publisher_nick, - const std::string& published_nick) = 0; -}; - -class PublishedNickKeySerializer : public PubSubStateKeySerializer { - public: - virtual std::string GetKey(const std::string& publisher_nick, - const std::string& published_nick) { - return published_nick; - } -}; - -class PublisherAndPublishedNicksKeySerializer - : public PubSubStateKeySerializer { - public: - virtual std::string GetKey(const std::string& publisher_nick, - const std::string& published_nick) { - return publisher_nick + ":" + published_nick; - } -}; - // A simple serialiazer where presence of item => true, lack of item // => false. class BoolStateSerializer : public PubSubStateSerializer { @@ -103,195 +57,11 @@ class BoolStateSerializer : public PubSubStateSerializer { return new XmlElement(state_name, true); } - virtual bool Parse(const XmlElement* state_elem) { - return state_elem != NULL; + virtual void Parse(const XmlElement* state_elem, bool *state_out) { + *state_out = state_elem != NULL; } }; -// Adapts PubSubClient to be specifically suited for pub sub call -// states. Signals state changes and keeps track of keys, which are -// normally nicks. -// TODO: Expose this as a generally useful class, not just -// private to hangouts. -template -class PubSubStateClient : public sigslot::has_slots<> { - public: - // Gets ownership of the serializers, but not the client. - PubSubStateClient(const std::string& publisher_nick, - PubSubClient* client, - const QName& state_name, - C default_state, - PubSubStateKeySerializer* key_serializer, - PubSubStateSerializer* state_serializer) - : publisher_nick_(publisher_nick), - client_(client), - state_name_(state_name), - default_state_(default_state) { - key_serializer_.reset(key_serializer); - state_serializer_.reset(state_serializer); - client_->SignalItems.connect( - this, &PubSubStateClient::OnItems); - client_->SignalPublishResult.connect( - this, &PubSubStateClient::OnPublishResult); - client_->SignalPublishError.connect( - this, &PubSubStateClient::OnPublishError); - client_->SignalRetractResult.connect( - this, &PubSubStateClient::OnRetractResult); - client_->SignalRetractError.connect( - this, &PubSubStateClient::OnRetractError); - } - - virtual ~PubSubStateClient() {} - - virtual void Publish(const std::string& published_nick, - const C& state, - std::string* task_id_out) { - std::string key = key_serializer_->GetKey(publisher_nick_, published_nick); - std::string itemid = state_name_.LocalPart() + ":" + key; - if (StatesEqual(state, default_state_)) { - client_->RetractItem(itemid, task_id_out); - } else { - XmlElement* state_elem = state_serializer_->Write(state_name_, state); - state_elem->AddAttr(QN_NICK, published_nick); - client_->PublishItem(itemid, state_elem, task_id_out); - } - }; - - sigslot::signal1&> SignalStateChange; - // Signal (task_id, item). item is NULL for retract. - sigslot::signal2 SignalPublishResult; - // Signal (task_id, item, error stanza). item is NULL for retract. - sigslot::signal3 SignalPublishError; - - protected: - // return false if retracted item (no info or state given) - virtual bool ParseStateItem(const PubSubItem& item, - StateItemInfo* info_out, - bool* state_out) { - const XmlElement* state_elem = item.elem->FirstNamed(state_name_); - if (state_elem == NULL) { - return false; - } - - info_out->publisher_nick = GetPublisherNickFromPubSubItem(item.elem); - info_out->published_nick = state_elem->Attr(QN_NICK); - *state_out = state_serializer_->Parse(state_elem); - return true; - }; - - virtual bool StatesEqual(C state1, C state2) { - return state1 == state2; - } - - PubSubClient* client() { return client_; } - - private: - void OnItems(PubSubClient* pub_sub_client, - const std::vector& items) { - for (std::vector::const_iterator item = items.begin(); - item != items.end(); ++item) { - OnItem(*item); - } - } - - void OnItem(const PubSubItem& item) { - const std::string& itemid = item.itemid; - StateItemInfo info; - C new_state; - - bool retracted = !ParseStateItem(item, &info, &new_state); - if (retracted) { - bool known_itemid = - (info_by_itemid_.find(itemid) != info_by_itemid_.end()); - if (!known_itemid) { - // Nothing to retract, and nothing to publish. - // Probably a different state type. - return; - } else { - info = info_by_itemid_[itemid]; - info_by_itemid_.erase(itemid); - new_state = default_state_; - } - } else { - // TODO: Assert new key matches the known key. It - // shouldn't change! - info_by_itemid_[itemid] = info; - } - - std::string key = key_serializer_->GetKey( - info.publisher_nick, info.published_nick); - bool has_old_state = (state_by_key_.find(key) != state_by_key_.end()); - C old_state = has_old_state ? state_by_key_[key] : default_state_; - if ((retracted && !has_old_state) || StatesEqual(new_state, old_state)) { - // Nothing change, so don't bother signalling. - return; - } - - if (retracted || StatesEqual(new_state, default_state_)) { - // We treat a default state similar to a retract. - state_by_key_.erase(key); - } else { - state_by_key_[key] = new_state; - } - - PubSubStateChange change; - if (!retracted) { - // Retracts do not have publisher information. - change.publisher_nick = info.publisher_nick; - } - change.published_nick = info.published_nick; - change.old_state = old_state; - change.new_state = new_state; - SignalStateChange(change); - } - - void OnPublishResult(PubSubClient* pub_sub_client, - const std::string& task_id, - const XmlElement* item) { - SignalPublishResult(task_id, item); - } - - void OnPublishError(PubSubClient* pub_sub_client, - const std::string& task_id, - const buzz::XmlElement* item, - const buzz::XmlElement* stanza) { - SignalPublishError(task_id, item, stanza); - } - - void OnRetractResult(PubSubClient* pub_sub_client, - const std::string& task_id) { - // There's no point in differentiating between publish and retract - // errors, so we simplify by making them both signal a publish - // result. - const XmlElement* item = NULL; - SignalPublishResult(task_id, item); - } - - void OnRetractError(PubSubClient* pub_sub_client, - const std::string& task_id, - const buzz::XmlElement* stanza) { - // There's no point in differentiating between publish and retract - // errors, so we simplify by making them both signal a publish - // error. - const XmlElement* item = NULL; - SignalPublishError(task_id, item, stanza); - } - - std::string publisher_nick_; - PubSubClient* client_; - const QName state_name_; - C default_state_; - talk_base::scoped_ptr key_serializer_; - talk_base::scoped_ptr > state_serializer_; - // key => state - std::map state_by_key_; - // itemid => StateItemInfo - std::map info_by_itemid_; -}; - class PresenterStateClient : public PubSubStateClient { public: PresenterStateClient(const std::string& publisher_nick, @@ -336,15 +106,17 @@ class PresenterStateClient : public PubSubStateClient { return false; } - info_out->publisher_nick = GetPublisherNickFromPubSubItem(item.elem); + info_out->publisher_nick = + client()->GetPublisherNickFromPubSubItem(item.elem); info_out->published_nick = presenter_elem->Attr(QN_NICK); *state_out = (presentation_item_elem->Attr( QN_PRESENTER_PRESENTATION_TYPE) != kNotPresenting); return true; } - virtual bool StatesEqual(bool state1, bool state2) { - return false; // Make every item trigger an event, even if state doesn't change. + virtual bool StatesEqual(const bool& state1, const bool& state2) { + // Make every item trigger an event, even if state doesn't change. + return false; } }; diff --git a/talk/xmpp/hangoutpubsubclient.h b/talk/xmpp/hangoutpubsubclient.h index a9986db159..2fcd691329 100644 --- a/talk/xmpp/hangoutpubsubclient.h +++ b/talk/xmpp/hangoutpubsubclient.h @@ -37,6 +37,7 @@ #include "talk/base/sigslotrepeater.h" #include "talk/xmpp/jid.h" #include "talk/xmpp/pubsubclient.h" +#include "talk/xmpp/pubsubstateclient.h" // Gives a high-level API for MUC call PubSub needs such as // presenter state, recording state, mute state, and remote mute. @@ -47,30 +48,6 @@ class Jid; class XmlElement; class XmppTaskParentInterface; -// To handle retracts correctly, we need to remember certain details -// about an item. We could just cache the entire XML element, but -// that would take more memory and require re-parsing. -struct StateItemInfo { - std::string published_nick; - std::string publisher_nick; -}; - -// Represents a PubSub state change. Usually, the key is the nick, -// but not always. It's a per-state-type thing. Currently documented -// at https://docs.google.com/a/google.com/document/d/ -// 1QyHu_ufyVdf0VICdfc_DtJbrOdrdIUm4eM73RZqnivI/edit?hl=en_US -template -struct PubSubStateChange { - // The nick of the user changing the state. - std::string publisher_nick; - // The nick of the user whose state is changing. - std::string published_nick; - C old_state; - C new_state; -}; - -template class PubSubStateClient; - // A client tied to a specific MUC jid and local nick. Provides ways // to get updates and publish state and events. Must call // RequestAll() to start getting updates. diff --git a/talk/xmpp/pubsubclient.cc b/talk/xmpp/pubsubclient.cc index 8d6d4c414a..b62758710c 100644 --- a/talk/xmpp/pubsubclient.cc +++ b/talk/xmpp/pubsubclient.cc @@ -134,4 +134,13 @@ void PubSubClient::OnRetractError(IqTask* task, SignalRetractError(this, retract_task->task_id(), stanza); } + +const std::string PubSubClient::GetPublisherNickFromPubSubItem( + const XmlElement* item_elem) { + if (item_elem == NULL) { + return ""; + } + + return Jid(item_elem->Attr(QN_ATTR_PUBLISHER)).resource(); +} } // namespace buzz diff --git a/talk/xmpp/pubsubclient.h b/talk/xmpp/pubsubclient.h index 099765a2d3..f0cd7a98f4 100644 --- a/talk/xmpp/pubsubclient.h +++ b/talk/xmpp/pubsubclient.h @@ -101,6 +101,9 @@ class PubSubClient : public sigslot::has_slots<> { void RetractItem(const std::string& itemid, std::string* task_id_out); + // Get the publisher nick if it exists from the pubsub item. + const std::string GetPublisherNickFromPubSubItem(const XmlElement* item_elem); + private: void OnRequestError(IqTask* task, const XmlElement* stanza); diff --git a/talk/xmpp/pubsubstateclient.cc b/talk/xmpp/pubsubstateclient.cc new file mode 100644 index 0000000000..5cd7b1a703 --- /dev/null +++ b/talk/xmpp/pubsubstateclient.cc @@ -0,0 +1,42 @@ +/* + * libjingle + * Copyright 2011, Google Inc. + * + * Redistribution and use in source and binary forms, with or without + * modification, are permitted provided that the following conditions are met: + * + * 1. Redistributions of source code must retain the above copyright notice, + * this list of conditions and the following disclaimer. + * 2. Redistributions in binary form must reproduce the above copyright notice, + * this list of conditions and the following disclaimer in the documentation + * and/or other materials provided with the distribution. + * 3. The name of the author may not be used to endorse or promote products + * derived from this software without specific prior written permission. + * + * THIS SOFTWARE IS PROVIDED BY THE AUTHOR ``AS IS'' AND ANY EXPRESS OR IMPLIED + * WARRANTIES, INCLUDING, BUT NOT LIMITED TO, THE IMPLIED WARRANTIES OF + * MERCHANTABILITY AND FITNESS FOR A PARTICULAR PURPOSE ARE DISCLAIMED. IN NO + * EVENT SHALL THE AUTHOR BE LIABLE FOR ANY DIRECT, INDIRECT, INCIDENTAL, + * SPECIAL, EXEMPLARY, OR CONSEQUENTIAL DAMAGES (INCLUDING, BUT NOT LIMITED TO, + * PROCUREMENT OF SUBSTITUTE GOODS OR SERVICES; LOSS OF USE, DATA, OR PROFITS; + * OR BUSINESS INTERRUPTION) HOWEVER CAUSED AND ON ANY THEORY OF LIABILITY, + * WHETHER IN CONTRACT, STRICT LIABILITY, OR TORT (INCLUDING NEGLIGENCE OR + * OTHERWISE) ARISING IN ANY WAY OUT OF THE USE OF THIS SOFTWARE, EVEN IF + * ADVISED OF THE POSSIBILITY OF SUCH DAMAGE. + */ + +#include "talk/xmpp/pubsubstateclient.h" + +namespace buzz { + +std::string PublishedNickKeySerializer::GetKey( + const std::string& publisher_nick, const std::string& published_nick) { + return published_nick; +} + +std::string PublisherAndPublishedNicksKeySerializer::GetKey( + const std::string& publisher_nick, const std::string& published_nick) { + return publisher_nick + ":" + published_nick; +} + +} // namespace buzz diff --git a/talk/xmpp/pubsubstateclient.h b/talk/xmpp/pubsubstateclient.h new file mode 100644 index 0000000000..f38658defd --- /dev/null +++ b/talk/xmpp/pubsubstateclient.h @@ -0,0 +1,287 @@ +/* + * libjingle + * Copyright 2011, Google Inc. + * + * Redistribution and use in source and binary forms, with or without + * modification, are permitted provided that the following conditions are met: + * + * 1. Redistributions of source code must retain the above copyright notice, + * this list of conditions and the following disclaimer. + * 2. Redistributions in binary form must reproduce the above copyright notice, + * this list of conditions and the following disclaimer in the documentation + * and/or other materials provided with the distribution. + * 3. The name of the author may not be used to endorse or promote products + * derived from this software without specific prior written permission. + * + * THIS SOFTWARE IS PROVIDED BY THE AUTHOR ``AS IS'' AND ANY EXPRESS OR IMPLIED + * WARRANTIES, INCLUDING, BUT NOT LIMITED TO, THE IMPLIED WARRANTIES OF + * MERCHANTABILITY AND FITNESS FOR A PARTICULAR PURPOSE ARE DISCLAIMED. IN NO + * EVENT SHALL THE AUTHOR BE LIABLE FOR ANY DIRECT, INDIRECT, INCIDENTAL, + * SPECIAL, EXEMPLARY, OR CONSEQUENTIAL DAMAGES (INCLUDING, BUT NOT LIMITED TO, + * PROCUREMENT OF SUBSTITUTE GOODS OR SERVICES; LOSS OF USE, DATA, OR PROFITS; + * OR BUSINESS INTERRUPTION) HOWEVER CAUSED AND ON ANY THEORY OF LIABILITY, + * WHETHER IN CONTRACT, STRICT LIABILITY, OR TORT (INCLUDING NEGLIGENCE OR + * OTHERWISE) ARISING IN ANY WAY OUT OF THE USE OF THIS SOFTWARE, EVEN IF + * ADVISED OF THE POSSIBILITY OF SUCH DAMAGE. + */ + +#ifndef TALK_XMPP_PUBSUBSTATECLIENT_H_ +#define TALK_XMPP_PUBSUBSTATECLIENT_H_ + +#include +#include +#include + +#include "talk/base/scoped_ptr.h" +#include "talk/base/sigslot.h" +#include "talk/base/sigslotrepeater.h" +#include "talk/xmpp/constants.h" +#include "talk/xmpp/jid.h" +#include "talk/xmpp/pubsubclient.h" +#include "talk/xmllite/qname.h" +#include "talk/xmllite/xmlelement.h" + +namespace buzz { + +// To handle retracts correctly, we need to remember certain details +// about an item. We could just cache the entire XML element, but +// that would take more memory and require re-parsing. +struct StateItemInfo { + std::string published_nick; + std::string publisher_nick; +}; + +// Represents a PubSub state change. Usually, the key is the nick, +// but not always. It's a per-state-type thing. Look below on how keys are +// computed. +template +struct PubSubStateChange { + // The nick of the user changing the state. + std::string publisher_nick; + // The nick of the user whose state is changing. + std::string published_nick; + C old_state; + C new_state; +}; + +// Knows how to handle specific states and XML. +template +class PubSubStateSerializer { + public: + virtual ~PubSubStateSerializer() {} + virtual XmlElement* Write(const QName& state_name, const C& state) = 0; + virtual void Parse(const XmlElement* state_elem, C* state_out) = 0; +}; + +// Knows how to create "keys" for states, which determines their +// uniqueness. Most states are per-nick, but block is +// per-blocker-and-blockee. This is independent of itemid, especially +// in the case of presenter state. +class PubSubStateKeySerializer { + public: + virtual ~PubSubStateKeySerializer() {} + virtual std::string GetKey(const std::string& publisher_nick, + const std::string& published_nick) = 0; +}; + +class PublishedNickKeySerializer : public PubSubStateKeySerializer { + public: + virtual std::string GetKey(const std::string& publisher_nick, + const std::string& published_nick); +}; + +class PublisherAndPublishedNicksKeySerializer + : public PubSubStateKeySerializer { + public: + virtual std::string GetKey(const std::string& publisher_nick, + const std::string& published_nick); +}; + +// Adapts PubSubClient to be specifically suited for pub sub call +// states. Signals state changes and keeps track of keys, which are +// normally nicks. +template +class PubSubStateClient : public sigslot::has_slots<> { + public: + // Gets ownership of the serializers, but not the client. + PubSubStateClient(const std::string& publisher_nick, + PubSubClient* client, + const QName& state_name, + C default_state, + PubSubStateKeySerializer* key_serializer, + PubSubStateSerializer* state_serializer) + : publisher_nick_(publisher_nick), + client_(client), + state_name_(state_name), + default_state_(default_state) { + key_serializer_.reset(key_serializer); + state_serializer_.reset(state_serializer); + client_->SignalItems.connect( + this, &PubSubStateClient::OnItems); + client_->SignalPublishResult.connect( + this, &PubSubStateClient::OnPublishResult); + client_->SignalPublishError.connect( + this, &PubSubStateClient::OnPublishError); + client_->SignalRetractResult.connect( + this, &PubSubStateClient::OnRetractResult); + client_->SignalRetractError.connect( + this, &PubSubStateClient::OnRetractError); + } + + virtual ~PubSubStateClient() {} + + virtual void Publish(const std::string& published_nick, + const C& state, + std::string* task_id_out) { + std::string key = key_serializer_->GetKey(publisher_nick_, published_nick); + std::string itemid = state_name_.LocalPart() + ":" + key; + if (StatesEqual(state, default_state_)) { + client_->RetractItem(itemid, task_id_out); + } else { + XmlElement* state_elem = state_serializer_->Write(state_name_, state); + state_elem->AddAttr(QN_NICK, published_nick); + client_->PublishItem(itemid, state_elem, task_id_out); + } + } + + sigslot::signal1&> SignalStateChange; + // Signal (task_id, item). item is NULL for retract. + sigslot::signal2 SignalPublishResult; + // Signal (task_id, item, error stanza). item is NULL for retract. + sigslot::signal3 SignalPublishError; + + protected: + // return false if retracted item (no info or state given) + virtual bool ParseStateItem(const PubSubItem& item, + StateItemInfo* info_out, + C* state_out) { + const XmlElement* state_elem = item.elem->FirstNamed(state_name_); + if (state_elem == NULL) { + return false; + } + + info_out->publisher_nick = + client_->GetPublisherNickFromPubSubItem(item.elem); + info_out->published_nick = state_elem->Attr(QN_NICK); + state_serializer_->Parse(state_elem, state_out); + return true; + } + + virtual bool StatesEqual(const C& state1, const C& state2) { + return state1 == state2; + } + + PubSubClient* client() { return client_; } + const QName& state_name() { return state_name_; } + + private: + void OnItems(PubSubClient* pub_sub_client, + const std::vector& items) { + for (std::vector::const_iterator item = items.begin(); + item != items.end(); ++item) { + OnItem(*item); + } + } + + void OnItem(const PubSubItem& item) { + const std::string& itemid = item.itemid; + StateItemInfo info; + C new_state; + + bool retracted = !ParseStateItem(item, &info, &new_state); + if (retracted) { + bool known_itemid = + (info_by_itemid_.find(itemid) != info_by_itemid_.end()); + if (!known_itemid) { + // Nothing to retract, and nothing to publish. + // Probably a different state type. + return; + } else { + info = info_by_itemid_[itemid]; + info_by_itemid_.erase(itemid); + new_state = default_state_; + } + } else { + // TODO: Assert new key matches the known key. It + // shouldn't change! + info_by_itemid_[itemid] = info; + } + + std::string key = key_serializer_->GetKey( + info.publisher_nick, info.published_nick); + bool has_old_state = (state_by_key_.find(key) != state_by_key_.end()); + C old_state = has_old_state ? state_by_key_[key] : default_state_; + if ((retracted && !has_old_state) || StatesEqual(new_state, old_state)) { + // Nothing change, so don't bother signalling. + return; + } + + if (retracted || StatesEqual(new_state, default_state_)) { + // We treat a default state similar to a retract. + state_by_key_.erase(key); + } else { + state_by_key_[key] = new_state; + } + + PubSubStateChange change; + if (!retracted) { + // Retracts do not have publisher information. + change.publisher_nick = info.publisher_nick; + } + change.published_nick = info.published_nick; + change.old_state = old_state; + change.new_state = new_state; + SignalStateChange(change); + } + + void OnPublishResult(PubSubClient* pub_sub_client, + const std::string& task_id, + const XmlElement* item) { + SignalPublishResult(task_id, item); + } + + void OnPublishError(PubSubClient* pub_sub_client, + const std::string& task_id, + const buzz::XmlElement* item, + const buzz::XmlElement* stanza) { + SignalPublishError(task_id, item, stanza); + } + + void OnRetractResult(PubSubClient* pub_sub_client, + const std::string& task_id) { + // There's no point in differentiating between publish and retract + // errors, so we simplify by making them both signal a publish + // result. + const XmlElement* item = NULL; + SignalPublishResult(task_id, item); + } + + void OnRetractError(PubSubClient* pub_sub_client, + const std::string& task_id, + const buzz::XmlElement* stanza) { + // There's no point in differentiating between publish and retract + // errors, so we simplify by making them both signal a publish + // error. + const XmlElement* item = NULL; + SignalPublishError(task_id, item, stanza); + } + + std::string publisher_nick_; + PubSubClient* client_; + const QName state_name_; + C default_state_; + talk_base::scoped_ptr key_serializer_; + talk_base::scoped_ptr > state_serializer_; + // key => state + std::map state_by_key_; + // itemid => StateItemInfo + std::map info_by_itemid_; + + DISALLOW_COPY_AND_ASSIGN(PubSubStateClient); +}; +} // namespace buzz + +#endif // TALK_XMPP_PUBSUBSTATECLIENT_H_