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

Remove playout delay lock.

Browse Source 
Now update the playout delay and related stats on the worker thread.

This was previously reviewed here:
https://webrtc-review.googlesource.com/c/src/+/172929/

With the exception of reducing unnecessarily broad
lock scope in one function in rtp_rtcp_impl.cc
and added comments in rtp_streams_synchronizer.h

Bug: webrtc:11489
Change-Id: I77807b5da2accfe774255d9409542d358f288993
Reviewed-on: https://webrtc-review.googlesource.com/c/src/+/174200
Commit-Queue: Tommi <tommi@webrtc.org>
Reviewed-by: Erik Språng <sprang@webrtc.org>
Cr-Commit-Position: refs/heads/master@{#31193}
This commit is contained in:
Tommi
2020-05-08 20:59:05 +02:00
committed by Commit Bot
parent 33d81a05eb
commit c623495fd1
4 changed files with 52 additions and 33 deletions
Download Patch File Download Diff File Expand all files Collapse all files

6
modules/rtp_rtcp/source/rtp_rtcp_impl.cc
View File

@ -774,8 +774,10 @@ std::vector<rtcp::TmmbItem> ModuleRtpRtcpImpl::BoundingSet(bool* tmmbr_owner) {
}
void ModuleRtpRtcpImpl::set_rtt_ms(int64_t rtt_ms) {
rtc::CritScope cs(&critical_section_rtt_);
rtt_ms_ = rtt_ms;
{
rtc::CritScope cs(&critical_section_rtt_);
rtt_ms_ = rtt_ms;
}
if (rtp_sender_) {
rtp_sender_->packet_history.SetRtt(rtt_ms);
}

5
video/rtp_streams_synchronizer.h
View File

@ -25,6 +25,11 @@ namespace webrtc {
class Syncable;
// TODO(bugs.webrtc.org/11489): Remove dependency on ProcessThread/Module.
// Instead make this a single threaded class, constructed on a TQ and
// post a 1 sec timer there. There shouldn't be a need for locking internally
// and the callback from this class, should occur on the construction TQ
// which in turn means that the callback doesn't need locking either.
class RtpStreamsSynchronizer : public Module {
public:
explicit RtpStreamsSynchronizer(Syncable* syncable_video);

49
video/video_receive_stream2.cc
View File

@ -274,6 +274,7 @@ VideoReceiveStream2::~VideoReceiveStream2() {
RTC_LOG(LS_INFO) << "~VideoReceiveStream2: " << config_.ToString();
Stop();
process_thread_->DeRegisterModule(&rtp_stream_sync_);
task_safety_flag_->SetNotAlive();
}
void VideoReceiveStream2::SignalNetworkState(NetworkState state) {
@ -477,8 +478,6 @@ bool VideoReceiveStream2::SetBaseMinimumPlayoutDelayMs(int delay_ms) {
return false;
}
// TODO(bugs.webrtc.org/11489): Consider posting to worker.
rtc::CritScope cs(&playout_delay_lock_);
base_minimum_playout_delay_ms_ = delay_ms;
UpdatePlayoutDelays();
return true;
@ -486,8 +485,6 @@ bool VideoReceiveStream2::SetBaseMinimumPlayoutDelayMs(int delay_ms) {
int VideoReceiveStream2::GetBaseMinimumPlayoutDelayMs() const {
RTC_DCHECK_RUN_ON(&worker_sequence_checker_);
rtc::CritScope cs(&playout_delay_lock_);
return base_minimum_playout_delay_ms_;
}
@ -549,21 +546,18 @@ void VideoReceiveStream2::OnCompleteFrame(
}
last_complete_frame_time_ms_ = time_now_ms;
// TODO(bugs.webrtc.org/11489): We grab the playout_delay_lock_ lock
// potentially twice. Consider checking both min/max and posting to worker if
// there's a change. If we always update playout delays on the worker, we
// don't need a lock.
const PlayoutDelay& playout_delay = frame->EncodedImage().playout_delay_;
if (playout_delay.min_ms >= 0) {
rtc::CritScope cs(&playout_delay_lock_);
frame_minimum_playout_delay_ms_ = playout_delay.min_ms;
UpdatePlayoutDelays();
}
if (playout_delay.max_ms >= 0) {
rtc::CritScope cs(&playout_delay_lock_);
frame_maximum_playout_delay_ms_ = playout_delay.max_ms;
UpdatePlayoutDelays();
if (playout_delay.min_ms >= 0 || playout_delay.max_ms >= 0) {
worker_thread_->PostTask(ToQueuedTask(
task_safety_flag_,
[min_ms = playout_delay.min_ms, max_ms = playout_delay.max_ms, this]() {
RTC_DCHECK_RUN_ON(&worker_sequence_checker_);
if (min_ms >= 0)
frame_minimum_playout_delay_ms_ = min_ms;
if (max_ms >= 0)
frame_maximum_playout_delay_ms_ = max_ms;
UpdatePlayoutDelays();
}));
}
int64_t last_continuous_pid = frame_buffer_->InsertFrame(std::move(frame));
@ -607,9 +601,21 @@ void VideoReceiveStream2::SetEstimatedPlayoutNtpTimestampMs(
}
void VideoReceiveStream2::SetMinimumPlayoutDelay(int delay_ms) {
RTC_DCHECK_RUN_ON(&module_process_sequence_checker_);
// TODO(bugs.webrtc.org/11489): Consider posting to worker.
rtc::CritScope cs(&playout_delay_lock_);
// TODO(bugs.webrtc.org/11489): Currently called back on the module process
// thread because of RtpStreamsSynchronizer or |rtp_stream_sync_|. Once we
// change RtpStreamsSynchronizer to be single threaded, this call should
// always occur on the worker thread. Use of |rtp_stream_sync_| should all
// move to the worker thread, which will remove a lot of locks and take
// blocking work off of the decoder thread.
if (!worker_thread_->IsCurrent()) {
RTC_DCHECK_RUN_ON(&module_process_sequence_checker_);
worker_thread_->PostTask(
ToQueuedTask(task_safety_flag_,
[delay_ms, this]() { SetMinimumPlayoutDelay(delay_ms); }));
return;
}
RTC_DCHECK_RUN_ON(&worker_sequence_checker_);
syncable_minimum_playout_delay_ms_ = delay_ms;
UpdatePlayoutDelays();
}
@ -731,6 +737,7 @@ bool VideoReceiveStream2::IsReceivingKeyFrame(int64_t timestamp_ms) const {
}
void VideoReceiveStream2::UpdatePlayoutDelays() const {
RTC_DCHECK_RUN_ON(&worker_sequence_checker_);
const int minimum_delay_ms =
std::max({frame_minimum_playout_delay_ms_, base_minimum_playout_delay_ms_,
syncable_minimum_playout_delay_ms_});

25
video/video_receive_stream2.h
View File

@ -26,6 +26,7 @@
#include "modules/video_coding/video_receiver2.h"
#include "rtc_base/synchronization/sequence_checker.h"
#include "rtc_base/task_queue.h"
#include "rtc_base/task_utils/pending_task_safety_flag.h"
#include "system_wrappers/include/clock.h"
#include "video/receive_statistics_proxy2.h"
#include "video/rtp_streams_synchronizer.h"
@ -134,8 +135,7 @@ class VideoReceiveStream2 : public webrtc::VideoReceiveStream,
void HandleEncodedFrame(std::unique_ptr<video_coding::EncodedFrame> frame)
RTC_RUN_ON(decode_queue_);
void HandleFrameBufferTimeout() RTC_RUN_ON(decode_queue_);
void UpdatePlayoutDelays() const
RTC_EXCLUSIVE_LOCKS_REQUIRED(playout_delay_lock_);
void UpdatePlayoutDelays() const;
void RequestKeyFrame(int64_t timestamp_ms) RTC_RUN_ON(decode_queue_);
void HandleKeyFrameGeneration(bool received_frame_is_keyframe, int64_t now_ms)
RTC_RUN_ON(decode_queue_);
@ -200,22 +200,23 @@ class VideoReceiveStream2 : public webrtc::VideoReceiveStream,
const int max_wait_for_keyframe_ms_;
const int max_wait_for_frame_ms_;
rtc::CriticalSection playout_delay_lock_;
// All of them tries to change current min_playout_delay on |timing_| but
// source of the change request is different in each case. Among them the
// biggest delay is used. -1 means use default value from the |timing_|.
//
// Minimum delay as decided by the RTP playout delay extension.
int frame_minimum_playout_delay_ms_ RTC_GUARDED_BY(playout_delay_lock_) = -1;
// Minimum delay as decided by the setLatency function in "webrtc/api".
int base_minimum_playout_delay_ms_ RTC_GUARDED_BY(playout_delay_lock_) = -1;
// Minimum delay as decided by the A/V synchronization feature.
int syncable_minimum_playout_delay_ms_ RTC_GUARDED_BY(playout_delay_lock_) =
int frame_minimum_playout_delay_ms_ RTC_GUARDED_BY(worker_sequence_checker_) =
-1;
// Minimum delay as decided by the setLatency function in "webrtc/api".
int base_minimum_playout_delay_ms_ RTC_GUARDED_BY(worker_sequence_checker_) =
-1;
// Minimum delay as decided by the A/V synchronization feature.
int syncable_minimum_playout_delay_ms_
RTC_GUARDED_BY(worker_sequence_checker_) = -1;
// Maximum delay as decided by the RTP playout delay extension.
int frame_maximum_playout_delay_ms_ RTC_GUARDED_BY(playout_delay_lock_) = -1;
int frame_maximum_playout_delay_ms_ RTC_GUARDED_BY(worker_sequence_checker_) =
-1;
// Function that is triggered with encoded frames, if not empty.
std::function<void(const RecordableEncodedFrame&)>
@ -225,6 +226,10 @@ class VideoReceiveStream2 : public webrtc::VideoReceiveStream,
// Defined last so they are destroyed before all other members.
rtc::TaskQueue decode_queue_;
// Used to signal destruction to potentially pending tasks.
PendingTaskSafetyFlag::Pointer task_safety_flag_ =
PendingTaskSafetyFlag::Create();
};
} // namespace internal
} // namespace webrtc