From 31791e7e2c2cfd28213449183479a7be92340404 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Niels=20M=C3=B6ller?= Date: Wed, 14 Mar 2018 11:27:26 +0100 Subject: [PATCH] Delete RED handling from RtpReceiverImpl::CheckPayloadChanged. Also delete the method RTPPayloadRegistry::red_payload_type() and remnants of RED support in RTPReceiverAudio. Bug: webrtc:8995,webrtc:5922 Change-Id: Iee310f5a8628ba70942e8c0277a856d2ca1f9b35 Reviewed-on: https://webrtc-review.googlesource.com/61500 Reviewed-by: Danil Chapovalov Reviewed-by: Oskar Sundbom Commit-Queue: Niels Moller Cr-Commit-Position: refs/heads/master@{#22425} --- .../rtp_rtcp/include/rtp_payload_registry.h | 7 ----- .../rtp_rtcp/source/rtp_payload_registry.cc | 11 -------- .../source/rtp_payload_registry_unittest.cc | 17 ------------ modules/rtp_rtcp/source/rtp_receiver_audio.cc | 16 ++---------- modules/rtp_rtcp/source/rtp_receiver_audio.h | 4 +-- modules/rtp_rtcp/source/rtp_receiver_impl.cc | 26 ++----------------- modules/rtp_rtcp/source/rtp_receiver_impl.h | 1 - .../rtp_rtcp/source/rtp_receiver_strategy.h | 1 - modules/rtp_rtcp/source/rtp_receiver_video.cc | 1 - modules/rtp_rtcp/source/rtp_receiver_video.h | 1 - 10 files changed, 5 insertions(+), 80 deletions(-) diff --git a/modules/rtp_rtcp/include/rtp_payload_registry.h b/modules/rtp_rtcp/include/rtp_payload_registry.h index 2c907c217c..fc2b5fb403 100644 --- a/modules/rtp_rtcp/include/rtp_payload_registry.h +++ b/modules/rtp_rtcp/include/rtp_payload_registry.h @@ -60,8 +60,6 @@ class RTPPayloadRegistry { // Returns true if the new media payload type has not changed. bool ReportMediaPayloadType(uint8_t media_payload_type); - int8_t red_payload_type() const { return GetPayloadTypeWithName("red"); } - int8_t last_received_payload_type() const { rtc::CritScope cs(&crit_sect_); return last_received_payload_type_; @@ -81,11 +79,6 @@ class RTPPayloadRegistry { void DeregisterAudioCodecOrRedTypeRegardlessOfPayloadType( const SdpAudioFormat& audio_format); - bool IsRtxInternal(const RTPHeader& header) const; - // Returns the payload type for the payload with name |payload_name|, or -1 if - // no such payload is registered. - int8_t GetPayloadTypeWithName(const char* payload_name) const; - rtc::CriticalSection crit_sect_; std::map payload_type_map_; int8_t last_received_payload_type_; diff --git a/modules/rtp_rtcp/source/rtp_payload_registry.cc b/modules/rtp_rtcp/source/rtp_payload_registry.cc index 8061628d27..5f9c1f940a 100644 --- a/modules/rtp_rtcp/source/rtp_payload_registry.cc +++ b/modules/rtp_rtcp/source/rtp_payload_registry.cc @@ -290,15 +290,4 @@ bool RTPPayloadRegistry::ReportMediaPayloadType(uint8_t media_payload_type) { return false; } -// Returns -1 if a payload with name |payload_name| is not registered. -int8_t RTPPayloadRegistry::GetPayloadTypeWithName( - const char* payload_name) const { - rtc::CritScope cs(&crit_sect_); - for (const auto& it : payload_type_map_) { - if (_stricmp(it.second.name, payload_name) == 0) - return it.first; - } - return -1; -} - } // namespace webrtc diff --git a/modules/rtp_rtcp/source/rtp_payload_registry_unittest.cc b/modules/rtp_rtcp/source/rtp_payload_registry_unittest.cc index 1b22d388e9..4e0434fc99 100644 --- a/modules/rtp_rtcp/source/rtp_payload_registry_unittest.cc +++ b/modules/rtp_rtcp/source/rtp_payload_registry_unittest.cc @@ -75,23 +75,6 @@ TEST(RtpPayloadRegistryTest, EXPECT_FALSE(rtp_payload_registry.PayloadTypeToPayload(payload_type)); } -TEST(RtpPayloadRegistryTest, AudioRedWorkProperly) { - RTPPayloadRegistry rtp_payload_registry; - bool new_payload_created = false; - const SdpAudioFormat red_format("red", 8000, 1); - - EXPECT_EQ(0, rtp_payload_registry.RegisterReceivePayload( - 127, red_format, &new_payload_created)); - EXPECT_TRUE(new_payload_created); - - EXPECT_EQ(127, rtp_payload_registry.red_payload_type()); - - const auto retrieved_payload = rtp_payload_registry.PayloadTypeToPayload(127); - EXPECT_TRUE(retrieved_payload); - EXPECT_TRUE(retrieved_payload->typeSpecific.is_audio()); - EXPECT_EQ(red_format, retrieved_payload->typeSpecific.audio_payload().format); -} - TEST(RtpPayloadRegistryTest, DoesNotAcceptSamePayloadTypeTwiceExceptIfPayloadIsCompatible) { constexpr int payload_type = 97; diff --git a/modules/rtp_rtcp/source/rtp_receiver_audio.cc b/modules/rtp_rtcp/source/rtp_receiver_audio.cc index 4f792952cc..e16771fe3b 100644 --- a/modules/rtp_rtcp/source/rtp_receiver_audio.cc +++ b/modules/rtp_rtcp/source/rtp_receiver_audio.cc @@ -137,7 +137,6 @@ int32_t RTPReceiverAudio::OnNewPayloadTypeCreated( int32_t RTPReceiverAudio::ParseRtpPacket(WebRtcRTPHeader* rtp_header, const PayloadUnion& specific_payload, - bool is_red, const uint8_t* payload, size_t payload_length, int64_t timestamp_ms) { @@ -158,7 +157,7 @@ int32_t RTPReceiverAudio::ParseRtpPacket(WebRtcRTPHeader* rtp_header, } return ParseAudioCodecSpecific(rtp_header, payload, payload_length, - specific_payload.audio_payload(), is_red); + specific_payload.audio_payload()); } RTPAliveType RTPReceiverAudio::ProcessDeadOrAlive( @@ -211,8 +210,7 @@ int32_t RTPReceiverAudio::ParseAudioCodecSpecific( WebRtcRTPHeader* rtp_header, const uint8_t* payload_data, size_t payload_length, - const AudioPayload& audio_specific, - bool is_red) { + const AudioPayload& audio_specific) { RTC_DCHECK_GE(payload_length, rtp_header->header.paddingLength); const size_t payload_data_length = payload_length - rtp_header->header.paddingLength; @@ -296,16 +294,6 @@ int32_t RTPReceiverAudio::ParseAudioCodecSpecific( } } } - // TODO(holmer): Break this out to have RED parsing handled generically. - RTC_DCHECK_GT(payload_data_length, 0); - if (is_red && !(payload_data[0] & 0x80)) { - // we recive only one frame packed in a RED packet remove the RED wrapper - rtp_header->header.payloadType = payload_data[0]; - - // only one frame in the RED strip the one byte to help NetEq - return data_callback_->OnReceivedPayloadData( - payload_data + 1, payload_data_length - 1, rtp_header); - } rtp_header->type.Audio.channel = audio_specific.format.num_channels; return data_callback_->OnReceivedPayloadData(payload_data, diff --git a/modules/rtp_rtcp/source/rtp_receiver_audio.h b/modules/rtp_rtcp/source/rtp_receiver_audio.h index a2112230fc..7e1103df91 100644 --- a/modules/rtp_rtcp/source/rtp_receiver_audio.h +++ b/modules/rtp_rtcp/source/rtp_receiver_audio.h @@ -46,7 +46,6 @@ class RTPReceiverAudio : public RTPReceiverStrategy, int32_t ParseRtpPacket(WebRtcRTPHeader* rtp_header, const PayloadUnion& specific_payload, - bool is_red, const uint8_t* packet, size_t payload_length, int64_t timestamp_ms) override; @@ -76,8 +75,7 @@ class RTPReceiverAudio : public RTPReceiverStrategy, int32_t ParseAudioCodecSpecific(WebRtcRTPHeader* rtp_header, const uint8_t* payload_data, size_t payload_length, - const AudioPayload& audio_specific, - bool is_red); + const AudioPayload& audio_specific); bool telephone_event_forward_to_decoder_; int8_t telephone_event_payload_type_; diff --git a/modules/rtp_rtcp/source/rtp_receiver_impl.cc b/modules/rtp_rtcp/source/rtp_receiver_impl.cc index 6ed4d7ce7d..c68d28ec02 100644 --- a/modules/rtp_rtcp/source/rtp_receiver_impl.cc +++ b/modules/rtp_rtcp/source/rtp_receiver_impl.cc @@ -171,9 +171,8 @@ bool RtpReceiverImpl::IncomingRtpPacket(const RTPHeader& rtp_header, CheckSSRCChanged(rtp_header); int8_t first_payload_byte = payload_length > 0 ? payload[0] : 0; - bool is_red = false; - if (CheckPayloadChanged(rtp_header, first_payload_byte, &is_red, + if (CheckPayloadChanged(rtp_header, first_payload_byte, &payload_specific) == -1) { if (payload_length == 0) { // OK, keep-alive packet. @@ -195,7 +194,7 @@ bool RtpReceiverImpl::IncomingRtpPacket(const RTPHeader& rtp_header, UpdateSources(audio_level); int32_t ret_val = rtp_media_receiver_->ParseRtpPacket( - &webrtc_rtp_header, payload_specific, is_red, payload, payload_length, + &webrtc_rtp_header, payload_specific, payload, payload_length, clock_->TimeInMilliseconds()); if (ret_val < 0) { @@ -335,7 +334,6 @@ void RtpReceiverImpl::CheckSSRCChanged(const RTPHeader& rtp_header) { // last known payload). int32_t RtpReceiverImpl::CheckPayloadChanged(const RTPHeader& rtp_header, const int8_t first_payload_byte, - bool* is_red, PayloadUnion* specific_payload) { bool re_initialize_decoder = false; @@ -350,24 +348,6 @@ int32_t RtpReceiverImpl::CheckPayloadChanged(const RTPHeader& rtp_header, // TODO(holmer): Remove this code when RED parsing has been broken out from // RtpReceiverAudio. if (payload_type != last_received_payload_type) { - if (rtp_payload_registry_->red_payload_type() == payload_type) { - // Get the real codec payload type. - payload_type = first_payload_byte & 0x7f; - *is_red = true; - - if (rtp_payload_registry_->red_payload_type() == payload_type) { - // Invalid payload type, traced by caller. If we proceeded here, - // this would be set as |_last_received_payload_type|, and we would no - // longer catch corrupt packets at this level. - return -1; - } - - // When we receive RED we need to check the real payload type. - if (payload_type == last_received_payload_type) { - rtp_media_receiver_->GetLastMediaSpecificPayload(specific_payload); - return 0; - } - } bool should_discard_changes = false; rtp_media_receiver_->CheckPayloadChanged( @@ -375,7 +355,6 @@ int32_t RtpReceiverImpl::CheckPayloadChanged(const RTPHeader& rtp_header, &should_discard_changes); if (should_discard_changes) { - *is_red = false; return 0; } @@ -405,7 +384,6 @@ int32_t RtpReceiverImpl::CheckPayloadChanged(const RTPHeader& rtp_header, } } else { rtp_media_receiver_->GetLastMediaSpecificPayload(specific_payload); - *is_red = false; } } // End critsect. diff --git a/modules/rtp_rtcp/source/rtp_receiver_impl.h b/modules/rtp_rtcp/source/rtp_receiver_impl.h index d41e4030c4..403231856d 100644 --- a/modules/rtp_rtcp/source/rtp_receiver_impl.h +++ b/modules/rtp_rtcp/source/rtp_receiver_impl.h @@ -74,7 +74,6 @@ class RtpReceiverImpl : public RtpReceiver { void CheckCSRC(const WebRtcRTPHeader& rtp_header); int32_t CheckPayloadChanged(const RTPHeader& rtp_header, const int8_t first_payload_byte, - bool* is_red, PayloadUnion* payload); void UpdateSources(const rtc::Optional& ssrc_audio_level); diff --git a/modules/rtp_rtcp/source/rtp_receiver_strategy.h b/modules/rtp_rtcp/source/rtp_receiver_strategy.h index 24c05f14fa..8d24eb170b 100644 --- a/modules/rtp_rtcp/source/rtp_receiver_strategy.h +++ b/modules/rtp_rtcp/source/rtp_receiver_strategy.h @@ -39,7 +39,6 @@ class RTPReceiverStrategy { // provides audio or video-specific data. virtual int32_t ParseRtpPacket(WebRtcRTPHeader* rtp_header, const PayloadUnion& specific_payload, - bool is_red, const uint8_t* payload, size_t payload_length, int64_t timestamp_ms) = 0; diff --git a/modules/rtp_rtcp/source/rtp_receiver_video.cc b/modules/rtp_rtcp/source/rtp_receiver_video.cc index d8117b51e5..314f290436 100644 --- a/modules/rtp_rtcp/source/rtp_receiver_video.cc +++ b/modules/rtp_rtcp/source/rtp_receiver_video.cc @@ -52,7 +52,6 @@ int32_t RTPReceiverVideo::OnNewPayloadTypeCreated( int32_t RTPReceiverVideo::ParseRtpPacket(WebRtcRTPHeader* rtp_header, const PayloadUnion& specific_payload, - bool is_red, const uint8_t* payload, size_t payload_length, int64_t timestamp_ms) { diff --git a/modules/rtp_rtcp/source/rtp_receiver_video.h b/modules/rtp_rtcp/source/rtp_receiver_video.h index bebae0a2c8..4a9f6535c1 100644 --- a/modules/rtp_rtcp/source/rtp_receiver_video.h +++ b/modules/rtp_rtcp/source/rtp_receiver_video.h @@ -27,7 +27,6 @@ class RTPReceiverVideo : public RTPReceiverStrategy { int32_t ParseRtpPacket(WebRtcRTPHeader* rtp_header, const PayloadUnion& specific_payload, - bool is_red, const uint8_t* packet, size_t packet_length, int64_t timestamp) override;