From 2c977b4cc57d9ddbf9cf6367dde5e91c4f2ba12a Mon Sep 17 00:00:00 2001 From: Bjorn Terelius Date: Thu, 22 Nov 2018 18:04:47 +0100 Subject: [PATCH] Remove RSID from stream configs in new event log format. MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit RSID is only useful if we store the RSID header extension. Since we don't do that at the moment, there is no need to store RSID in the stream configs. Bug: webrtc:8111 Change-Id: I978f335d05984346f225c4781a8bfaa228f3f4c8 Reviewed-on: https://webrtc-review.googlesource.com/c/111759 Commit-Queue: Björn Terelius Reviewed-by: Niels Moller Cr-Commit-Position: refs/heads/master@{#25763} --- .../rtc_event_log_encoder_new_format.cc | 8 ------- logging/rtc_event_log/rtc_event_log2.proto | 22 +++++-------------- .../rtc_event_log/rtc_event_log_parser_new.cc | 12 ---------- .../rtc_event_log_unittest_helper.cc | 1 - 4 files changed, 5 insertions(+), 38 deletions(-) diff --git a/logging/rtc_event_log/encoder/rtc_event_log_encoder_new_format.cc b/logging/rtc_event_log/encoder/rtc_event_log_encoder_new_format.cc index a5cd177fb6..79dc379f48 100644 --- a/logging/rtc_event_log/encoder/rtc_event_log_encoder_new_format.cc +++ b/logging/rtc_event_log/encoder/rtc_event_log_encoder_new_format.cc @@ -1023,8 +1023,6 @@ void RtcEventLogEncoderNewFormat::EncodeAudioRecvStreamConfig( proto_batch->set_timestamp_ms(base_event->timestamp_ms()); proto_batch->set_remote_ssrc(base_event->config().remote_ssrc); proto_batch->set_local_ssrc(base_event->config().local_ssrc); - if (!base_event->config().rsid.empty()) - proto_batch->set_rsid(base_event->config().rsid); rtclog2::RtpHeaderExtensionConfig* proto_config = proto_batch->mutable_header_extensions(); @@ -1043,8 +1041,6 @@ void RtcEventLogEncoderNewFormat::EncodeAudioSendStreamConfig( event_stream->add_audio_send_stream_configs(); proto_batch->set_timestamp_ms(base_event->timestamp_ms()); proto_batch->set_ssrc(base_event->config().local_ssrc); - if (!base_event->config().rsid.empty()) - proto_batch->set_rsid(base_event->config().rsid); rtclog2::RtpHeaderExtensionConfig* proto_config = proto_batch->mutable_header_extensions(); @@ -1264,8 +1260,6 @@ void RtcEventLogEncoderNewFormat::EncodeVideoRecvStreamConfig( proto_batch->set_remote_ssrc(base_event->config().remote_ssrc); proto_batch->set_local_ssrc(base_event->config().local_ssrc); proto_batch->set_rtx_ssrc(base_event->config().rtx_ssrc); - if (!base_event->config().rsid.empty()) - proto_batch->set_rsid(base_event->config().rsid); rtclog2::RtpHeaderExtensionConfig* proto_config = proto_batch->mutable_header_extensions(); @@ -1285,8 +1279,6 @@ void RtcEventLogEncoderNewFormat::EncodeVideoSendStreamConfig( proto_batch->set_timestamp_ms(base_event->timestamp_ms()); proto_batch->set_ssrc(base_event->config().local_ssrc); proto_batch->set_rtx_ssrc(base_event->config().rtx_ssrc); - if (!base_event->config().rsid.empty()) - proto_batch->set_rsid(base_event->config().rsid); rtclog2::RtpHeaderExtensionConfig* proto_config = proto_batch->mutable_header_extensions(); diff --git a/logging/rtc_event_log/rtc_event_log2.proto b/logging/rtc_event_log/rtc_event_log2.proto index adaefd64cf..995853b565 100644 --- a/logging/rtc_event_log/rtc_event_log2.proto +++ b/logging/rtc_event_log/rtc_event_log2.proto @@ -321,12 +321,9 @@ message VideoRecvStreamConfig { // optional - required if RTX is configured. SSRC for the RTX stream. optional uint32 rtx_ssrc = 4; - // optional - RTP source stream ID - optional bytes rsid = 5; - // IDs for the header extension we care about. Only required if there are // header extensions configured. - optional RtpHeaderExtensionConfig header_extensions = 6; + optional RtpHeaderExtensionConfig header_extensions = 5; // TODO(terelius): Do we need codec-payload mapping? If so and rtx_ssrc is // used, we also need a map between RTP payload type and RTX payload type. @@ -343,12 +340,9 @@ message VideoSendStreamConfig { // optional - required if RTX is configured. SSRC for the RTX stream. optional uint32 rtx_ssrc = 3; - // optional - RTP source stream ID - optional bytes rsid = 4; - // IDs for the header extension we care about. Only required if there are // header extensions configured. - optional RtpHeaderExtensionConfig header_extensions = 5; + optional RtpHeaderExtensionConfig header_extensions = 4; // TODO(terelius): Do we need codec-payload mapping? If so and rtx_ssrc is // used, we also need a map between RTP payload type and RTX payload type. @@ -366,12 +360,9 @@ message AudioRecvStreamConfig { // Field number 4 reserved for RTX SSRC. - // optional - RTP source stream ID. - optional bytes rsid = 5; - // IDs for the header extension we care about. Only required if there are // header extensions configured. - optional RtpHeaderExtensionConfig header_extensions = 6; + optional RtpHeaderExtensionConfig header_extensions = 5; // TODO(terelius): Do we need codec-payload mapping? If so and rtx_ssrc is // used, we also need a map between RTP payload type and RTX payload type. @@ -384,14 +375,11 @@ message AudioSendStreamConfig { // required - Synchronization source (stream identifier) for outgoing stream. optional uint32 ssrc = 2; - // Field number 3 reserved for RTX SSRC - - // optional - RTP source stream ID - optional bytes rsid = 4; + // Field number 3 reserved for RTX SSRC. // IDs for the header extension we care about. Only required if there are // header extensions configured. - optional RtpHeaderExtensionConfig header_extensions = 5; + optional RtpHeaderExtensionConfig header_extensions = 4; // TODO(terelius): Do we need codec-payload mapping? If so and rtx_ssrc is // used, we also need a map between RTP payload type and RTX payload type. diff --git a/logging/rtc_event_log/rtc_event_log_parser_new.cc b/logging/rtc_event_log/rtc_event_log_parser_new.cc index cadea745f9..7278ea18b1 100644 --- a/logging/rtc_event_log/rtc_event_log_parser_new.cc +++ b/logging/rtc_event_log/rtc_event_log_parser_new.cc @@ -2358,9 +2358,6 @@ void ParsedRtcEventLogNew::StoreVideoRecvConfig( if (proto.has_rtx_ssrc()) { stream.config.rtx_ssrc = proto.rtx_ssrc(); } - if (proto.has_rsid()) { - stream.config.rsid = proto.rsid(); - } if (proto.has_header_extensions()) { stream.config.rtp_extensions = GetRuntimeRtpHeaderExtensionConfig(proto.header_extensions()); @@ -2379,9 +2376,6 @@ void ParsedRtcEventLogNew::StoreVideoSendConfig( if (proto.has_rtx_ssrc()) { config.rtx_ssrc = proto.rtx_ssrc(); } - if (proto.has_rsid()) { - config.rsid = proto.rsid(); - } if (proto.has_header_extensions()) { config.rtp_extensions = GetRuntimeRtpHeaderExtensionConfig(proto.header_extensions()); @@ -2399,9 +2393,6 @@ void ParsedRtcEventLogNew::StoreAudioRecvConfig( stream.config.remote_ssrc = proto.remote_ssrc(); RTC_CHECK(proto.has_local_ssrc()); stream.config.local_ssrc = proto.local_ssrc(); - if (proto.has_rsid()) { - stream.config.rsid = proto.rsid(); - } if (proto.has_header_extensions()) { stream.config.rtp_extensions = GetRuntimeRtpHeaderExtensionConfig(proto.header_extensions()); @@ -2416,9 +2407,6 @@ void ParsedRtcEventLogNew::StoreAudioSendConfig( stream.timestamp_us = proto.timestamp_ms() * 1000; RTC_CHECK(proto.has_ssrc()); stream.config.local_ssrc = proto.ssrc(); - if (proto.has_rsid()) { - stream.config.rsid = proto.rsid(); - } if (proto.has_header_extensions()) { stream.config.rtp_extensions = GetRuntimeRtpHeaderExtensionConfig(proto.header_extensions()); 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 a0c5d3205c..e87808d125 100644 --- a/logging/rtc_event_log/rtc_event_log_unittest_helper.cc +++ b/logging/rtc_event_log/rtc_event_log_unittest_helper.cc @@ -762,7 +762,6 @@ void VerifyLoggedStreamConfig(const rtclog::StreamConfig& original_config, EXPECT_EQ(original_config.local_ssrc, logged_config.local_ssrc); EXPECT_EQ(original_config.remote_ssrc, logged_config.remote_ssrc); EXPECT_EQ(original_config.rtx_ssrc, logged_config.rtx_ssrc); - EXPECT_EQ(original_config.rsid, logged_config.rsid); EXPECT_EQ(original_config.rtp_extensions.size(), logged_config.rtp_extensions.size());