Merge min_ms and max_ms accessors in PlayoutDelayOracle

to reduce CriticalSection enterencies and
avoid potentional synchronisation issues.

R=isheriff@chromium.org

Review URL: https://codereview.webrtc.org/2312143002 .

Cr-Commit-Position: refs/heads/master@{#14101}
This commit is contained in:
Danil Chapovalov
2016-09-07 13:29:47 +02:00
parent 126ee727a0
commit 9881cb2874
4 changed files with 21 additions and 28 deletions

View File

@ -21,8 +21,7 @@ PlayoutDelayOracle::PlayoutDelayOracle()
: high_sequence_number_(0),
send_playout_delay_(false),
ssrc_(0),
min_playout_delay_ms_(-1),
max_playout_delay_ms_(-1) {}
playout_delay_{-1, -1} {}
PlayoutDelayOracle::~PlayoutDelayOracle() {}
@ -35,16 +34,16 @@ void PlayoutDelayOracle::UpdateRequest(uint32_t ssrc,
RTC_DCHECK_LE(playout_delay.min_ms, playout_delay.max_ms);
int64_t unwrapped_seq_num = unwrapper_.Unwrap(seq_num);
if (playout_delay.min_ms >= 0 &&
playout_delay.min_ms != min_playout_delay_ms_) {
playout_delay.min_ms != playout_delay_.min_ms) {
send_playout_delay_ = true;
min_playout_delay_ms_ = playout_delay.min_ms;
playout_delay_.min_ms = playout_delay.min_ms;
high_sequence_number_ = unwrapped_seq_num;
}
if (playout_delay.max_ms >= 0 &&
playout_delay.max_ms != max_playout_delay_ms_) {
playout_delay.max_ms != playout_delay_.max_ms) {
send_playout_delay_ = true;
max_playout_delay_ms_ = playout_delay.max_ms;
playout_delay_.max_ms = playout_delay.max_ms;
high_sequence_number_ = unwrapped_seq_num;
}
ssrc_ = ssrc;

View File

@ -42,16 +42,10 @@ class PlayoutDelayOracle {
return send_playout_delay_;
}
// Returns current minimum playout delay in milliseconds.
int min_playout_delay_ms() const {
// Returns current playout delay.
PlayoutDelay playout_delay() const {
rtc::CritScope lock(&crit_sect_);
return min_playout_delay_ms_;
}
// Returns current maximum playout delay in milliseconds.
int max_playout_delay_ms() const {
rtc::CritScope lock(&crit_sect_);
return max_playout_delay_ms_;
return playout_delay_;
}
// Updates the application requested playout delay, current ssrc
@ -75,10 +69,8 @@ class PlayoutDelayOracle {
uint32_t ssrc_ GUARDED_BY(crit_sect_);
// Sequence number unwrapper.
SequenceNumberUnwrapper unwrapper_ GUARDED_BY(crit_sect_);
// Min playout delay value on the next frame if |send_playout_delay_| is set.
int min_playout_delay_ms_ GUARDED_BY(crit_sect_);
// Max playout delay value on the next frame if |send_playout_delay_| is set.
int max_playout_delay_ms_ GUARDED_BY(crit_sect_);
// Playout delay values on the next frame if |send_playout_delay_| is set.
PlayoutDelay playout_delay_ GUARDED_BY(crit_sect_);
RTC_DISALLOW_COPY_AND_ASSIGN(PlayoutDelayOracle);
};

View File

@ -38,16 +38,16 @@ class PlayoutDelayOracleTest : public ::testing::Test {
TEST_F(PlayoutDelayOracleTest, DisabledByDefault) {
EXPECT_FALSE(playout_delay_oracle_.send_playout_delay());
EXPECT_EQ(playout_delay_oracle_.min_playout_delay_ms(), -1);
EXPECT_EQ(playout_delay_oracle_.max_playout_delay_ms(), -1);
EXPECT_EQ(-1, playout_delay_oracle_.playout_delay().min_ms);
EXPECT_EQ(-1, playout_delay_oracle_.playout_delay().max_ms);
}
TEST_F(PlayoutDelayOracleTest, SendPlayoutDelayUntilSeqNumberExceeds) {
PlayoutDelay playout_delay = {kMinPlayoutDelay, kMaxPlayoutDelay};
playout_delay_oracle_.UpdateRequest(kSsrc, playout_delay, kSequenceNumber);
EXPECT_TRUE(playout_delay_oracle_.send_playout_delay());
EXPECT_EQ(playout_delay_oracle_.min_playout_delay_ms(), kMinPlayoutDelay);
EXPECT_EQ(playout_delay_oracle_.max_playout_delay_ms(), kMaxPlayoutDelay);
EXPECT_EQ(kMinPlayoutDelay, playout_delay_oracle_.playout_delay().min_ms);
EXPECT_EQ(kMaxPlayoutDelay, playout_delay_oracle_.playout_delay().max_ms);
// Oracle indicates playout delay should be sent if highest sequence number
// acked is lower than the sequence number of the first packet containing

View File

@ -481,8 +481,9 @@ bool RTPSender::SendOutgoingData(FrameType frame_type,
// on what the oracle indicates.
{
rtc::CritScope lock(&send_critsect_);
if (playout_delay_active_ != playout_delay_oracle_.send_playout_delay()) {
playout_delay_active_ = playout_delay_oracle_.send_playout_delay();
bool send_playout_delay = playout_delay_oracle_.send_playout_delay();
if (playout_delay_active_ != send_playout_delay) {
playout_delay_active_ = send_playout_delay;
rtp_header_extension_map_.SetActive(kRtpExtensionPlayoutDelay,
playout_delay_active_);
}
@ -1177,11 +1178,12 @@ uint16_t RTPSender::BuildRtpHeaderExtension(uint8_t* data_buffer,
block_length = BuildTransportSequenceNumberExtension(
extension_data, transport_sequence_number_);
break;
case kRtpExtensionPlayoutDelay:
case kRtpExtensionPlayoutDelay: {
PlayoutDelay playout_delay = playout_delay_oracle_.playout_delay();
block_length = BuildPlayoutDelayExtension(
extension_data, playout_delay_oracle_.min_playout_delay_ms(),
playout_delay_oracle_.max_playout_delay_ms());
extension_data, playout_delay.min_ms, playout_delay.max_ms);
break;
}
default:
assert(false);
}