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

Revert "Ship ability to opt-in to VP9/AV1 simulcast."

Browse Source 
This reverts commit 75990b9a8f98ea2d597a31472fb778ec4d55f698.

Reason for revert: Breaks downstream, a use case of having three VP9
encodings, scalability mode only specified on the first layer
(L2T2_KEY) and the other two layers not having a scalability mode but
also being active=false appears to trigger a DCHECK in
call/rtp_video_sender.cc:501. More investigation needed

Original change's description:
> Ship ability to opt-in to VP9/AV1 simulcast.
>
> With this unflagging, an app can opt-in to simulcast when using multiple
> encodings by specifying RTCRtpEncodingParameters.scalabilityMode. This
> ensures backwards-compat with apps relying on 3 encodings to mean SVC
> who traditionally have not specified scalabilityMode.
>
> It fixes the spec/API bug of asking for simulcast and not getting
> simulcast. The field trial exists only as a kill-switch with a TODO to
> remove it.
>
> This ships initial support, however note that the VP9/AV1 simulcast uses
> SimulcastRateAllocator (just like VP8/H264 simulcast). This rate
> allocator uses more kbps than SvcRateAllocator. This should be revisited
> to avoid significant higher bitrates, for example when comparing VP9
> simulcast to VP9 SVC.
>
> Shipping the ability for apps to opt-in makes it easier to exercise
> these new code paths and allows initial feedback from developers, but
> due to the high bitrate (= same bitrate as VP8/H264 simulcast today)
> many apps may find that VP9 SVC is still more beneficial for BW reasons.
>
> Bug: webrtc:14884, webrtc:15005
> Change-Id: I748aae1adb47acc8a6b79b5852cff6aa47a46f5d
> Reviewed-on: https://webrtc-review.googlesource.com/c/src/+/298046
> Commit-Queue: Henrik Boström <hbos@webrtc.org>
> Reviewed-by: Erik Språng <sprang@webrtc.org>
> Reviewed-by: Evan Shrubsole <eshr@google.com>
> Reviewed-by: Ilya Nikolaevskiy <ilnik@webrtc.org>
> Cr-Commit-Position: refs/heads/main@{#39601}

Bug: webrtc:14884, webrtc:15005
Change-Id: Ic8f77e6a2971f493d6cd8c23faecd435058a8847
No-Presubmit: true
No-Tree-Checks: true
No-Try: true
Reviewed-on: https://webrtc-review.googlesource.com/c/src/+/298440
Bot-Commit: rubber-stamper@appspot.gserviceaccount.com <rubber-stamper@appspot.gserviceaccount.com>
Reviewed-by: Ilya Nikolaevskiy <ilnik@webrtc.org>
Commit-Queue: Henrik Boström <hbos@webrtc.org>
Cr-Commit-Position: refs/heads/main@{#39605}
This commit is contained in:
Henrik Boström 2023-03-20 12:36:29 +00:00 committed by WebRTC LUCI CQ
parent 934a88a460
commit 75ea06f0fa
3 changed files with 19 additions and 11 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

6
media/engine/webrtc_video_engine.cc
View File

@ -2502,9 +2502,9 @@ WebRtcVideoChannel::WebRtcVideoSendStream::CreateVideoEncoderConfig(
// `legacy_scalability_mode` and codec used.
encoder_config.number_of_streams = parameters_.config.rtp.ssrcs.size();
bool legacy_scalability_mode = true;
// TODO(https://crbug.com/webrtc/14884): This is only used as a kill-switch
// in case of serious bugs - when this reaches Stable, delete the field trial.
if (!call_->trials().IsDisabled("WebRTC-AllowDisablingLegacyScalability")) {
// TODO(https://crbug.com/webrtc/14884): When simulcast VP9 is ready to ship,
// don't require a field trial to set `legacy_scalability_mode` to false here.
if (call_->trials().IsEnabled("WebRTC-AllowDisablingLegacyScalability")) {
for (const webrtc::RtpEncodingParameters& encoding :
rtp_parameters_.encodings) {
if (encoding.scalability_mode.has_value()) {

10
media/engine/webrtc_video_engine_unittest.cc
View File

@ -8463,9 +8463,7 @@ TEST_F(WebRtcVideoChannelTest,
webrtc::ScalabilityModeFromString(kDefaultScalabilityModeStr);
EXPECT_EQ(2, stream->num_encoder_reconfigurations());
webrtc::VideoEncoderConfig encoder_config = stream->GetEncoderConfig().Copy();
// When `scalability_mode` has been specified on any encoding, we do not
// reduce the number of simulcast streams.
EXPECT_EQ(3u, encoder_config.number_of_streams);
EXPECT_EQ(1u, encoder_config.number_of_streams);
EXPECT_THAT(encoder_config.simulcast_layers,
ElementsAre(Field(&webrtc::VideoStream::scalability_mode,
ScalabilityMode::kL3T3),
@ -8478,11 +8476,7 @@ TEST_F(WebRtcVideoChannelTest,
// VideoStreams are created appropriately for the simulcast case.
EXPECT_THAT(stream->GetVideoStreams(),
ElementsAre(Field(&webrtc::VideoStream::scalability_mode,
ScalabilityMode::kL3T3),
Field(&webrtc::VideoStream::scalability_mode,
kDefaultScalabilityMode),
Field(&webrtc::VideoStream::scalability_mode,
kDefaultScalabilityMode)));
ScalabilityMode::kL3T3)));
// GetParameters.
parameters = send_channel_->GetRtpSendParameters(last_ssrc_);

14
pc/peer_connection_simulcast_unittest.cc
View File

@ -60,6 +60,7 @@
#include "rtc_base/thread.h"
#include "rtc_base/unique_id_generator.h"
#include "system_wrappers/include/metrics.h"
#include "test/field_trial.h"
#include "test/gmock.h"
#include "test/gtest.h"
@ -1431,8 +1432,13 @@ TEST_F(PeerConnectionSimulcastWithMediaFlowTests,
Optional(std::string("L3T3_KEY")));
}
// TODO(https://crbug.com/webrtc/14884): A field trial shouldn't be needed to
// get spec-compliant behavior!
TEST_F(PeerConnectionSimulcastWithMediaFlowTests,
SendingThreeEncodings_VP9_Simulcast) {
test::ScopedFieldTrials field_trials(
"WebRTC-AllowDisablingLegacyScalability/Enabled/");
rtc::scoped_refptr<PeerConnectionTestWrapper> local_pc_wrapper = CreatePc();
rtc::scoped_refptr<PeerConnectionTestWrapper> remote_pc_wrapper = CreatePc();
ExchangeIceCandidates(local_pc_wrapper, remote_pc_wrapper);
@ -1497,9 +1503,17 @@ TEST_F(PeerConnectionSimulcastWithMediaFlowTests,
EXPECT_THAT(*outbound_rtps[2]->scalability_mode, StrEq("L1T3"));
}
// TODO(https://crbug.com/webrtc/15005): A field trial shouldn't be needed to
// get spec-compliant behavior! The same field trial is also used for VP9
// simulcast (https://crbug.com/webrtc/14884).
TEST_F(PeerConnectionSimulcastWithMediaFlowTests,
SendingThreeEncodings_AV1_Simulcast) {
test::ScopedFieldTrials field_trials(
"WebRTC-AllowDisablingLegacyScalability/Enabled/");
rtc::scoped_refptr<PeerConnectionTestWrapper> local_pc_wrapper = CreatePc();
// TODO(https://crbug.com/webrtc/15011): Expand testing support for AV1 or
// allow compile time checks so that gates like this isn't needed at runtime.
if (!HasSenderVideoCodecCapability(local_pc_wrapper, "AV1")) {
RTC_LOG(LS_WARNING) << "\n***\nAV1 is not available, skipping test.\n***";
return;