admin/webrtc_m130
1
0
Fork 0
Code Issues Pull Requests Actions Packages Projects Releases Wiki Activity

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

Browse Source 
This reverts commit f31cc08ba01ed403e89255b5f3f38d5dbdde855e.

Reason for revert: Reland with fixes

Original change's description:
> 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}

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

Change-Id: I5a3908a8c45f7faedab6f009b22df81d674e13a0
No-Presubmit: true
No-Tree-Checks: true
No-Try: true
Bug: webrtc:10886
Reviewed-on: https://webrtc-review.googlesource.com/c/src/+/150653
Reviewed-by: Johannes Kron <kron@webrtc.org>
Commit-Queue: Johannes Kron <kron@webrtc.org>
Cr-Commit-Position: refs/heads/master@{#28985}
This commit is contained in:
Johannes Kron 2019-08-28 12:40:45 +00:00 committed by Commit Bot
parent f31cc08ba0
commit 87bed4793f
2 changed files with 64 additions and 25 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

22
video/video_receive_stream.cc
View File

@ -578,14 +578,12 @@ void VideoReceiveStream::OnCompleteFrame(
last_complete_frame_time_ms_ = time_now_ms;
const PlayoutDelay& playout_delay = frame->EncodedImage().playout_delay_;
if (playout_delay.min_ms >= 0) {
// 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) {
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();
}
@ -763,17 +761,19 @@ void VideoReceiveStream::HandleFrameBufferTimeout() {
}
void VideoReceiveStream::UpdatePlayoutDelays() const {
const int minimum_delay_ms =
int minimum_delay_ms =
std::max({frame_minimum_playout_delay_ms_, base_minimum_playout_delay_ms_,
syncable_minimum_playout_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) {
// 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);
}
}
std::vector<webrtc::RtpSource> VideoReceiveStream::GetSources() const {

67
video/video_receive_stream_unittest.cc
View File

@ -168,7 +168,7 @@ TEST_F(VideoReceiveStreamTest, CreateFrameFromH264FmtpSpropAndIdr) {
}
TEST_F(VideoReceiveStreamTest, PlayoutDelay) {
const PlayoutDelay kPlayoutDelayMs = {123, 321};
const PlayoutDelay kPlayoutDelayMs = {123, 621};
std::unique_ptr<FrameObjectFake> test_frame(new FrameObjectFake());
test_frame->id.picture_id = 0;
test_frame->SetPlayoutDelay(kPlayoutDelayMs);
@ -196,9 +196,10 @@ TEST_F(VideoReceiveStreamTest, PlayoutDelay) {
EXPECT_EQ(123, timing_->min_playout_delay());
}
TEST_F(VideoReceiveStreamTest, PlayoutDelayPreservesDefaultMaxValue) {
TEST_F(VideoReceiveStreamTest, PlayoutDelayPreservesDefaultValues) {
const int default_min_playout_latency = timing_->min_playout_delay();
const int default_max_playout_latency = timing_->max_playout_delay();
const PlayoutDelay kPlayoutDelayMs = {123, -1};
const PlayoutDelay kPlayoutDelayMs = {-1, -1};
std::unique_ptr<FrameObjectFake> test_frame(new FrameObjectFake());
test_frame->id.picture_id = 0;
@ -206,26 +207,64 @@ TEST_F(VideoReceiveStreamTest, PlayoutDelayPreservesDefaultMaxValue) {
video_receive_stream_->OnCompleteFrame(std::move(test_frame));
// 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());
// Ensure that -1 preserves default minimum and maximum value from |timing_|.
EXPECT_EQ(default_min_playout_latency, timing_->min_playout_delay());
EXPECT_EQ(default_max_playout_latency, timing_->max_playout_delay());
}
TEST_F(VideoReceiveStreamTest, PlayoutDelayPreservesDefaultMinValue) {
const int default_min_playout_latency = timing_->min_playout_delay();
const PlayoutDelay kPlayoutDelayMs = {-1, 321};
TEST_F(VideoReceiveStreamTest, ZeroMinMaxPlayoutDelayOverridesSyncAndBase) {
const PlayoutDelay kPlayoutDelayMs = {0, 0};
std::unique_ptr<FrameObjectFake> test_frame(new FrameObjectFake());
test_frame->id.picture_id = 0;
test_frame->SetPlayoutDelay(kPlayoutDelayMs);
video_receive_stream_->OnCompleteFrame(std::move(test_frame));
video_receive_stream_->SetMinimumPlayoutDelay(400);
EXPECT_EQ(400, timing_->min_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());
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());
}
class VideoReceiveStreamTestWithFakeDecoder : public ::testing::Test {