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

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

Browse Source 
This reverts commit 341434e4da2c193b8842917d73afed6eea3a4332.

Reason for revert: another attempt to land with Chromium
test updated to accept both error messages by CL:
https://chromium-review.googlesource.com/c/chromium/src/+/2228545

Original change's description:
> Revert "Added mid to error messages reported during SDP apply."
>
> 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}

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

# Not skipping CQ checks because this is a reland.

Bug: webrtc:10139
Change-Id: I603d3891c43ac396bf0ba98c6de189663235c8af
Reviewed-on: https://webrtc-review.googlesource.com/c/src/+/176448
Reviewed-by: Tommi <tommi@webrtc.org>
Commit-Queue: Yura Yaroshevich <yura.yaroshevich@gmail.com>
Cr-Commit-Position: refs/heads/master@{#31445}
This commit is contained in:
Yura Yaroshevich
2020-06-03 21:15:22 +00:00
committed by Commit Bot
parent 10594c3c46
commit bc1f910456
3 changed files with 109 additions and 48 deletions
Download Patch File Download Diff File Expand all files Collapse all files

106
pc/channel.cc
View File

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

42
pc/jsep_transport_controller.cc
View File

@ -654,7 +654,8 @@ 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.");
"Failed to process the bundled m= section with mid='" +
content_info.name + "'.");
}
continue;
}
@ -706,9 +707,10 @@ RTCError JsepTransportController::ApplyDescription_n(
}
if (!error.ok()) {
LOG_AND_RETURN_ERROR(RTCErrorType::INVALID_PARAMETER,
"Failed to apply the description for " +
content_info.name + ": " + error.message());
LOG_AND_RETURN_ERROR(
RTCErrorType::INVALID_PARAMETER,
"Failed to apply the description for m= section with mid='" +
content_info.name + "': " + error.message());
}
}
if (type == SdpType::kAnswer) {
@ -727,11 +729,11 @@ RTCError JsepTransportController::ValidateAndMaybeUpdateBundleGroup(
// The BUNDLE group containing a MID that no m= section has is invalid.
if (new_bundle_group) {
for (const auto& content_name : new_bundle_group->content_names()) {
for (const std::string& 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.");
}
}
}
@ -743,18 +745,21 @@ RTCError JsepTransportController::ValidateAndMaybeUpdateBundleGroup(
if (new_bundle_group) {
// The BUNDLE group in answer should be a subset of offered group.
for (const auto& content_name : new_bundle_group->content_names()) {
for (const std::string& 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 that was "
"not in the offered group.");
"The BUNDLE group in answer contains a MID='" +
content_name +
"' that was "
"not in the offered group.");
}
}
}
if (bundle_group_) {
for (const auto& content_name : bundle_group_->content_names()) {
for (const std::string& 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 ||
@ -762,8 +767,9 @@ 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 " + content_name +
" from already-established BUNDLE group.");
"Answer cannot remove m= section with mid='" +
content_name +
"' from already-established BUNDLE group.");
}
}
}
@ -798,9 +804,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:" + content_name + " should be rejected.");
return RTCError(RTCErrorType::INVALID_PARAMETER,
"The m= section with mid='" + content_name +
"' should be rejected.");
}
}
}
@ -815,8 +821,8 @@ RTCError JsepTransportController::ValidateContent(
content_info.type == cricket::MediaProtocolType::kRtp &&
!content_info.media_description()->rtcp_mux()) {
return RTCError(RTCErrorType::INVALID_PARAMETER,
"The m= section:" + content_info.name +
" is invalid. RTCP-MUX is not "
"The m= section with mid='" + 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,10 +1118,11 @@ TEST_P(PeerConnectionMediaTest, MediaEngineErrorPropagatedToClients) {
std::string error;
ASSERT_FALSE(caller->SetRemoteDescription(callee->CreateAnswerAndSetAsLocal(),
&error));
EXPECT_EQ(
"Failed to set remote answer sdp: Failed to set remote video description "
"send parameters.",
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);
}
// Tests that if the underlying video encoder fails once then subsequent