Check that PC is configured for media before doing media operations.

If media_engine is not passed in init parameters, the PC can't handle
media, but can be used for datachannels. This CL adds testing that
datachannels work without media engine, and adds failure returns
to PeerConnection APIs that manipulate media when media engine is
not present.

Bug: webrtc:13931
Change-Id: Iecdf17a0a0bb89e0ad39eb74d6ed077303b875c2
Reviewed-on: https://webrtc-review.googlesource.com/c/src/+/261246
Commit-Queue: Harald Alvestrand <hta@webrtc.org>
Reviewed-by: Henrik Boström <hbos@webrtc.org>
Cr-Commit-Position: refs/heads/main@{#36778}
This commit is contained in:
Harald Alvestrand
2022-05-05 07:37:41 +00:00
committed by WebRTC LUCI CQ
parent 3c2359c663
commit 35ba0c5cd5
5 changed files with 79 additions and 12 deletions

View File

@ -202,7 +202,7 @@ TEST_P(DataChannelIntegrationTest, EndToEndCallWithSctpDataChannel) {
// data channel only, and sends messages of various sizes. // data channel only, and sends messages of various sizes.
TEST_P(DataChannelIntegrationTest, TEST_P(DataChannelIntegrationTest,
EndToEndCallWithSctpDataChannelVariousSizes) { EndToEndCallWithSctpDataChannelVariousSizes) {
ASSERT_TRUE(CreatePeerConnectionWrappers()); ASSERT_TRUE(CreatePeerConnectionWrappersWithoutMediaEngine());
ConnectFakeSignaling(); ConnectFakeSignaling();
// Expect that data channel created on caller side will show up for callee as // Expect that data channel created on caller side will show up for callee as
// well. // well.
@ -241,7 +241,7 @@ TEST_P(DataChannelIntegrationTest,
// data channel only, and sends empty messages // data channel only, and sends empty messages
TEST_P(DataChannelIntegrationTest, TEST_P(DataChannelIntegrationTest,
EndToEndCallWithSctpDataChannelEmptyMessages) { EndToEndCallWithSctpDataChannelEmptyMessages) {
ASSERT_TRUE(CreatePeerConnectionWrappers()); ASSERT_TRUE(CreatePeerConnectionWrappersWithoutMediaEngine());
ConnectFakeSignaling(); ConnectFakeSignaling();
// Expect that data channel created on caller side will show up for callee as // Expect that data channel created on caller side will show up for callee as
// well. // well.
@ -291,7 +291,7 @@ TEST_P(DataChannelIntegrationTest,
// this test does not use TURN. // this test does not use TURN.
const size_t kLowestSafePayloadSizeLimit = 1225; const size_t kLowestSafePayloadSizeLimit = 1225;
ASSERT_TRUE(CreatePeerConnectionWrappers()); ASSERT_TRUE(CreatePeerConnectionWrappersWithoutMediaEngine());
ConnectFakeSignaling(); ConnectFakeSignaling();
// Expect that data channel created on caller side will show up for callee as // Expect that data channel created on caller side will show up for callee as
// well. // well.
@ -328,7 +328,7 @@ TEST_P(DataChannelIntegrationTest, EndToEndCallWithSctpDataChannelHarmfulMtu) {
// The size of the smallest message that fails to be delivered. // The size of the smallest message that fails to be delivered.
const size_t kMessageSizeThatIsNotDelivered = 1157; const size_t kMessageSizeThatIsNotDelivered = 1157;
ASSERT_TRUE(CreatePeerConnectionWrappers()); ASSERT_TRUE(CreatePeerConnectionWrappersWithoutMediaEngine());
ConnectFakeSignaling(); ConnectFakeSignaling();
caller()->CreateDataChannel(); caller()->CreateDataChannel();
caller()->CreateAndSetAndSignalOffer(); caller()->CreateAndSetAndSignalOffer();
@ -429,7 +429,7 @@ TEST_P(DataChannelIntegrationTest, StressTestUnorderedSctpDataChannel) {
virtual_socket_server()->set_delay_stddev(5); virtual_socket_server()->set_delay_stddev(5);
virtual_socket_server()->UpdateDelayDistribution(); virtual_socket_server()->UpdateDelayDistribution();
// Normal procedure, but with unordered data channel config. // Normal procedure, but with unordered data channel config.
ASSERT_TRUE(CreatePeerConnectionWrappers()); ASSERT_TRUE(CreatePeerConnectionWrappersWithoutMediaEngine());
ConnectFakeSignaling(); ConnectFakeSignaling();
webrtc::DataChannelInit init; webrtc::DataChannelInit init;
init.ordered = false; init.ordered = false;

View File

@ -829,11 +829,16 @@ bool PeerConnection::AddStream(MediaStreamInterface* local_stream) {
RTC_CHECK(!IsUnifiedPlan()) << "AddStream is not available with Unified Plan " RTC_CHECK(!IsUnifiedPlan()) << "AddStream is not available with Unified Plan "
"SdpSemantics. Please use AddTrack instead."; "SdpSemantics. Please use AddTrack instead.";
TRACE_EVENT0("webrtc", "PeerConnection::AddStream"); TRACE_EVENT0("webrtc", "PeerConnection::AddStream");
if (!ConfiguredForMedia()) {
RTC_LOG(LS_ERROR) << "AddStream: Not configured for media";
return false;
}
return sdp_handler_->AddStream(local_stream); return sdp_handler_->AddStream(local_stream);
} }
void PeerConnection::RemoveStream(MediaStreamInterface* local_stream) { void PeerConnection::RemoveStream(MediaStreamInterface* local_stream) {
RTC_DCHECK_RUN_ON(signaling_thread()); RTC_DCHECK_RUN_ON(signaling_thread());
RTC_DCHECK(ConfiguredForMedia());
RTC_CHECK(!IsUnifiedPlan()) << "RemoveStream is not available with Unified " RTC_CHECK(!IsUnifiedPlan()) << "RemoveStream is not available with Unified "
"Plan SdpSemantics. Please use RemoveTrack " "Plan SdpSemantics. Please use RemoveTrack "
"instead."; "instead.";
@ -846,6 +851,10 @@ RTCErrorOr<rtc::scoped_refptr<RtpSenderInterface>> PeerConnection::AddTrack(
const std::vector<std::string>& stream_ids) { const std::vector<std::string>& stream_ids) {
RTC_DCHECK_RUN_ON(signaling_thread()); RTC_DCHECK_RUN_ON(signaling_thread());
TRACE_EVENT0("webrtc", "PeerConnection::AddTrack"); TRACE_EVENT0("webrtc", "PeerConnection::AddTrack");
if (!ConfiguredForMedia()) {
LOG_AND_RETURN_ERROR(RTCErrorType::UNSUPPORTED_OPERATION,
"Not configured for media");
}
if (!track) { if (!track) {
LOG_AND_RETURN_ERROR(RTCErrorType::INVALID_PARAMETER, "Track is null."); LOG_AND_RETURN_ERROR(RTCErrorType::INVALID_PARAMETER, "Track is null.");
} }
@ -874,6 +883,10 @@ RTCErrorOr<rtc::scoped_refptr<RtpSenderInterface>> PeerConnection::AddTrack(
RTCError PeerConnection::RemoveTrackOrError( RTCError PeerConnection::RemoveTrackOrError(
rtc::scoped_refptr<RtpSenderInterface> sender) { rtc::scoped_refptr<RtpSenderInterface> sender) {
RTC_DCHECK_RUN_ON(signaling_thread()); RTC_DCHECK_RUN_ON(signaling_thread());
if (!ConfiguredForMedia()) {
LOG_AND_RETURN_ERROR(RTCErrorType::UNSUPPORTED_OPERATION,
"Not configured for media");
}
if (!sender) { if (!sender) {
LOG_AND_RETURN_ERROR(RTCErrorType::INVALID_PARAMETER, "Sender is null."); LOG_AND_RETURN_ERROR(RTCErrorType::INVALID_PARAMETER, "Sender is null.");
} }
@ -923,6 +936,11 @@ PeerConnection::FindTransceiverBySender(
RTCErrorOr<rtc::scoped_refptr<RtpTransceiverInterface>> RTCErrorOr<rtc::scoped_refptr<RtpTransceiverInterface>>
PeerConnection::AddTransceiver( PeerConnection::AddTransceiver(
rtc::scoped_refptr<MediaStreamTrackInterface> track) { rtc::scoped_refptr<MediaStreamTrackInterface> track) {
if (!ConfiguredForMedia()) {
LOG_AND_RETURN_ERROR(RTCErrorType::UNSUPPORTED_OPERATION,
"Not configured for media");
}
return AddTransceiver(track, RtpTransceiverInit()); return AddTransceiver(track, RtpTransceiverInit());
} }
@ -944,6 +962,10 @@ PeerConnection::AddTransceiver(
rtc::scoped_refptr<MediaStreamTrackInterface> track, rtc::scoped_refptr<MediaStreamTrackInterface> track,
const RtpTransceiverInit& init) { const RtpTransceiverInit& init) {
RTC_DCHECK_RUN_ON(signaling_thread()); RTC_DCHECK_RUN_ON(signaling_thread());
if (!ConfiguredForMedia()) {
LOG_AND_RETURN_ERROR(RTCErrorType::UNSUPPORTED_OPERATION,
"Not configured for media");
}
RTC_CHECK(IsUnifiedPlan()) RTC_CHECK(IsUnifiedPlan())
<< "AddTransceiver is only available with Unified Plan SdpSemantics"; << "AddTransceiver is only available with Unified Plan SdpSemantics";
if (!track) { if (!track) {
@ -970,6 +992,10 @@ RTCErrorOr<rtc::scoped_refptr<RtpTransceiverInterface>>
PeerConnection::AddTransceiver(cricket::MediaType media_type, PeerConnection::AddTransceiver(cricket::MediaType media_type,
const RtpTransceiverInit& init) { const RtpTransceiverInit& init) {
RTC_DCHECK_RUN_ON(signaling_thread()); RTC_DCHECK_RUN_ON(signaling_thread());
if (!ConfiguredForMedia()) {
LOG_AND_RETURN_ERROR(RTCErrorType::UNSUPPORTED_OPERATION,
"Not configured for media");
}
RTC_CHECK(IsUnifiedPlan()) RTC_CHECK(IsUnifiedPlan())
<< "AddTransceiver is only available with Unified Plan SdpSemantics"; << "AddTransceiver is only available with Unified Plan SdpSemantics";
if (!(media_type == cricket::MEDIA_TYPE_AUDIO || if (!(media_type == cricket::MEDIA_TYPE_AUDIO ||
@ -987,6 +1013,10 @@ PeerConnection::AddTransceiver(
const RtpTransceiverInit& init, const RtpTransceiverInit& init,
bool update_negotiation_needed) { bool update_negotiation_needed) {
RTC_DCHECK_RUN_ON(signaling_thread()); RTC_DCHECK_RUN_ON(signaling_thread());
if (!ConfiguredForMedia()) {
LOG_AND_RETURN_ERROR(RTCErrorType::UNSUPPORTED_OPERATION,
"Not configured for media");
}
RTC_DCHECK((media_type == cricket::MEDIA_TYPE_AUDIO || RTC_DCHECK((media_type == cricket::MEDIA_TYPE_AUDIO ||
media_type == cricket::MEDIA_TYPE_VIDEO)); media_type == cricket::MEDIA_TYPE_VIDEO));
if (track) { if (track) {
@ -1095,6 +1125,10 @@ rtc::scoped_refptr<RtpSenderInterface> PeerConnection::CreateSender(
const std::string& kind, const std::string& kind,
const std::string& stream_id) { const std::string& stream_id) {
RTC_DCHECK_RUN_ON(signaling_thread()); RTC_DCHECK_RUN_ON(signaling_thread());
if (!ConfiguredForMedia()) {
RTC_LOG(LS_ERROR) << "Not configured for media";
return nullptr;
}
RTC_CHECK(!IsUnifiedPlan()) << "CreateSender is not available with Unified " RTC_CHECK(!IsUnifiedPlan()) << "CreateSender is not available with Unified "
"Plan SdpSemantics. Please use AddTransceiver " "Plan SdpSemantics. Please use AddTransceiver "
"instead."; "instead.";
@ -1677,6 +1711,10 @@ void PeerConnection::AddAdaptationResource(
call_->AddAdaptationResource(resource); call_->AddAdaptationResource(resource);
} }
bool PeerConnection::ConfiguredForMedia() const {
return context_->channel_manager()->media_engine();
}
bool PeerConnection::StartRtcEventLog(std::unique_ptr<RtcEventLogOutput> output, bool PeerConnection::StartRtcEventLog(std::unique_ptr<RtcEventLogOutput> output,
int64_t output_period_ms) { int64_t output_period_ms) {
return worker_thread()->Invoke<bool>( return worker_thread()->Invoke<bool>(

View File

@ -430,6 +430,10 @@ class PeerConnection : public PeerConnectionInternal,
RTC_RUN_ON(network_thread()); RTC_RUN_ON(network_thread());
void TeardownDataChannelTransport_n() override RTC_RUN_ON(network_thread()); void TeardownDataChannelTransport_n() override RTC_RUN_ON(network_thread());
const FieldTrialsView& trials() const override { return *trials_; }
bool ConfiguredForMedia() const;
// Functions made public for testing. // Functions made public for testing.
void ReturnHistogramVeryQuicklyForTesting() { void ReturnHistogramVeryQuicklyForTesting() {
RTC_DCHECK_RUN_ON(signaling_thread()); RTC_DCHECK_RUN_ON(signaling_thread());
@ -437,8 +441,6 @@ class PeerConnection : public PeerConnectionInternal,
} }
void RequestUsagePatternReportForTesting(); void RequestUsagePatternReportForTesting();
const FieldTrialsView& trials() const override { return *trials_; }
protected: protected:
// Available for rtc::scoped_refptr creation // Available for rtc::scoped_refptr creation
PeerConnection(rtc::scoped_refptr<ConnectionContext> context, PeerConnection(rtc::scoped_refptr<ConnectionContext> context,

View File

@ -3675,6 +3675,13 @@ TEST_P(PeerConnectionIntegrationTest, NewTracksDoNotCauseNewCandidates) {
ASSERT_TRUE_WAIT(SignalingStateStable(), kDefaultTimeout); ASSERT_TRUE_WAIT(SignalingStateStable(), kDefaultTimeout);
} }
TEST_P(PeerConnectionIntegrationTest, MediaCallWithoutMediaEngineFails) {
ASSERT_TRUE(CreatePeerConnectionWrappersWithoutMediaEngine());
// AddTrack should fail.
EXPECT_FALSE(
caller()->pc()->AddTrack(caller()->CreateLocalAudioTrack(), {}).ok());
}
INSTANTIATE_TEST_SUITE_P( INSTANTIATE_TEST_SUITE_P(
PeerConnectionIntegrationTest, PeerConnectionIntegrationTest,
PeerConnectionIntegrationInteropTest, PeerConnectionIntegrationInteropTest,

View File

@ -735,7 +735,8 @@ class PeerConnectionIntegrationWrapper : public webrtc::PeerConnectionObserver,
rtc::Thread* worker_thread, rtc::Thread* worker_thread,
std::unique_ptr<webrtc::FakeRtcEventLogFactory> event_log_factory, std::unique_ptr<webrtc::FakeRtcEventLogFactory> event_log_factory,
bool reset_encoder_factory, bool reset_encoder_factory,
bool reset_decoder_factory) { bool reset_decoder_factory,
bool create_media_engine) {
// There's an error in this test code if Init ends up being called twice. // There's an error in this test code if Init ends up being called twice.
RTC_DCHECK(!peer_connection_); RTC_DCHECK(!peer_connection_);
RTC_DCHECK(!peer_connection_factory_); RTC_DCHECK(!peer_connection_factory_);
@ -782,8 +783,10 @@ class PeerConnectionIntegrationWrapper : public webrtc::PeerConnectionObserver,
media_deps.trials = pc_factory_dependencies.trials.get(); media_deps.trials = pc_factory_dependencies.trials.get();
pc_factory_dependencies.media_engine = if (create_media_engine) {
cricket::CreateMediaEngine(std::move(media_deps)); pc_factory_dependencies.media_engine =
cricket::CreateMediaEngine(std::move(media_deps));
}
pc_factory_dependencies.call_factory = webrtc::CreateCallFactory(); pc_factory_dependencies.call_factory = webrtc::CreateCallFactory();
if (event_log_factory) { if (event_log_factory) {
event_log_factory_ = event_log_factory.get(); event_log_factory_ = event_log_factory.get();
@ -1435,7 +1438,8 @@ class PeerConnectionIntegrationBaseTest : public ::testing::Test {
webrtc::PeerConnectionDependencies dependencies, webrtc::PeerConnectionDependencies dependencies,
std::unique_ptr<webrtc::FakeRtcEventLogFactory> event_log_factory, std::unique_ptr<webrtc::FakeRtcEventLogFactory> event_log_factory,
bool reset_encoder_factory, bool reset_encoder_factory,
bool reset_decoder_factory) { bool reset_decoder_factory,
bool create_media_engine = true) {
RTCConfiguration modified_config; RTCConfiguration modified_config;
if (config) { if (config) {
modified_config = *config; modified_config = *config;
@ -1451,7 +1455,7 @@ class PeerConnectionIntegrationBaseTest : public ::testing::Test {
if (!client->Init(options, &modified_config, std::move(dependencies), if (!client->Init(options, &modified_config, std::move(dependencies),
network_thread_.get(), worker_thread_.get(), network_thread_.get(), worker_thread_.get(),
std::move(event_log_factory), reset_encoder_factory, std::move(event_log_factory), reset_encoder_factory,
reset_decoder_factory)) { reset_decoder_factory, create_media_engine)) {
return nullptr; return nullptr;
} }
return client; return client;
@ -1590,6 +1594,22 @@ class PeerConnectionIntegrationBaseTest : public ::testing::Test {
return caller_ && callee_; return caller_ && callee_;
} }
bool CreatePeerConnectionWrappersWithoutMediaEngine() {
caller_ = CreatePeerConnectionWrapper(
"Caller", nullptr, nullptr, webrtc::PeerConnectionDependencies(nullptr),
nullptr,
/*reset_encoder_factory=*/false,
/*reset_decoder_factory=*/false,
/*create_media_engine=*/false);
callee_ = CreatePeerConnectionWrapper(
"Callee", nullptr, nullptr, webrtc::PeerConnectionDependencies(nullptr),
nullptr,
/*reset_encoder_factory=*/false,
/*reset_decoder_factory=*/false,
/*create_media_engine=*/false);
return caller_ && callee_;
}
cricket::TestTurnServer* CreateTurnServer( cricket::TestTurnServer* CreateTurnServer(
rtc::SocketAddress internal_address, rtc::SocketAddress internal_address,
rtc::SocketAddress external_address, rtc::SocketAddress external_address,