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

Recreate the stream when switching from standard to legacy API.

Browse Source 
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}
This commit is contained in:
Henrik Boström 2023-07-26 14:01:59 +02:00 committed by WebRTC LUCI CQ
parent 9130431b54
commit 0145db4091
2 changed files with 2 additions and 7 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

2
call/rtp_video_sender.cc
View File

@ -498,7 +498,7 @@ void RtpVideoSender::SetActiveModules(const std::vector<bool>& active_modules) {
void RtpVideoSender::SetActiveModulesLocked( void RtpVideoSender::SetActiveModulesLocked(
const std::vector<bool>& active_modules) { const std::vector<bool>& active_modules) {
RTC_DCHECK_RUN_ON(&transport_checker_); 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; active_ = false;
for (size_t i = 0; i < active_modules.size(); ++i) { for (size_t i = 0; i < active_modules.size(); ++i) {
if (active_modules[i]) { if (active_modules[i]) {

7
media/engine/webrtc_video_engine.cc
View File

@ -2166,14 +2166,9 @@ void WebRtcVideoSendChannel::WebRtcVideoSendStream::ReconfigureEncoder(
// layers specified by `scalability_mode`), the number of streams can change. // layers specified by `scalability_mode`), the number of streams can change.
bool num_streams_changed = parameters_.encoder_config.number_of_streams != bool num_streams_changed = parameters_.encoder_config.number_of_streams !=
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); 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 // The app is switching between legacy and standard modes, recreate instead
// of reconfiguring to avoid number of streams not matching in lower layers. // of reconfiguring to avoid number of streams not matching in lower layers.
RecreateWebRtcStream(); RecreateWebRtcStream();