Delete RtpReceiverImpl::CheckPayloadChanged.

Also delete related code in RtpReceiverAudio, RtpReceiverVideo and
RtpPayloadRegistry.

Only intended change in behavior is that packets with unknown payload
types are not discarded at this level of the stack. They are discarded
higher up, in Channel::ReceivePacket (audio) and
RtpVideoStreamReceiver::ReceivePacket (video).

Bug: webrtc:8995
Change-Id: I807997120bb40a95b0575c55db6e20a0cac651bf
Reviewed-on: https://webrtc-review.googlesource.com/92087
Commit-Queue: Niels Moller <nisse@webrtc.org>
Reviewed-by: Danil Chapovalov <danilchap@webrtc.org>
Cr-Commit-Position: refs/heads/master@{#24196}
This commit is contained in:
Niels Möller
2018-08-06 12:40:58 +02:00
committed by Commit Bot
parent 0047bce0a9
commit fd77b78821
9 changed files with 6 additions and 118 deletions

View File

@ -45,20 +45,6 @@ class RTPPayloadRegistry {
absl::optional<RtpUtility::Payload> PayloadTypeToPayload(
uint8_t payload_type) const;
void ResetLastReceivedPayloadTypes() {
rtc::CritScope cs(&crit_sect_);
last_received_payload_type_ = -1;
}
int8_t last_received_payload_type() const {
rtc::CritScope cs(&crit_sect_);
return last_received_payload_type_;
}
void set_last_received_payload_type(int8_t last_received_payload_type) {
rtc::CritScope cs(&crit_sect_);
last_received_payload_type_ = last_received_payload_type;
}
private:
// Prunes the payload type map of the specific payload type, if it exists.
void DeregisterAudioCodecOrRedTypeRegardlessOfPayloadType(
@ -66,7 +52,6 @@ class RTPPayloadRegistry {
rtc::CriticalSection crit_sect_;
std::map<int, RtpUtility::Payload> payload_type_map_;
int8_t last_received_payload_type_;
// As a first step in splitting this class up in separate cases for audio and
// video, DCHECK that no instance is used for both audio and video.

View File

@ -91,7 +91,7 @@ bool IsPayloadTypeValid(int8_t payload_type) {
} // namespace
RTPPayloadRegistry::RTPPayloadRegistry() : last_received_payload_type_(-1) {}
RTPPayloadRegistry::RTPPayloadRegistry() = default;
RTPPayloadRegistry::~RTPPayloadRegistry() = default;
@ -112,10 +112,6 @@ void RTPPayloadRegistry::SetAudioReceivePayloads(
payload_type_map_.emplace(rtp_payload_type,
CreatePayloadType(audio_format));
}
// Clear the value of last received payload type since it might mean
// something else now.
last_received_payload_type_ = -1;
}
int32_t RTPPayloadRegistry::RegisterReceivePayload(
@ -153,9 +149,7 @@ int32_t RTPPayloadRegistry::RegisterReceivePayload(
RTC_DCHECK(insert_status.second); // Insertion succeeded.
*created_new_payload = true;
// Successful set of payload type, clear the value of last received payload
// type since it might mean something else.
last_received_payload_type_ = -1;
// Successful set of payload type.
return 0;
}
@ -186,9 +180,7 @@ int32_t RTPPayloadRegistry::RegisterReceivePayload(
video_codec.plType, CreatePayloadType(video_codec));
RTC_DCHECK(insert_status.second); // Insertion succeeded.
// Successful set of payload type, clear the value of last received payload
// type since it might mean something else.
last_received_payload_type_ = -1;
// Successful set of payload type.
return 0;
}

View File

@ -149,21 +149,6 @@ TEST(RtpPayloadRegistryTest,
<< "Not compatible; both payloads should be kept.";
}
TEST(RtpPayloadRegistryTest,
LastReceivedCodecTypesAreResetWhenRegisteringNewPayloadTypes) {
RTPPayloadRegistry rtp_payload_registry;
rtp_payload_registry.set_last_received_payload_type(17);
EXPECT_EQ(17, rtp_payload_registry.last_received_payload_type());
bool ignored;
constexpr int payload_type = 34;
const SdpAudioFormat audio_format("name", 44000, 1);
EXPECT_EQ(0, rtp_payload_registry.RegisterReceivePayload(
payload_type, audio_format, &ignored));
EXPECT_EQ(-1, rtp_payload_registry.last_received_payload_type());
}
class ParameterizedRtpPayloadRegistryTest
: public ::testing::TestWithParam<int> {};

View File

@ -130,13 +130,6 @@ RTPAliveType RTPReceiverAudio::ProcessDeadOrAlive(
}
}
void RTPReceiverAudio::CheckPayloadChanged(int8_t payload_type,
PayloadUnion* /* specific_payload */,
bool* should_discard_changes) {
*should_discard_changes =
TelephoneEventPayloadType(payload_type) || CNGPayloadType(payload_type);
}
// We are not allowed to have any critsects when calling data_callback.
int32_t RTPReceiverAudio::ParseAudioCodecSpecific(
WebRtcRTPHeader* rtp_header,

View File

@ -41,12 +41,6 @@ class RTPReceiverAudio : public RTPReceiverStrategy {
int32_t OnNewPayloadTypeCreated(int payload_type,
const SdpAudioFormat& audio_format) override;
// We need to look out for special payload types here and sometimes reset
// statistics. In addition we sometimes need to tweak the frequency.
void CheckPayloadChanged(int8_t payload_type,
PayloadUnion* specific_payload,
bool* should_discard_changes) override;
private:
int32_t ParseAudioCodecSpecific(WebRtcRTPHeader* rtp_header,
const uint8_t* payload_data,

View File

@ -150,15 +150,10 @@ bool RtpReceiverImpl::IncomingRtpPacket(const RTPHeader& rtp_header,
// Trigger our callbacks.
CheckSSRCChanged(rtp_header);
if (CheckPayloadChanged(rtp_header, &payload_specific) == -1) {
if (payload_length == 0) {
// OK, keep-alive packet.
return true;
}
RTC_LOG(LS_WARNING) << "Receiving invalid payload type.";
return false;
if (payload_length == 0) {
// OK, keep-alive packet.
return true;
}
WebRtcRTPHeader webrtc_rtp_header{};
webrtc_rtp_header.header = rtp_header;
CheckCSRC(webrtc_rtp_header);
@ -248,47 +243,6 @@ void RtpReceiverImpl::CheckSSRCChanged(const RTPHeader& rtp_header) {
ssrc_ = rtp_header.ssrc;
}
// Implementation note: must not hold critsect when called.
// TODO(phoglund): Move as much as possible of this code path into the media
// specific receivers. Basically this method goes through a lot of trouble to
// compute something which is only used by the media specific parts later. If
// this code path moves we can get rid of some of the rtp_receiver ->
// media_specific interface (such as CheckPayloadChange, possibly get/set
// last known payload).
int32_t RtpReceiverImpl::CheckPayloadChanged(const RTPHeader& rtp_header,
PayloadUnion* specific_payload) {
int8_t payload_type = rtp_header.payloadType;
{
rtc::CritScope lock(&critical_section_rtp_receiver_);
int8_t last_received_payload_type =
rtp_payload_registry_->last_received_payload_type();
// TODO(holmer): Remove this code when RED parsing has been broken out from
// RtpReceiverAudio.
if (payload_type != last_received_payload_type) {
bool should_discard_changes = false;
rtp_media_receiver_->CheckPayloadChanged(payload_type, specific_payload,
&should_discard_changes);
if (should_discard_changes) {
return 0;
}
const auto payload =
rtp_payload_registry_->PayloadTypeToPayload(payload_type);
if (!payload) {
// Not a registered payload type.
return -1;
}
rtp_payload_registry_->set_last_received_payload_type(payload_type);
}
} // End critsect.
return 0;
}
// Implementation note: must not hold critsect when called.
void RtpReceiverImpl::CheckCSRC(const WebRtcRTPHeader& rtp_header) {
const uint8_t num_csrcs = rtp_header.header.numCSRCs;

View File

@ -66,8 +66,6 @@ class RtpReceiverImpl : public RtpReceiver {
private:
void CheckSSRCChanged(const RTPHeader& rtp_header);
void CheckCSRC(const WebRtcRTPHeader& rtp_header);
int32_t CheckPayloadChanged(const RTPHeader& rtp_header,
PayloadUnion* payload);
void UpdateSources(const absl::optional<uint8_t>& ssrc_audio_level);
void RemoveOutdatedSources(int64_t now_ms);

View File

@ -19,11 +19,4 @@ RTPReceiverStrategy::RTPReceiverStrategy(RtpData* data_callback)
RTPReceiverStrategy::~RTPReceiverStrategy() = default;
void RTPReceiverStrategy::CheckPayloadChanged(int8_t payload_type,
PayloadUnion* specific_payload,
bool* should_discard_changes) {
// Default: Keep changes.
*should_discard_changes = false;
}
} // namespace webrtc

View File

@ -50,12 +50,6 @@ class RTPReceiverStrategy {
int payload_type,
const SdpAudioFormat& audio_format) = 0;
// Checks if the payload type has changed, and returns whether we should
// reset statistics and/or discard this packet.
virtual void CheckPayloadChanged(int8_t payload_type,
PayloadUnion* specific_payload,
bool* should_discard_changes);
protected:
// The data callback is where we should send received payload data.
// See ParseRtpPacket. This class does not claim ownership of the callback.