From ba661fe11a56c1a1c3c865228968e26bd195007d Mon Sep 17 00:00:00 2001 From: Bjorn Terelius Date: Mon, 10 Dec 2018 19:29:18 +0100 Subject: [PATCH] Remove support for having multiple SSRCs in an RtcEventVideoSendStreamConfig. MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit This has been deprecated for a long time. Simulcast streams are now logged as one RtcEventVideoSendStreamConfig per SSRC instead of one RtcEventVideoSendStreamConfig containing a group of SSRCs Bug: webrtc:8111 Change-Id: I4da62a4b2151a841413cde222a5154638dbb2e47 Reviewed-on: https://webrtc-review.googlesource.com/c/113811 Commit-Queue: Björn Terelius Reviewed-by: Elad Alon Cr-Commit-Position: refs/heads/master@{#25957} --- .../rtc_event_log/rtc_event_log_parser_new.cc | 95 ++++++++----------- .../rtc_event_log/rtc_event_log_parser_new.h | 13 +-- .../rtc_event_log_unittest_helper.cc | 6 +- 3 files changed, 46 insertions(+), 68 deletions(-) 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 b0d9504d0c..6dab85325f 100644 --- a/logging/rtc_event_log/rtc_event_log_parser_new.cc +++ b/logging/rtc_event_log/rtc_event_log_parser_new.cc @@ -788,15 +788,6 @@ LoggedRtcpPacket::LoggedRtcpPacket(uint64_t timestamp_us, LoggedRtcpPacket::LoggedRtcpPacket(const LoggedRtcpPacket& rhs) = default; LoggedRtcpPacket::~LoggedRtcpPacket() = default; -LoggedVideoSendConfig::LoggedVideoSendConfig() = default; -LoggedVideoSendConfig::LoggedVideoSendConfig( - int64_t timestamp_us, - const std::vector& configs) - : timestamp_us(timestamp_us), configs(configs) {} -LoggedVideoSendConfig::LoggedVideoSendConfig(const LoggedVideoSendConfig& rhs) = - default; -LoggedVideoSendConfig::~LoggedVideoSendConfig() = default; - ParsedRtcEventLogNew::~ParsedRtcEventLogNew() = default; ParsedRtcEventLogNew::LoggedRtpStreamIncoming::LoggedRtpStreamIncoming() = @@ -1146,19 +1137,17 @@ void ParsedRtcEventLogNew::StoreParsedLegacyEvent(const rtclog::Event& event) { break; } case rtclog::Event::VIDEO_SENDER_CONFIG_EVENT: { - std::vector configs = GetVideoSendConfig(event); - video_send_configs_.emplace_back(GetTimestamp(event), configs); - for (const auto& config : configs) { - if (!config.rtp_extensions.empty()) { - outgoing_rtp_extensions_maps_[config.local_ssrc] = - RtpHeaderExtensionMap(config.rtp_extensions); - outgoing_rtp_extensions_maps_[config.rtx_ssrc] = - RtpHeaderExtensionMap(config.rtp_extensions); - } - outgoing_video_ssrcs_.insert(config.local_ssrc); - outgoing_video_ssrcs_.insert(config.rtx_ssrc); - outgoing_rtx_ssrcs_.insert(config.rtx_ssrc); + rtclog::StreamConfig config = GetVideoSendConfig(event); + video_send_configs_.emplace_back(GetTimestamp(event), config); + if (!config.rtp_extensions.empty()) { + outgoing_rtp_extensions_maps_[config.local_ssrc] = + RtpHeaderExtensionMap(config.rtp_extensions); + outgoing_rtp_extensions_maps_[config.rtx_ssrc] = + RtpHeaderExtensionMap(config.rtp_extensions); } + outgoing_video_ssrcs_.insert(config.local_ssrc); + outgoing_video_ssrcs_.insert(config.rtx_ssrc); + outgoing_rtx_ssrcs_.insert(config.rtx_ssrc); break; } case rtclog::Event::AUDIO_RECEIVER_CONFIG_EVENT: { @@ -1467,42 +1456,40 @@ rtclog::StreamConfig ParsedRtcEventLogNew::GetVideoReceiveConfig( return config; } -std::vector ParsedRtcEventLogNew::GetVideoSendConfig( +rtclog::StreamConfig ParsedRtcEventLogNew::GetVideoSendConfig( const rtclog::Event& event) const { - std::vector configs; + rtclog::StreamConfig config; RTC_CHECK(event.has_type()); RTC_CHECK_EQ(event.type(), rtclog::Event::VIDEO_SENDER_CONFIG_EVENT); RTC_CHECK(event.has_video_sender_config()); const rtclog::VideoSendConfig& sender_config = event.video_sender_config(); - if (sender_config.rtx_ssrcs_size() > 0 && - sender_config.ssrcs_size() != sender_config.rtx_ssrcs_size()) { - RTC_LOG(WARNING) - << "VideoSendConfig is configured for RTX but the number of " - "SSRCs doesn't match the number of RTX SSRCs."; - } - configs.resize(sender_config.ssrcs_size()); - for (int i = 0; i < sender_config.ssrcs_size(); i++) { - // Get SSRCs. - configs[i].local_ssrc = sender_config.ssrcs(i); - if (sender_config.rtx_ssrcs_size() > 0 && - i < sender_config.rtx_ssrcs_size()) { - RTC_CHECK(sender_config.has_rtx_payload_type()); - configs[i].rtx_ssrc = sender_config.rtx_ssrcs(i); - } - // Get header extensions. - GetHeaderExtensions(&configs[i].rtp_extensions, - sender_config.header_extensions()); + RTC_CHECK_EQ(sender_config.ssrcs_size(), 1) + << "VideoSendStreamConfig no longer stores multiple SSRCs. If you are " + "analyzing a very old log, try building the parser from the same " + "WebRTC version."; - // Get the codec. - RTC_CHECK(sender_config.has_encoder()); - RTC_CHECK(sender_config.encoder().has_name()); - RTC_CHECK(sender_config.encoder().has_payload_type()); - configs[i].codecs.emplace_back( - sender_config.encoder().name(), sender_config.encoder().payload_type(), - sender_config.has_rtx_payload_type() ? sender_config.rtx_payload_type() - : 0); + // Get SSRCs. + config.local_ssrc = sender_config.ssrcs(0); + + if (sender_config.has_rtx_payload_type()) { + RTC_CHECK_EQ(sender_config.rtx_ssrcs_size(), 1); + config.rtx_ssrc = sender_config.rtx_ssrcs(0); + } else { + RTC_CHECK_EQ(sender_config.rtx_ssrcs_size(), 0); } - return configs; + // Get header extensions. + GetHeaderExtensions(&config.rtp_extensions, + sender_config.header_extensions()); + + // Get the codec. + RTC_CHECK(sender_config.has_encoder()); + RTC_CHECK(sender_config.encoder().has_name()); + RTC_CHECK(sender_config.encoder().has_payload_type()); + config.codecs.emplace_back( + sender_config.encoder().name(), sender_config.encoder().payload_type(), + sender_config.has_rtx_payload_type() ? sender_config.rtx_payload_type() + : 0); + return config; } rtclog::StreamConfig ParsedRtcEventLogNew::GetAudioReceiveConfig( @@ -2457,17 +2444,15 @@ void ParsedRtcEventLogNew::StoreVideoSendConfig( LoggedVideoSendConfig stream; RTC_CHECK(proto.has_timestamp_ms()); stream.timestamp_us = proto.timestamp_ms() * 1000; - rtclog::StreamConfig config; RTC_CHECK(proto.has_ssrc()); - config.local_ssrc = proto.ssrc(); + stream.config.local_ssrc = proto.ssrc(); if (proto.has_rtx_ssrc()) { - config.rtx_ssrc = proto.rtx_ssrc(); + stream.config.rtx_ssrc = proto.rtx_ssrc(); } if (proto.has_header_extensions()) { - config.rtp_extensions = + stream.config.rtp_extensions = GetRuntimeRtpHeaderExtensionConfig(proto.header_extensions()); } - stream.configs.push_back(config); video_send_configs_.push_back(stream); } diff --git a/logging/rtc_event_log/rtc_event_log_parser_new.h b/logging/rtc_event_log/rtc_event_log_parser_new.h index ae97f95ed8..b2156016e3 100644 --- a/logging/rtc_event_log/rtc_event_log_parser_new.h +++ b/logging/rtc_event_log/rtc_event_log_parser_new.h @@ -450,17 +450,15 @@ struct LoggedVideoRecvConfig { }; struct LoggedVideoSendConfig { - LoggedVideoSendConfig(); - LoggedVideoSendConfig(int64_t timestamp_us, - const std::vector& configs); - LoggedVideoSendConfig(const LoggedVideoSendConfig&); - ~LoggedVideoSendConfig(); + LoggedVideoSendConfig() = default; + LoggedVideoSendConfig(int64_t timestamp_us, const rtclog::StreamConfig config) + : timestamp_us(timestamp_us), config(config) {} int64_t log_time_us() const { return timestamp_us; } int64_t log_time_ms() const { return timestamp_us / 1000; } int64_t timestamp_us; - std::vector configs; + rtclog::StreamConfig config; }; template @@ -922,8 +920,7 @@ class ParsedRtcEventLogNew { size_t* length) const; rtclog::StreamConfig GetVideoReceiveConfig(const rtclog::Event& event) const; - std::vector GetVideoSendConfig( - const rtclog::Event& event) const; + rtclog::StreamConfig GetVideoSendConfig(const rtclog::Event& event) const; rtclog::StreamConfig GetAudioReceiveConfig(const rtclog::Event& event) const; rtclog::StreamConfig GetAudioSendConfig(const rtclog::Event& event) const; 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 a92e6663a4..e30d989a76 100644 --- a/logging/rtc_event_log/rtc_event_log_unittest_helper.cc +++ b/logging/rtc_event_log/rtc_event_log_unittest_helper.cc @@ -1023,11 +1023,7 @@ void EventVerifier::VerifyLoggedVideoSendConfig( const RtcEventVideoSendStreamConfig& original_event, const LoggedVideoSendConfig& logged_event) const { EXPECT_EQ(original_event.timestamp_ms(), logged_event.log_time_ms()); - // TODO(terelius): In the past, we allowed storing multiple RtcStreamConfigs - // in the same RtcEventVideoSendStreamConfig. Look into whether we should drop - // backwards compatibility in the parser. - ASSERT_EQ(logged_event.configs.size(), 1u); - VerifyLoggedStreamConfig(original_event.config(), logged_event.configs[0]); + VerifyLoggedStreamConfig(original_event.config(), logged_event.config); } } // namespace test