diff --git a/logging/rtc_event_log/encoder/rtc_event_log_encoder_legacy.cc b/logging/rtc_event_log/encoder/rtc_event_log_encoder_legacy.cc index 7ef8d677c6..1db2bf8490 100644 --- a/logging/rtc_event_log/encoder/rtc_event_log_encoder_legacy.cc +++ b/logging/rtc_event_log/encoder/rtc_event_log_encoder_legacy.cc @@ -571,14 +571,14 @@ std::string RtcEventLogEncoderLegacy::EncodeRtcpPacketOutgoing( std::string RtcEventLogEncoderLegacy::EncodeRtpPacketIncoming( const RtcEventRtpPacketIncoming& event) { return EncodeRtpPacket(event.timestamp_us_, event.header_, - event.packet_length_, PacedPacketInfo::kNotAProbe, + event.packet_length(), PacedPacketInfo::kNotAProbe, true); } std::string RtcEventLogEncoderLegacy::EncodeRtpPacketOutgoing( const RtcEventRtpPacketOutgoing& event) { return EncodeRtpPacket(event.timestamp_us_, event.header_, - event.packet_length_, event.probe_cluster_id_, false); + event.packet_length(), event.probe_cluster_id_, false); } std::string RtcEventLogEncoderLegacy::EncodeVideoReceiveStreamConfig( diff --git a/logging/rtc_event_log/events/rtc_event_rtp_packet_incoming.cc b/logging/rtc_event_log/events/rtc_event_rtp_packet_incoming.cc index 8e15d02175..d0e511825f 100644 --- a/logging/rtc_event_log/events/rtc_event_rtp_packet_incoming.cc +++ b/logging/rtc_event_log/events/rtc_event_rtp_packet_incoming.cc @@ -17,13 +17,20 @@ namespace webrtc { RtcEventRtpPacketIncoming::RtcEventRtpPacketIncoming( const RtpPacketReceived& packet) - : packet_length_(packet.size()) { + : payload_length_(packet.payload_size()), + header_length_(packet.headers_size()), + padding_length_(packet.padding_size()) { header_.CopyHeaderFrom(packet); + RTC_DCHECK_EQ(packet.size(), + payload_length_ + header_length_ + padding_length_); } RtcEventRtpPacketIncoming::RtcEventRtpPacketIncoming( const RtcEventRtpPacketIncoming& other) - : RtcEvent(other.timestamp_us_), packet_length_(other.packet_length_) { + : RtcEvent(other.timestamp_us_), + payload_length_(other.payload_length_), + header_length_(other.header_length_), + padding_length_(other.padding_length_) { header_.CopyHeaderFrom(other.header_); } diff --git a/logging/rtc_event_log/events/rtc_event_rtp_packet_incoming.h b/logging/rtc_event_log/events/rtc_event_rtp_packet_incoming.h index 6b141b6d57..edc506b90c 100644 --- a/logging/rtc_event_log/events/rtc_event_rtp_packet_incoming.h +++ b/logging/rtc_event_log/events/rtc_event_rtp_packet_incoming.h @@ -31,8 +31,14 @@ class RtcEventRtpPacketIncoming final : public RtcEvent { std::unique_ptr Copy() const override; + size_t packet_length() const { + return payload_length_ + header_length_ + padding_length_; + } + RtpPacket header_; // Only the packet's header will be stored here. - const size_t packet_length_; // Length before stripping away all but header. + const size_t payload_length_; // Media payload, excluding header and padding. + const size_t header_length_; // RTP header. + const size_t padding_length_; // RTP padding. private: RtcEventRtpPacketIncoming(const RtcEventRtpPacketIncoming& other); diff --git a/logging/rtc_event_log/events/rtc_event_rtp_packet_outgoing.cc b/logging/rtc_event_log/events/rtc_event_rtp_packet_outgoing.cc index 103440ad6e..3f86c3f0fc 100644 --- a/logging/rtc_event_log/events/rtc_event_rtp_packet_outgoing.cc +++ b/logging/rtc_event_log/events/rtc_event_rtp_packet_outgoing.cc @@ -18,14 +18,21 @@ namespace webrtc { RtcEventRtpPacketOutgoing::RtcEventRtpPacketOutgoing( const RtpPacketToSend& packet, int probe_cluster_id) - : packet_length_(packet.size()), probe_cluster_id_(probe_cluster_id) { + : payload_length_(packet.payload_size()), + header_length_(packet.headers_size()), + padding_length_(packet.padding_size()), + probe_cluster_id_(probe_cluster_id) { header_.CopyHeaderFrom(packet); + RTC_DCHECK_EQ(packet.size(), + payload_length_ + header_length_ + padding_length_); } RtcEventRtpPacketOutgoing::RtcEventRtpPacketOutgoing( const RtcEventRtpPacketOutgoing& other) : RtcEvent(other.timestamp_us_), - packet_length_(other.packet_length_), + payload_length_(other.payload_length_), + header_length_(other.header_length_), + padding_length_(other.padding_length_), probe_cluster_id_(other.probe_cluster_id_) { header_.CopyHeaderFrom(other.header_); } diff --git a/logging/rtc_event_log/events/rtc_event_rtp_packet_outgoing.h b/logging/rtc_event_log/events/rtc_event_rtp_packet_outgoing.h index 171a21c420..c21e6133c4 100644 --- a/logging/rtc_event_log/events/rtc_event_rtp_packet_outgoing.h +++ b/logging/rtc_event_log/events/rtc_event_rtp_packet_outgoing.h @@ -32,8 +32,14 @@ class RtcEventRtpPacketOutgoing final : public RtcEvent { std::unique_ptr Copy() const override; + size_t packet_length() const { + return payload_length_ + header_length_ + padding_length_; + } + RtpPacket header_; // Only the packet's header will be stored here. - const size_t packet_length_; // Length before stripping away all but header. + const size_t payload_length_; // Media payload, excluding header and padding. + const size_t header_length_; // RTP header. + const size_t padding_length_; // RTP padding. const int probe_cluster_id_; private: diff --git a/logging/rtc_event_log/rtc_event_log2.proto b/logging/rtc_event_log/rtc_event_log2.proto index d1e2f30d0c..213aee7fe4 100644 --- a/logging/rtc_event_log/rtc_event_log2.proto +++ b/logging/rtc_event_log/rtc_event_log2.proto @@ -63,14 +63,15 @@ message IncomingRtpPackets { // TODO(terelius/dinor): Add CSRCs. Field number 7 reserved for this purpose. - // required - The size (in bytes) of the packet including header, payload - // and padding. - optional uint32 packet_size = 8; + // required - The size (in bytes) of the media payload, not including + // RTP header or padding. The packet size is the sum of payload, header and + // padding. + optional uint32 payload_size = 8; // required - The size (in bytes) of the RTP header. optional uint32 header_size = 9; - // required - The size (in bytes) of the RTP header. + // required - The size (in bytes) of the padding. optional uint32 padding_size = 10; // optional - required if the batch contains delta encoded events. @@ -94,7 +95,7 @@ message IncomingRtpPackets { optional bytes rtp_timestamp_deltas = 105; // Field number 107 reserved for CSRC. optional bytes ssrc_deltas = 106; - optional bytes packet_size_deltas = 108; + optional bytes payload_size_deltas = 108; optional bytes header_size_deltas = 109; optional bytes padding_size_deltas = 110; // Field number 111-114 reserved for future use. @@ -126,14 +127,15 @@ message OutgoingRtpPackets { // TODO(terelius/dinor): Add CSRCs. Field number 7 reserved for this purpose. - // required - The size (in bytes) of the packet including header, payload - // and padding. - optional uint32 packet_size = 8; + // required - The size (in bytes) of the media payload, not including + // RTP header or padding. The packet size is the sum of payload, header and + // padding. + optional uint32 payload_size = 8; // required - The size (in bytes) of the RTP header. optional uint32 header_size = 9; - // required - The size (in bytes) of the RTP header. + // required - The size (in bytes) of the padding. optional uint32 padding_size = 10; // optional - required if the batch contains delta encoded events. @@ -157,7 +159,7 @@ message OutgoingRtpPackets { optional bytes rtp_timestamp_deltas = 105; optional bytes ssrc_deltas = 106; // Field number 107 reserved for CSRC. - optional bytes packet_size_deltas = 108; + optional bytes payload_size_deltas = 108; optional bytes header_size_deltas = 109; optional bytes padding_size_deltas = 110; // Field number 111-114 reserved for future use. diff --git a/logging/rtc_event_log/rtc_event_log_unittest_helper.cc b/logging/rtc_event_log/rtc_event_log_unittest_helper.cc index db51d7d6ed..581e79182b 100644 --- a/logging/rtc_event_log/rtc_event_log_unittest_helper.cc +++ b/logging/rtc_event_log/rtc_event_log_unittest_helper.cc @@ -655,17 +655,12 @@ void VerifyLoggedRtpPacketIncoming( EXPECT_EQ(original_event.header_.headers_size(), logged_event.rtp.header_length); - EXPECT_EQ(original_event.packet_length_, logged_event.rtp.total_length); + EXPECT_EQ(original_event.packet_length(), logged_event.rtp.total_length); - if ((original_event.header_.data()[0] & 0x20) != 0) { // has padding - // Currently, RTC eventlog encoder-parser can only maintain padding length - // if packet is full padding. - // TODO(webrtc:9730): Change the condition to something like - // original_event.padding_length_ != logged_event.rtp.header.paddingLength. - EXPECT_EQ( - original_event.packet_length_ - original_event.header_.headers_size(), - logged_event.rtp.header.paddingLength); - } + // Currently, RTC eventlog encoder-parser can only maintain padding length + // if packet is full padding. + EXPECT_EQ(original_event.padding_length_, + logged_event.rtp.header.paddingLength); VerifyLoggedRtpHeader(original_event.header_, logged_event.rtp.header); } @@ -678,17 +673,12 @@ void VerifyLoggedRtpPacketOutgoing( EXPECT_EQ(original_event.header_.headers_size(), logged_event.rtp.header_length); - EXPECT_EQ(original_event.packet_length_, logged_event.rtp.total_length); + EXPECT_EQ(original_event.packet_length(), logged_event.rtp.total_length); - if ((original_event.header_.data()[0] & 0x20) != 0) { // has padding - // Currently, RTC eventlog encoder-parser can only maintain padding length - // if packet is full padding. - // TODO(webrtc:9730): Change the condition to something like - // original_event.padding_length_ != logged_event.rtp.header.paddingLength. - EXPECT_EQ( - original_event.packet_length_ - original_event.header_.headers_size(), - logged_event.rtp.header.paddingLength); - } + // Currently, RTC eventlog encoder-parser can only maintain padding length + // if packet is full padding. + EXPECT_EQ(original_event.padding_length_, + logged_event.rtp.header.paddingLength); // TODO(terelius): Probe cluster ID isn't parsed, used or tested. Unless // someone has a strong reason to keep it, it'll be removed.