Reland of Add framerate to VideoSinkWants and ability to signal on overuse (patchset #1 id:1 of https://codereview.webrtc.org/2783183003/ )
Reason for revert:
Seem to be a flaky test rather than an issue with this cl. Creating reland, will add code to reduce flakiness to that test.
Original issue's description:
> Revert of Add framerate to VideoSinkWants and ability to signal on overuse (patchset #8 id:410001 of https://codereview.webrtc.org/2781433002/ )
>
> Reason for revert:
> This has resulted in failure of CallPerfTest.ReceivesCpuOveruseAndUnderuse test on the Win7 build bot https://build.chromium.org/p/client.webrtc.perf/builders/Win7/builds/1780
>
> Original issue's description:
> > Reland of Add framerate to VideoSinkWants and ability to signal on overuse (patchset #1 id:1 of https://codereview.webrtc.org/2764133002/ )
> >
> > Reason for revert:
> > Found issue with test case, will add fix to reland cl.
> >
> > Original issue's description:
> > > Revert of Add framerate to VideoSinkWants and ability to signal on overuse (patchset #14 id:250001 of https://codereview.webrtc.org/2716643002/ )
> > >
> > > Reason for revert:
> > > Breaks perf tests:
> > > https://build.chromium.org/p/client.webrtc.perf/builders/Win7/builds/1679
> > > https://build.chromium.org/p/client.webrtc.perf/builders/Android32%20Tests%20%28L%20Nexus5%29/builds/2325
> > >
> > > Original issue's description:
> > > > Add framerate to VideoSinkWants and ability to signal on overuse
> > > >
> > > > In ViEEncoder, try to reduce framerate instead of resolution if the
> > > > current degradation preference is maintain-resolution rather than
> > > > balanced.
> > > >
> > > > BUG=webrtc:4172
> > > >
> > > > Review-Url: https://codereview.webrtc.org/2716643002
> > > > Cr-Commit-Position: refs/heads/master@{#17327}
> > > > Committed: https://chromium.googlesource.com/external/webrtc/+/72acf2526177bb4dbb5103cd6e165eb4361a5ae6
> > >
> > > TBR=nisse@webrtc.org,magjed@webrtc.org,kthelgason@webrtc.org,ilnik@webrtc.org,stefan@webrtc.org,sprang@webrtc.org
> > > # Skipping CQ checks because original CL landed less than 1 days ago.
> > > NOPRESUBMIT=true
> > > NOTREECHECKS=true
> > > NOTRY=true
> > > BUG=webrtc:4172
> > >
> > > Review-Url: https://codereview.webrtc.org/2764133002
> > > Cr-Commit-Position: refs/heads/master@{#17331}
> > > Committed: https://chromium.googlesource.com/external/webrtc/+/8b45b11144c968b4173215c76f78c710c9a2ed0b
> >
> > TBR=nisse@webrtc.org,magjed@webrtc.org,kthelgason@webrtc.org,ilnik@webrtc.org,stefan@webrtc.org,skvlad@webrtc.org
> > # Not skipping CQ checks because original CL landed more than 1 days ago.
> > BUG=webrtc:4172
> >
> > Review-Url: https://codereview.webrtc.org/2781433002
> > Cr-Commit-Position: refs/heads/master@{#17474}
> > Committed: https://chromium.googlesource.com/external/webrtc/+/3ea3c77e93121b1ab9d5e46641e6764f2cca0d51
>
> TBR=ilnik@webrtc.org,stefan@webrtc.org,asapersson@webrtc.org,sprang@webrtc.org
> # Skipping CQ checks because original CL landed less than 1 days ago.
> NOPRESUBMIT=true
> NOTREECHECKS=true
> NOTRY=true
> BUG=webrtc:4172
>
> Review-Url: https://codereview.webrtc.org/2783183003
> Cr-Commit-Position: refs/heads/master@{#17477}
> Committed: https://chromium.googlesource.com/external/webrtc/+/f9ed235c9b7248694edcb46feb1f29ce7456ab59
R=ilnik@webrtc.org,stefan@webrtc.org
BUG=webrtc:4172
Review-Url: https://codereview.webrtc.org/2789823002
Cr-Commit-Position: refs/heads/master@{#17498}
diff --git a/webrtc/media/engine/webrtcvideoengine2_unittest.cc b/webrtc/media/engine/webrtcvideoengine2_unittest.cc
index 0589610..1868834 100644
--- a/webrtc/media/engine/webrtcvideoengine2_unittest.cc
+++ b/webrtc/media/engine/webrtcvideoengine2_unittest.cc
@@ -2115,8 +2115,8 @@
// Trigger overuse.
rtc::VideoSinkWants wants;
- wants.max_pixel_count = rtc::Optional<int>(
- send_stream->GetLastWidth() * send_stream->GetLastHeight() - 1);
+ wants.max_pixel_count =
+ send_stream->GetLastWidth() * send_stream->GetLastHeight() - 1;
send_stream->InjectVideoSinkWants(wants);
EXPECT_TRUE(capturer.CaptureCustomFrame(1280, 720, cricket::FOURCC_I420));
EXPECT_EQ(2, send_stream->GetNumberOfSwappedFrames());
@@ -2124,8 +2124,8 @@
EXPECT_EQ(720 * 3 / 4, send_stream->GetLastHeight());
// Trigger overuse again.
- wants.max_pixel_count = rtc::Optional<int>(
- send_stream->GetLastWidth() * send_stream->GetLastHeight() - 1);
+ wants.max_pixel_count =
+ send_stream->GetLastWidth() * send_stream->GetLastHeight() - 1;
send_stream->InjectVideoSinkWants(wants);
EXPECT_TRUE(capturer.CaptureCustomFrame(1280, 720, cricket::FOURCC_I420));
EXPECT_EQ(3, send_stream->GetNumberOfSwappedFrames());
@@ -2143,7 +2143,7 @@
send_stream->GetLastWidth() * send_stream->GetLastHeight();
// Cap the max to 4x the pixel count (assuming max 1/2 x 1/2 scale downs)
// of the current stream, so we don't take too large steps.
- wants.max_pixel_count = rtc::Optional<int>(current_pixel_count * 4);
+ wants.max_pixel_count = current_pixel_count * 4;
// Default step down is 3/5 pixel count, so go up by 5/3.
wants.target_pixel_count = rtc::Optional<int>((current_pixel_count * 5) / 3);
send_stream->InjectVideoSinkWants(wants);
@@ -2155,7 +2155,7 @@
// Trigger underuse again, should go back up to full resolution.
current_pixel_count =
send_stream->GetLastWidth() * send_stream->GetLastHeight();
- wants.max_pixel_count = rtc::Optional<int>(current_pixel_count * 4);
+ wants.max_pixel_count = current_pixel_count * 4;
wants.target_pixel_count = rtc::Optional<int>((current_pixel_count * 5) / 3);
send_stream->InjectVideoSinkWants(wants);
EXPECT_TRUE(capturer.CaptureCustomFrame(1284, 724, cricket::FOURCC_I420));
@@ -2199,8 +2199,8 @@
// Trigger overuse.
rtc::VideoSinkWants wants;
- wants.max_pixel_count = rtc::Optional<int>(
- send_stream->GetLastWidth() * send_stream->GetLastHeight() - 1);
+ wants.max_pixel_count =
+ send_stream->GetLastWidth() * send_stream->GetLastHeight() - 1;
send_stream->InjectVideoSinkWants(wants);
EXPECT_TRUE(capturer.CaptureCustomFrame(1280, 720, cricket::FOURCC_I420));
EXPECT_EQ(2, send_stream->GetNumberOfSwappedFrames());
@@ -2242,6 +2242,7 @@
void WebRtcVideoChannel2Test::TestCpuAdaptation(bool enable_overuse,
bool is_screenshare) {
+ const int kDefaultFps = 30;
cricket::VideoCodec codec = GetEngineCodec("VP8");
cricket::VideoSendParameters parameters;
parameters.codecs.push_back(codec);
@@ -2263,14 +2264,17 @@
options.is_screencast = rtc::Optional<bool>(is_screenshare);
EXPECT_TRUE(channel_->SetVideoSend(last_ssrc_, true, &options, &capturer));
cricket::VideoFormat capture_format = capturer.GetSupportedFormats()->front();
+ capture_format.interval = rtc::kNumNanosecsPerSec / kDefaultFps;
EXPECT_EQ(cricket::CS_RUNNING, capturer.Start(capture_format));
EXPECT_TRUE(channel_->SetSend(true));
FakeVideoSendStream* send_stream = fake_call_->GetVideoSendStreams().front();
- if (!enable_overuse || is_screenshare) {
+ if (!enable_overuse) {
EXPECT_FALSE(send_stream->resolution_scaling_enabled());
+ EXPECT_FALSE(send_stream->framerate_scaling_enabled());
+ EXPECT_EQ(is_screenshare, send_stream->framerate_scaling_enabled());
EXPECT_TRUE(capturer.CaptureFrame());
EXPECT_EQ(1, send_stream->GetNumberOfSwappedFrames());
@@ -2282,33 +2286,59 @@
return;
}
- EXPECT_TRUE(send_stream->resolution_scaling_enabled());
+ if (is_screenshare) {
+ EXPECT_FALSE(send_stream->resolution_scaling_enabled());
+ EXPECT_TRUE(send_stream->framerate_scaling_enabled());
+ } else {
+ EXPECT_TRUE(send_stream->resolution_scaling_enabled());
+ EXPECT_FALSE(send_stream->framerate_scaling_enabled());
+ }
+
// Trigger overuse.
ASSERT_EQ(1u, fake_call_->GetVideoSendStreams().size());
rtc::VideoSinkWants wants;
- wants.max_pixel_count =
- rtc::Optional<int>(capture_format.width * capture_format.height - 1);
+ if (is_screenshare) {
+ wants.max_framerate_fps = (kDefaultFps * 2) / 3;
+ } else {
+ wants.max_pixel_count = capture_format.width * capture_format.height - 1;
+ }
send_stream->InjectVideoSinkWants(wants);
- EXPECT_TRUE(capturer.CaptureFrame());
- EXPECT_EQ(1, send_stream->GetNumberOfSwappedFrames());
+ for (int i = 0; i < kDefaultFps; ++i)
+ EXPECT_TRUE(capturer.CaptureFrame());
- EXPECT_TRUE(capturer.CaptureFrame());
- EXPECT_EQ(2, send_stream->GetNumberOfSwappedFrames());
-
- EXPECT_LT(send_stream->GetLastWidth(), capture_format.width);
- EXPECT_LT(send_stream->GetLastHeight(), capture_format.height);
+ if (is_screenshare) {
+ // Drops every third frame.
+ EXPECT_EQ(kDefaultFps * 2 / 3, send_stream->GetNumberOfSwappedFrames());
+ EXPECT_EQ(send_stream->GetLastWidth(), capture_format.width);
+ EXPECT_EQ(send_stream->GetLastHeight(), capture_format.height);
+ } else {
+ EXPECT_EQ(kDefaultFps, send_stream->GetNumberOfSwappedFrames());
+ EXPECT_LT(send_stream->GetLastWidth(), capture_format.width);
+ EXPECT_LT(send_stream->GetLastHeight(), capture_format.height);
+ }
// Trigger underuse which should go back to normal resolution.
int last_pixel_count =
send_stream->GetLastWidth() * send_stream->GetLastHeight();
- wants.max_pixel_count = rtc::Optional<int>(last_pixel_count * 4);
- wants.target_pixel_count = rtc::Optional<int>((last_pixel_count * 5) / 3);
+ if (is_screenshare) {
+ wants.max_framerate_fps = kDefaultFps;
+ } else {
+ wants.max_pixel_count = last_pixel_count * 4;
+ wants.target_pixel_count.emplace((last_pixel_count * 5) / 3);
+ }
send_stream->InjectVideoSinkWants(wants);
- EXPECT_TRUE(capturer.CaptureFrame());
- EXPECT_EQ(3, send_stream->GetNumberOfSwappedFrames());
+ for (int i = 0; i < kDefaultFps; ++i)
+ EXPECT_TRUE(capturer.CaptureFrame());
+
+ if (is_screenshare) {
+ EXPECT_EQ(kDefaultFps + (kDefaultFps * 2 / 3),
+ send_stream->GetNumberOfSwappedFrames());
+ } else {
+ EXPECT_EQ(kDefaultFps * 2, send_stream->GetNumberOfSwappedFrames());
+ }
EXPECT_EQ(capture_format.width, send_stream->GetLastWidth());
EXPECT_EQ(capture_format.height, send_stream->GetLastHeight());