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

VP9 Simulcast test: Update comments to reflect L1T3.

Browse Source 
Looked in to this some more and had a chat with Evan, and L1T3 being
reported in getStats() is a real sign that L1T3 is used. This CL updates
the comments of the VP9 simulcast test to reflect that this is what
we are getting, not SVC, even if layers are being dropped etc.

Bug: webrtc:14884
Change-Id: I15eac981625302480ce337879138537c0ad73664
Reviewed-on: https://webrtc-review.googlesource.com/c/src/+/295540
Commit-Queue: Evan Shrubsole <eshr@webrtc.org>
Reviewed-by: Ilya Nikolaevskiy <ilnik@webrtc.org>
Auto-Submit: Henrik Boström <hbos@webrtc.org>
Reviewed-by: Evan Shrubsole <eshr@webrtc.org>
Cr-Commit-Position: refs/heads/main@{#39421}
This commit is contained in:
Henrik Boström 2023-02-28 16:04:21 +01:00 committed by WebRTC LUCI CQ
parent 227574804f
commit c65f5fd90f
1 changed files with 8 additions and 11 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

19
pc/peer_connection_simulcast_unittest.cc
View File

@ -1162,8 +1162,7 @@ TEST_F(PeerConnectionSimulcastWithMediaFlowTests,
} }
// TODO(https://crbug.com/webrtc/14884): Support VP9 simulcast and update this // TODO(https://crbug.com/webrtc/14884): Support VP9 simulcast and update this
// test to EXPECT three encodings of L1T3, not the VP9 SVC legacy fallback path // test to EXPECT three RTP streams of L1T3, not the single RTP we get today.
// that happens today which is wrong.
TEST_F(PeerConnectionSimulcastWithMediaFlowTests, TEST_F(PeerConnectionSimulcastWithMediaFlowTests,
SendingThreeEncodings_VP9_Simulcast) { SendingThreeEncodings_VP9_Simulcast) {
rtc::scoped_refptr<PeerConnectionTestWrapper> local_pc_wrapper = CreatePc(); rtc::scoped_refptr<PeerConnectionTestWrapper> local_pc_wrapper = CreatePc();
@ -1194,24 +1193,22 @@ TEST_F(PeerConnectionSimulcastWithMediaFlowTests,
remote_pc_wrapper->WaitForConnection(); remote_pc_wrapper->WaitForConnection();
// We want to EXPECT to get 3 "outbound-rtps", but because VP9 simulcast is // We want to EXPECT to get 3 "outbound-rtps", but because VP9 simulcast is
// not supported yet (webrtc:14884), we expect a single RTP stream with SVC. // not supported yet (webrtc:14884), we expect a single RTP stream. We get
// Due to bugs, the fallback to SVC is not reported in either getStats() or // "L1T3" so we do avoid SVC fallback, but the other two layers are dropped
// getParameters(). This test expects what we see, not what we want to see. // and some parts of the code still assume SVC which could lead to other bugs
// such as invalid bitrate configurations.
// Legacy SVC fallback only has a single RTP stream.
EXPECT_TRUE_WAIT(HasOutboundRtpBytesSent(local_pc_wrapper, 1u), EXPECT_TRUE_WAIT(HasOutboundRtpBytesSent(local_pc_wrapper, 1u),
kLongTimeoutForRampingUp.ms()); kLongTimeoutForRampingUp.ms());
// Legacy SVC fallback uses "L3T3_KEY" but the `scalability_mode` returned by
// the API seems to reflect what we asked for, not what we got.
rtc::scoped_refptr<const RTCStatsReport> report = GetStats(local_pc_wrapper); rtc::scoped_refptr<const RTCStatsReport> report = GetStats(local_pc_wrapper);
std::vector<const RTCOutboundRTPStreamStats*> outbound_rtps = std::vector<const RTCOutboundRTPStreamStats*> outbound_rtps =
report->GetStatsOfType<RTCOutboundRTPStreamStats>(); report->GetStatsOfType<RTCOutboundRTPStreamStats>();
// The fact that we only have a single RTP is a sign SVC is used.
ASSERT_THAT(outbound_rtps, SizeIs(1u)); ASSERT_THAT(outbound_rtps, SizeIs(1u));
EXPECT_THAT(GetCurrentCodecMimeType(report, *outbound_rtps[0]), EXPECT_THAT(GetCurrentCodecMimeType(report, *outbound_rtps[0]),
StrCaseEq("video/VP9")); StrCaseEq("video/VP9"));
// In SVC we should see "L3T3_KEY" but we see the "L1T3" that we asked for. // `scalability_mode` in stats does reflect LibvpxVp9Encoder settings!
EXPECT_EQ(*outbound_rtps[0]->scalability_mode, "L1T3"); EXPECT_EQ(*outbound_rtps[0]->scalability_mode, "L1T3");
// What GetParameters() is returning though is most likely just reflecting
// what was set, not what was configured.
parameters = sender->GetParameters(); parameters = sender->GetParameters();
ASSERT_EQ(parameters.encodings.size(), 3u); ASSERT_EQ(parameters.encodings.size(), 3u);
EXPECT_THAT(parameters.encodings[0].scalability_mode, EXPECT_THAT(parameters.encodings[0].scalability_mode,