[Stats] Add minimum RTCReceivedRtpStreamStats with jitter and packetsLost

Spec: https://www.w3.org/TR/webrtc-stats/#receivedrtpstats-dict*

    According to the spec, |RTCReceivedRtpStreamStats| is the base class for |RTCInboundRtpStreamStats| and |RTCRemoteInboundRtpStreamStats|. This structure isn't visible in JavaScript but it's important to bring it up to spec for the C++ part. This CL adds the barebone |RTCReceivedRtpStreamStats| with a bunch of TODOs for later migrations.

    This commit makes the minimum |RTCReceivedRtpStreamStats| and rebase |RTCInboundRtpStreamStats| and |RTCRemoteInboundRtpStreamStats| to use the new class as the parent class.

    This commit also moves |jitter| and |packets_lost| to |RTCReceivedRtpStreamStats|, from |RTCInboundRtpStreamStats| and |RTCRemoteInboundRtpStreamStats|. Moving these two first because they are the two that exist in both subclasses for now.

Bug: webrtc:12532
Change-Id: I0ec74fd241f16c1e1a6498b6baa621ca0489f279
Reviewed-on: https://webrtc-review.googlesource.com/c/src/+/210340
Commit-Queue: Henrik Boström <hbos@webrtc.org>
Reviewed-by: Henrik Boström <hbos@webrtc.org>
Reviewed-by: Harald Alvestrand <hta@webrtc.org>
Cr-Commit-Position: refs/heads/master@{#33435}

api/stats/rtcstats_objects.h

@@ -374,34 +374,46 @@ class RTC_EXPORT RTCRTPStreamStats : public RTCStats {
   ~RTCRTPStreamStats() override;
 
   RTCStatsMember<uint32_t> ssrc;
-  // TODO(hbos): Remote case not supported by |RTCStatsCollector|.
-  // crbug.com/657855, 657856
-  RTCStatsMember<bool> is_remote;          // = false
-  RTCStatsMember<std::string> media_type;  // renamed to kind.
   RTCStatsMember<std::string> kind;
   RTCStatsMember<std::string> track_id;
   RTCStatsMember<std::string> transport_id;
   RTCStatsMember<std::string> codec_id;
-  // FIR and PLI counts are only defined for |media_type == "video"|.
+
-  RTCStatsMember<uint32_t> fir_count;
+  // Obsolete
-  RTCStatsMember<uint32_t> pli_count;
+  RTCStatsMember<std::string> media_type;  // renamed to kind.
-  // TODO(hbos): NACK count should be collected by |RTCStatsCollector| for both
-  // audio and video but is only defined in the "video" case. crbug.com/657856
-  RTCStatsMember<uint32_t> nack_count;
-  // TODO(hbos): Not collected by |RTCStatsCollector|. crbug.com/657854
-  // SLI count is only defined for |media_type == "video"|.
-  RTCStatsMember<uint32_t> sli_count;
-  RTCStatsMember<uint64_t> qp_sum;
 
  protected:
   RTCRTPStreamStats(const std::string& id, int64_t timestamp_us);
   RTCRTPStreamStats(std::string&& id, int64_t timestamp_us);
 };
 
+class RTC_EXPORT RTCReceivedRtpStreamStats : public RTCRTPStreamStats {
+ public:
+  WEBRTC_RTCSTATS_DECL();
+
+  RTCReceivedRtpStreamStats(const RTCReceivedRtpStreamStats& other);
+  ~RTCReceivedRtpStreamStats() override;
+
+  // TODO(hbos) The following fields need to be added and migrated
+  // both from RTCInboundRtpStreamStats and RTCRemoteInboundRtpStreamStats:
+  // packetsReceived, packetsDiscarded, packetsRepaired, burstPacketsLost,
+  // burstPacketDiscarded, burstLossCount, burstDiscardCount, burstLossRate,
+  // burstDiscardRate, gapLossRate, gapDiscardRate, framesDropped,
+  // partialFramesLost, fullFramesLost
+  // crbug.com/webrtc/12532
+  RTCStatsMember<double> jitter;
+  RTCStatsMember<int32_t> packets_lost;  // Signed per RFC 3550
+
+ protected:
+  RTCReceivedRtpStreamStats(const std::string&& id, int64_t timestamp_us);
+  RTCReceivedRtpStreamStats(std::string&& id, int64_t timestamp_us);
+};
+
 // https://w3c.github.io/webrtc-stats/#inboundrtpstats-dict*
 // TODO(hbos): Support the remote case |is_remote = true|.
 // https://bugs.webrtc.org/7065
-class RTC_EXPORT RTCInboundRTPStreamStats final : public RTCRTPStreamStats {
+class RTC_EXPORT RTCInboundRTPStreamStats final
+    : public RTCReceivedRtpStreamStats {
  public:
   WEBRTC_RTCSTATS_DECL();
 
@@ -415,9 +427,7 @@ class RTC_EXPORT RTCInboundRTPStreamStats final : public RTCRTPStreamStats {
   RTCStatsMember<uint64_t> fec_packets_discarded;
   RTCStatsMember<uint64_t> bytes_received;
   RTCStatsMember<uint64_t> header_bytes_received;
-  RTCStatsMember<int32_t> packets_lost;  // Signed per RFC 3550
   RTCStatsMember<double> last_packet_received_timestamp;
-  RTCStatsMember<double> jitter;
   RTCStatsMember<double> jitter_buffer_delay;
   RTCStatsMember<uint64_t> jitter_buffer_emitted_count;
   RTCStatsMember<uint64_t> total_samples_received;
@@ -469,6 +479,16 @@ class RTC_EXPORT RTCInboundRTPStreamStats final : public RTCRTPStreamStats {
   // TODO(hbos): This is only implemented for video; implement it for audio as
   // well.
   RTCStatsMember<std::string> decoder_implementation;
+  // FIR and PLI counts are only defined for |media_type == "video"|.
+  RTCStatsMember<uint32_t> fir_count;
+  RTCStatsMember<uint32_t> pli_count;
+  // TODO(hbos): NACK count should be collected by |RTCStatsCollector| for both
+  // audio and video but is only defined in the "video" case. crbug.com/657856
+  RTCStatsMember<uint32_t> nack_count;
+  RTCStatsMember<uint64_t> qp_sum;
+
+  // Obsolete
+  RTCStatsMember<bool> is_remote;  // = false
 };
 
 // https://w3c.github.io/webrtc-stats/#outboundrtpstats-dict*
@@ -517,18 +537,21 @@ class RTC_EXPORT RTCOutboundRTPStreamStats final : public RTCRTPStreamStats {
   // TODO(hbos): This is only implemented for video; implement it for audio as
   // well.
   RTCStatsMember<std::string> encoder_implementation;
+  // FIR and PLI counts are only defined for |media_type == "video"|.
+  RTCStatsMember<uint32_t> fir_count;
+  RTCStatsMember<uint32_t> pli_count;
+  // TODO(hbos): NACK count should be collected by |RTCStatsCollector| for both
+  // audio and video but is only defined in the "video" case. crbug.com/657856
+  RTCStatsMember<uint32_t> nack_count;
+  RTCStatsMember<uint64_t> qp_sum;
+
+  // Obsolete
+  RTCStatsMember<bool> is_remote;  // = false
 };
 
-// TODO(https://crbug.com/webrtc/10671): Refactor the stats dictionaries to have
-// the same hierarchy as in the spec; implement RTCReceivedRtpStreamStats.
-// Several metrics are shared between "outbound-rtp", "remote-inbound-rtp",
-// "inbound-rtp" and "remote-outbound-rtp". In the spec there is a hierarchy of
-// dictionaries that minimizes defining the same metrics in multiple places.
-// From JavaScript this hierarchy is not observable and the spec's hierarchy is
-// purely editorial. In C++ non-final classes in the hierarchy could be used to
-// refer to different stats objects within the hierarchy.
 // https://w3c.github.io/webrtc-stats/#remoteinboundrtpstats-dict*
-class RTC_EXPORT RTCRemoteInboundRtpStreamStats final : public RTCStats {
+class RTC_EXPORT RTCRemoteInboundRtpStreamStats final
+    : public RTCReceivedRtpStreamStats {
  public:
   WEBRTC_RTCSTATS_DECL();
 
@@ -537,17 +560,6 @@ class RTC_EXPORT RTCRemoteInboundRtpStreamStats final : public RTCStats {
   RTCRemoteInboundRtpStreamStats(const RTCRemoteInboundRtpStreamStats& other);
   ~RTCRemoteInboundRtpStreamStats() override;
 
-  // In the spec RTCRemoteInboundRtpStreamStats inherits from RTCRtpStreamStats
-  // and RTCReceivedRtpStreamStats. The members here are listed based on where
-  // they are defined in the spec.
-  // RTCRtpStreamStats
-  RTCStatsMember<uint32_t> ssrc;
-  RTCStatsMember<std::string> kind;
-  RTCStatsMember<std::string> transport_id;
-  RTCStatsMember<std::string> codec_id;
-  // RTCReceivedRtpStreamStats
-  RTCStatsMember<int32_t> packets_lost;
-  RTCStatsMember<double> jitter;
   // TODO(hbos): The following RTCReceivedRtpStreamStats metrics should also be
   // implemented: packetsReceived, packetsDiscarded, packetsRepaired,
   // burstPacketsLost, burstPacketsDiscarded, burstLossCount, burstDiscardCount,

pc/rtc_stats_integrationtest.cc

@@ -770,7 +770,6 @@ class RTCStatsReportVerifier {
   void VerifyRTCRTPStreamStats(const RTCRTPStreamStats& stream,
                                RTCStatsVerifier* verifier) {
     verifier->TestMemberIsDefined(stream.ssrc);
-    verifier->TestMemberIsDefined(stream.is_remote);
     verifier->TestMemberIsDefined(stream.media_type);
     verifier->TestMemberIsDefined(stream.kind);
     verifier->TestMemberIsIDReference(stream.track_id,
@@ -778,16 +777,6 @@ class RTCStatsReportVerifier {
     verifier->TestMemberIsIDReference(stream.transport_id,
                                       RTCTransportStats::kType);
     verifier->TestMemberIsIDReference(stream.codec_id, RTCCodecStats::kType);
-    if (stream.media_type.is_defined() && *stream.media_type == "video") {
-      verifier->TestMemberIsNonNegative<uint32_t>(stream.fir_count);
-      verifier->TestMemberIsNonNegative<uint32_t>(stream.pli_count);
-      verifier->TestMemberIsNonNegative<uint32_t>(stream.nack_count);
-    } else {
-      verifier->TestMemberIsUndefined(stream.fir_count);
-      verifier->TestMemberIsUndefined(stream.pli_count);
-      verifier->TestMemberIsUndefined(stream.nack_count);
-    }
-    verifier->TestMemberIsUndefined(stream.sli_count);
   }
 
   bool VerifyRTCInboundRTPStreamStats(
@@ -820,6 +809,7 @@ class RTCStatsReportVerifier {
     // this test. See RFC 3550.
     verifier.TestMemberIsNonNegative<int32_t>(inbound_stream.packets_lost);
     verifier.TestMemberIsDefined(inbound_stream.last_packet_received_timestamp);
+    verifier.TestMemberIsDefined(inbound_stream.is_remote);
     if (inbound_stream.frames_received.ValueOrDefault(0) > 0) {
       verifier.TestMemberIsNonNegative<uint32_t>(inbound_stream.frame_width);
       verifier.TestMemberIsNonNegative<uint32_t>(inbound_stream.frame_height);
@@ -852,7 +842,13 @@ class RTCStatsReportVerifier {
       verifier.TestMemberIsUndefined(inbound_stream.total_audio_energy);
       verifier.TestMemberIsUndefined(inbound_stream.total_samples_duration);
       verifier.TestMemberIsNonNegative<int32_t>(inbound_stream.frames_received);
+      verifier.TestMemberIsNonNegative<uint32_t>(inbound_stream.fir_count);
+      verifier.TestMemberIsNonNegative<uint32_t>(inbound_stream.pli_count);
+      verifier.TestMemberIsNonNegative<uint32_t>(inbound_stream.nack_count);
     } else {
+      verifier.TestMemberIsUndefined(inbound_stream.fir_count);
+      verifier.TestMemberIsUndefined(inbound_stream.pli_count);
+      verifier.TestMemberIsUndefined(inbound_stream.nack_count);
       verifier.TestMemberIsNonNegative<double>(inbound_stream.jitter);
       verifier.TestMemberIsNonNegative<double>(
           inbound_stream.jitter_buffer_delay);
@@ -925,16 +921,23 @@ class RTCStatsReportVerifier {
         *outbound_stream.media_type == "video") {
       verifier.TestMemberIsIDReference(outbound_stream.media_source_id,
                                        RTCVideoSourceStats::kType);
+      verifier.TestMemberIsNonNegative<uint32_t>(outbound_stream.fir_count);
+      verifier.TestMemberIsNonNegative<uint32_t>(outbound_stream.pli_count);
+      verifier.TestMemberIsNonNegative<uint32_t>(outbound_stream.nack_count);
       if (*outbound_stream.frames_encoded > 0) {
         verifier.TestMemberIsNonNegative<uint64_t>(outbound_stream.qp_sum);
       } else {
         verifier.TestMemberIsUndefined(outbound_stream.qp_sum);
       }
     } else {
+      verifier.TestMemberIsUndefined(outbound_stream.fir_count);
+      verifier.TestMemberIsUndefined(outbound_stream.pli_count);
+      verifier.TestMemberIsUndefined(outbound_stream.nack_count);
       verifier.TestMemberIsIDReference(outbound_stream.media_source_id,
                                        RTCAudioSourceStats::kType);
       verifier.TestMemberIsUndefined(outbound_stream.qp_sum);
     }
+    verifier.TestMemberIsDefined(outbound_stream.is_remote);
     verifier.TestMemberIsOptionalIDReference(
         outbound_stream.remote_id, RTCRemoteInboundRtpStreamStats::kType);
     verifier.TestMemberIsNonNegative<uint32_t>(outbound_stream.packets_sent);

stats/rtcstats_objects.cc

@@ -547,17 +547,11 @@ RTCPeerConnectionStats::~RTCPeerConnectionStats() {}
 // clang-format off
 WEBRTC_RTCSTATS_IMPL(RTCRTPStreamStats, RTCStats, "rtp",
     &ssrc,
-    &is_remote,
-    &media_type,
     &kind,
     &track_id,
     &transport_id,
     &codec_id,
-    &fir_count,
+    &media_type)
-    &pli_count,
-    &nack_count,
-    &sli_count,
-    &qp_sum)
 // clang-format on
 
 RTCRTPStreamStats::RTCRTPStreamStats(const std::string& id,
@@ -567,46 +561,57 @@ RTCRTPStreamStats::RTCRTPStreamStats(const std::string& id,
 RTCRTPStreamStats::RTCRTPStreamStats(std::string&& id, int64_t timestamp_us)
     : RTCStats(std::move(id), timestamp_us),
       ssrc("ssrc"),
-      is_remote("isRemote", false),
-      media_type("mediaType"),
       kind("kind"),
       track_id("trackId"),
       transport_id("transportId"),
       codec_id("codecId"),
-      fir_count("firCount"),
+      media_type("mediaType") {}
-      pli_count("pliCount"),
-      nack_count("nackCount"),
-      sli_count("sliCount"),
-      qp_sum("qpSum") {}
 
 RTCRTPStreamStats::RTCRTPStreamStats(const RTCRTPStreamStats& other)
     : RTCStats(other.id(), other.timestamp_us()),
       ssrc(other.ssrc),
-      is_remote(other.is_remote),
-      media_type(other.media_type),
       kind(other.kind),
       track_id(other.track_id),
       transport_id(other.transport_id),
       codec_id(other.codec_id),
-      fir_count(other.fir_count),
+      media_type(other.media_type) {}
-      pli_count(other.pli_count),
-      nack_count(other.nack_count),
-      sli_count(other.sli_count),
-      qp_sum(other.qp_sum) {}
 
 RTCRTPStreamStats::~RTCRTPStreamStats() {}
 
 // clang-format off
 WEBRTC_RTCSTATS_IMPL(
-    RTCInboundRTPStreamStats, RTCRTPStreamStats, "inbound-rtp",
+    RTCReceivedRtpStreamStats, RTCRTPStreamStats, "received-rtp",
+    &jitter,
+    &packets_lost)
+// clang-format on
+
+RTCReceivedRtpStreamStats::RTCReceivedRtpStreamStats(const std::string&& id,
+                                                     int64_t timestamp_us)
+    : RTCReceivedRtpStreamStats(std::string(id), timestamp_us) {}
+
+RTCReceivedRtpStreamStats::RTCReceivedRtpStreamStats(std::string&& id,
+                                                     int64_t timestamp_us)
+    : RTCRTPStreamStats(std::move(id), timestamp_us),
+      jitter("jitter"),
+      packets_lost("packetsLost") {}
+
+RTCReceivedRtpStreamStats::RTCReceivedRtpStreamStats(
+    const RTCReceivedRtpStreamStats& other)
+    : RTCRTPStreamStats(other),
+      jitter(other.jitter),
+      packets_lost(other.packets_lost) {}
+
+RTCReceivedRtpStreamStats::~RTCReceivedRtpStreamStats() {}
+
+// clang-format off
+WEBRTC_RTCSTATS_IMPL(
+    RTCInboundRTPStreamStats, RTCReceivedRtpStreamStats, "inbound-rtp",
     &packets_received,
     &fec_packets_received,
     &fec_packets_discarded,
     &bytes_received,
     &header_bytes_received,
-    &packets_lost,
     &last_packet_received_timestamp,
-    &jitter,
     &jitter_buffer_delay,
     &jitter_buffer_emitted_count,
     &total_samples_received,
@@ -642,7 +647,12 @@ WEBRTC_RTCSTATS_IMPL(
     &total_squared_inter_frame_delay,
     &content_type,
     &estimated_playout_timestamp,
-    &decoder_implementation)
+    &decoder_implementation,
+    &fir_count,
+    &pli_count,
+    &nack_count,
+    &qp_sum,
+    &is_remote)
 // clang-format on
 
 RTCInboundRTPStreamStats::RTCInboundRTPStreamStats(const std::string& id,
@@ -651,15 +661,13 @@ RTCInboundRTPStreamStats::RTCInboundRTPStreamStats(const std::string& id,
 
 RTCInboundRTPStreamStats::RTCInboundRTPStreamStats(std::string&& id,
                                                    int64_t timestamp_us)
-    : RTCRTPStreamStats(std::move(id), timestamp_us),
+    : RTCReceivedRtpStreamStats(std::move(id), timestamp_us),
       packets_received("packetsReceived"),
       fec_packets_received("fecPacketsReceived"),
       fec_packets_discarded("fecPacketsDiscarded"),
       bytes_received("bytesReceived"),
       header_bytes_received("headerBytesReceived"),
-      packets_lost("packetsLost"),
       last_packet_received_timestamp("lastPacketReceivedTimestamp"),
-      jitter("jitter"),
       jitter_buffer_delay("jitterBufferDelay"),
       jitter_buffer_emitted_count("jitterBufferEmittedCount"),
       total_samples_received("totalSamplesReceived"),
@@ -695,19 +703,22 @@ RTCInboundRTPStreamStats::RTCInboundRTPStreamStats(std::string&& id,
       total_squared_inter_frame_delay("totalSquaredInterFrameDelay"),
       content_type("contentType"),
       estimated_playout_timestamp("estimatedPlayoutTimestamp"),
-      decoder_implementation("decoderImplementation") {}
+      decoder_implementation("decoderImplementation"),
+      fir_count("firCount"),
+      pli_count("pliCount"),
+      nack_count("nackCount"),
+      qp_sum("qpSum"),
+      is_remote("isRemote") {}
 
 RTCInboundRTPStreamStats::RTCInboundRTPStreamStats(
     const RTCInboundRTPStreamStats& other)
-    : RTCRTPStreamStats(other),
+    : RTCReceivedRtpStreamStats(other),
       packets_received(other.packets_received),
       fec_packets_received(other.fec_packets_received),
       fec_packets_discarded(other.fec_packets_discarded),
       bytes_received(other.bytes_received),
       header_bytes_received(other.header_bytes_received),
-      packets_lost(other.packets_lost),
       last_packet_received_timestamp(other.last_packet_received_timestamp),
-      jitter(other.jitter),
       jitter_buffer_delay(other.jitter_buffer_delay),
       jitter_buffer_emitted_count(other.jitter_buffer_emitted_count),
       total_samples_received(other.total_samples_received),
@@ -744,7 +755,12 @@ RTCInboundRTPStreamStats::RTCInboundRTPStreamStats(
       total_squared_inter_frame_delay(other.total_squared_inter_frame_delay),
       content_type(other.content_type),
       estimated_playout_timestamp(other.estimated_playout_timestamp),
-      decoder_implementation(other.decoder_implementation) {}
+      decoder_implementation(other.decoder_implementation),
+      fir_count(other.fir_count),
+      pli_count(other.pli_count),
+      nack_count(other.nack_count),
+      qp_sum(other.qp_sum),
+      is_remote(other.is_remote) {}
 
 RTCInboundRTPStreamStats::~RTCInboundRTPStreamStats() {}
 
@@ -773,7 +789,12 @@ WEBRTC_RTCSTATS_IMPL(
     &quality_limitation_reason,
     &quality_limitation_resolution_changes,
     &content_type,
-    &encoder_implementation)
+    &encoder_implementation,
+    &fir_count,
+    &pli_count,
+    &nack_count,
+    &qp_sum,
+    &is_remote)
 // clang-format on
 
 RTCOutboundRTPStreamStats::RTCOutboundRTPStreamStats(const std::string& id,
@@ -806,7 +827,12 @@ RTCOutboundRTPStreamStats::RTCOutboundRTPStreamStats(std::string&& id,
       quality_limitation_resolution_changes(
           "qualityLimitationResolutionChanges"),
       content_type("contentType"),
-      encoder_implementation("encoderImplementation") {}
+      encoder_implementation("encoderImplementation"),
+      fir_count("firCount"),
+      pli_count("pliCount"),
+      nack_count("nackCount"),
+      qp_sum("qpSum"),
+      is_remote("isRemote") {}
 
 RTCOutboundRTPStreamStats::RTCOutboundRTPStreamStats(
     const RTCOutboundRTPStreamStats& other)
@@ -834,7 +860,12 @@ RTCOutboundRTPStreamStats::RTCOutboundRTPStreamStats(
       quality_limitation_resolution_changes(
           other.quality_limitation_resolution_changes),
       content_type(other.content_type),
-      encoder_implementation(other.encoder_implementation) {}
+      encoder_implementation(other.encoder_implementation),
+      fir_count(other.fir_count),
+      pli_count(other.pli_count),
+      nack_count(other.nack_count),
+      qp_sum(other.qp_sum),
+      is_remote(other.is_remote) {}
 
 RTCOutboundRTPStreamStats::~RTCOutboundRTPStreamStats() {}
 
@@ -845,8 +876,6 @@ WEBRTC_RTCSTATS_IMPL(
     &kind,
     &transport_id,
     &codec_id,
-    &packets_lost,
-    &jitter,
     &local_id,
     &round_trip_time,
     &fraction_lost,
@@ -862,13 +891,7 @@ RTCRemoteInboundRtpStreamStats::RTCRemoteInboundRtpStreamStats(
 RTCRemoteInboundRtpStreamStats::RTCRemoteInboundRtpStreamStats(
     std::string&& id,
     int64_t timestamp_us)
-    : RTCStats(std::move(id), timestamp_us),
+    : RTCReceivedRtpStreamStats(std::move(id), timestamp_us),
-      ssrc("ssrc"),
-      kind("kind"),
-      transport_id("transportId"),
-      codec_id("codecId"),
-      packets_lost("packetsLost"),
-      jitter("jitter"),
       local_id("localId"),
       round_trip_time("roundTripTime"),
       fraction_lost("fractionLost"),
@@ -877,13 +900,7 @@ RTCRemoteInboundRtpStreamStats::RTCRemoteInboundRtpStreamStats(
 
 RTCRemoteInboundRtpStreamStats::RTCRemoteInboundRtpStreamStats(
     const RTCRemoteInboundRtpStreamStats& other)
-    : RTCStats(other),
+    : RTCReceivedRtpStreamStats(other),
-      ssrc(other.ssrc),
-      kind(other.kind),
-      transport_id(other.transport_id),
-      codec_id(other.codec_id),
-      packets_lost(other.packets_lost),
-      jitter(other.jitter),
       local_id(other.local_id),
       round_trip_time(other.round_trip_time),
       fraction_lost(other.fraction_lost),