Revert "Fix echo return loss stats and add to RTCAudioSourceStats."
This reverts commit a27cfbffdfa0bf359628d2164db5b9d6321f9c9c. Reason for revert: WebRtcBrowserTest.RunsAudioVideoWebRTCCallInTwoTabsGetStatsPromise failing. Original change's description: > Fix echo return loss stats and add to RTCAudioSourceStats. > > This solves two problems: > * Echo return loss stats weren't being gathered in Chrome, because they > need to be taken from the audio processor attached to the track > rather than the audio send stream. > * The standardized location is in RTCAudioSourceStats, not > RTCMediaStreamTrackStats. For now, will populate the stats in both > locations. > > Bug: webrtc:12770 > Change-Id: I47eaf7f2b50b914a1be84156aa831e27497d07e3 > Reviewed-on: https://webrtc-review.googlesource.com/c/src/+/223182 > Commit-Queue: Taylor Brandstetter <deadbeef@webrtc.org> > Reviewed-by: Henrik Boström <hbos@webrtc.org> > Cr-Commit-Position: refs/heads/master@{#34344} TBR=deadbeef@webrtc.org,hbos@webrtc.org,hbos@chromium.org,webrtc-scoped@luci-project-accounts.iam.gserviceaccount.com Change-Id: I6b2587d762f005adef67c0d5121f1b58c3b76688 No-Presubmit: true No-Tree-Checks: true No-Try: true Bug: webrtc:12770 Reviewed-on: https://webrtc-review.googlesource.com/c/src/+/223068 Reviewed-by: Evan Shrubsole <eshr@google.com> Reviewed-by: Henrik Boström <hbos@webrtc.org> Commit-Queue: Evan Shrubsole <eshr@google.com> Cr-Commit-Position: refs/heads/master@{#34352}
This commit is contained in:
committed by
WebRTC LUCI CQ
parent
9e2b3155ee
commit
fe6580fb87
@ -629,8 +629,6 @@ class RTC_EXPORT RTCAudioSourceStats final : public RTCMediaSourceStats {
|
||||
RTCStatsMember<double> audio_level;
|
||||
RTCStatsMember<double> total_audio_energy;
|
||||
RTCStatsMember<double> total_samples_duration;
|
||||
RTCStatsMember<double> echo_return_loss;
|
||||
RTCStatsMember<double> echo_return_loss_enhancement;
|
||||
};
|
||||
|
||||
// https://w3c.github.io/webrtc-stats/#dom-rtcvideosourcestats
|
||||
|
||||
@ -743,22 +743,10 @@ const std::string& ProduceIceCandidateStats(int64_t timestamp_us,
|
||||
return stats->id();
|
||||
}
|
||||
|
||||
template <typename StatsType>
|
||||
void SetAudioProcessingStats(StatsType* stats,
|
||||
const AudioProcessingStats& apm_stats) {
|
||||
if (apm_stats.echo_return_loss) {
|
||||
stats->echo_return_loss = *apm_stats.echo_return_loss;
|
||||
}
|
||||
if (apm_stats.echo_return_loss_enhancement) {
|
||||
stats->echo_return_loss_enhancement =
|
||||
*apm_stats.echo_return_loss_enhancement;
|
||||
}
|
||||
}
|
||||
|
||||
std::unique_ptr<RTCMediaStreamTrackStats>
|
||||
ProduceMediaStreamTrackStatsFromVoiceSenderInfo(
|
||||
int64_t timestamp_us,
|
||||
AudioTrackInterface& audio_track,
|
||||
const AudioTrackInterface& audio_track,
|
||||
const cricket::VoiceSenderInfo& voice_sender_info,
|
||||
int attachment_id) {
|
||||
std::unique_ptr<RTCMediaStreamTrackStats> audio_track_stats(
|
||||
@ -773,17 +761,13 @@ ProduceMediaStreamTrackStatsFromVoiceSenderInfo(
|
||||
attachment_id);
|
||||
audio_track_stats->remote_source = false;
|
||||
audio_track_stats->detached = false;
|
||||
// Audio processor may be attached to either the track or the send
|
||||
// stream, so look in both places.
|
||||
SetAudioProcessingStats(audio_track_stats.get(),
|
||||
voice_sender_info.apm_statistics);
|
||||
auto audio_processor(audio_track.GetAudioProcessor());
|
||||
if (audio_processor.get()) {
|
||||
// The |has_remote_tracks| argument is obsolete; makes no difference if it's
|
||||
// set to true or false.
|
||||
AudioProcessorInterface::AudioProcessorStatistics ap_stats =
|
||||
audio_processor->GetStats(/*has_remote_tracks=*/false);
|
||||
SetAudioProcessingStats(audio_track_stats.get(), ap_stats.apm_statistics);
|
||||
if (voice_sender_info.apm_statistics.echo_return_loss) {
|
||||
audio_track_stats->echo_return_loss =
|
||||
*voice_sender_info.apm_statistics.echo_return_loss;
|
||||
}
|
||||
if (voice_sender_info.apm_statistics.echo_return_loss_enhancement) {
|
||||
audio_track_stats->echo_return_loss_enhancement =
|
||||
*voice_sender_info.apm_statistics.echo_return_loss_enhancement;
|
||||
}
|
||||
return audio_track_stats;
|
||||
}
|
||||
@ -1673,8 +1657,6 @@ void RTCStatsCollector::ProduceMediaSourceStats_s(
|
||||
// create separate media source stats objects on a per-attachment basis.
|
||||
std::unique_ptr<RTCMediaSourceStats> media_source_stats;
|
||||
if (track->kind() == MediaStreamTrackInterface::kAudioKind) {
|
||||
AudioTrackInterface* audio_track =
|
||||
static_cast<AudioTrackInterface*>(track.get());
|
||||
auto audio_source_stats = std::make_unique<RTCAudioSourceStats>(
|
||||
RTCMediaSourceStatsIDFromKindAndAttachment(
|
||||
cricket::MEDIA_TYPE_AUDIO, sender_internal->AttachmentId()),
|
||||
@ -1695,21 +1677,8 @@ void RTCStatsCollector::ProduceMediaSourceStats_s(
|
||||
voice_sender_info->total_input_energy;
|
||||
audio_source_stats->total_samples_duration =
|
||||
voice_sender_info->total_input_duration;
|
||||
SetAudioProcessingStats(audio_source_stats.get(),
|
||||
voice_sender_info->apm_statistics);
|
||||
}
|
||||
}
|
||||
// Audio processor may be attached to either the track or the send
|
||||
// stream, so look in both places.
|
||||
auto audio_processor(audio_track->GetAudioProcessor());
|
||||
if (audio_processor.get()) {
|
||||
// The |has_remote_tracks| argument is obsolete; makes no difference
|
||||
// if it's set to true or false.
|
||||
AudioProcessorInterface::AudioProcessorStatistics ap_stats =
|
||||
audio_processor->GetStats(/*has_remote_tracks=*/false);
|
||||
SetAudioProcessingStats(audio_source_stats.get(),
|
||||
ap_stats.apm_statistics);
|
||||
}
|
||||
media_source_stats = std::move(audio_source_stats);
|
||||
} else {
|
||||
RTC_DCHECK_EQ(MediaStreamTrackInterface::kVideoKind, track->kind());
|
||||
|
||||
@ -200,34 +200,14 @@ std::unique_ptr<cricket::Candidate> CreateFakeCandidate(
|
||||
return candidate;
|
||||
}
|
||||
|
||||
class FakeAudioProcessor : public AudioProcessorInterface {
|
||||
public:
|
||||
FakeAudioProcessor() {}
|
||||
~FakeAudioProcessor() {}
|
||||
|
||||
private:
|
||||
AudioProcessorInterface::AudioProcessorStatistics GetStats(
|
||||
bool has_recv_streams) override {
|
||||
AudioProcessorStatistics stats;
|
||||
stats.apm_statistics.echo_return_loss = 2.0;
|
||||
stats.apm_statistics.echo_return_loss_enhancement = 3.0;
|
||||
return stats;
|
||||
}
|
||||
};
|
||||
|
||||
class FakeAudioTrackForStats : public MediaStreamTrack<AudioTrackInterface> {
|
||||
public:
|
||||
static rtc::scoped_refptr<FakeAudioTrackForStats> Create(
|
||||
const std::string& id,
|
||||
MediaStreamTrackInterface::TrackState state,
|
||||
bool create_fake_audio_processor) {
|
||||
MediaStreamTrackInterface::TrackState state) {
|
||||
rtc::scoped_refptr<FakeAudioTrackForStats> audio_track_stats(
|
||||
new rtc::RefCountedObject<FakeAudioTrackForStats>(id));
|
||||
audio_track_stats->set_state(state);
|
||||
if (create_fake_audio_processor) {
|
||||
audio_track_stats->processor_ =
|
||||
rtc::make_ref_counted<FakeAudioProcessor>();
|
||||
}
|
||||
return audio_track_stats;
|
||||
}
|
||||
|
||||
@ -242,11 +222,8 @@ class FakeAudioTrackForStats : public MediaStreamTrack<AudioTrackInterface> {
|
||||
void RemoveSink(webrtc::AudioTrackSinkInterface* sink) override {}
|
||||
bool GetSignalLevel(int* level) override { return false; }
|
||||
rtc::scoped_refptr<AudioProcessorInterface> GetAudioProcessor() override {
|
||||
return processor_;
|
||||
return nullptr;
|
||||
}
|
||||
|
||||
private:
|
||||
rtc::scoped_refptr<FakeAudioProcessor> processor_;
|
||||
};
|
||||
|
||||
class FakeVideoTrackSourceForStats : public VideoTrackSourceInterface {
|
||||
@ -331,11 +308,9 @@ class FakeVideoTrackForStats : public MediaStreamTrack<VideoTrackInterface> {
|
||||
rtc::scoped_refptr<MediaStreamTrackInterface> CreateFakeTrack(
|
||||
cricket::MediaType media_type,
|
||||
const std::string& track_id,
|
||||
MediaStreamTrackInterface::TrackState track_state,
|
||||
bool create_fake_audio_processor = false) {
|
||||
MediaStreamTrackInterface::TrackState track_state) {
|
||||
if (media_type == cricket::MEDIA_TYPE_AUDIO) {
|
||||
return FakeAudioTrackForStats::Create(track_id, track_state,
|
||||
create_fake_audio_processor);
|
||||
return FakeAudioTrackForStats::Create(track_id, track_state);
|
||||
} else {
|
||||
RTC_DCHECK_EQ(media_type, cricket::MEDIA_TYPE_VIDEO);
|
||||
return FakeVideoTrackForStats::Create(track_id, track_state, nullptr);
|
||||
@ -2605,9 +2580,6 @@ TEST_F(RTCStatsCollectorTest, RTCAudioSourceStatsCollectedForSenderWithTrack) {
|
||||
voice_media_info.senders[0].audio_level = 32767; // [0,32767]
|
||||
voice_media_info.senders[0].total_input_energy = 2.0;
|
||||
voice_media_info.senders[0].total_input_duration = 3.0;
|
||||
voice_media_info.senders[0].apm_statistics.echo_return_loss = 42.0;
|
||||
voice_media_info.senders[0].apm_statistics.echo_return_loss_enhancement =
|
||||
52.0;
|
||||
auto* voice_media_channel = pc_->AddVoiceChannel("AudioMid", "TransportName");
|
||||
voice_media_channel->SetStats(voice_media_info);
|
||||
stats_->SetupLocalTrackAndSender(cricket::MEDIA_TYPE_AUDIO,
|
||||
@ -2623,8 +2595,6 @@ TEST_F(RTCStatsCollectorTest, RTCAudioSourceStatsCollectedForSenderWithTrack) {
|
||||
expected_audio.audio_level = 1.0; // [0,1]
|
||||
expected_audio.total_audio_energy = 2.0;
|
||||
expected_audio.total_samples_duration = 3.0;
|
||||
expected_audio.echo_return_loss = 42.0;
|
||||
expected_audio.echo_return_loss_enhancement = 52.0;
|
||||
|
||||
ASSERT_TRUE(report->Get(expected_audio.id()));
|
||||
EXPECT_EQ(report->Get(expected_audio.id())->cast_to<RTCAudioSourceStats>(),
|
||||
@ -3086,64 +3056,6 @@ TEST_F(RTCStatsCollectorTest,
|
||||
EXPECT_FALSE(report->Get("RTCVideoSource_42"));
|
||||
}
|
||||
|
||||
// Test collecting echo return loss stats from the audio processor attached to
|
||||
// the track, rather than the voice sender info.
|
||||
TEST_F(RTCStatsCollectorTest, CollectEchoReturnLossFromTrackAudioProcessor) {
|
||||
rtc::scoped_refptr<MediaStream> local_stream =
|
||||
MediaStream::Create("LocalStreamId");
|
||||
pc_->mutable_local_streams()->AddStream(local_stream);
|
||||
|
||||
// Local audio track
|
||||
rtc::scoped_refptr<MediaStreamTrackInterface> local_audio_track =
|
||||
CreateFakeTrack(cricket::MEDIA_TYPE_AUDIO, "LocalAudioTrackID",
|
||||
MediaStreamTrackInterface::kEnded,
|
||||
/*create_fake_audio_processor=*/true);
|
||||
local_stream->AddTrack(
|
||||
static_cast<AudioTrackInterface*>(local_audio_track.get()));
|
||||
|
||||
cricket::VoiceSenderInfo voice_sender_info_ssrc1;
|
||||
voice_sender_info_ssrc1.local_stats.push_back(cricket::SsrcSenderInfo());
|
||||
voice_sender_info_ssrc1.local_stats[0].ssrc = 1;
|
||||
|
||||
stats_->CreateMockRtpSendersReceiversAndChannels(
|
||||
{std::make_pair(local_audio_track.get(), voice_sender_info_ssrc1)}, {},
|
||||
{}, {}, {local_stream->id()}, {});
|
||||
|
||||
rtc::scoped_refptr<const RTCStatsReport> report = stats_->GetStatsReport();
|
||||
|
||||
RTCMediaStreamTrackStats expected_local_audio_track_ssrc1(
|
||||
IdForType<RTCMediaStreamTrackStats>(report), report->timestamp_us(),
|
||||
RTCMediaStreamTrackKind::kAudio);
|
||||
expected_local_audio_track_ssrc1.track_identifier = local_audio_track->id();
|
||||
expected_local_audio_track_ssrc1.media_source_id =
|
||||
"RTCAudioSource_11"; // Attachment ID = SSRC + 10
|
||||
expected_local_audio_track_ssrc1.remote_source = false;
|
||||
expected_local_audio_track_ssrc1.ended = true;
|
||||
expected_local_audio_track_ssrc1.detached = false;
|
||||
expected_local_audio_track_ssrc1.echo_return_loss = 2.0;
|
||||
expected_local_audio_track_ssrc1.echo_return_loss_enhancement = 3.0;
|
||||
ASSERT_TRUE(report->Get(expected_local_audio_track_ssrc1.id()))
|
||||
<< "Did not find " << expected_local_audio_track_ssrc1.id() << " in "
|
||||
<< report->ToJson();
|
||||
EXPECT_EQ(expected_local_audio_track_ssrc1,
|
||||
report->Get(expected_local_audio_track_ssrc1.id())
|
||||
->cast_to<RTCMediaStreamTrackStats>());
|
||||
|
||||
RTCAudioSourceStats expected_audio("RTCAudioSource_11",
|
||||
report->timestamp_us());
|
||||
expected_audio.track_identifier = "LocalAudioTrackID";
|
||||
expected_audio.kind = "audio";
|
||||
expected_audio.audio_level = 0;
|
||||
expected_audio.total_audio_energy = 0;
|
||||
expected_audio.total_samples_duration = 0;
|
||||
expected_audio.echo_return_loss = 2.0;
|
||||
expected_audio.echo_return_loss_enhancement = 3.0;
|
||||
|
||||
ASSERT_TRUE(report->Get(expected_audio.id()));
|
||||
EXPECT_EQ(report->Get(expected_audio.id())->cast_to<RTCAudioSourceStats>(),
|
||||
expected_audio);
|
||||
}
|
||||
|
||||
TEST_F(RTCStatsCollectorTest, GetStatsWithSenderSelector) {
|
||||
ExampleStatsGraph graph = SetupExampleStatsGraphForSelectorTests();
|
||||
// Expected stats graph when filtered by sender:
|
||||
|
||||
@ -1082,12 +1082,6 @@ class RTCStatsReportVerifier {
|
||||
verifier.TestMemberIsNonNegative<double>(audio_source.audio_level);
|
||||
verifier.TestMemberIsPositive<double>(audio_source.total_audio_energy);
|
||||
verifier.TestMemberIsPositive<double>(audio_source.total_samples_duration);
|
||||
// TODO(hbos): |echo_return_loss| and |echo_return_loss_enhancement| are
|
||||
// flaky on msan bot (sometimes defined, sometimes undefined). Should the
|
||||
// test run until available or is there a way to have it always be
|
||||
// defined? crbug.com/627816
|
||||
verifier.MarkMemberTested(audio_source.echo_return_loss, true);
|
||||
verifier.MarkMemberTested(audio_source.echo_return_loss_enhancement, true);
|
||||
return verifier.ExpectAllMembersSuccessfullyTested();
|
||||
}
|
||||
|
||||
|
||||
@ -987,9 +987,7 @@ RTCMediaSourceStats::~RTCMediaSourceStats() {}
|
||||
WEBRTC_RTCSTATS_IMPL(RTCAudioSourceStats, RTCMediaSourceStats, "media-source",
|
||||
&audio_level,
|
||||
&total_audio_energy,
|
||||
&total_samples_duration,
|
||||
&echo_return_loss,
|
||||
&echo_return_loss_enhancement)
|
||||
&total_samples_duration)
|
||||
// clang-format on
|
||||
|
||||
RTCAudioSourceStats::RTCAudioSourceStats(const std::string& id,
|
||||
@ -1000,17 +998,13 @@ RTCAudioSourceStats::RTCAudioSourceStats(std::string&& id, int64_t timestamp_us)
|
||||
: RTCMediaSourceStats(std::move(id), timestamp_us),
|
||||
audio_level("audioLevel"),
|
||||
total_audio_energy("totalAudioEnergy"),
|
||||
total_samples_duration("totalSamplesDuration"),
|
||||
echo_return_loss("echoReturnLoss"),
|
||||
echo_return_loss_enhancement("echoReturnLossEnhancement") {}
|
||||
total_samples_duration("totalSamplesDuration") {}
|
||||
|
||||
RTCAudioSourceStats::RTCAudioSourceStats(const RTCAudioSourceStats& other)
|
||||
: RTCMediaSourceStats(other),
|
||||
audio_level(other.audio_level),
|
||||
total_audio_energy(other.total_audio_energy),
|
||||
total_samples_duration(other.total_samples_duration),
|
||||
echo_return_loss(other.echo_return_loss),
|
||||
echo_return_loss_enhancement(other.echo_return_loss_enhancement) {}
|
||||
total_samples_duration(other.total_samples_duration) {}
|
||||
|
||||
RTCAudioSourceStats::~RTCAudioSourceStats() {}
|
||||
|
||||
|
||||
Reference in New Issue
Block a user