Prevent channels being set on stopped transceiver.
Fixing bug that allows a channel to be set on a stopped transceiver.
This CL contains the following refactoring:
1. Extracted ChannelInterface from BaseChannel
2. Unified SetXxxMediaChannel (Voice, Video) into SetMediaChannel
Bug: webrtc:9932
Change-Id: I2fbf00c823b7848ad4f2acb6e80b1b58ac45ee38
Reviewed-on: https://webrtc-review.googlesource.com/c/110564
Reviewed-by: Seth Hampson <shampson@webrtc.org>
Reviewed-by: Steve Anton <steveanton@webrtc.org>
Commit-Queue: Amit Hilbuch <amithi@webrtc.org>
Cr-Commit-Position: refs/heads/master@{#25641}diff --git a/media/base/mediachannel.cc b/media/base/mediachannel.cc
index a5dadd2..0634b2e 100644
--- a/media/base/mediachannel.cc
+++ b/media/base/mediachannel.cc
@@ -87,6 +87,10 @@
return params;
}
+cricket::MediaType VoiceMediaChannel::media_type() const {
+ return cricket::MediaType::MEDIA_TYPE_AUDIO;
+}
+
VideoSendParameters::VideoSendParameters() = default;
VideoSendParameters::~VideoSendParameters() = default;
@@ -96,11 +100,19 @@
return params;
}
+cricket::MediaType VideoMediaChannel::media_type() const {
+ return cricket::MediaType::MEDIA_TYPE_VIDEO;
+}
+
DataMediaChannel::DataMediaChannel() = default;
DataMediaChannel::DataMediaChannel(const MediaConfig& config)
: MediaChannel(config) {}
DataMediaChannel::~DataMediaChannel() = default;
+cricket::MediaType DataMediaChannel::media_type() const {
+ return cricket::MediaType::MEDIA_TYPE_DATA;
+}
+
bool DataMediaChannel::GetStats(DataMediaInfo* info) {
return true;
}
diff --git a/media/base/mediachannel.h b/media/base/mediachannel.h
index 69c7b00..108ae26 100644
--- a/media/base/mediachannel.h
+++ b/media/base/mediachannel.h
@@ -184,6 +184,8 @@
MediaChannel();
~MediaChannel() override;
+ virtual cricket::MediaType media_type() const = 0;
+
// Sets the abstract interface class for sending RTP/RTCP data and
// interface for media transport (experimental). If media transport is
// provided, it should be used instead of RTP/RTCP.
@@ -700,6 +702,8 @@
explicit VoiceMediaChannel(const MediaConfig& config)
: MediaChannel(config) {}
~VoiceMediaChannel() override {}
+
+ cricket::MediaType media_type() const override;
virtual bool SetSendParameters(const AudioSendParameters& params) = 0;
virtual bool SetRecvParameters(const AudioRecvParameters& params) = 0;
virtual webrtc::RtpParameters GetRtpSendParameters(uint32_t ssrc) const = 0;
@@ -772,6 +776,7 @@
: MediaChannel(config) {}
~VideoMediaChannel() override {}
+ cricket::MediaType media_type() const override;
virtual bool SetSendParameters(const VideoSendParameters& params) = 0;
virtual bool SetRecvParameters(const VideoRecvParameters& params) = 0;
virtual webrtc::RtpParameters GetRtpSendParameters(uint32_t ssrc) const = 0;
@@ -883,6 +888,7 @@
explicit DataMediaChannel(const MediaConfig& config);
~DataMediaChannel() override;
+ cricket::MediaType media_type() const override;
virtual bool SetSendParameters(const DataSendParameters& params) = 0;
virtual bool SetRecvParameters(const DataRecvParameters& params) = 0;
diff --git a/pc/BUILD.gn b/pc/BUILD.gn
index 97045ba..4be6052 100644
--- a/pc/BUILD.gn
+++ b/pc/BUILD.gn
@@ -31,6 +31,7 @@
sources = [
"channel.cc",
"channel.h",
+ "channelinterface.h",
"channelmanager.cc",
"channelmanager.h",
"dtlssrtptransport.cc",
@@ -380,6 +381,7 @@
"test/fakevideotrackrenderer.h",
"test/fakevideotracksource.h",
"test/framegeneratorcapturervideotracksource.h",
+ "test/mock_channelinterface.h",
"test/mock_datachannel.h",
"test/mock_rtpreceiverinternal.h",
"test/mock_rtpsenderinternal.h",
@@ -460,6 +462,7 @@
"rtpmediautils_unittest.cc",
"rtpparametersconversion_unittest.cc",
"rtpsenderreceiver_unittest.cc",
+ "rtptransceiver_unittest.cc",
"sctputils_unittest.cc",
"statscollector_unittest.cc",
"test/fakeaudiocapturemodule_unittest.cc",
diff --git a/pc/channel.cc b/pc/channel.cc
index 50fa567..0f857cd 100644
--- a/pc/channel.cc
+++ b/pc/channel.cc
@@ -702,7 +702,7 @@
break;
}
case MSG_FIRSTPACKETRECEIVED: {
- SignalFirstPacketReceived(this);
+ SignalFirstPacketReceived_(this);
break;
}
}
diff --git a/pc/channel.h b/pc/channel.h
index 65d6480..5644bb4 100644
--- a/pc/channel.h
+++ b/pc/channel.h
@@ -29,6 +29,7 @@
#include "media/base/streamparams.h"
#include "p2p/base/dtlstransportinternal.h"
#include "p2p/base/packettransportinternal.h"
+#include "pc/channelinterface.h"
#include "pc/dtlssrtptransport.h"
#include "pc/mediasession.h"
#include "pc/rtptransport.h"
@@ -48,7 +49,6 @@
namespace cricket {
struct CryptoParams;
-class MediaContentDescription;
// BaseChannel contains logic common to voice and video, including enable,
// marshaling calls to a worker and network threads, and connection and media
@@ -68,7 +68,8 @@
// vtable, and the media channel's thread using BaseChannel as the
// NetworkInterface.
-class BaseChannel : public rtc::MessageHandler,
+class BaseChannel : public ChannelInterface,
+ public rtc::MessageHandler,
public sigslot::has_slots<>,
public MediaChannel::NetworkInterface,
public webrtc::RtpPacketSinkInterface {
@@ -94,10 +95,10 @@
rtc::Thread* worker_thread() const { return worker_thread_; }
rtc::Thread* network_thread() const { return network_thread_; }
- const std::string& content_name() const { return content_name_; }
+ const std::string& content_name() const override { return content_name_; }
// TODO(deadbeef): This is redundant; remove this.
- const std::string& transport_name() const { return transport_name_; }
- bool enabled() const { return enabled_; }
+ const std::string& transport_name() const override { return transport_name_; }
+ bool enabled() const override { return enabled_; }
// This function returns true if using SRTP (DTLS-based keying or SDES).
bool srtp_active() const {
@@ -110,17 +111,17 @@
// encryption, an SrtpTransport for SDES or a DtlsSrtpTransport for DTLS-SRTP.
// This can be called from any thread and it hops to the network thread
// internally. It would replace the |SetTransports| and its variants.
- bool SetRtpTransport(webrtc::RtpTransportInternal* rtp_transport);
+ bool SetRtpTransport(webrtc::RtpTransportInternal* rtp_transport) override;
// Channel control
bool SetLocalContent(const MediaContentDescription* content,
webrtc::SdpType type,
- std::string* error_desc);
+ std::string* error_desc) override;
bool SetRemoteContent(const MediaContentDescription* content,
webrtc::SdpType type,
- std::string* error_desc);
+ std::string* error_desc) override;
- bool Enable(bool enable);
+ bool Enable(bool enable) override;
// TODO(zhihuang): These methods are used for testing and can be removed.
bool AddRecvStream(const StreamParams& sp);
@@ -140,7 +141,9 @@
void SignalDtlsSrtpSetupFailure_s(bool rtcp);
// Used for latency measurements.
- sigslot::signal1<BaseChannel*> SignalFirstPacketReceived;
+ sigslot::signal1<ChannelInterface*>& SignalFirstPacketReceived() override {
+ return SignalFirstPacketReceived_;
+ }
// Forward SignalSentPacket to worker thread.
sigslot::signal1<const rtc::SentPacket&> SignalSentPacket;
@@ -176,8 +179,6 @@
int SetOption(SocketType type, rtc::Socket::Option o, int val) override;
int SetOption_n(SocketType type, rtc::Socket::Option o, int val);
- virtual cricket::MediaType media_type() = 0;
-
// RtpPacketSinkInterface overrides.
void OnRtpPacket(const webrtc::RtpPacketReceived& packet) override;
@@ -187,9 +188,9 @@
transport_name_ = transport_name;
}
- protected:
- virtual MediaChannel* media_channel() const { return media_channel_.get(); }
+ MediaChannel* media_channel() const override { return media_channel_.get(); }
+ protected:
bool was_ever_writable() const { return was_ever_writable_; }
void set_local_content_direction(webrtc::RtpTransceiverDirection direction) {
local_content_direction_ = direction;
@@ -306,6 +307,7 @@
rtc::Thread* const network_thread_;
rtc::Thread* const signaling_thread_;
rtc::AsyncInvoker invoker_;
+ sigslot::signal1<ChannelInterface*> SignalFirstPacketReceived_;
const std::string content_name_;
@@ -363,7 +365,9 @@
return static_cast<VoiceMediaChannel*>(BaseChannel::media_channel());
}
- cricket::MediaType media_type() override { return cricket::MEDIA_TYPE_AUDIO; }
+ cricket::MediaType media_type() const override {
+ return cricket::MEDIA_TYPE_AUDIO;
+ }
private:
// overrides from BaseChannel
@@ -402,7 +406,9 @@
void FillBitrateInfo(BandwidthEstimationInfo* bwe_info);
- cricket::MediaType media_type() override { return cricket::MEDIA_TYPE_VIDEO; }
+ cricket::MediaType media_type() const override {
+ return cricket::MEDIA_TYPE_VIDEO;
+ }
private:
// overrides from BaseChannel
@@ -454,7 +460,9 @@
// That occurs when the channel is enabled, the transport is writable,
// both local and remote descriptions are set, and the channel is unblocked.
sigslot::signal1<bool> SignalReadyToSendData;
- cricket::MediaType media_type() override { return cricket::MEDIA_TYPE_DATA; }
+ cricket::MediaType media_type() const override {
+ return cricket::MEDIA_TYPE_DATA;
+ }
protected:
// downcasts a MediaChannel.
diff --git a/pc/channelinterface.h b/pc/channelinterface.h
new file mode 100644
index 0000000..8e4109a
--- /dev/null
+++ b/pc/channelinterface.h
@@ -0,0 +1,68 @@
+/*
+ * Copyright 2018 The WebRTC project authors. All Rights Reserved.
+ *
+ * Use of this source code is governed by a BSD-style license
+ * that can be found in the LICENSE file in the root of the source
+ * tree. An additional intellectual property rights grant can be found
+ * in the file PATENTS. All contributing project authors may
+ * be found in the AUTHORS file in the root of the source tree.
+ */
+
+#ifndef PC_CHANNELINTERFACE_H_
+#define PC_CHANNELINTERFACE_H_
+
+#include <string>
+
+#include "api/jsep.h"
+#include "api/mediatypes.h"
+#include "media/base/mediachannel.h"
+#include "pc/rtptransportinternal.h"
+
+namespace cricket {
+
+class MediaContentDescription;
+
+// ChannelInterface contains methods common to voice, video and data channels.
+// As more methods are added to BaseChannel, they should be included in the
+// interface as well.
+class ChannelInterface {
+ public:
+ virtual cricket::MediaType media_type() const = 0;
+
+ virtual MediaChannel* media_channel() const = 0;
+
+ // TODO(deadbeef): This is redundant; remove this.
+ virtual const std::string& transport_name() const = 0;
+
+ virtual const std::string& content_name() const = 0;
+
+ virtual bool enabled() const = 0;
+
+ // Enables or disables this channel
+ virtual bool Enable(bool enable) = 0;
+
+ // Used for latency measurements.
+ virtual sigslot::signal1<ChannelInterface*>& SignalFirstPacketReceived() = 0;
+
+ // Channel control
+ virtual bool SetLocalContent(const MediaContentDescription* content,
+ webrtc::SdpType type,
+ std::string* error_desc) = 0;
+ virtual bool SetRemoteContent(const MediaContentDescription* content,
+ webrtc::SdpType type,
+ std::string* error_desc) = 0;
+
+ // Set an RTP level transport.
+ // Some examples:
+ // * An RtpTransport without encryption.
+ // * An SrtpTransport for SDES.
+ // * A DtlsSrtpTransport for DTLS-SRTP.
+ virtual bool SetRtpTransport(webrtc::RtpTransportInternal* rtp_transport) = 0;
+
+ protected:
+ virtual ~ChannelInterface() = default;
+};
+
+} // namespace cricket
+
+#endif // PC_CHANNELINTERFACE_H_
diff --git a/pc/peerconnection.cc b/pc/peerconnection.cc
index 3fa5ca5..47ecb4e 100644
--- a/pc/peerconnection.cc
+++ b/pc/peerconnection.cc
@@ -1253,7 +1253,7 @@
auto new_sender =
CreateSender(media_type, track->id(), track, adjusted_stream_ids, {});
if (track->kind() == MediaStreamTrackInterface::kAudioKind) {
- new_sender->internal()->SetVoiceMediaChannel(voice_media_channel());
+ new_sender->internal()->SetMediaChannel(voice_media_channel());
GetAudioTransceiver()->internal()->AddSender(new_sender);
const RtpSenderInfo* sender_info =
FindSenderInfo(local_audio_sender_infos_,
@@ -1263,7 +1263,7 @@
}
} else {
RTC_DCHECK_EQ(MediaStreamTrackInterface::kVideoKind, track->kind());
- new_sender->internal()->SetVideoMediaChannel(video_media_channel());
+ new_sender->internal()->SetMediaChannel(video_media_channel());
GetVideoTransceiver()->internal()->AddSender(new_sender);
const RtpSenderInfo* sender_info =
FindSenderInfo(local_video_sender_infos_,
@@ -1593,14 +1593,14 @@
if (kind == MediaStreamTrackInterface::kAudioKind) {
auto* audio_sender = new AudioRtpSender(
worker_thread(), rtc::CreateRandomUuid(), stats_.get());
- audio_sender->SetVoiceMediaChannel(voice_media_channel());
+ audio_sender->SetMediaChannel(voice_media_channel());
new_sender = RtpSenderProxyWithInternal<RtpSenderInternal>::Create(
signaling_thread(), audio_sender);
GetAudioTransceiver()->internal()->AddSender(new_sender);
} else if (kind == MediaStreamTrackInterface::kVideoKind) {
auto* video_sender =
new VideoRtpSender(worker_thread(), rtc::CreateRandomUuid());
- video_sender->SetVideoMediaChannel(video_media_channel());
+ video_sender->SetMediaChannel(video_media_channel());
new_sender = RtpSenderProxyWithInternal<RtpSenderInternal>::Create(
signaling_thread(), video_sender);
GetVideoTransceiver()->internal()->AddSender(new_sender);
@@ -2736,11 +2736,11 @@
const cricket::ContentGroup* bundle_group) {
RTC_DCHECK(IsUnifiedPlan());
RTC_DCHECK(transceiver);
- cricket::BaseChannel* channel = transceiver->internal()->channel();
+ cricket::ChannelInterface* channel = transceiver->internal()->channel();
if (content.rejected) {
if (channel) {
transceiver->internal()->SetChannel(nullptr);
- DestroyBaseChannel(channel);
+ DestroyChannelInterface(channel);
}
} else {
if (!channel) {
@@ -3482,7 +3482,7 @@
// the constructor taking stream IDs instead.
auto* audio_receiver = new AudioRtpReceiver(
worker_thread(), remote_sender_info.sender_id, streams);
- audio_receiver->SetVoiceMediaChannel(voice_media_channel());
+ audio_receiver->SetMediaChannel(voice_media_channel());
audio_receiver->SetupMediaChannel(remote_sender_info.first_ssrc);
auto receiver = RtpReceiverProxyWithInternal<RtpReceiverInternal>::Create(
signaling_thread(), audio_receiver);
@@ -3500,7 +3500,7 @@
// the constructor taking stream IDs instead.
auto* video_receiver = new VideoRtpReceiver(
worker_thread(), remote_sender_info.sender_id, streams);
- video_receiver->SetVideoMediaChannel(video_media_channel());
+ video_receiver->SetMediaChannel(video_media_channel());
video_receiver->SetupMediaChannel(remote_sender_info.first_ssrc);
auto receiver = RtpReceiverProxyWithInternal<RtpReceiverInternal>::Create(
signaling_thread(), video_receiver);
@@ -3543,7 +3543,7 @@
// Normal case; we've never seen this track before.
auto new_sender = CreateSender(cricket::MEDIA_TYPE_AUDIO, track->id(), track,
{stream->id()}, {});
- new_sender->internal()->SetVoiceMediaChannel(voice_media_channel());
+ new_sender->internal()->SetMediaChannel(voice_media_channel());
GetAudioTransceiver()->internal()->AddSender(new_sender);
// If the sender has already been configured in SDP, we call SetSsrc,
// which will connect the sender to the underlying transport. This can
@@ -3588,7 +3588,7 @@
// Normal case; we've never seen this track before.
auto new_sender = CreateSender(cricket::MEDIA_TYPE_VIDEO, track->id(), track,
{stream->id()}, {});
- new_sender->internal()->SetVideoMediaChannel(video_media_channel());
+ new_sender->internal()->SetMediaChannel(video_media_channel());
GetVideoTransceiver()->internal()->AddSender(new_sender);
const RtpSenderInfo* sender_info =
FindSenderInfo(local_video_sender_infos_, stream->id(), track->id());
@@ -4966,10 +4966,10 @@
}
}
-cricket::BaseChannel* PeerConnection::GetChannel(
+cricket::ChannelInterface* PeerConnection::GetChannel(
const std::string& content_name) {
for (auto transceiver : transceivers_) {
- cricket::BaseChannel* channel = transceiver->internal()->channel();
+ cricket::ChannelInterface* channel = transceiver->internal()->channel();
if (channel && channel->content_name() == content_name) {
return channel;
}
@@ -5086,7 +5086,7 @@
for (auto transceiver : transceivers_) {
const ContentInfo* content_info =
FindMediaSectionForTransceiver(transceiver, sdesc);
- cricket::BaseChannel* channel = transceiver->internal()->channel();
+ cricket::ChannelInterface* channel = transceiver->internal()->channel();
if (!channel || !content_info || content_info->rejected) {
continue;
}
@@ -5421,7 +5421,7 @@
const {
std::map<std::string, std::string> transport_names_by_mid;
for (auto transceiver : transceivers_) {
- cricket::BaseChannel* channel = transceiver->internal()->channel();
+ cricket::ChannelInterface* channel = transceiver->internal()->channel();
if (channel) {
transport_names_by_mid[channel->content_name()] =
channel->transport_name();
@@ -5598,7 +5598,7 @@
void PeerConnection::EnableSending() {
for (auto transceiver : transceivers_) {
- cricket::BaseChannel* channel = transceiver->internal()->channel();
+ cricket::ChannelInterface* channel = transceiver->internal()->channel();
if (channel && !channel->enabled()) {
channel->Enable(true);
}
@@ -6484,7 +6484,7 @@
const std::string PeerConnection::GetTransportName(
const std::string& content_name) {
- cricket::BaseChannel* channel = GetChannel(content_name);
+ cricket::ChannelInterface* channel = GetChannel(content_name);
if (channel) {
return channel->transport_name();
}
@@ -6503,17 +6503,17 @@
transceiver) {
RTC_DCHECK(transceiver);
- cricket::BaseChannel* channel = transceiver->internal()->channel();
+ cricket::ChannelInterface* channel = transceiver->internal()->channel();
if (channel) {
transceiver->internal()->SetChannel(nullptr);
- DestroyBaseChannel(channel);
+ DestroyChannelInterface(channel);
}
}
void PeerConnection::DestroyDataChannel() {
if (rtp_data_channel_) {
OnDataChannelDestroyed();
- DestroyBaseChannel(rtp_data_channel_);
+ DestroyChannelInterface(rtp_data_channel_);
rtp_data_channel_ = nullptr;
}
@@ -6538,7 +6538,8 @@
}
}
-void PeerConnection::DestroyBaseChannel(cricket::BaseChannel* channel) {
+void PeerConnection::DestroyChannelInterface(
+ cricket::ChannelInterface* channel) {
RTC_DCHECK(channel);
switch (channel->media_type()) {
case cricket::MEDIA_TYPE_AUDIO:
diff --git a/pc/peerconnection.h b/pc/peerconnection.h
index fe4d777..b5ae9d2 100644
--- a/pc/peerconnection.h
+++ b/pc/peerconnection.h
@@ -719,7 +719,7 @@
SessionError session_error() const { return session_error_; }
const std::string& session_error_desc() const { return session_error_desc_; }
- cricket::BaseChannel* GetChannel(const std::string& content_name);
+ cricket::ChannelInterface* GetChannel(const std::string& content_name);
// Get current SSL role used by SCTP's underlying transport.
bool GetSctpSslRole(rtc::SSLRole* role);
@@ -922,9 +922,9 @@
// Destroys the RTP data channel and/or the SCTP data channel and clears it.
void DestroyDataChannel();
- // Destroys the given BaseChannel. The channel cannot be accessed after this
- // method is called.
- void DestroyBaseChannel(cricket::BaseChannel* channel);
+ // Destroys the given ChannelInterface.
+ // The channel cannot be accessed after this method is called.
+ void DestroyChannelInterface(cricket::ChannelInterface* channel);
// JsepTransportController::Observer override.
//
diff --git a/pc/peerconnection_ice_unittest.cc b/pc/peerconnection_ice_unittest.cc
index 8fde875..e015abf 100644
--- a/pc/peerconnection_ice_unittest.cc
+++ b/pc/peerconnection_ice_unittest.cc
@@ -212,7 +212,12 @@
PeerConnection* pc = static_cast<PeerConnection*>(pc_proxy->internal());
for (auto transceiver : pc->GetTransceiversInternal()) {
if (transceiver->media_type() == cricket::MEDIA_TYPE_AUDIO) {
- cricket::BaseChannel* channel = transceiver->internal()->channel();
+ // TODO(amithi): This test seems to be using a method that should not
+ // be public |rtp_packet_transport|. Because the test is not mocking
+ // the channels or transceiver, workaround will be to |static_cast|
+ // the channel until the method is rewritten.
+ cricket::BaseChannel* channel = static_cast<cricket::BaseChannel*>(
+ transceiver->internal()->channel());
if (channel) {
auto dtls_transport = static_cast<cricket::DtlsTransportInternal*>(
channel->rtp_packet_transport());
diff --git a/pc/rtcstatscollector.cc b/pc/rtcstatscollector.cc
index 32e0b65..871cff9 100644
--- a/pc/rtcstatscollector.cc
+++ b/pc/rtcstatscollector.cc
@@ -1426,7 +1426,7 @@
stats.transceiver = transceiver->internal();
stats.media_type = media_type;
- cricket::BaseChannel* channel = transceiver->internal()->channel();
+ cricket::ChannelInterface* channel = transceiver->internal()->channel();
if (!channel) {
// The remaining fields require a BaseChannel.
continue;
diff --git a/pc/rtpreceiver.cc b/pc/rtpreceiver.cc
index acea930..1916a73 100644
--- a/pc/rtpreceiver.cc
+++ b/pc/rtpreceiver.cc
@@ -268,9 +268,10 @@
}
}
-void AudioRtpReceiver::SetVoiceMediaChannel(
- cricket::VoiceMediaChannel* voice_media_channel) {
- media_channel_ = voice_media_channel;
+void AudioRtpReceiver::SetMediaChannel(cricket::MediaChannel* media_channel) {
+ RTC_DCHECK(media_channel == nullptr ||
+ media_channel->media_type() == media_type());
+ media_channel_ = static_cast<cricket::VoiceMediaChannel*>(media_channel);
}
void AudioRtpReceiver::NotifyFirstPacketReceived() {
@@ -443,9 +444,10 @@
}
}
-void VideoRtpReceiver::SetVideoMediaChannel(
- cricket::VideoMediaChannel* video_media_channel) {
- media_channel_ = video_media_channel;
+void VideoRtpReceiver::SetMediaChannel(cricket::MediaChannel* media_channel) {
+ RTC_DCHECK(media_channel == nullptr ||
+ media_channel->media_type() == media_type());
+ media_channel_ = static_cast<cricket::VideoMediaChannel*>(media_channel);
}
void VideoRtpReceiver::NotifyFirstPacketReceived() {
diff --git a/pc/rtpreceiver.h b/pc/rtpreceiver.h
index 85be8c2..06331a6 100644
--- a/pc/rtpreceiver.h
+++ b/pc/rtpreceiver.h
@@ -34,14 +34,10 @@
virtual void Stop() = 0;
// Sets the underlying MediaEngine channel associated with this RtpSender.
- // SetVoiceMediaChannel should be used for audio RtpSenders and
- // SetVideoMediaChannel should be used for video RtpSenders. Must call the
- // appropriate SetXxxMediaChannel(nullptr) before the media channel is
- // destroyed.
- virtual void SetVoiceMediaChannel(
- cricket::VoiceMediaChannel* voice_media_channel) = 0;
- virtual void SetVideoMediaChannel(
- cricket::VideoMediaChannel* video_media_channel) = 0;
+ // A VoiceMediaChannel should be used for audio RtpSenders and
+ // a VideoMediaChannel should be used for video RtpSenders.
+ // Must call SetMediaChannel(nullptr) before the media channel is destroyed.
+ virtual void SetMediaChannel(cricket::MediaChannel* media_channel) = 0;
// Configures the RtpReceiver with the underlying media channel, with the
// given SSRC as the stream identifier. If |ssrc| is 0, the receiver will
@@ -130,13 +126,8 @@
void SetStreams(const std::vector<rtc::scoped_refptr<MediaStreamInterface>>&
streams) override;
void SetObserver(RtpReceiverObserverInterface* observer) override;
- void SetVoiceMediaChannel(
- cricket::VoiceMediaChannel* voice_media_channel) override;
- void SetVideoMediaChannel(
- cricket::VideoMediaChannel* video_media_channel) override {
- RTC_NOTREACHED();
- }
+ void SetMediaChannel(cricket::MediaChannel* media_channel) override;
std::vector<RtpSource> GetSources() const override;
int AttachmentId() const override { return attachment_id_; }
@@ -217,13 +208,7 @@
void SetObserver(RtpReceiverObserverInterface* observer) override;
- void SetVoiceMediaChannel(
- cricket::VoiceMediaChannel* voice_media_channel) override {
- RTC_NOTREACHED();
- }
-
- void SetVideoMediaChannel(
- cricket::VideoMediaChannel* video_media_channel) override;
+ void SetMediaChannel(cricket::MediaChannel* media_channel) override;
int AttachmentId() const override { return attachment_id_; }
diff --git a/pc/rtpsender.cc b/pc/rtpsender.cc
index abd1748..76fdca6 100644
--- a/pc/rtpsender.cc
+++ b/pc/rtpsender.cc
@@ -386,9 +386,10 @@
stopped_ = true;
}
-void AudioRtpSender::SetVoiceMediaChannel(
- cricket::VoiceMediaChannel* voice_media_channel) {
- media_channel_ = voice_media_channel;
+void AudioRtpSender::SetMediaChannel(cricket::MediaChannel* media_channel) {
+ RTC_DCHECK(media_channel == nullptr ||
+ media_channel->media_type() == media_type());
+ media_channel_ = static_cast<cricket::VoiceMediaChannel*>(media_channel);
}
void AudioRtpSender::SetAudioSend() {
@@ -629,9 +630,10 @@
stopped_ = true;
}
-void VideoRtpSender::SetVideoMediaChannel(
- cricket::VideoMediaChannel* video_media_channel) {
- media_channel_ = video_media_channel;
+void VideoRtpSender::SetMediaChannel(cricket::MediaChannel* media_channel) {
+ RTC_DCHECK(media_channel == nullptr ||
+ media_channel->media_type() == media_type());
+ media_channel_ = static_cast<cricket::VideoMediaChannel*>(media_channel);
}
void VideoRtpSender::SetVideoSend() {
diff --git a/pc/rtpsender.h b/pc/rtpsender.h
index 2a8289f..07fae6b 100644
--- a/pc/rtpsender.h
+++ b/pc/rtpsender.h
@@ -36,14 +36,10 @@
class RtpSenderInternal : public RtpSenderInterface {
public:
// Sets the underlying MediaEngine channel associated with this RtpSender.
- // SetVoiceMediaChannel should be used for audio RtpSenders and
- // SetVideoMediaChannel should be used for video RtpSenders. Must call the
- // appropriate SetXxxMediaChannel(nullptr) before the media channel is
- // destroyed.
- virtual void SetVoiceMediaChannel(
- cricket::VoiceMediaChannel* voice_media_channel) = 0;
- virtual void SetVideoMediaChannel(
- cricket::VideoMediaChannel* video_media_channel) = 0;
+ // A VoiceMediaChannel should be used for audio RtpSenders and
+ // a VideoMediaChannel should be used for video RtpSenders.
+ // Must call SetMediaChannel(nullptr) before the media channel is destroyed.
+ virtual void SetMediaChannel(cricket::MediaChannel* media_channel) = 0;
// Used to set the SSRC of the sender, once a local description has been set.
// If |ssrc| is 0, this indiates that the sender should disconnect from the
@@ -156,13 +152,7 @@
int AttachmentId() const override { return attachment_id_; }
- void SetVoiceMediaChannel(
- cricket::VoiceMediaChannel* voice_media_channel) override;
-
- void SetVideoMediaChannel(
- cricket::VideoMediaChannel* video_media_channel) override {
- RTC_NOTREACHED();
- }
+ void SetMediaChannel(cricket::MediaChannel* media_channel) override;
private:
// TODO(nisse): Since SSRC == 0 is technically valid, figure out
@@ -253,13 +243,7 @@
void Stop() override;
int AttachmentId() const override { return attachment_id_; }
- void SetVoiceMediaChannel(
- cricket::VoiceMediaChannel* voice_media_channel) override {
- RTC_NOTREACHED();
- }
-
- void SetVideoMediaChannel(
- cricket::VideoMediaChannel* video_media_channel) override;
+ void SetMediaChannel(cricket::MediaChannel* media_channel) override;
private:
bool can_send_track() const { return track_ && ssrc_; }
diff --git a/pc/rtpsenderreceiver_unittest.cc b/pc/rtpsenderreceiver_unittest.cc
index fa42615..b5a513b 100644
--- a/pc/rtpsenderreceiver_unittest.cc
+++ b/pc/rtpsenderreceiver_unittest.cc
@@ -154,7 +154,7 @@
new AudioRtpSender(worker_thread_, audio_track_->id(), nullptr);
ASSERT_TRUE(audio_rtp_sender_->SetTrack(audio_track_));
audio_rtp_sender_->set_stream_ids({local_stream_->id()});
- audio_rtp_sender_->SetVoiceMediaChannel(voice_media_channel_);
+ audio_rtp_sender_->SetMediaChannel(voice_media_channel_);
audio_rtp_sender_->SetSsrc(kAudioSsrc);
audio_rtp_sender_->GetOnDestroyedSignal()->connect(
this, &RtpSenderReceiverTest::OnAudioSenderDestroyed);
@@ -163,7 +163,7 @@
void CreateAudioRtpSenderWithNoTrack() {
audio_rtp_sender_ = new AudioRtpSender(worker_thread_, /*id=*/"", nullptr);
- audio_rtp_sender_->SetVoiceMediaChannel(voice_media_channel_);
+ audio_rtp_sender_->SetMediaChannel(voice_media_channel_);
}
void OnAudioSenderDestroyed() { audio_sender_destroyed_signal_fired_ = true; }
@@ -191,13 +191,13 @@
video_rtp_sender_ = new VideoRtpSender(worker_thread_, video_track_->id());
ASSERT_TRUE(video_rtp_sender_->SetTrack(video_track_));
video_rtp_sender_->set_stream_ids({local_stream_->id()});
- video_rtp_sender_->SetVideoMediaChannel(video_media_channel_);
+ video_rtp_sender_->SetMediaChannel(video_media_channel_);
video_rtp_sender_->SetSsrc(ssrc);
VerifyVideoChannelInput(ssrc);
}
void CreateVideoRtpSenderWithNoTrack() {
video_rtp_sender_ = new VideoRtpSender(worker_thread_, /*id=*/"");
- video_rtp_sender_->SetVideoMediaChannel(video_media_channel_);
+ video_rtp_sender_->SetMediaChannel(video_media_channel_);
}
void DestroyAudioRtpSender() {
@@ -214,7 +214,7 @@
std::vector<rtc::scoped_refptr<MediaStreamInterface>> streams = {}) {
audio_rtp_receiver_ = new AudioRtpReceiver(
rtc::Thread::Current(), kAudioTrackId, std::move(streams));
- audio_rtp_receiver_->SetVoiceMediaChannel(voice_media_channel_);
+ audio_rtp_receiver_->SetMediaChannel(voice_media_channel_);
audio_rtp_receiver_->SetupMediaChannel(kAudioSsrc);
audio_track_ = audio_rtp_receiver_->audio_track();
VerifyVoiceChannelOutput();
@@ -224,7 +224,7 @@
std::vector<rtc::scoped_refptr<MediaStreamInterface>> streams = {}) {
video_rtp_receiver_ = new VideoRtpReceiver(
rtc::Thread::Current(), kVideoTrackId, std::move(streams));
- video_rtp_receiver_->SetVideoMediaChannel(video_media_channel_);
+ video_rtp_receiver_->SetMediaChannel(video_media_channel_);
video_rtp_receiver_->SetupMediaChannel(kVideoSsrc);
video_track_ = video_rtp_receiver_->video_track();
VerifyVideoChannelOutput();
@@ -664,7 +664,7 @@
cricket::StreamParams stream_params =
cricket::CreateSimStreamParams("cname", ssrcs);
voice_media_channel_->AddSendStream(stream_params);
- audio_rtp_sender_->SetVoiceMediaChannel(voice_media_channel_);
+ audio_rtp_sender_->SetMediaChannel(voice_media_channel_);
audio_rtp_sender_->SetSsrc(1);
params = audio_rtp_sender_->GetParameters();
@@ -903,7 +903,7 @@
cricket::StreamParams stream_params =
cricket::CreateSimStreamParams("cname", ssrcs);
video_media_channel_->AddSendStream(stream_params);
- video_rtp_sender_->SetVideoMediaChannel(video_media_channel_);
+ video_rtp_sender_->SetMediaChannel(video_media_channel_);
video_rtp_sender_->SetSsrc(kVideoSsrcSimulcast);
params = video_rtp_sender_->GetParameters();
@@ -938,7 +938,7 @@
cricket::StreamParams stream_params =
cricket::CreateSimStreamParams("cname", ssrcs);
video_media_channel_->AddSendStream(stream_params);
- video_rtp_sender_->SetVideoMediaChannel(video_media_channel_);
+ video_rtp_sender_->SetMediaChannel(video_media_channel_);
video_rtp_sender_->SetSsrc(kVideoSsrcSimulcast);
params = video_rtp_sender_->GetParameters();
@@ -1332,7 +1332,7 @@
video_rtp_sender_ = new VideoRtpSender(worker_thread_, video_track_->id());
ASSERT_TRUE(video_rtp_sender_->SetTrack(video_track_));
video_rtp_sender_->set_stream_ids({local_stream_->id()});
- video_rtp_sender_->SetVideoMediaChannel(video_media_channel_);
+ video_rtp_sender_->SetMediaChannel(video_media_channel_);
video_track_->set_enabled(true);
// Sender is not ready to send (no SSRC) so no option should have been set.
diff --git a/pc/rtptransceiver.cc b/pc/rtptransceiver.cc
index 0fe8ea6..8b56b8b 100644
--- a/pc/rtptransceiver.cc
+++ b/pc/rtptransceiver.cc
@@ -38,52 +38,45 @@
Stop();
}
-void RtpTransceiver::SetChannel(cricket::BaseChannel* channel) {
+void RtpTransceiver::SetChannel(cricket::ChannelInterface* channel) {
+ // Cannot set a non-null channel on a stopped transceiver.
+ if (stopped_ && channel) {
+ return;
+ }
+
if (channel) {
RTC_DCHECK_EQ(media_type(), channel->media_type());
}
if (channel_) {
- channel_->SignalFirstPacketReceived.disconnect(this);
+ channel_->SignalFirstPacketReceived().disconnect(this);
}
channel_ = channel;
if (channel_) {
- channel_->SignalFirstPacketReceived.connect(
+ channel_->SignalFirstPacketReceived().connect(
this, &RtpTransceiver::OnFirstPacketReceived);
}
for (auto sender : senders_) {
- if (media_type() == cricket::MEDIA_TYPE_AUDIO) {
- auto* voice_channel = static_cast<cricket::VoiceChannel*>(channel);
- sender->internal()->SetVoiceMediaChannel(
- voice_channel ? voice_channel->media_channel() : nullptr);
- } else {
- auto* video_channel = static_cast<cricket::VideoChannel*>(channel);
- sender->internal()->SetVideoMediaChannel(
- video_channel ? video_channel->media_channel() : nullptr);
- }
+ sender->internal()->SetMediaChannel(channel_ ? channel_->media_channel()
+ : nullptr);
}
for (auto receiver : receivers_) {
- if (!channel) {
+ if (!channel_) {
receiver->internal()->Stop();
}
- if (media_type() == cricket::MEDIA_TYPE_AUDIO) {
- auto* voice_channel = static_cast<cricket::VoiceChannel*>(channel);
- receiver->internal()->SetVoiceMediaChannel(
- voice_channel ? voice_channel->media_channel() : nullptr);
- } else {
- auto* video_channel = static_cast<cricket::VideoChannel*>(channel);
- receiver->internal()->SetVideoMediaChannel(
- video_channel ? video_channel->media_channel() : nullptr);
- }
+
+ receiver->internal()->SetMediaChannel(channel_ ? channel_->media_channel()
+ : nullptr);
}
}
void RtpTransceiver::AddSender(
rtc::scoped_refptr<RtpSenderProxyWithInternal<RtpSenderInternal>> sender) {
+ RTC_DCHECK(!stopped_);
RTC_DCHECK(!unified_plan_);
RTC_DCHECK(sender);
RTC_DCHECK_EQ(media_type(), sender->media_type());
@@ -109,6 +102,7 @@
void RtpTransceiver::AddReceiver(
rtc::scoped_refptr<RtpReceiverProxyWithInternal<RtpReceiverInternal>>
receiver) {
+ RTC_DCHECK(!stopped_);
RTC_DCHECK(!unified_plan_);
RTC_DCHECK(receiver);
RTC_DCHECK_EQ(media_type(), receiver->media_type());
@@ -152,7 +146,7 @@
return mid_;
}
-void RtpTransceiver::OnFirstPacketReceived(cricket::BaseChannel* channel) {
+void RtpTransceiver::OnFirstPacketReceived(cricket::ChannelInterface*) {
for (auto receiver : receivers_) {
receiver->internal()->NotifyFirstPacketReceived();
}
diff --git a/pc/rtptransceiver.h b/pc/rtptransceiver.h
index 3e0d433..07db196 100644
--- a/pc/rtptransceiver.h
+++ b/pc/rtptransceiver.h
@@ -70,11 +70,11 @@
// Returns the Voice/VideoChannel set for this transceiver. May be null if
// the transceiver is not in the currently set local/remote description.
- cricket::BaseChannel* channel() const { return channel_; }
+ cricket::ChannelInterface* channel() const { return channel_; }
// Sets the Voice/VideoChannel. The caller must pass in the correct channel
// implementation based on the type of the transceiver.
- void SetChannel(cricket::BaseChannel* channel);
+ void SetChannel(cricket::ChannelInterface* channel);
// Adds an RtpSender of the appropriate type to be owned by this transceiver.
// Must not be null.
@@ -177,7 +177,7 @@
void SetCodecPreferences(rtc::ArrayView<RtpCodecCapability> codecs) override;
private:
- void OnFirstPacketReceived(cricket::BaseChannel* channel);
+ void OnFirstPacketReceived(cricket::ChannelInterface* channel);
const bool unified_plan_;
const cricket::MediaType media_type_;
@@ -196,7 +196,7 @@
bool created_by_addtrack_ = false;
bool has_ever_been_used_to_send_ = false;
- cricket::BaseChannel* channel_ = nullptr;
+ cricket::ChannelInterface* channel_ = nullptr;
};
BEGIN_SIGNALING_PROXY_MAP(RtpTransceiver)
diff --git a/pc/rtptransceiver_unittest.cc b/pc/rtptransceiver_unittest.cc
new file mode 100644
index 0000000..b57d212
--- /dev/null
+++ b/pc/rtptransceiver_unittest.cc
@@ -0,0 +1,71 @@
+/*
+ * Copyright 2018 The WebRTC project authors. All Rights Reserved.
+ *
+ * Use of this source code is governed by a BSD-style license
+ * that can be found in the LICENSE file in the root of the source
+ * tree. An additional intellectual property rights grant can be found
+ * in the file PATENTS. All contributing project authors may
+ * be found in the AUTHORS file in the root of the source tree.
+ */
+
+// This file contains tests for |RtpTransceiver|.
+
+#include "pc/rtptransceiver.h"
+#include "rtc_base/gunit.h"
+#include "test/gmock.h"
+#include "test/mock_channelinterface.h"
+
+using ::testing::Return;
+using ::testing::ReturnRef;
+
+namespace webrtc {
+
+// Checks that a channel cannot be set on a stopped |RtpTransceiver|.
+TEST(RtpTransceiverTest, CannotSetChannelOnStoppedTransceiver) {
+ RtpTransceiver transceiver(cricket::MediaType::MEDIA_TYPE_AUDIO);
+ cricket::MockChannelInterface channel1;
+ sigslot::signal1<cricket::ChannelInterface*> signal;
+ EXPECT_CALL(channel1, media_type())
+ .WillRepeatedly(Return(cricket::MediaType::MEDIA_TYPE_AUDIO));
+ EXPECT_CALL(channel1, SignalFirstPacketReceived())
+ .WillRepeatedly(ReturnRef(signal));
+
+ transceiver.SetChannel(&channel1);
+ EXPECT_EQ(&channel1, transceiver.channel());
+
+ // Stop the transceiver.
+ transceiver.Stop();
+ EXPECT_EQ(&channel1, transceiver.channel());
+
+ cricket::MockChannelInterface channel2;
+ EXPECT_CALL(channel2, media_type())
+ .WillRepeatedly(Return(cricket::MediaType::MEDIA_TYPE_AUDIO));
+
+ // Channel can no longer be set, so this call should be a no-op.
+ transceiver.SetChannel(&channel2);
+ EXPECT_EQ(&channel1, transceiver.channel());
+}
+
+// Checks that a channel can be unset on a stopped |RtpTransceiver|
+TEST(RtpTransceiverTest, CanUnsetChannelOnStoppedTransceiver) {
+ RtpTransceiver transceiver(cricket::MediaType::MEDIA_TYPE_VIDEO);
+ cricket::MockChannelInterface channel;
+ sigslot::signal1<cricket::ChannelInterface*> signal;
+ EXPECT_CALL(channel, media_type())
+ .WillRepeatedly(Return(cricket::MediaType::MEDIA_TYPE_VIDEO));
+ EXPECT_CALL(channel, SignalFirstPacketReceived())
+ .WillRepeatedly(ReturnRef(signal));
+
+ transceiver.SetChannel(&channel);
+ EXPECT_EQ(&channel, transceiver.channel());
+
+ // Stop the transceiver.
+ transceiver.Stop();
+ EXPECT_EQ(&channel, transceiver.channel());
+
+ // Set the channel to |nullptr|.
+ transceiver.SetChannel(nullptr);
+ EXPECT_EQ(nullptr, transceiver.channel());
+}
+
+} // namespace webrtc
diff --git a/pc/test/mock_channelinterface.h b/pc/test/mock_channelinterface.h
new file mode 100644
index 0000000..4c32a8f
--- /dev/null
+++ b/pc/test/mock_channelinterface.h
@@ -0,0 +1,47 @@
+/*
+ * Copyright 2018 The WebRTC project authors. All Rights Reserved.
+ *
+ * Use of this source code is governed by a BSD-style license
+ * that can be found in the LICENSE file in the root of the source
+ * tree. An additional intellectual property rights grant can be found
+ * in the file PATENTS. All contributing project authors may
+ * be found in the AUTHORS file in the root of the source tree.
+ */
+
+#ifndef PC_TEST_MOCK_CHANNELINTERFACE_H_
+#define PC_TEST_MOCK_CHANNELINTERFACE_H_
+
+#include <string>
+
+#include "pc/channelinterface.h"
+#include "test/gmock.h"
+
+namespace cricket {
+
+// Mock class for BaseChannel.
+// Use this class in unit tests to avoid dependecy on a specific
+// implementation of BaseChannel.
+class MockChannelInterface : public cricket::ChannelInterface {
+ public:
+ MOCK_CONST_METHOD0(media_type, cricket::MediaType());
+ MOCK_CONST_METHOD0(media_channel, MediaChannel*());
+ MOCK_CONST_METHOD0(transport_name, const std::string&());
+ MOCK_CONST_METHOD0(content_name, const std::string&());
+ MOCK_CONST_METHOD0(enabled, bool());
+ MOCK_METHOD1(Enable, bool(bool));
+ MOCK_METHOD0(SignalFirstPacketReceived,
+ sigslot::signal1<ChannelInterface*>&());
+ MOCK_METHOD3(SetLocalContent,
+ bool(const cricket::MediaContentDescription*,
+ webrtc::SdpType,
+ std::string*));
+ MOCK_METHOD3(SetRemoteContent,
+ bool(const cricket::MediaContentDescription*,
+ webrtc::SdpType,
+ std::string*));
+ MOCK_METHOD1(SetRtpTransport, bool(webrtc::RtpTransportInternal*));
+};
+
+} // namespace cricket
+
+#endif // PC_TEST_MOCK_CHANNELINTERFACE_H_
diff --git a/pc/test/mock_rtpreceiverinternal.h b/pc/test/mock_rtpreceiverinternal.h
index 850d5a9..84b0d71 100644
--- a/pc/test/mock_rtpreceiverinternal.h
+++ b/pc/test/mock_rtpreceiverinternal.h
@@ -41,8 +41,7 @@
// RtpReceiverInternal methods.
MOCK_METHOD0(Stop, void());
- MOCK_METHOD1(SetVoiceMediaChannel, void(cricket::VoiceMediaChannel*));
- MOCK_METHOD1(SetVideoMediaChannel, void(cricket::VideoMediaChannel*));
+ MOCK_METHOD1(SetMediaChannel, void(cricket::MediaChannel*));
MOCK_METHOD1(SetupMediaChannel, void(uint32_t));
MOCK_CONST_METHOD0(ssrc, uint32_t());
MOCK_METHOD0(NotifyFirstPacketReceived, void());
diff --git a/pc/test/mock_rtpsenderinternal.h b/pc/test/mock_rtpsenderinternal.h
index 5e14f7a..dd4f694 100644
--- a/pc/test/mock_rtpsenderinternal.h
+++ b/pc/test/mock_rtpsenderinternal.h
@@ -39,8 +39,7 @@
rtc::scoped_refptr<FrameEncryptorInterface>());
// RtpSenderInternal methods.
- MOCK_METHOD1(SetVoiceMediaChannel, void(cricket::VoiceMediaChannel*));
- MOCK_METHOD1(SetVideoMediaChannel, void(cricket::VideoMediaChannel*));
+ MOCK_METHOD1(SetMediaChannel, void(cricket::MediaChannel*));
MOCK_METHOD1(SetSsrc, void(uint32_t));
MOCK_METHOD1(set_stream_ids, void(const std::vector<std::string>&));
MOCK_METHOD1(set_init_send_encodings,
