Reland "Delete PacketReceiver::DeliverPacket from all implementations"

This reverts commit f2a083f262.

Reason for revert: Test problem fixed in https://webrtc-review.googlesource.com/c/src/+/291333.

Original change's description:
> Revert "Delete PacketReceiver::DeliverPacket from all implementations"
>
> This reverts commit 897ea04db5.
>
> Reason for revert: Speculative revert as it could be the reason why perf tests started failing: https://ci.chromium.org/p/webrtc/g/perf/console?limit=200
>
> Original change's description:
> > Delete PacketReceiver::DeliverPacket from all implementations
> >
> > And fix tests that still depend on extensions to be known by the receiver.
> >
> > Change-Id: I62227829af81af07769189e547f1cdb8ed4d06b3
> >
> > Bug: webrtc:7135,webrtc:14795
> > Change-Id: I62227829af81af07769189e547f1cdb8ed4d06b3
> > Reviewed-on: https://webrtc-review.googlesource.com/c/src/+/290996
> > Commit-Queue: Per Kjellander <perkj@webrtc.org>
> > Reviewed-by: Danil Chapovalov <danilchap@webrtc.org>
> > Reviewed-by: Erik Språng <sprang@webrtc.org>
> > Cr-Commit-Position: refs/heads/main@{#39184}
>
> Bug: webrtc:7135,webrtc:14795,b/266658815
> Change-Id: I9d03f4952938d176ffee110a707acadc1846457c
> No-Presubmit: true
> No-Tree-Checks: true
> No-Try: true
> Reviewed-on: https://webrtc-review.googlesource.com/c/src/+/291400
> Commit-Queue: Andrey Logvin <landrey@webrtc.org>
> Bot-Commit: rubber-stamper@appspot.gserviceaccount.com <rubber-stamper@appspot.gserviceaccount.com>
> Owners-Override: Andrey Logvin <landrey@webrtc.org>
> Reviewed-by: Jeremy Leconte <jleconte@google.com>
> Cr-Commit-Position: refs/heads/main@{#39189}

Bug: webrtc:7135,webrtc:14795,b/266658815
Change-Id: Ia640f4342a1f42012ba5295003e17aef7613ad80
Reviewed-on: https://webrtc-review.googlesource.com/c/src/+/291440
Reviewed-by: Erik Språng <sprang@webrtc.org>
Reviewed-by: Andrey Logvin <landrey@webrtc.org>
Commit-Queue: Per Kjellander <perkj@webrtc.org>
Cr-Commit-Position: refs/heads/main@{#39199}
This commit is contained in:
Per K 2023-01-25 15:19:11 +01:00 committed by WebRTC LUCI CQ
parent 7a67dce582
commit 664cf14f9f
13 changed files with 30 additions and 220 deletions

View File

@ -110,6 +110,11 @@ class BitrateEstimatorTest : public test::CallTest {
virtual void SetUp() {
SendTask(task_queue(), [this]() {
RegisterRtpExtension(
RtpExtension(RtpExtension::kTimestampOffsetUri, kTOFExtensionId));
RegisterRtpExtension(
RtpExtension(RtpExtension::kAbsSendTimeUri, kASTExtensionId));
CreateCalls();
CreateSendTransport(BuiltInNetworkBehaviorConfig(), /*observer=*/nullptr);

View File

@ -241,11 +241,6 @@ class Call final : public webrtc::Call,
TaskQueueBase* network_thread() const override;
TaskQueueBase* worker_thread() const override;
// Implements PacketReceiver.
DeliveryStatus DeliverPacket(MediaType media_type,
rtc::CopyOnWriteBuffer packet,
int64_t packet_time_us) override;
void DeliverRtcpPacket(rtc::CopyOnWriteBuffer packet) override;
void DeliverRtpPacket(
@ -339,9 +334,6 @@ class Call final : public webrtc::Call,
void DeliverRtcp(MediaType media_type, rtc::CopyOnWriteBuffer packet)
RTC_RUN_ON(network_thread_);
DeliveryStatus DeliverRtp(MediaType media_type,
rtc::CopyOnWriteBuffer packet,
int64_t packet_time_us) RTC_RUN_ON(worker_thread_);
AudioReceiveStreamImpl* FindAudioStreamForSyncGroup(
absl::string_view sync_group) RTC_RUN_ON(worker_thread_);
@ -351,7 +343,6 @@ class Call final : public webrtc::Call,
MediaType media_type)
RTC_RUN_ON(worker_thread_);
bool IdentifyReceivedPacket(RtpPacketReceived& packet);
bool RegisterReceiveStream(uint32_t ssrc, ReceiveStreamInterface* stream);
bool UnregisterReceiveStream(uint32_t ssrc);
@ -1475,57 +1466,6 @@ void Call::DeliverRtpPacket(
}
}
PacketReceiver::DeliveryStatus Call::DeliverRtp(MediaType media_type,
rtc::CopyOnWriteBuffer packet,
int64_t packet_time_us) {
// TODO(perkj, https://bugs.webrtc.org/7135): Deprecate this method and
// direcly use DeliverRtpPacket.
TRACE_EVENT0("webrtc", "Call::DeliverRtp");
RTC_DCHECK_NE(media_type, MediaType::ANY);
RtpPacketReceived parsed_packet;
if (!parsed_packet.Parse(std::move(packet)))
return DELIVERY_PACKET_ERROR;
if (packet_time_us != -1) {
parsed_packet.set_arrival_time(Timestamp::Micros(packet_time_us));
} else {
parsed_packet.set_arrival_time(clock_->CurrentTime());
}
if (!IdentifyReceivedPacket(parsed_packet))
return DELIVERY_UNKNOWN_SSRC;
if (media_type == MediaType::VIDEO) {
parsed_packet.set_payload_type_frequency(kVideoPayloadTypeFrequency);
}
DeliverRtpPacket(media_type, std::move(parsed_packet),
[](const webrtc::RtpPacketReceived& packet) {
// If IdentifyReceivedPacket returns true, a packet is
// expected to be demuxable.
RTC_DCHECK_NOTREACHED();
return false;
});
return DELIVERY_OK;
}
PacketReceiver::DeliveryStatus Call::DeliverPacket(
MediaType media_type,
rtc::CopyOnWriteBuffer packet,
int64_t packet_time_us) {
if (IsRtcpPacket(packet)) {
RTC_DCHECK_RUN_ON(network_thread_);
worker_thread_->PostTask(SafeTask(
task_safety_.flag(), [this, packet = std::move(packet)]() mutable {
RTC_DCHECK_RUN_ON(worker_thread_);
DeliverRtcpPacket(std::move(packet));
}));
return DELIVERY_OK;
}
RTC_DCHECK_RUN_ON(worker_thread_);
return DeliverRtp(media_type, std::move(packet), packet_time_us);
}
void Call::NotifyBweOfReceivedPacket(const RtpPacketReceived& packet,
MediaType media_type) {
RTC_DCHECK_RUN_ON(worker_thread_);
@ -1549,19 +1489,6 @@ void Call::NotifyBweOfReceivedPacket(const RtpPacketReceived& packet,
}
}
bool Call::IdentifyReceivedPacket(RtpPacketReceived& packet) {
RTC_DCHECK_RUN_ON(&receive_11993_checker_);
auto it = receive_rtp_config_.find(packet.Ssrc());
if (it == receive_rtp_config_.end()) {
RTC_DLOG(LS_WARNING) << "receive_rtp_config_ lookup failed for ssrc "
<< packet.Ssrc();
return false;
}
packet.IdentifyExtensions(it->second->GetRtpExtensionMap());
return true;
}
bool Call::RegisterReceiveStream(uint32_t ssrc,
ReceiveStreamInterface* stream) {
RTC_DCHECK_RUN_ON(&receive_11993_checker_);

View File

@ -346,25 +346,6 @@ void DegradedCall::OnSentPacket(const rtc::SentPacket& sent_packet) {
call_->OnSentPacket(sent_packet);
}
PacketReceiver::DeliveryStatus DegradedCall::DeliverPacket(
MediaType media_type,
rtc::CopyOnWriteBuffer packet,
int64_t packet_time_us) {
RTC_DCHECK_RUN_ON(&received_packet_sequence_checker_);
PacketReceiver::DeliveryStatus status = receive_pipe_->DeliverPacket(
media_type, std::move(packet), packet_time_us);
// This is not optimal, but there are many places where there are thread
// checks that fail if we're not using the worker thread call into this
// method. If we want to fix this we probably need a task queue to do handover
// of all overriden methods, which feels like overkill for the current use
// case.
// By just having this thread call out via the Process() method we work around
// that, with the tradeoff that a non-zero delay may become a little larger
// than anticipated at very low packet rates.
receive_pipe_->Process();
return status;
}
void DegradedCall::DeliverRtpPacket(
MediaType media_type,
RtpPacketReceived packet,

View File

@ -113,9 +113,6 @@ class DegradedCall : public Call, private PacketReceiver {
protected:
// Implements PacketReceiver.
DeliveryStatus DeliverPacket(MediaType media_type,
rtc::CopyOnWriteBuffer packet,
int64_t packet_time_us) override;
void DeliverRtpPacket(
MediaType media_type,
RtpPacketReceived packet,

View File

@ -191,16 +191,6 @@ bool FakeNetworkPipe::SendRtcp(const uint8_t* packet,
return true;
}
PacketReceiver::DeliveryStatus FakeNetworkPipe::DeliverPacket(
MediaType media_type,
rtc::CopyOnWriteBuffer packet,
int64_t packet_time_us) {
return EnqueuePacket(std::move(packet), absl::nullopt, false, media_type,
packet_time_us)
? PacketReceiver::DELIVERY_OK
: PacketReceiver::DELIVERY_PACKET_ERROR;
}
void FakeNetworkPipe::DeliverRtpPacket(
MediaType media_type,
RtpPacketReceived packet,
@ -393,10 +383,6 @@ void FakeNetworkPipe::DeliverNetworkPacket(NetworkPacket* packet) {
<< packet.Ssrc() << " seq : " << packet.SequenceNumber();
return false;
});
} else {
receiver_->DeliverPacket(packet->media_type(),
std::move(*packet->raw_packet()),
packet_time_us);
}
}
}

View File

@ -162,12 +162,6 @@ class FakeNetworkPipe : public SimulatedPacketReceiverInterface {
OnUndemuxablePacketHandler undemuxable_packet_handler) override;
void DeliverRtcpPacket(rtc::CopyOnWriteBuffer packet) override;
// TODO(perkj, https://bugs.webrtc.org/7135): Remove once implementations
// dont use it.
PacketReceiver::DeliveryStatus DeliverPacket(MediaType media_type,
rtc::CopyOnWriteBuffer packet,
int64_t packet_time_us) override;
// Processes the network queues and trigger PacketReceiver::IncomingPacket for
// packets ready to be delivered.
void Process() override;

View File

@ -31,10 +31,6 @@ using ::testing::WithArg;
namespace webrtc {
class MockReceiver : public PacketReceiver {
public:
MOCK_METHOD(DeliveryStatus,
DeliverPacket,
(MediaType, rtc::CopyOnWriteBuffer, int64_t),
(override));
MOCK_METHOD(void,
DeliverRtcpPacket,
(rtc::CopyOnWriteBuffer packet),

View File

@ -20,26 +20,8 @@ namespace webrtc {
class PacketReceiver {
public:
enum DeliveryStatus {
DELIVERY_OK,
DELIVERY_UNKNOWN_SSRC,
DELIVERY_PACKET_ERROR,
};
// TODO(perkj, https://bugs.webrtc.org/7135): Remove this method. This method
// is no longer used by PeerConnections. Some tests still use it.
virtual DeliveryStatus DeliverPacket(MediaType media_type,
rtc::CopyOnWriteBuffer packet,
int64_t packet_time_us) {
RTC_CHECK_NOTREACHED();
}
// Demux RTCP packets. Must be called on the worker thread.
virtual void DeliverRtcpPacket(rtc::CopyOnWriteBuffer packet) {
// TODO(perkj, https://bugs.webrtc.org/7135): Implement in FakeCall and
// FakeNetworkPipe.
RTC_CHECK_NOTREACHED();
}
virtual void DeliverRtcpPacket(rtc::CopyOnWriteBuffer packet) = 0;
// Invoked once when a packet packet is received that can not be demuxed.
// If the method returns true, a new attempt is made to demux the packet.
@ -50,11 +32,7 @@ class PacketReceiver {
virtual void DeliverRtpPacket(
MediaType media_type,
RtpPacketReceived packet,
OnUndemuxablePacketHandler undemuxable_packet_handler) {
// TODO(perkj, https://bugs.webrtc.org/7135): Implement in FakeCall and
// FakeNetworkPipe.
RTC_CHECK_NOTREACHED();
}
OnUndemuxablePacketHandler undemuxable_packet_handler) = 0;
protected:
virtual ~PacketReceiver() {}

View File

@ -665,21 +665,6 @@ webrtc::PacketReceiver* FakeCall::Receiver() {
return this;
}
webrtc::PacketReceiver::DeliveryStatus FakeCall::DeliverPacket(
webrtc::MediaType media_type,
rtc::CopyOnWriteBuffer packet,
int64_t packet_time_us) {
RTC_DCHECK(webrtc::IsRtpPacket(packet));
uint32_t ssrc = ParseRtpSsrc(packet);
webrtc::Timestamp arrival_time =
packet_time_us > -1 ? webrtc::Timestamp::Micros(packet_time_us)
: webrtc::Timestamp::Zero();
if (DeliverPacketInternal(media_type, ssrc, packet, arrival_time)) {
return DELIVERY_OK;
}
return DELIVERY_UNKNOWN_SSRC;
}
void FakeCall::DeliverRtpPacket(
webrtc::MediaType media_type,
webrtc::RtpPacketReceived packet,

View File

@ -442,10 +442,6 @@ class FakeCall final : public webrtc::Call, public webrtc::PacketReceiver {
webrtc::PacketReceiver* Receiver() override;
DeliveryStatus DeliverPacket(webrtc::MediaType media_type,
rtc::CopyOnWriteBuffer packet,
int64_t packet_time_us) override;
void DeliverRtcpPacket(rtc::CopyOnWriteBuffer packet) override {}
void DeliverRtpPacket(

View File

@ -40,18 +40,6 @@ MediaType Demuxer::GetMediaType(const uint8_t* packet_data,
return MediaType::ANY;
}
DirectTransport::DirectTransport(
TaskQueueBase* task_queue,
std::unique_ptr<SimulatedPacketReceiverInterface> pipe,
Call* send_call,
const std::map<uint8_t, MediaType>& payload_type_map)
: DirectTransport(task_queue,
std::move(pipe),
send_call,
payload_type_map,
{},
{}) {}
DirectTransport::DirectTransport(
TaskQueueBase* task_queue,
std::unique_ptr<SimulatedPacketReceiverInterface> pipe,
@ -63,7 +51,6 @@ DirectTransport::DirectTransport(
task_queue_(task_queue),
demuxer_(payload_type_map),
fake_network_(std::move(pipe)),
use_legacy_send_(audio_extensions.empty() && video_extensions.empty()),
audio_extensions_(audio_extensions),
video_extensions_(video_extensions) {
Start();
@ -89,30 +76,27 @@ bool DirectTransport::SendRtp(const uint8_t* data,
send_call_->OnSentPacket(sent_packet);
}
if (use_legacy_send_) {
LegacySendPacket(data, length);
} else {
const RtpHeaderExtensionMap* extensions = nullptr;
MediaType media_type = demuxer_.GetMediaType(data, length);
switch (demuxer_.GetMediaType(data, length)) {
case webrtc::MediaType::AUDIO:
extensions = &audio_extensions_;
break;
case webrtc::MediaType::VIDEO:
extensions = &video_extensions_;
break;
default:
RTC_CHECK_NOTREACHED();
}
RtpPacketReceived packet(extensions, Timestamp::Micros(rtc::TimeMicros()));
if (media_type == MediaType::VIDEO) {
packet.set_payload_type_frequency(kVideoPayloadTypeFrequency);
}
RTC_CHECK(packet.Parse(rtc::CopyOnWriteBuffer(data, length)));
fake_network_->DeliverRtpPacket(
media_type, std::move(packet),
[](const RtpPacketReceived& packet) { return false; });
const RtpHeaderExtensionMap* extensions = nullptr;
MediaType media_type = demuxer_.GetMediaType(data, length);
switch (demuxer_.GetMediaType(data, length)) {
case webrtc::MediaType::AUDIO:
extensions = &audio_extensions_;
break;
case webrtc::MediaType::VIDEO:
extensions = &video_extensions_;
break;
default:
RTC_CHECK_NOTREACHED();
}
RtpPacketReceived packet(extensions, Timestamp::Micros(rtc::TimeMicros()));
if (media_type == MediaType::VIDEO) {
packet.set_payload_type_frequency(kVideoPayloadTypeFrequency);
}
RTC_CHECK(packet.Parse(rtc::CopyOnWriteBuffer(data, length)));
fake_network_->DeliverRtpPacket(
media_type, std::move(packet),
[](const RtpPacketReceived& packet) { return false; });
MutexLock lock(&process_lock_);
if (!next_process_task_.Running())
ProcessPackets();
@ -120,24 +104,13 @@ bool DirectTransport::SendRtp(const uint8_t* data,
}
bool DirectTransport::SendRtcp(const uint8_t* data, size_t length) {
if (use_legacy_send_) {
LegacySendPacket(data, length);
} else {
fake_network_->DeliverRtcpPacket(rtc::CopyOnWriteBuffer(data, length));
}
fake_network_->DeliverRtcpPacket(rtc::CopyOnWriteBuffer(data, length));
MutexLock lock(&process_lock_);
if (!next_process_task_.Running())
ProcessPackets();
return true;
}
void DirectTransport::LegacySendPacket(const uint8_t* data, size_t length) {
MediaType media_type = demuxer_.GetMediaType(data, length);
int64_t send_time_us = rtc::TimeMicros();
fake_network_->DeliverPacket(media_type, rtc::CopyOnWriteBuffer(data, length),
send_time_us);
}
int DirectTransport::GetAverageDelayMs() {
return fake_network_->AverageDelay();
}

View File

@ -44,14 +44,6 @@ class Demuxer {
// same task-queue - the one that's passed in via the constructor.
class DirectTransport : public Transport {
public:
// TODO(perkj, https://bugs.webrtc.org/7135): Remove header once downstream
// projects have been updated.
[[deprecated("Use ctor that provide header extensions.")]] DirectTransport(
TaskQueueBase* task_queue,
std::unique_ptr<SimulatedPacketReceiverInterface> pipe,
Call* send_call,
const std::map<uint8_t, MediaType>& payload_type_map);
DirectTransport(TaskQueueBase* task_queue,
std::unique_ptr<SimulatedPacketReceiverInterface> pipe,
Call* send_call,
@ -85,7 +77,6 @@ class DirectTransport : public Transport {
const Demuxer demuxer_;
const std::unique_ptr<SimulatedPacketReceiverInterface> fake_network_;
const bool use_legacy_send_;
const RtpHeaderExtensionMap audio_extensions_;
const RtpHeaderExtensionMap video_extensions_;
};

View File

@ -108,7 +108,8 @@ TEST_F(SsrcEndToEndTest, UnknownRtpPacketTriggersUndemuxablePacketHandler) {
std::make_unique<FakeNetworkPipe>(
Clock::GetRealTimeClock(), std::make_unique<SimulatedNetwork>(
BuiltInNetworkBehaviorConfig())),
receiver_call_.get(), payload_type_map_);
receiver_call_.get(), payload_type_map_, GetRegisteredExtensions(),
GetRegisteredExtensions());
input_observer =
std::make_unique<PacketInputObserver>(receiver_call_->Receiver());
send_transport->SetReceiver(input_observer.get());