Fix AudioSendStream reconfigure - stop processing during unconfigured state
When Reconfiguring there's a call to ResetSenderCongestionControlObjects followed by a later call to RegisterSenderCongestionControlObjects which happen on the worker thread, while enqueuing packets is happening on a different thread. If packets are enqueued in between these calls we get a null dereference of the `rtp_packet_pacer_` This change fixes it by pausing processing of incoming audio in the interim state Bug: webrtc:358290775 Change-Id: I77cebfb131545ce2a6fdb26105dd999da3f7c443 Reviewed-on: https://webrtc-review.googlesource.com/c/src/+/359080 Reviewed-by: Danil Chapovalov <danilchap@webrtc.org> Reviewed-by: Jakob Ivarsson <jakobi@webrtc.org> Commit-Queue: Danil Chapovalov <danilchap@webrtc.org> Cr-Commit-Position: refs/heads/main@{#42815}
This commit is contained in:
parent
f2d31361d9
commit
f009e38fe0
@ -313,11 +313,27 @@ class RtpPacketSenderProxy : public RtpPacketSender {
|
|||||||
void EnqueuePackets(
|
void EnqueuePackets(
|
||||||
std::vector<std::unique_ptr<RtpPacketToSend>> packets) override {
|
std::vector<std::unique_ptr<RtpPacketToSend>> packets) override {
|
||||||
MutexLock lock(&mutex_);
|
MutexLock lock(&mutex_);
|
||||||
|
|
||||||
|
// Since we allow having an instance with no rtp_packet_pacer_ set we
|
||||||
|
// should handle calls to member functions in this state gracefully rather
|
||||||
|
// than null dereferencing.
|
||||||
|
if (!rtp_packet_pacer_) {
|
||||||
|
RTC_DLOG(LS_WARNING)
|
||||||
|
<< "Dropping packets queued while rtp_packet_pacer_ is null.";
|
||||||
|
return;
|
||||||
|
}
|
||||||
rtp_packet_pacer_->EnqueuePackets(std::move(packets));
|
rtp_packet_pacer_->EnqueuePackets(std::move(packets));
|
||||||
}
|
}
|
||||||
|
|
||||||
void RemovePacketsForSsrc(uint32_t ssrc) override {
|
void RemovePacketsForSsrc(uint32_t ssrc) override {
|
||||||
MutexLock lock(&mutex_);
|
MutexLock lock(&mutex_);
|
||||||
|
|
||||||
|
// Since we allow having an instance with no rtp_packet_pacer_ set we
|
||||||
|
// should handle calls to member functions in this state gracefully rather
|
||||||
|
// than null dereferencing.
|
||||||
|
if (!rtp_packet_pacer_) {
|
||||||
|
return;
|
||||||
|
}
|
||||||
rtp_packet_pacer_->RemovePacketsForSsrc(ssrc);
|
rtp_packet_pacer_->RemovePacketsForSsrc(ssrc);
|
||||||
}
|
}
|
||||||
|
|
||||||
|
|||||||
@ -362,6 +362,25 @@ TEST_F(ChannelSendTest, UsedRateIsLargerofLastTwoFrames) {
|
|||||||
EXPECT_EQ(used_rate_3, used_rate_2);
|
EXPECT_EQ(used_rate_3, used_rate_2);
|
||||||
}
|
}
|
||||||
|
|
||||||
|
// Test that we gracefully handle packets while the congestion control objects
|
||||||
|
// are not configured. This can happen during calls
|
||||||
|
// AudioSendStream::ConfigureStream
|
||||||
|
TEST_F(ChannelSendTest, EnqueuePacketsGracefullyHandlesNonInitializedPacer) {
|
||||||
|
EXPECT_CALL(transport_, SendRtp).Times(1);
|
||||||
|
channel_->StartSend();
|
||||||
|
channel_->ResetSenderCongestionControlObjects();
|
||||||
|
// This should trigger a packet, but congestion control is not configured
|
||||||
|
// so it should be dropped
|
||||||
|
ProcessNextFrame();
|
||||||
|
ProcessNextFrame();
|
||||||
|
|
||||||
|
channel_->RegisterSenderCongestionControlObjects(&transport_controller_);
|
||||||
|
// Now that we reconfigured the congestion control objects the new frame
|
||||||
|
// should be processed
|
||||||
|
ProcessNextFrame();
|
||||||
|
ProcessNextFrame();
|
||||||
|
}
|
||||||
|
|
||||||
} // namespace
|
} // namespace
|
||||||
} // namespace voe
|
} // namespace voe
|
||||||
} // namespace webrtc
|
} // namespace webrtc
|
||||||
|
|||||||
Loading…
x
Reference in New Issue
Block a user