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}
diff --git a/api/stats/rtcstats_objects.h b/api/stats/rtcstats_objects.h
index fe6d658..cb8e25a 100644
--- a/api/stats/rtcstats_objects.h
+++ b/api/stats/rtcstats_objects.h
@@ -629,6 +629,8 @@
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
diff --git a/pc/rtc_stats_collector.cc b/pc/rtc_stats_collector.cc
index 4b38abc..2049d83 100644
--- a/pc/rtc_stats_collector.cc
+++ b/pc/rtc_stats_collector.cc
@@ -743,10 +743,22 @@
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,
- const AudioTrackInterface& audio_track,
+ AudioTrackInterface& audio_track,
const cricket::VoiceSenderInfo& voice_sender_info,
int attachment_id) {
std::unique_ptr<RTCMediaStreamTrackStats> audio_track_stats(
@@ -761,13 +773,17 @@
attachment_id);
audio_track_stats->remote_source = false;
audio_track_stats->detached = false;
- 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;
+ // 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);
}
return audio_track_stats;
}
@@ -1657,6 +1673,8 @@
// 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()),
@@ -1677,8 +1695,21 @@
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());
diff --git a/pc/rtc_stats_collector_unittest.cc b/pc/rtc_stats_collector_unittest.cc
index 92f7377..3488460 100644
--- a/pc/rtc_stats_collector_unittest.cc
+++ b/pc/rtc_stats_collector_unittest.cc
@@ -200,14 +200,34 @@
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) {
+ MediaStreamTrackInterface::TrackState state,
+ bool create_fake_audio_processor) {
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;
}
@@ -222,8 +242,11 @@
void RemoveSink(webrtc::AudioTrackSinkInterface* sink) override {}
bool GetSignalLevel(int* level) override { return false; }
rtc::scoped_refptr<AudioProcessorInterface> GetAudioProcessor() override {
- return nullptr;
+ return processor_;
}
+
+ private:
+ rtc::scoped_refptr<FakeAudioProcessor> processor_;
};
class FakeVideoTrackSourceForStats : public VideoTrackSourceInterface {
@@ -308,9 +331,11 @@
rtc::scoped_refptr<MediaStreamTrackInterface> CreateFakeTrack(
cricket::MediaType media_type,
const std::string& track_id,
- MediaStreamTrackInterface::TrackState track_state) {
+ MediaStreamTrackInterface::TrackState track_state,
+ bool create_fake_audio_processor = false) {
if (media_type == cricket::MEDIA_TYPE_AUDIO) {
- return FakeAudioTrackForStats::Create(track_id, track_state);
+ return FakeAudioTrackForStats::Create(track_id, track_state,
+ create_fake_audio_processor);
} else {
RTC_DCHECK_EQ(media_type, cricket::MEDIA_TYPE_VIDEO);
return FakeVideoTrackForStats::Create(track_id, track_state, nullptr);
@@ -2580,6 +2605,9 @@
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,
@@ -2595,6 +2623,8 @@
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>(),
@@ -3056,6 +3086,64 @@
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:
diff --git a/pc/rtc_stats_integrationtest.cc b/pc/rtc_stats_integrationtest.cc
index 032cbe9..90be600 100644
--- a/pc/rtc_stats_integrationtest.cc
+++ b/pc/rtc_stats_integrationtest.cc
@@ -1082,6 +1082,12 @@
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();
}
diff --git a/stats/rtcstats_objects.cc b/stats/rtcstats_objects.cc
index 0db833c..a2d7aa0 100644
--- a/stats/rtcstats_objects.cc
+++ b/stats/rtcstats_objects.cc
@@ -987,7 +987,9 @@
WEBRTC_RTCSTATS_IMPL(RTCAudioSourceStats, RTCMediaSourceStats, "media-source",
&audio_level,
&total_audio_energy,
- &total_samples_duration)
+ &total_samples_duration,
+ &echo_return_loss,
+ &echo_return_loss_enhancement)
// clang-format on
RTCAudioSourceStats::RTCAudioSourceStats(const std::string& id,
@@ -998,13 +1000,17 @@
: RTCMediaSourceStats(std::move(id), timestamp_us),
audio_level("audioLevel"),
total_audio_energy("totalAudioEnergy"),
- total_samples_duration("totalSamplesDuration") {}
+ total_samples_duration("totalSamplesDuration"),
+ echo_return_loss("echoReturnLoss"),
+ echo_return_loss_enhancement("echoReturnLossEnhancement") {}
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) {}
+ total_samples_duration(other.total_samples_duration),
+ echo_return_loss(other.echo_return_loss),
+ echo_return_loss_enhancement(other.echo_return_loss_enhancement) {}
RTCAudioSourceStats::~RTCAudioSourceStats() {}