Delete UMA histograms relating to Plan B vs Unified Plan.
Plan B having been deleted from Chrome, there is no need to collect UMAs relating to Plan B vs Unified Plan setups. Bug: chromium:1357994 Change-Id: Icb5d16823ea9d849798583cd1c82683014b8a15c Reviewed-on: https://webrtc-review.googlesource.com/c/src/+/275309 Commit-Queue: Mirko Bonadei <mbonadei@webrtc.org> Commit-Queue: Henrik Boström <hbos@webrtc.org> Reviewed-by: Harald Alvestrand <hta@webrtc.org> Cr-Commit-Position: refs/heads/main@{#38069}
This commit is contained in:
parent
39f216bf1d
commit
41263fab8f
@ -112,36 +112,6 @@ enum SdpSemanticRequested {
|
||||
kSdpSemanticRequestMax
|
||||
};
|
||||
|
||||
// These values are persisted to logs. Entries should not be renumbered and
|
||||
// numeric values should never be reused.
|
||||
enum SdpSemanticNegotiated {
|
||||
kSdpSemanticNegotiatedNone = 0,
|
||||
kSdpSemanticNegotiatedPlanB = 1,
|
||||
kSdpSemanticNegotiatedUnifiedPlan = 2,
|
||||
kSdpSemanticNegotiatedMixed = 3,
|
||||
kSdpSemanticNegotiatedMax
|
||||
};
|
||||
|
||||
// Metric which records the format of the received SDP for tracking how much the
|
||||
// difference between Plan B and Unified Plan affect users.
|
||||
// These values are persisted to logs. Entries should not be renumbered and
|
||||
// numeric values should never be reused.
|
||||
enum SdpFormatReceived {
|
||||
// No audio or video tracks. This is worth special casing since it seems to be
|
||||
// the most common scenario (data-channel only).
|
||||
kSdpFormatReceivedNoTracks = 0,
|
||||
// No more than one audio and one video track. Should be compatible with both
|
||||
// Plan B and Unified Plan endpoints.
|
||||
kSdpFormatReceivedSimple = 1,
|
||||
// More than one audio track or more than one video track in the Plan B format
|
||||
// (e.g., one audio media section with multiple streams).
|
||||
kSdpFormatReceivedComplexPlanB = 2,
|
||||
// More than one audio track or more than one video track in the Unified Plan
|
||||
// format (e.g., two audio media sections).
|
||||
kSdpFormatReceivedComplexUnifiedPlan = 3,
|
||||
kSdpFormatReceivedMax
|
||||
};
|
||||
|
||||
// Metric for counting the outcome of adding an ICE candidate
|
||||
// These values are persisted to logs. Entries should not be renumbered and
|
||||
// numeric values should never be reused.
|
||||
|
||||
@ -2559,50 +2559,6 @@ bool PeerConnection::ValidateBundleSettings(
|
||||
return true;
|
||||
}
|
||||
|
||||
void PeerConnection::ReportSdpFormatReceived(
|
||||
const SessionDescriptionInterface& remote_description) {
|
||||
int num_audio_mlines = 0;
|
||||
int num_video_mlines = 0;
|
||||
int num_audio_tracks = 0;
|
||||
int num_video_tracks = 0;
|
||||
for (const ContentInfo& content :
|
||||
remote_description.description()->contents()) {
|
||||
cricket::MediaType media_type = content.media_description()->type();
|
||||
int num_tracks = std::max(
|
||||
1, static_cast<int>(content.media_description()->streams().size()));
|
||||
if (media_type == cricket::MEDIA_TYPE_AUDIO) {
|
||||
num_audio_mlines += 1;
|
||||
num_audio_tracks += num_tracks;
|
||||
} else if (media_type == cricket::MEDIA_TYPE_VIDEO) {
|
||||
num_video_mlines += 1;
|
||||
num_video_tracks += num_tracks;
|
||||
}
|
||||
}
|
||||
SdpFormatReceived format = kSdpFormatReceivedNoTracks;
|
||||
if (num_audio_mlines > 1 || num_video_mlines > 1) {
|
||||
format = kSdpFormatReceivedComplexUnifiedPlan;
|
||||
} else if (num_audio_tracks > 1 || num_video_tracks > 1) {
|
||||
format = kSdpFormatReceivedComplexPlanB;
|
||||
} else if (num_audio_tracks > 0 || num_video_tracks > 0) {
|
||||
format = kSdpFormatReceivedSimple;
|
||||
}
|
||||
switch (remote_description.GetType()) {
|
||||
case SdpType::kOffer:
|
||||
// Historically only offers were counted.
|
||||
RTC_HISTOGRAM_ENUMERATION("WebRTC.PeerConnection.SdpFormatReceived",
|
||||
format, kSdpFormatReceivedMax);
|
||||
break;
|
||||
case SdpType::kAnswer:
|
||||
RTC_HISTOGRAM_ENUMERATION("WebRTC.PeerConnection.SdpFormatReceivedAnswer",
|
||||
format, kSdpFormatReceivedMax);
|
||||
break;
|
||||
default:
|
||||
RTC_LOG(LS_ERROR) << "Can not report SdpFormatReceived for "
|
||||
<< SdpTypeToString(remote_description.GetType());
|
||||
break;
|
||||
}
|
||||
}
|
||||
|
||||
void PeerConnection::ReportSdpBundleUsage(
|
||||
const SessionDescriptionInterface& remote_description) {
|
||||
RTC_DCHECK_RUN_ON(signaling_thread());
|
||||
|
||||
@ -371,10 +371,6 @@ class PeerConnection : public PeerConnectionInternal,
|
||||
void AddRemoteCandidate(const std::string& mid,
|
||||
const cricket::Candidate& candidate) override;
|
||||
|
||||
// Report the UMA metric SdpFormatReceived for the given remote description.
|
||||
void ReportSdpFormatReceived(
|
||||
const SessionDescriptionInterface& remote_description) override;
|
||||
|
||||
// Report the UMA metric BundleUsage for the given remote description.
|
||||
void ReportSdpBundleUsage(
|
||||
const SessionDescriptionInterface& remote_description) override;
|
||||
|
||||
@ -53,10 +53,6 @@ class PeerConnectionSdpMethods {
|
||||
virtual const PeerConnectionInterface::RTCConfiguration* configuration()
|
||||
const = 0;
|
||||
|
||||
// Report the UMA metric SdpFormatReceived for the given remote description.
|
||||
virtual void ReportSdpFormatReceived(
|
||||
const SessionDescriptionInterface& remote_description) = 0;
|
||||
|
||||
// Report the UMA metric BundleUsage for the given remote description.
|
||||
virtual void ReportSdpBundleUsage(
|
||||
const SessionDescriptionInterface& remote_description) = 0;
|
||||
|
||||
@ -1811,10 +1811,6 @@ TEST_F(PeerConnectionMsidSignalingTest, UnifiedPlanTalkingToOurself) {
|
||||
auto* answer = caller->pc()->remote_description();
|
||||
EXPECT_EQ(cricket::kMsidSignalingMediaSection,
|
||||
answer->description()->msid_signaling());
|
||||
// Check that this is counted correctly
|
||||
EXPECT_METRIC_THAT(
|
||||
metrics::Samples("WebRTC.PeerConnection.SdpSemanticNegotiated"),
|
||||
ElementsAre(Pair(kSdpSemanticNegotiatedUnifiedPlan, 2)));
|
||||
}
|
||||
|
||||
TEST_F(PeerConnectionMsidSignalingTest, PlanBOfferToUnifiedPlanAnswer) {
|
||||
@ -1890,92 +1886,6 @@ TEST_F(PeerConnectionMsidSignalingTest, PureUnifiedPlanToUs) {
|
||||
answer->description()->msid_signaling());
|
||||
}
|
||||
|
||||
// Test that the correct UMA metrics are reported for simple/complex SDP.
|
||||
|
||||
class SdpFormatReceivedTest : public PeerConnectionRtpTestUnifiedPlan {};
|
||||
|
||||
#ifdef WEBRTC_HAVE_SCTP
|
||||
TEST_F(SdpFormatReceivedTest, DataChannelOnlyIsReportedAsNoTracks) {
|
||||
auto caller = CreatePeerConnectionWithUnifiedPlan();
|
||||
caller->CreateDataChannel("dc");
|
||||
auto callee = CreatePeerConnectionWithUnifiedPlan();
|
||||
|
||||
ASSERT_TRUE(callee->SetRemoteDescription(caller->CreateOffer()));
|
||||
// Note that only the callee does ReportSdpFormatReceived.
|
||||
EXPECT_METRIC_THAT(
|
||||
metrics::Samples("WebRTC.PeerConnection.SdpFormatReceived"),
|
||||
ElementsAre(Pair(kSdpFormatReceivedNoTracks, 1)));
|
||||
}
|
||||
#endif // WEBRTC_HAVE_SCTP
|
||||
|
||||
TEST_F(SdpFormatReceivedTest, SimpleUnifiedPlanIsReportedAsSimple) {
|
||||
auto caller = CreatePeerConnectionWithUnifiedPlan();
|
||||
caller->AddAudioTrack("audio");
|
||||
caller->AddVideoTrack("video");
|
||||
auto callee = CreatePeerConnectionWithPlanB();
|
||||
|
||||
ASSERT_TRUE(callee->SetRemoteDescription(caller->CreateOffer()));
|
||||
// Note that only the callee does ReportSdpFormatReceived.
|
||||
EXPECT_METRIC_THAT(
|
||||
metrics::Samples("WebRTC.PeerConnection.SdpFormatReceived"),
|
||||
ElementsAre(Pair(kSdpFormatReceivedSimple, 1)));
|
||||
}
|
||||
|
||||
TEST_F(SdpFormatReceivedTest, SimplePlanBIsReportedAsSimple) {
|
||||
auto caller = CreatePeerConnectionWithPlanB();
|
||||
caller->AddVideoTrack("video"); // Video only.
|
||||
auto callee = CreatePeerConnectionWithUnifiedPlan();
|
||||
|
||||
ASSERT_TRUE(callee->SetRemoteDescription(caller->CreateOffer()));
|
||||
// Note that only the callee does ReportSdpFormatReceived.
|
||||
EXPECT_METRIC_THAT(
|
||||
metrics::Samples("WebRTC.PeerConnection.SdpFormatReceived"),
|
||||
ElementsAre(Pair(kSdpFormatReceivedSimple, 1)));
|
||||
}
|
||||
|
||||
TEST_F(SdpFormatReceivedTest, ComplexUnifiedIsReportedAsComplexUnifiedPlan) {
|
||||
auto caller = CreatePeerConnectionWithUnifiedPlan();
|
||||
caller->AddAudioTrack("audio1");
|
||||
caller->AddAudioTrack("audio2");
|
||||
caller->AddVideoTrack("video");
|
||||
auto callee = CreatePeerConnectionWithPlanB();
|
||||
|
||||
ASSERT_TRUE(callee->SetRemoteDescription(caller->CreateOffer()));
|
||||
// Note that only the callee does ReportSdpFormatReceived.
|
||||
EXPECT_METRIC_THAT(
|
||||
metrics::Samples("WebRTC.PeerConnection.SdpFormatReceived"),
|
||||
ElementsAre(Pair(kSdpFormatReceivedComplexUnifiedPlan, 1)));
|
||||
}
|
||||
|
||||
TEST_F(SdpFormatReceivedTest, ComplexPlanBIsReportedAsComplexPlanB) {
|
||||
auto caller = CreatePeerConnectionWithPlanB();
|
||||
caller->AddVideoTrack("video1");
|
||||
caller->AddVideoTrack("video2");
|
||||
auto callee = CreatePeerConnectionWithUnifiedPlan();
|
||||
|
||||
// This fails since Unified Plan cannot set a session description with
|
||||
// multiple "Plan B tracks" in the same media section. But we still expect the
|
||||
// SDP Format to be recorded.
|
||||
ASSERT_FALSE(callee->SetRemoteDescription(caller->CreateOffer()));
|
||||
// Note that only the callee does ReportSdpFormatReceived.
|
||||
EXPECT_METRIC_THAT(
|
||||
metrics::Samples("WebRTC.PeerConnection.SdpFormatReceived"),
|
||||
ElementsAre(Pair(kSdpFormatReceivedComplexPlanB, 1)));
|
||||
}
|
||||
|
||||
TEST_F(SdpFormatReceivedTest, AnswerIsReported) {
|
||||
auto caller = CreatePeerConnectionWithPlanB();
|
||||
caller->AddAudioTrack("audio");
|
||||
caller->AddVideoTrack("video");
|
||||
auto callee = CreatePeerConnectionWithUnifiedPlan();
|
||||
|
||||
ASSERT_TRUE(callee->SetRemoteDescription(caller->CreateOfferAndSetAsLocal()));
|
||||
ASSERT_TRUE(caller->SetRemoteDescription(callee->CreateAnswer()));
|
||||
EXPECT_METRIC_THAT(
|
||||
metrics::Samples("WebRTC.PeerConnection.SdpFormatReceivedAnswer"),
|
||||
ElementsAre(Pair(kSdpFormatReceivedSimple, 1)));
|
||||
}
|
||||
|
||||
// Sender setups in a call.
|
||||
|
||||
TEST_P(PeerConnectionRtpTest, CreateTwoSendersWithSameTrack) {
|
||||
|
||||
@ -799,7 +799,6 @@ class SdpOfferAnswerHandler::RemoteDescriptionOperation {
|
||||
void ReportOfferAnswerUma() {
|
||||
RTC_DCHECK(ok());
|
||||
if (type_ == SdpType::kOffer || type_ == SdpType::kAnswer) {
|
||||
handler_->pc_->ReportSdpFormatReceived(*desc_.get());
|
||||
handler_->pc_->ReportSdpBundleUsage(*desc_.get());
|
||||
}
|
||||
}
|
||||
@ -2200,8 +2199,6 @@ void SdpOfferAnswerHandler::DoSetLocalDescription(
|
||||
// MaybeStartGathering...
|
||||
context_->network_thread()->BlockingCall(
|
||||
[this] { port_allocator()->DiscardCandidatePool(); });
|
||||
// Make UMA notes about what was agreed to.
|
||||
ReportNegotiatedSdpSemantics(*local_description());
|
||||
}
|
||||
|
||||
observer->OnSetLocalDescriptionComplete(RTCError::OK());
|
||||
@ -2404,8 +2401,6 @@ void SdpOfferAnswerHandler::SetRemoteDescriptionPostProcess(bool was_answer) {
|
||||
// MaybeStartGathering...
|
||||
context_->network_thread()->BlockingCall(
|
||||
[this] { port_allocator()->DiscardCandidatePool(); });
|
||||
// Make UMA notes about what was agreed to.
|
||||
ReportNegotiatedSdpSemantics(*remote_description());
|
||||
}
|
||||
|
||||
pc_->NoteUsageEvent(UsageEvent::SET_REMOTE_DESCRIPTION_SUCCEEDED);
|
||||
@ -4698,30 +4693,6 @@ void SdpOfferAnswerHandler::RemoveUnusedChannels(
|
||||
}
|
||||
}
|
||||
|
||||
void SdpOfferAnswerHandler::ReportNegotiatedSdpSemantics(
|
||||
const SessionDescriptionInterface& answer) {
|
||||
SdpSemanticNegotiated semantics_negotiated;
|
||||
switch (answer.description()->msid_signaling()) {
|
||||
case 0:
|
||||
semantics_negotiated = kSdpSemanticNegotiatedNone;
|
||||
break;
|
||||
case cricket::kMsidSignalingMediaSection:
|
||||
semantics_negotiated = kSdpSemanticNegotiatedUnifiedPlan;
|
||||
break;
|
||||
case cricket::kMsidSignalingSsrcAttribute:
|
||||
semantics_negotiated = kSdpSemanticNegotiatedPlanB;
|
||||
break;
|
||||
case cricket::kMsidSignalingMediaSection |
|
||||
cricket::kMsidSignalingSsrcAttribute:
|
||||
semantics_negotiated = kSdpSemanticNegotiatedMixed;
|
||||
break;
|
||||
default:
|
||||
RTC_DCHECK_NOTREACHED();
|
||||
}
|
||||
RTC_HISTOGRAM_ENUMERATION("WebRTC.PeerConnection.SdpSemanticNegotiated",
|
||||
semantics_negotiated, kSdpSemanticNegotiatedMax);
|
||||
}
|
||||
|
||||
void SdpOfferAnswerHandler::UpdateEndedRemoteMediaStreams() {
|
||||
RTC_DCHECK_RUN_ON(signaling_thread());
|
||||
std::vector<rtc::scoped_refptr<MediaStreamInterface>> streams_to_remove;
|
||||
|
||||
@ -491,10 +491,6 @@ class SdpOfferAnswerHandler : public SdpStateProvider {
|
||||
// `desc` can be null. This means that all channels are deleted.
|
||||
void RemoveUnusedChannels(const cricket::SessionDescription* desc);
|
||||
|
||||
// Report inferred negotiated SDP semantics from a local/remote answer to the
|
||||
// UMA observer.
|
||||
void ReportNegotiatedSdpSemantics(const SessionDescriptionInterface& answer);
|
||||
|
||||
// Finds remote MediaStreams without any tracks and removes them from
|
||||
// `remote_streams_` and notifies the observer that the MediaStreams no longer
|
||||
// exist.
|
||||
|
||||
@ -294,9 +294,6 @@ class FakePeerConnectionBase : public PeerConnectionInternal {
|
||||
return nullptr;
|
||||
}
|
||||
|
||||
void ReportSdpFormatReceived(
|
||||
const SessionDescriptionInterface& remote_description) override {}
|
||||
|
||||
void ReportSdpBundleUsage(
|
||||
const SessionDescriptionInterface& remote_description) override {}
|
||||
|
||||
|
||||
@ -202,10 +202,6 @@ class MockPeerConnectionInternal : public PeerConnectionInternal {
|
||||
configuration,
|
||||
(),
|
||||
(const, override));
|
||||
MOCK_METHOD(void,
|
||||
ReportSdpFormatReceived,
|
||||
(const SessionDescriptionInterface&),
|
||||
(override));
|
||||
MOCK_METHOD(void,
|
||||
ReportSdpBundleUsage,
|
||||
(const SessionDescriptionInterface&),
|
||||
|
||||
Loading…
x
Reference in New Issue
Block a user