Revert "Preserve min and max playout delay from RTP header extension"

This reverts commit 85ba9972c42544c4771e394c9aa1d20bf5d09a91.

Reason for revert: Audio might be more out of sync than needed due to jitter in received video stream.

Original change's description:
> Preserve min and max playout delay from RTP header extension
> 
> Audio and video synchronization can sometimes override the minimum
> and maximum playout delay that is set through the RTP header
> extension. This CL makes sure that the playout delay always is
> within the limits set by the RTP header extension.
> 
> Bug: webrtc:10886
> Change-Id: Ie2dd4b901c4ed178759b555a8be04bd8b8f63bda
> Reviewed-on: https://webrtc-review.googlesource.com/c/src/+/150645
> Commit-Queue: Johannes Kron <kron@webrtc.org>
> Reviewed-by: Stefan Holmer <stefan@webrtc.org>
> Cr-Commit-Position: refs/heads/master@{#28980}

TBR=stefan@webrtc.org,kron@webrtc.org

Change-Id: I417e440d8a7e04ab3e19faa4454b704d2b971cd7
No-Presubmit: true
No-Tree-Checks: true
No-Try: true
Bug: webrtc:10886
Reviewed-on: https://webrtc-review.googlesource.com/c/src/+/150652
Reviewed-by: Johannes Kron <kron@webrtc.org>
Commit-Queue: Johannes Kron <kron@webrtc.org>
Cr-Commit-Position: refs/heads/master@{#28984}
This commit is contained in:
Johannes Kron 2019-08-28 12:38:28 +00:00 committed by Commit Bot
parent fdd2340311
commit f31cc08ba0
2 changed files with 25 additions and 64 deletions

View File

@ -578,12 +578,14 @@ void VideoReceiveStream::OnCompleteFrame(
last_complete_frame_time_ms_ = time_now_ms;
const PlayoutDelay& playout_delay = frame->EncodedImage().playout_delay_;
// Both |min_ms| and |max_ms| must be valid if PlayoutDelay is set.
RTC_DCHECK((playout_delay.min_ms >= 0 && playout_delay.max_ms >= 0) ||
(playout_delay.min_ms < 0 && playout_delay.max_ms < 0));
if (playout_delay.min_ms >= 0 && playout_delay.max_ms >= 0) {
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();
}
@ -761,19 +763,17 @@ void VideoReceiveStream::HandleFrameBufferTimeout() {
}
void VideoReceiveStream::UpdatePlayoutDelays() const {
int minimum_delay_ms =
const int minimum_delay_ms =
std::max({frame_minimum_playout_delay_ms_, base_minimum_playout_delay_ms_,
syncable_minimum_playout_delay_ms_});
const int maximum_delay_ms = frame_maximum_playout_delay_ms_;
if (maximum_delay_ms >= 0) {
// Make sure that minimum_delay_ms <= maximum_delay_ms.
minimum_delay_ms = std::min(minimum_delay_ms, maximum_delay_ms);
timing_->set_max_playout_delay(maximum_delay_ms);
}
if (minimum_delay_ms >= 0) {
timing_->set_min_playout_delay(minimum_delay_ms);
}
const int maximum_delay_ms = frame_maximum_playout_delay_ms_;
if (maximum_delay_ms >= 0) {
timing_->set_max_playout_delay(maximum_delay_ms);
}
}
std::vector<webrtc::RtpSource> VideoReceiveStream::GetSources() const {

View File

@ -168,7 +168,7 @@ TEST_F(VideoReceiveStreamTest, CreateFrameFromH264FmtpSpropAndIdr) {
}
TEST_F(VideoReceiveStreamTest, PlayoutDelay) {
const PlayoutDelay kPlayoutDelayMs = {123, 621};
const PlayoutDelay kPlayoutDelayMs = {123, 321};
std::unique_ptr<FrameObjectFake> test_frame(new FrameObjectFake());
test_frame->id.picture_id = 0;
test_frame->SetPlayoutDelay(kPlayoutDelayMs);
@ -196,10 +196,9 @@ TEST_F(VideoReceiveStreamTest, PlayoutDelay) {
EXPECT_EQ(123, timing_->min_playout_delay());
}
TEST_F(VideoReceiveStreamTest, PlayoutDelayPreservesDefaultValues) {
const int default_min_playout_latency = timing_->min_playout_delay();
TEST_F(VideoReceiveStreamTest, PlayoutDelayPreservesDefaultMaxValue) {
const int default_max_playout_latency = timing_->max_playout_delay();
const PlayoutDelay kPlayoutDelayMs = {-1, -1};
const PlayoutDelay kPlayoutDelayMs = {123, -1};
std::unique_ptr<FrameObjectFake> test_frame(new FrameObjectFake());
test_frame->id.picture_id = 0;
@ -207,64 +206,26 @@ TEST_F(VideoReceiveStreamTest, PlayoutDelayPreservesDefaultValues) {
video_receive_stream_->OnCompleteFrame(std::move(test_frame));
// Ensure that -1 preserves default minimum and maximum value from |timing_|.
EXPECT_EQ(default_min_playout_latency, timing_->min_playout_delay());
// Ensure that -1 preserves default maximum value from |timing_|.
EXPECT_EQ(kPlayoutDelayMs.min_ms, timing_->min_playout_delay());
EXPECT_NE(kPlayoutDelayMs.max_ms, timing_->max_playout_delay());
EXPECT_EQ(default_max_playout_latency, timing_->max_playout_delay());
}
TEST_F(VideoReceiveStreamTest, ZeroMinMaxPlayoutDelayOverridesSyncAndBase) {
const PlayoutDelay kPlayoutDelayMs = {0, 0};
TEST_F(VideoReceiveStreamTest, PlayoutDelayPreservesDefaultMinValue) {
const int default_min_playout_latency = timing_->min_playout_delay();
const PlayoutDelay kPlayoutDelayMs = {-1, 321};
std::unique_ptr<FrameObjectFake> test_frame(new FrameObjectFake());
test_frame->id.picture_id = 0;
test_frame->SetPlayoutDelay(kPlayoutDelayMs);
video_receive_stream_->SetMinimumPlayoutDelay(400);
EXPECT_EQ(400, timing_->min_playout_delay());
video_receive_stream_->OnCompleteFrame(std::move(test_frame));
EXPECT_EQ(0, timing_->min_playout_delay());
EXPECT_EQ(0, timing_->max_playout_delay());
video_receive_stream_->SetBaseMinimumPlayoutDelayMs(1234);
EXPECT_EQ(0, timing_->min_playout_delay());
EXPECT_EQ(0, timing_->max_playout_delay());
}
TEST_F(VideoReceiveStreamTest, PlayoutDelayFromFrameIsCached) {
// Expect the playout delay from one frame to be used until there's a new
// frame with a valid value.
const PlayoutDelay kPlayoutDelay1Ms = {100, 1000};
const PlayoutDelay kPlayoutDelay2Ms = {120, 900};
// Frame 1 with playout delay set.
std::unique_ptr<FrameObjectFake> frame1(new FrameObjectFake());
frame1->id.picture_id = 0;
frame1->SetPlayoutDelay(kPlayoutDelay1Ms);
video_receive_stream_->OnCompleteFrame(std::move(frame1));
EXPECT_EQ(kPlayoutDelay1Ms.min_ms, timing_->min_playout_delay());
EXPECT_EQ(kPlayoutDelay1Ms.max_ms, timing_->max_playout_delay());
// Frame 2 without playout delay set.
std::unique_ptr<FrameObjectFake> frame2_without_playout_delay(
new FrameObjectFake());
frame2_without_playout_delay->id.picture_id = 1;
video_receive_stream_->OnCompleteFrame(
std::move(frame2_without_playout_delay));
video_receive_stream_->SetBaseMinimumPlayoutDelayMs(40);
video_receive_stream_->SetMinimumPlayoutDelay(50);
EXPECT_EQ(kPlayoutDelay1Ms.min_ms, timing_->min_playout_delay());
EXPECT_EQ(kPlayoutDelay1Ms.max_ms, timing_->max_playout_delay());
// Frame 3 with tighter playout delay bounds.
std::unique_ptr<FrameObjectFake> frame3(new FrameObjectFake());
frame3->id.picture_id = 2;
frame3->SetPlayoutDelay(kPlayoutDelay2Ms);
video_receive_stream_->OnCompleteFrame(std::move(frame3));
EXPECT_EQ(kPlayoutDelay2Ms.min_ms, timing_->min_playout_delay());
EXPECT_EQ(kPlayoutDelay2Ms.max_ms, timing_->max_playout_delay());
// Ensure that -1 preserves default minimum value from |timing_|.
EXPECT_NE(kPlayoutDelayMs.min_ms, timing_->min_playout_delay());
EXPECT_EQ(kPlayoutDelayMs.max_ms, timing_->max_playout_delay());
EXPECT_EQ(default_min_playout_latency, timing_->min_playout_delay());
}
class VideoReceiveStreamTestWithFakeDecoder : public ::testing::Test {