zqz/platform-external-webrtc
1
0
Fork 0
Code Issues Pull Requests Actions Packages Projects Releases Wiki Activity

Revert "Added mid to error messages reported during SDP apply."

Browse Source 
This reverts commit d2890e8833796f13c4a1243769be966bebdfcaa7.

Reason for revert: speculative: WebRtcBrowserTest.NegotiateUnsupportedVideoCodec broken on all FYI bots, example: https://ci.chromium.org/p/chromium/builders/webrtc.fyi/WebRTC%20Chromium%20FYI%20Linux%20Tester/6659

Original change's description:
> Added mid to error messages reported during SDP apply.
> 
> Bug: webrtc:10139
> Change-Id: I7462b632e00a2da7b189b63022d30f594700b68a
> Reviewed-on: https://webrtc-review.googlesource.com/c/src/+/176400
> Reviewed-by: Tommi <tommi@webrtc.org>
> Commit-Queue: Yura Yaroshevich <yura.yaroshevich@gmail.com>
> Cr-Commit-Position: refs/heads/master@{#31421}

TBR=tommi@webrtc.org,yura.yaroshevich@gmail.com

Change-Id: I18972815df10e2bd7b914ad82df9596009c2fecc
No-Presubmit: true
No-Tree-Checks: true
No-Try: true
Bug: webrtc:10139
Reviewed-on: https://webrtc-review.googlesource.com/c/src/+/176418
Reviewed-by: Olga Sharonova <olka@webrtc.org>
Commit-Queue: Olga Sharonova <olka@webrtc.org>
Cr-Commit-Position: refs/heads/master@{#31425}
This commit is contained in:
Olga Sharonova
2020-06-03 13:16:39 +00:00
committed by Commit Bot
parent 636865e05d
commit 341434e4da
3 changed files with 48 additions and 109 deletions
Download Patch File Download Diff File Expand all files Collapse all files

106
pc/channel.cc
View File

@ -604,8 +604,7 @@ bool BaseChannel::UpdateLocalStreams_w(const std::vector<StreamParams>& streams,
if (!media_channel()->RemoveSendStream(old_stream.first_ssrc())) {
rtc::StringBuilder desc;
desc << "Failed to remove send stream with ssrc "
<< old_stream.first_ssrc() << " from m-section with mid='"
<< content_name() << "'.";
<< old_stream.first_ssrc() << ".";
SafeSetError(desc.str(), error_desc);
ret = false;
}
@ -631,8 +630,7 @@ bool BaseChannel::UpdateLocalStreams_w(const std::vector<StreamParams>& streams,
if (new_stream.has_ssrcs() && new_stream.has_rids()) {
rtc::StringBuilder desc;
desc << "Failed to add send stream: " << new_stream.first_ssrc()
<< " into m-section with mid='" << content_name()
<< "'. Stream has both SSRCs and RIDs.";
<< ". Stream has both SSRCs and RIDs.";
SafeSetError(desc.str(), error_desc);
ret = false;
continue;
@ -651,8 +649,7 @@ bool BaseChannel::UpdateLocalStreams_w(const std::vector<StreamParams>& streams,
<< " into " << ToString();
} else {
rtc::StringBuilder desc;
desc << "Failed to add send stream ssrc: " << new_stream.first_ssrc()
<< " into m-section with mid='" << content_name() << "'";
desc << "Failed to add send stream ssrc: " << new_stream.first_ssrc();
SafeSetError(desc.str(), error_desc);
ret = false;
}
@ -682,8 +679,7 @@ bool BaseChannel::UpdateRemoteStreams_w(
} else {
rtc::StringBuilder desc;
desc << "Failed to remove remote stream with ssrc "
<< old_stream.first_ssrc() << " from m-section with mid='"
<< content_name() << "'.";
<< old_stream.first_ssrc() << ".";
SafeSetError(desc.str(), error_desc);
ret = false;
}
@ -866,11 +862,8 @@ bool VoiceChannel::SetLocalContent_w(const MediaContentDescription* content,
audio, rtp_header_extensions,
webrtc::RtpTransceiverDirectionHasRecv(audio->direction()), &recv_params);
if (!media_channel()->SetRecvParameters(recv_params)) {
SafeSetError(
"Failed to set local audio description recv parameters for m-section "
"with mid='" +
content_name() + "'.",
error_desc);
SafeSetError("Failed to set local audio description recv parameters.",
error_desc);
return false;
}
@ -892,11 +885,7 @@ bool VoiceChannel::SetLocalContent_w(const MediaContentDescription* content,
// description too (without a remote description, we won't be able
// to send them anyway).
if (!UpdateLocalStreams_w(audio->streams(), type, error_desc)) {
SafeSetError(
"Failed to set local audio description streams for m-section with "
"mid='" +
content_name() + "'.",
error_desc);
SafeSetError("Failed to set local audio description streams.", error_desc);
return false;
}
@ -931,11 +920,8 @@ bool VoiceChannel::SetRemoteContent_w(const MediaContentDescription* content,
bool parameters_applied = media_channel()->SetSendParameters(send_params);
if (!parameters_applied) {
SafeSetError(
"Failed to set remote audio description send parameters for m-section "
"with mid='" +
content_name() + "'.",
error_desc);
SafeSetError("Failed to set remote audio description send parameters.",
error_desc);
return false;
}
last_send_params_ = send_params;
@ -956,11 +942,7 @@ bool VoiceChannel::SetRemoteContent_w(const MediaContentDescription* content,
// description too (without a local description, we won't be able to
// recv them anyway).
if (!UpdateRemoteStreams_w(audio->streams(), type, error_desc)) {
SafeSetError(
"Failed to set remote audio description streams for m-section with "
"mid='" +
content_name() + "'.",
error_desc);
SafeSetError("Failed to set remote audio description streams.", error_desc);
return false;
}
@ -1048,9 +1030,7 @@ bool VideoChannel::SetLocalContent_w(const MediaContentDescription* content,
needs_send_params_update = true;
} else if (recv_codec->packetization != send_codec.packetization) {
SafeSetError(
"Failed to set local answer due to invalid codec packetization "
"specified in m-section with mid='" +
content_name() + "'.",
"Failed to set local answer due to invalid codec packetization.",
error_desc);
return false;
}
@ -1059,11 +1039,8 @@ bool VideoChannel::SetLocalContent_w(const MediaContentDescription* content,
}
if (!media_channel()->SetRecvParameters(recv_params)) {
SafeSetError(
"Failed to set local video description recv parameters for m-section "
"with mid='" +
content_name() + "'.",
error_desc);
SafeSetError("Failed to set local video description recv parameters.",
error_desc);
return false;
}
@ -1082,9 +1059,7 @@ bool VideoChannel::SetLocalContent_w(const MediaContentDescription* content,
if (needs_send_params_update) {
if (!media_channel()->SetSendParameters(send_params)) {
SafeSetError("Failed to set send parameters for m-section with mid='" +
content_name() + "'.",
error_desc);
SafeSetError("Failed to set send parameters.", error_desc);
return false;
}
last_send_params_ = send_params;
@ -1095,11 +1070,7 @@ bool VideoChannel::SetLocalContent_w(const MediaContentDescription* content,
// description too (without a remote description, we won't be able
// to send them anyway).
if (!UpdateLocalStreams_w(video->streams(), type, error_desc)) {
SafeSetError(
"Failed to set local video description streams for m-section with "
"mid='" +
content_name() + "'.",
error_desc);
SafeSetError("Failed to set local video description streams.", error_desc);
return false;
}
@ -1147,9 +1118,7 @@ bool VideoChannel::SetRemoteContent_w(const MediaContentDescription* content,
needs_recv_params_update = true;
} else if (send_codec->packetization != recv_codec.packetization) {
SafeSetError(
"Failed to set remote answer due to invalid codec packetization "
"specifid in m-section with mid='" +
content_name() + "'.",
"Failed to set remote answer due to invalid codec packetization.",
error_desc);
return false;
}
@ -1158,20 +1127,15 @@ bool VideoChannel::SetRemoteContent_w(const MediaContentDescription* content,
}
if (!media_channel()->SetSendParameters(send_params)) {
SafeSetError(
"Failed to set remote video description send parameters for m-section "
"with mid='" +
content_name() + "'.",
error_desc);
SafeSetError("Failed to set remote video description send parameters.",
error_desc);
return false;
}
last_send_params_ = send_params;
if (needs_recv_params_update) {
if (!media_channel()->SetRecvParameters(recv_params)) {
SafeSetError("Failed to set recv parameters for m-section with mid='" +
content_name() + "'.",
error_desc);
SafeSetError("Failed to set recv parameters.", error_desc);
return false;
}
last_recv_params_ = recv_params;
@ -1193,11 +1157,7 @@ bool VideoChannel::SetRemoteContent_w(const MediaContentDescription* content,
// description too (without a local description, we won't be able to
// recv them anyway).
if (!UpdateRemoteStreams_w(video->streams(), type, error_desc)) {
SafeSetError(
"Failed to set remote video description streams for m-section with "
"mid='" +
content_name() + "'.",
error_desc);
SafeSetError("Failed to set remote video description streams.", error_desc);
return false;
}
set_remote_content_direction(content->direction());
@ -1288,11 +1248,8 @@ bool RtpDataChannel::SetLocalContent_w(const MediaContentDescription* content,
data, rtp_header_extensions,
webrtc::RtpTransceiverDirectionHasRecv(data->direction()), &recv_params);
if (!media_channel()->SetRecvParameters(recv_params)) {
SafeSetError(
"Failed to set remote data description recv parameters for m-section "
"with mid='" +
content_name() + "'.",
error_desc);
SafeSetError("Failed to set remote data description recv parameters.",
error_desc);
return false;
}
for (const DataCodec& codec : data->codecs()) {
@ -1311,11 +1268,7 @@ bool RtpDataChannel::SetLocalContent_w(const MediaContentDescription* content,
// description too (without a remote description, we won't be able
// to send them anyway).
if (!UpdateLocalStreams_w(data->streams(), type, error_desc)) {
SafeSetError(
"Failed to set local data description streams for m-section with "
"mid='" +
content_name() + "'.",
error_desc);
SafeSetError("Failed to set local data description streams.", error_desc);
return false;
}
@ -1357,11 +1310,8 @@ bool RtpDataChannel::SetRemoteContent_w(const MediaContentDescription* content,
data, rtp_header_extensions,
webrtc::RtpTransceiverDirectionHasRecv(data->direction()), &send_params);
if (!media_channel()->SetSendParameters(send_params)) {
SafeSetError(
"Failed to set remote data description send parameters for m-section "
"with mid='" +
content_name() + "'.",
error_desc);
SafeSetError("Failed to set remote data description send parameters.",
error_desc);
return false;
}
last_send_params_ = send_params;
@ -1371,11 +1321,7 @@ bool RtpDataChannel::SetRemoteContent_w(const MediaContentDescription* content,
// description too (without a local description, we won't be able to
// recv them anyway).
if (!UpdateRemoteStreams_w(data->streams(), type, error_desc)) {
SafeSetError(
"Failed to set remote data description streams for m-section with "
"mid='" +
content_name() + "'.",
error_desc);
SafeSetError("Failed to set remote data description streams.", error_desc);
return false;
}

42
pc/jsep_transport_controller.cc
View File

@ -654,8 +654,7 @@ RTCError JsepTransportController::ApplyDescription_n(
if (IsBundled(content_info.name) && content_info.name != *bundled_mid()) {
if (!HandleBundledContent(content_info)) {
return RTCError(RTCErrorType::INVALID_PARAMETER,
"Failed to process the bundled m= section with mid='" +
content_info.name + "'.");
"Failed to process the bundled m= section.");
}
continue;
}
@ -707,10 +706,9 @@ RTCError JsepTransportController::ApplyDescription_n(
}
if (!error.ok()) {
LOG_AND_RETURN_ERROR(
RTCErrorType::INVALID_PARAMETER,
"Failed to apply the description for m= section with mid='" +
content_info.name + "': " + error.message());
LOG_AND_RETURN_ERROR(RTCErrorType::INVALID_PARAMETER,
"Failed to apply the description for " +
content_info.name + ": " + error.message());
}
}
if (type == SdpType::kAnswer) {
@ -729,11 +727,11 @@ RTCError JsepTransportController::ValidateAndMaybeUpdateBundleGroup(
// The BUNDLE group containing a MID that no m= section has is invalid.
if (new_bundle_group) {
for (const std::string& content_name : new_bundle_group->content_names()) {
for (const auto& content_name : new_bundle_group->content_names()) {
if (!description->GetContentByName(content_name)) {
return RTCError(RTCErrorType::INVALID_PARAMETER,
"The BUNDLE group contains MID='" + content_name +
"' matching no m= section.");
"The BUNDLE group contains MID:" + content_name +
" matching no m= section.");
}
}
}
@ -745,21 +743,18 @@ RTCError JsepTransportController::ValidateAndMaybeUpdateBundleGroup(
if (new_bundle_group) {
// The BUNDLE group in answer should be a subset of offered group.
for (const std::string& content_name :
new_bundle_group->content_names()) {
for (const auto& content_name : new_bundle_group->content_names()) {
if (!offered_bundle_group ||
!offered_bundle_group->HasContentName(content_name)) {
return RTCError(RTCErrorType::INVALID_PARAMETER,
"The BUNDLE group in answer contains a MID='" +
content_name +
"' that was "
"not in the offered group.");
"The BUNDLE group in answer contains a MID that was "
"not in the offered group.");
}
}
}
if (bundle_group_) {
for (const std::string& content_name : bundle_group_->content_names()) {
for (const auto& content_name : bundle_group_->content_names()) {
// An answer that removes m= sections from pre-negotiated BUNDLE group
// without rejecting it, is invalid.
if (!new_bundle_group ||
@ -767,9 +762,8 @@ RTCError JsepTransportController::ValidateAndMaybeUpdateBundleGroup(
auto* content_info = description->GetContentByName(content_name);
if (!content_info || !content_info->rejected) {
return RTCError(RTCErrorType::INVALID_PARAMETER,
"Answer cannot remove m= section with mid='" +
content_name +
"' from already-established BUNDLE group.");
"Answer cannot remove m= section " + content_name +
" from already-established BUNDLE group.");
}
}
}
@ -804,9 +798,9 @@ RTCError JsepTransportController::ValidateAndMaybeUpdateBundleGroup(
for (const auto& content_name : bundle_group_->content_names()) {
auto other_content = description->GetContentByName(content_name);
if (!other_content->rejected) {
return RTCError(RTCErrorType::INVALID_PARAMETER,
"The m= section with mid='" + content_name +
"' should be rejected.");
return RTCError(
RTCErrorType::INVALID_PARAMETER,
"The m= section:" + content_name + " should be rejected.");
}
}
}
@ -821,8 +815,8 @@ RTCError JsepTransportController::ValidateContent(
content_info.type == cricket::MediaProtocolType::kRtp &&
!content_info.media_description()->rtcp_mux()) {
return RTCError(RTCErrorType::INVALID_PARAMETER,
"The m= section with mid='" + content_info.name +
"' is invalid. RTCP-MUX is not "
"The m= section:" + content_info.name +
" is invalid. RTCP-MUX is not "
"enabled when it is required.");
}
return RTCError::OK();

9
pc/peer_connection_media_unittest.cc
View File

@ -1118,11 +1118,10 @@ TEST_P(PeerConnectionMediaTest, MediaEngineErrorPropagatedToClients) {
std::string error;
ASSERT_FALSE(caller->SetRemoteDescription(callee->CreateAnswerAndSetAsLocal(),
&error));
EXPECT_EQ(std::string("Failed to set remote answer sdp: Failed to set remote "
"video description "
"send parameters for m-section with mid='") +
(IsUnifiedPlan() ? "1" : "video") + "'.",
error);
EXPECT_EQ(
"Failed to set remote answer sdp: Failed to set remote video description "
"send parameters.",
error);
}
// Tests that if the underlying video encoder fails once then subsequent