Recreate the stream when switching from standard to legacy API.
ReconfigureEncoder() is supposed to recreate the send stream when
switching between legacy and standard API paths to ensure that the
upper and lower layers agree on the number of streams that exist
(legacy = 3 encodings but 1 stream, standard = same as encodings).
This successfully happened when going from standard to legacy but due
to a bug in the condition this did not happen when going from legacy to
standard because `scalability_mode_used` is always false here (even
though the standard path does use a scalability mode).
As a consequence, SetRtpParameters()'s call to UpdateSendState()
resulted in a DCHECK-crash. In release builds we still avoid IOOB
because active_modules.size() < rtp_streams.size() but to avoid mistakes
like this happening again in the future, the DCHECK is promoted to a
CHECK.
The fix is to remove the scalability mode condition which didn't make
sense anyway - changing scalability mode does not require recreation but
recreation is necessary when number of streams change, whether or not
scalability mode changed.
TESTED = Using Simulcast Playground and switching back and forth
between standard and legacy and changing scalability modes and
confirming from stats, see https://crbug.com/1467455.
Bug: chromium:1467455
Change-Id: Ide29742972ba83f2e0a11f135ab9b39c39d4eb49
Reviewed-on: https://webrtc-review.googlesource.com/c/src/+/313280
Reviewed-by: Erik Språng <sprang@webrtc.org>
Commit-Queue: Henrik Boström <hbos@webrtc.org>
Cr-Commit-Position: refs/heads/main@{#40477}
diff --git a/call/rtp_video_sender.cc b/call/rtp_video_sender.cc
index 7e88b59..b7616d1 100644
--- a/call/rtp_video_sender.cc
+++ b/call/rtp_video_sender.cc
@@ -498,7 +498,7 @@
void RtpVideoSender::SetActiveModulesLocked(
const std::vector<bool>& active_modules) {
RTC_DCHECK_RUN_ON(&transport_checker_);
- RTC_DCHECK_EQ(rtp_streams_.size(), active_modules.size());
+ RTC_CHECK_EQ(rtp_streams_.size(), active_modules.size());
active_ = false;
for (size_t i = 0; i < active_modules.size(); ++i) {
if (active_modules[i]) {
diff --git a/media/engine/webrtc_video_engine.cc b/media/engine/webrtc_video_engine.cc
index af5d0c7..35a5eb4 100644
--- a/media/engine/webrtc_video_engine.cc
+++ b/media/engine/webrtc_video_engine.cc
@@ -2166,14 +2166,9 @@
// layers specified by `scalability_mode`), the number of streams can change.
bool num_streams_changed = parameters_.encoder_config.number_of_streams !=
encoder_config.number_of_streams;
- bool scalability_mode_used = !codec_settings.codec.scalability_modes.empty();
- bool scalability_modes = absl::c_any_of(
- rtp_parameters_.encodings,
- [](const auto& e) { return e.scalability_mode.has_value(); });
-
parameters_.encoder_config = std::move(encoder_config);
- if (num_streams_changed && (scalability_mode_used != scalability_modes)) {
+ if (num_streams_changed) {
// The app is switching between legacy and standard modes, recreate instead
// of reconfiguring to avoid number of streams not matching in lower layers.
RecreateWebRtcStream();
