admin/webrtc_m130
1
0
Fork 0
Code Issues Pull Requests Actions Packages Projects Releases Wiki Activity

Cleanup RtcpReceiver from passing TransportFeedback via older interface

Browse Source 
Bug: webrtc:8239
Change-Id: Ibc289627cc89bda86f3e2c7c0c11d0ec2ae95087
Reviewed-on: https://webrtc-review.googlesource.com/c/src/+/305783
Reviewed-by: Åsa Persson <asapersson@webrtc.org>
Commit-Queue: Danil Chapovalov <danilchap@webrtc.org>
Cr-Commit-Position: refs/heads/main@{#40102}
This commit is contained in:
Danil Chapovalov 2023-05-19 15:10:55 +02:00 committed by WebRTC LUCI CQ
parent 15feded162
commit 718601a1f8
6 changed files with 10 additions and 63 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

1
call/rtp_transport_controller_send.h
View File

@ -119,7 +119,6 @@ class RtpTransportControllerSend final
// Implements TransportFeedbackObserver interface
void OnAddPacket(const RtpPacketSendInfo& packet_info) override;
void OnTransportFeedback(const rtcp::TransportFeedback& feedback) override {}
// Implements NetworkStateEstimateObserver interface
void OnRemoteNetworkEstimate(NetworkStateEstimate estimate) override;

7
modules/rtp_rtcp/include/rtp_rtcp_defines.h
View File

@ -232,14 +232,9 @@ class NetworkStateEstimateObserver {
class TransportFeedbackObserver {
public:
TransportFeedbackObserver() {}
virtual ~TransportFeedbackObserver() {}
virtual ~TransportFeedbackObserver() = default;
virtual void OnAddPacket(const RtpPacketSendInfo& packet_info) = 0;
// TODO(bugs.webrtc.org/8239): Remove this function in favor of receiving
// feedback message via `NetworkLinkRtcpObserver` interface.
virtual void OnTransportFeedback(const rtcp::TransportFeedback& feedback) {}
};
// Interface for PacketRouter to send rtcp feedback on behalf of

10
modules/rtp_rtcp/source/rtcp_receiver.cc
View File

@ -149,8 +149,6 @@ RTCPReceiver::RTCPReceiver(const RtpRtcpInterface::Configuration& config,
rtcp_intra_frame_observer_(config.intra_frame_callback),
rtcp_loss_notification_observer_(config.rtcp_loss_notification_observer),
network_state_estimate_observer_(config.network_state_estimate_observer),
deprecated_transport_feedback_observer_(
config.transport_feedback_callback),
bitrate_allocation_observer_(config.bitrate_allocation_observer),
report_interval_(config.rtcp_report_interval_ms > 0
? TimeDelta::Millis(config.rtcp_report_interval_ms)
@ -180,8 +178,6 @@ RTCPReceiver::RTCPReceiver(const RtpRtcpInterface::Configuration& config,
rtcp_intra_frame_observer_(config.intra_frame_callback),
rtcp_loss_notification_observer_(config.rtcp_loss_notification_observer),
network_state_estimate_observer_(config.network_state_estimate_observer),
deprecated_transport_feedback_observer_(
config.transport_feedback_callback),
bitrate_allocation_observer_(config.bitrate_allocation_observer),
report_interval_(config.rtcp_report_interval_ms > 0
? TimeDelta::Millis(config.rtcp_report_interval_ms)
@ -1194,12 +1190,6 @@ void RTCPReceiver::TriggerCallbacksFromRtcpPacket(
rtp_rtcp_->OnReceivedRtcpReportBlocks(packet_information.report_blocks);
}
if (deprecated_transport_feedback_observer_ &&
(packet_information.packet_type_flags & kRtcpTransportFeedback)) {
deprecated_transport_feedback_observer_->OnTransportFeedback(
*packet_information.transport_feedback);
}
if (network_state_estimate_observer_ &&
packet_information.network_state_estimate) {
network_state_estimate_observer_->OnRemoteNetworkEstimate(

1
modules/rtp_rtcp/source/rtcp_receiver.h
View File

@ -360,7 +360,6 @@ class RTCPReceiver final {
RtcpIntraFrameObserver* const rtcp_intra_frame_observer_;
RtcpLossNotificationObserver* const rtcp_loss_notification_observer_;
NetworkStateEstimateObserver* const network_state_estimate_observer_;
TransportFeedbackObserver* const deprecated_transport_feedback_observer_;
VideoBitrateAllocationObserver* const bitrate_allocation_observer_;
const TimeDelta report_interval_;

50
modules/rtp_rtcp/source/rtcp_receiver_unittest.cc
View File

@ -100,15 +100,6 @@ class MockReportBlockDataObserverImpl : public ReportBlockDataObserver {
MOCK_METHOD(void, OnReportBlockDataUpdated, (ReportBlockData), (override));
};
class MockTransportFeedbackObserver : public TransportFeedbackObserver {
public:
MOCK_METHOD(void, OnAddPacket, (const RtpPacketSendInfo&), (override));
MOCK_METHOD(void,
OnTransportFeedback,
(const rtcp::TransportFeedback&),
(override));
};
class MockModuleRtpRtcp : public RTCPReceiver::ModuleRtpRtcp {
public:
MOCK_METHOD(void, SetTmmbn, (std::vector<rtcp::TmmbItem>), (override));
@ -157,7 +148,6 @@ struct ReceiverMocks {
StrictMock<MockRtcpBandwidthObserver> bandwidth_observer;
StrictMock<MockRtcpIntraFrameObserver> intra_frame_observer;
StrictMock<MockRtcpLossNotificationObserver> rtcp_loss_notification_observer;
StrictMock<MockTransportFeedbackObserver> transport_feedback_observer;
StrictMock<MockVideoBitrateAllocationObserver> bitrate_allocation_observer;
StrictMock<MockModuleRtpRtcp> rtp_rtcp_impl;
NiceMock<MockNetworkLinkRtcpObserver> network_link_rtcp_observer;
@ -173,7 +163,6 @@ RtpRtcpInterface::Configuration DefaultConfiguration(ReceiverMocks* mocks) {
config.intra_frame_callback = &mocks->intra_frame_observer;
config.rtcp_loss_notification_observer =
&mocks->rtcp_loss_notification_observer;
config.transport_feedback_callback = &mocks->transport_feedback_observer;
config.bitrate_allocation_observer = &mocks->bitrate_allocation_observer;
config.rtcp_report_interval_ms = kRtcpIntervalMs;
config.local_media_ssrc = kReceiverMainSsrc;
@ -556,7 +545,6 @@ TEST(RtcpReceiverTest, NotifiesNetworkLinkObserverOnReportBlocks) {
ReceiverMocks mocks;
RtpRtcpInterface::Configuration config = DefaultConfiguration(&mocks);
config.bandwidth_callback = nullptr;
config.transport_feedback_callback = nullptr;
config.network_link_rtcp_observer = &mocks.network_link_rtcp_observer;
RTCPReceiver receiver(config, &mocks.rtp_rtcp_impl);
receiver.SetRemoteSSRC(kSenderSsrc);
@ -1781,30 +1769,10 @@ TEST(RtcpReceiverTest, GetReportBlockDataAfterTwoReportBlocksOfDifferentSsrcs) {
report_block_datas[1].extended_highest_sequence_number());
}
TEST(RtcpReceiverTest, ReceivesTransportFeedback) {
ReceiverMocks mocks;
RTCPReceiver receiver(DefaultConfiguration(&mocks), &mocks.rtp_rtcp_impl);
receiver.SetRemoteSSRC(kSenderSsrc);
rtcp::TransportFeedback packet;
packet.SetMediaSsrc(kReceiverMainSsrc);
packet.SetSenderSsrc(kSenderSsrc);
packet.SetBase(1, Timestamp::Millis(1));
packet.AddReceivedPacket(1, Timestamp::Millis(1));
EXPECT_CALL(
mocks.transport_feedback_observer,
OnTransportFeedback(AllOf(
Property(&rtcp::TransportFeedback::media_ssrc, kReceiverMainSsrc),
Property(&rtcp::TransportFeedback::sender_ssrc, kSenderSsrc))));
receiver.IncomingPacket(packet.Build());
}
TEST(RtcpReceiverTest, NotifiesNetworkLinkObserverOnTransportFeedback) {
ReceiverMocks mocks;
RtpRtcpInterface::Configuration config = DefaultConfiguration(&mocks);
config.bandwidth_callback = nullptr;
config.transport_feedback_callback = nullptr;
config.network_link_rtcp_observer = &mocks.network_link_rtcp_observer;
RTCPReceiver receiver(config, &mocks.rtp_rtcp_impl);
receiver.SetRemoteSSRC(kSenderSsrc);
@ -1831,7 +1799,6 @@ TEST(RtcpReceiverTest,
ReceiverMocks mocks;
RtpRtcpInterface::Configuration config = DefaultConfiguration(&mocks);
config.bandwidth_callback = nullptr;
config.transport_feedback_callback = nullptr;
config.network_link_rtcp_observer = &mocks.network_link_rtcp_observer;
RTCPReceiver receiver(config, &mocks.rtp_rtcp_impl);
receiver.SetRemoteSSRC(kSenderSsrc);
@ -1851,7 +1818,6 @@ TEST(RtcpReceiverTest,
ReceiverMocks mocks;
RtpRtcpInterface::Configuration config = DefaultConfiguration(&mocks);
config.bandwidth_callback = nullptr;
config.transport_feedback_callback = nullptr;
config.network_link_rtcp_observer = &mocks.network_link_rtcp_observer;
RTCPReceiver receiver(config, &mocks.rtp_rtcp_impl);
receiver.SetRemoteSSRC(kSenderSsrc);
@ -1885,7 +1851,6 @@ TEST(RtcpReceiverTest, NotifiesNetworkLinkObserverOnRemb) {
ReceiverMocks mocks;
RtpRtcpInterface::Configuration config = DefaultConfiguration(&mocks);
config.bandwidth_callback = nullptr;
config.transport_feedback_callback = nullptr;
config.network_link_rtcp_observer = &mocks.network_link_rtcp_observer;
RTCPReceiver receiver(config, &mocks.rtp_rtcp_impl);
receiver.SetRemoteSSRC(kSenderSsrc);
@ -1902,7 +1867,10 @@ TEST(RtcpReceiverTest, NotifiesNetworkLinkObserverOnRemb) {
TEST(RtcpReceiverTest, HandlesInvalidTransportFeedback) {
ReceiverMocks mocks;
RTCPReceiver receiver(DefaultConfiguration(&mocks), &mocks.rtp_rtcp_impl);
RtpRtcpInterface::Configuration config = DefaultConfiguration(&mocks);
config.bandwidth_callback = nullptr;
config.network_link_rtcp_observer = &mocks.network_link_rtcp_observer;
RTCPReceiver receiver(config, &mocks.rtp_rtcp_impl);
receiver.SetRemoteSSRC(kSenderSsrc);
// Send a compound packet with a TransportFeedback followed by something else.
@ -1912,10 +1880,10 @@ TEST(RtcpReceiverTest, HandlesInvalidTransportFeedback) {
packet->SetBase(1, Timestamp::Millis(1));
packet->AddReceivedPacket(1, Timestamp::Millis(1));
static uint32_t kBitrateBps = 50000;
static constexpr DataRate kBitrate = DataRate::BitsPerSec(50'000);
auto remb = std::make_unique<rtcp::Remb>();
remb->SetSenderSsrc(kSenderSsrc);
remb->SetBitrateBps(kBitrateBps);
remb->SetBitrateBps(kBitrate.bps());
rtcp::CompoundPacket compound;
compound.Append(std::move(packet));
compound.Append(std::move(remb));
@ -1927,10 +1895,10 @@ TEST(RtcpReceiverTest, HandlesInvalidTransportFeedback) {
42);
// Stress no transport feedback is expected.
EXPECT_CALL(mocks.transport_feedback_observer, OnTransportFeedback).Times(0);
EXPECT_CALL(mocks.network_link_rtcp_observer, OnTransportFeedback).Times(0);
// But remb should be processed and cause a callback
EXPECT_CALL(mocks.bandwidth_observer,
OnReceivedEstimatedBitrate(kBitrateBps));
EXPECT_CALL(mocks.network_link_rtcp_observer,
OnReceiverEstimatedMaxBitrate(_, kBitrate));
receiver.IncomingPacket(built_packet);
}

4
modules/rtp_rtcp/source/rtp_sender_egress_unittest.cc
View File

@ -63,10 +63,6 @@ class MockSendPacketObserver : public SendPacketObserver {
class MockTransportFeedbackObserver : public TransportFeedbackObserver {
public:
MOCK_METHOD(void, OnAddPacket, (const RtpPacketSendInfo&), (override));
MOCK_METHOD(void,
OnTransportFeedback,
(const rtcp::TransportFeedback&),
(override));
};
class MockStreamDataCountersCallback : public StreamDataCountersCallback {