zqz/platform-external-webrtc
1
0
Fork 0
Code Issues Pull Requests Actions Packages Projects Releases Wiki Activity

In RtcpTransceiver functions with callback avoid relying on PostTaskAndReply

Browse Source 
deprecated version guarantees (using PostTaskAndReply) callback task will run on the task queue,
and thus doesn't guarantee to run it if task queue is destroyed,

new callback versions instead guarantee callback will always run,
but may run off the task queue if task queue is destroyed.

Both keep guarantee observer callbacks will not run after on_destroyed/on_removed is called.

Bug: None
Change-Id: I61bf52127f3084c0186aa8bc89037bf9296801d8
Reviewed-on: https://webrtc-review.googlesource.com/c/107305
Commit-Queue: Danil Chapovalov <danilchap@webrtc.org>
Reviewed-by: Niels Moller <nisse@webrtc.org>
Cr-Commit-Position: refs/heads/master@{#25301}
This commit is contained in:
Danil Chapovalov
2018-10-22 14:22:31 +02:00
committed by Commit Bot
parent b0ab2ce256
commit 98f5f6cdea
4 changed files with 42 additions and 15 deletions
Download Patch File Download Diff File Expand all files Collapse all files

1
modules/rtp_rtcp/BUILD.gn
View File

@ -243,6 +243,7 @@ rtc_source_set("rtcp_transceiver") {
"../../api:transport_api",
"../../api/video:video_bitrate_allocation",
"../../rtc_base:checks",
"../../rtc_base:deprecation",
"../../rtc_base:rtc_base_approved",
"../../rtc_base:rtc_cancelable_task",
"../../rtc_base:rtc_task_queue",

20
modules/rtp_rtcp/source/rtcp_transceiver.cc
View File

@ -46,6 +46,13 @@ void RtcpTransceiver::Stop(std::unique_ptr<rtc::QueuedTask> on_destroyed) {
RTC_DCHECK(!rtcp_transceiver_);
}
void RtcpTransceiver::Stop(std::function<void()> on_destroyed) {
RTC_DCHECK(rtcp_transceiver_);
task_queue_->PostTask(rtc::NewClosure(
Destructor{std::move(rtcp_transceiver_)}, std::move(on_destroyed)));
RTC_DCHECK(!rtcp_transceiver_);
}
void RtcpTransceiver::AddMediaReceiverRtcpObserver(
uint32_t remote_ssrc,
MediaReceiverRtcpObserver* observer) {
@ -68,6 +75,19 @@ void RtcpTransceiver::RemoveMediaReceiverRtcpObserver(
task_queue_->PostTaskAndReply(std::move(remove), std::move(on_removed));
}
void RtcpTransceiver::RemoveMediaReceiverRtcpObserver(
uint32_t remote_ssrc,
MediaReceiverRtcpObserver* observer,
std::function<void()> on_removed) {
RTC_CHECK(rtcp_transceiver_);
RtcpTransceiverImpl* ptr = rtcp_transceiver_.get();
auto remove = [ptr, remote_ssrc, observer] {
ptr->RemoveMediaReceiverRtcpObserver(remote_ssrc, observer);
};
task_queue_->PostTask(
rtc::NewClosure(std::move(remove), std::move(on_removed)));
}
void RtcpTransceiver::SetReadyToSend(bool ready) {
RTC_CHECK(rtcp_transceiver_);
RtcpTransceiverImpl* ptr = rtcp_transceiver_.get();

10
modules/rtp_rtcp/source/rtcp_transceiver.h
View File

@ -11,6 +11,7 @@
#ifndef MODULES_RTP_RTCP_SOURCE_RTCP_TRANSCEIVER_H_
#define MODULES_RTP_RTCP_SOURCE_RTCP_TRANSCEIVER_H_
#include <functional>
#include <memory>
#include <string>
#include <vector>
@ -18,6 +19,7 @@
#include "modules/rtp_rtcp/source/rtcp_transceiver_config.h"
#include "modules/rtp_rtcp/source/rtcp_transceiver_impl.h"
#include "rtc_base/copyonwritebuffer.h"
#include "rtc_base/deprecation.h"
#include "rtc_base/task_queue.h"
namespace webrtc {
@ -42,18 +44,24 @@ class RtcpTransceiver : public RtcpFeedbackSenderInterface {
// Note that interfaces provided in constructor or registered with AddObserver
// still might be used by the transceiver on the task queue
// until |on_destroyed| runs.
RTC_DEPRECATED
void Stop(std::unique_ptr<rtc::QueuedTask> on_destroyed);
void Stop(std::function<void()> on_destroyed);
// Registers observer to be notified about incoming rtcp packets.
// Calls to observer will be done on the |config.task_queue|.
void AddMediaReceiverRtcpObserver(uint32_t remote_ssrc,
MediaReceiverRtcpObserver* observer);
// Deregisters the observer. Might return before observer is deregistered.
// Posts |on_removed| task when observer is deregistered.
// Runs |on_removed| when observer is deregistered.
RTC_DEPRECATED
void RemoveMediaReceiverRtcpObserver(
uint32_t remote_ssrc,
MediaReceiverRtcpObserver* observer,
std::unique_ptr<rtc::QueuedTask> on_removed);
void RemoveMediaReceiverRtcpObserver(uint32_t remote_ssrc,
MediaReceiverRtcpObserver* observer,
std::function<void()> on_removed);
// Enables/disables sending rtcp packets eventually.
// Packets may be sent after the SetReadyToSend(false) returns, but no new

26
modules/rtp_rtcp/source/rtcp_transceiver_unittest.cc
View File

@ -166,12 +166,11 @@ TEST(RtcpTransceiverTest, DoesntPostToRtcpObserverAfterCallToRemove) {
rtcp_transceiver.AddMediaReceiverRtcpObserver(kRemoteSsrc, observer.get());
rtcp_transceiver.ReceivePacket(CreateSenderReport(kRemoteSsrc, 1));
rtcp_transceiver.RemoveMediaReceiverRtcpObserver(
kRemoteSsrc, observer.get(),
/*on_removed=*/rtc::NewClosure([&] {
observer.reset();
observer_deleted.Set();
}));
rtcp_transceiver.RemoveMediaReceiverRtcpObserver(kRemoteSsrc, observer.get(),
/*on_removed=*/[&] {
observer.reset();
observer_deleted.Set();
});
rtcp_transceiver.ReceivePacket(CreateSenderReport(kRemoteSsrc, 2));
EXPECT_TRUE(observer_deleted.Wait(kTimeoutMs));
@ -192,12 +191,11 @@ TEST(RtcpTransceiverTest, RemoveMediaReceiverRtcpObserverIsNonBlocking) {
rtc::Event queue_blocker(false, false);
rtc::Event observer_deleted(false, false);
queue.PostTask([&] { EXPECT_TRUE(queue_blocker.Wait(kTimeoutMs)); });
rtcp_transceiver.RemoveMediaReceiverRtcpObserver(
kRemoteSsrc, observer.get(),
/*on_removed=*/rtc::NewClosure([&] {
observer.reset();
observer_deleted.Set();
}));
rtcp_transceiver.RemoveMediaReceiverRtcpObserver(kRemoteSsrc, observer.get(),
/*on_removed=*/[&] {
observer.reset();
observer_deleted.Set();
});
EXPECT_THAT(observer, Not(IsNull()));
queue_blocker.Set();
@ -244,10 +242,10 @@ TEST(RtcpTransceiverTest, DoesntSendPacketsAfterStopCallback) {
auto rtcp_transceiver = absl::make_unique<RtcpTransceiver>(config);
rtc::Event done(false, false);
rtcp_transceiver->SendCompoundPacket();
rtcp_transceiver->Stop(rtc::NewClosure([&] {
rtcp_transceiver->Stop([&] {
EXPECT_CALL(outgoing_transport, SendRtcp).Times(0);
done.Set();
}));
});
rtcp_transceiver = nullptr;
EXPECT_TRUE(done.Wait(kTimeoutMs));
}