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

Fix first encoding's maxBitrate being ignored when scalability is set.

Browse Source 
EncoderStreamFactory has two code paths for creating a stream: the
"simulcast path" and the "default path". Only the former cares about
encoding paramter's maxBitrate. The latter assumes that
`encoder_config.max_bitrate_bps` already encompasses the maxBitrate of
the first encoding, but this is not always the case.

As of M113, when scalability mode is specified, {active,inactive} does
not count as simulcast stream but as a default stream represented by
encoding[0].

The problem is that `encoder_config.max_bitrate_bps` only includes
`encodings[0].max_bitrate_bps` when `encodings.size() == 1` which isn't
the case here.

This CL fixes the problem by making the "create default stream" code
path look at the first encoding's maxBitrate and remove existing
assumptions that `encoder_config.max_bitrate_bps` encompasses
`encodings[0].max_bitrate_bps`. This is a step in the right direction
since we're trying to remove all special cases and have encodings map
1:1 with SSRCs, so the "max bps of entire stream" should indeed be a
separate limit than the per-encoding limits and it was confusing that
sometimes it included and sometimes it excluded encoding[0]'s limit.

This issue did not happen in {inactive,active} since that code path
counts as "simulcast stream", so "default stream" is only ever
applicable for index 0.

TESTED=Simulcast Playground, see https://crbug.com/1455962.

Bug: chromium:1455962
Change-Id: I7c44925b780623b5979751e8959e972293648a3d
Reviewed-on: https://webrtc-review.googlesource.com/c/src/+/313282
Commit-Queue: Henrik Boström <hbos@webrtc.org>
Reviewed-by: Ilya Nikolaevskiy <ilnik@webrtc.org>
Reviewed-by: Erik Språng <sprang@webrtc.org>
Cr-Commit-Position: refs/heads/main@{#40482}
This commit is contained in:
Henrik Boström 2023-07-27 13:48:18 +02:00 committed by WebRTC LUCI CQ
parent 6143ec939a
commit b90cd91983
3 changed files with 47 additions and 42 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

41
media/engine/webrtc_video_engine.cc
View File

@ -352,18 +352,6 @@ bool IsCodecDisabledForSimulcast(bool legacy_scalability_mode,
return false;
}
// Returns its smallest positive argument. If neither argument is positive,
// returns an arbitrary nonpositive value.
int MinPositive(int a, int b) {
if (a <= 0) {
return b;
}
if (b <= 0) {
return a;
}
return std::min(a, b);
}
bool IsLayerActive(const webrtc::RtpEncodingParameters& layer) {
return layer.active &&
(!layer.max_bitrate_bps || *layer.max_bitrate_bps > 0) &&
@ -2042,29 +2030,20 @@ WebRtcVideoSendChannel::WebRtcVideoSendStream::CreateVideoEncoderConfig(
}
// parameters_.max_bitrate comes from the max bitrate set at the SDP
// (m-section) level with the attribute "b=AS." Note that we override this
// value below if the RtpParameters max bitrate set with
// RtpSender::SetParameters has a lower value.
// (m-section) level with the attribute "b=AS." Note that stream max bitrate
// is the RtpSender's max bitrate, but each individual encoding may also have
// its own max bitrate specified by SetParameters.
int stream_max_bitrate = parameters_.max_bitrate_bps;
// When simulcast is enabled (when there are multiple encodings),
// encodings[i].max_bitrate_bps will be enforced by
// encoder_config.simulcast_layers[i].max_bitrate_bps. Otherwise, it's
// enforced by stream_max_bitrate, taking the minimum of the two maximums
// (one coming from SDP, the other coming from RtpParameters).
if (rtp_parameters_.encodings[0].max_bitrate_bps &&
rtp_parameters_.encodings.size() == 1) {
stream_max_bitrate =
MinPositive(*(rtp_parameters_.encodings[0].max_bitrate_bps),
parameters_.max_bitrate_bps);
}
// The codec max bitrate comes from the "x-google-max-bitrate" parameter
// attribute set in the SDP for a specific codec. As done in
// WebRtcVideoSendChannel::SetSendParameters, this value does not override the
// stream max_bitrate set above.
// attribute set in the SDP for a specific codec. It only has an effect if
// max bitrate is not specified via "b=AS" and doesn't apply in singlecast
// if the encoding has a max bitrate.
int codec_max_bitrate_kbps;
bool is_single_encoding_with_max_bitrate =
rtp_parameters_.encodings.size() == 1 &&
rtp_parameters_.encodings[0].max_bitrate_bps.value_or(0) > 0;
if (codec.GetParam(kCodecParamMaxBitrate, &codec_max_bitrate_kbps) &&
stream_max_bitrate == -1) {
stream_max_bitrate == -1 && !is_single_encoding_with_max_bitrate) {
stream_max_bitrate = codec_max_bitrate_kbps * 1000;
}
encoder_config.max_bitrate_bps = stream_max_bitrate;

25
video/config/encoder_stream_factory.cc
View File

@ -174,10 +174,27 @@ EncoderStreamFactory::CreateDefaultVideoStreams(
const absl::optional<webrtc::DataRate>& experimental_min_bitrate) const {
std::vector<webrtc::VideoStream> layers;
// The max bitrate specified by the API.
// - `encoder_config.simulcast_layers[0].max_bitrate_bps` comes from the first
// RtpEncodingParamters, which is the encoding of this stream.
// - `encoder_config.max_bitrate_bps` comes from SDP; "b=AS" or conditionally
// "x-google-max-bitrate".
// If `api_max_bitrate_bps` has a value then it is positive.
absl::optional<int> api_max_bitrate_bps;
if (encoder_config.simulcast_layers[0].max_bitrate_bps > 0) {
api_max_bitrate_bps = encoder_config.simulcast_layers[0].max_bitrate_bps;
}
if (encoder_config.max_bitrate_bps > 0) {
api_max_bitrate_bps =
api_max_bitrate_bps.has_value()
? std::min(encoder_config.max_bitrate_bps, *api_max_bitrate_bps)
: encoder_config.max_bitrate_bps;
}
// For unset max bitrates set default bitrate for non-simulcast.
int max_bitrate_bps =
(encoder_config.max_bitrate_bps > 0)
? encoder_config.max_bitrate_bps
api_max_bitrate_bps.has_value()
? *api_max_bitrate_bps
: GetMaxDefaultVideoBitrateKbps(width, height, is_screenshare_) *
1000;
@ -189,7 +206,7 @@ EncoderStreamFactory::CreateDefaultVideoStreams(
// Use set min bitrate.
min_bitrate_bps = encoder_config.simulcast_layers[0].min_bitrate_bps;
// If only min bitrate is configured, make sure max is above min.
if (encoder_config.max_bitrate_bps <= 0)
if (!api_max_bitrate_bps.has_value())
max_bitrate_bps = std::max(min_bitrate_bps, max_bitrate_bps);
}
int max_framerate = (encoder_config.simulcast_layers[0].max_framerate > 0)
@ -253,7 +270,7 @@ EncoderStreamFactory::CreateDefaultVideoStreams(
sum_max_bitrates_kbps += spatial_layer.maxBitrate;
}
RTC_DCHECK_GE(sum_max_bitrates_kbps, 0);
if (encoder_config.max_bitrate_bps <= 0) {
if (!api_max_bitrate_bps.has_value()) {
max_bitrate_bps = sum_max_bitrates_kbps * 1000;
} else {
max_bitrate_bps =

23
video/video_stream_encoder.cc
View File

@ -1106,8 +1106,7 @@ void VideoStreamEncoder::ReconfigureEncoder() {
// or/and can be provided by encoder. In presence of both set of
// limits, the final set is derived as their intersection.
int min_bitrate_bps;
if (encoder_config_.simulcast_layers.empty() ||
encoder_config_.simulcast_layers[0].min_bitrate_bps <= 0) {
if (encoder_config_.simulcast_layers[0].min_bitrate_bps <= 0) {
min_bitrate_bps = encoder_bitrate_limits->min_bitrate_bps;
} else {
min_bitrate_bps = std::max(encoder_bitrate_limits->min_bitrate_bps,
@ -1115,10 +1114,20 @@ void VideoStreamEncoder::ReconfigureEncoder() {
}
int max_bitrate_bps;
// We don't check encoder_config_.simulcast_layers[0].max_bitrate_bps
// here since encoder_config_.max_bitrate_bps is derived from it (as
// well as from other inputs).
if (encoder_config_.max_bitrate_bps <= 0) {
// The API max bitrate comes from both `encoder_config_.max_bitrate_bps`
// and `encoder_config_.simulcast_layers[0].max_bitrate_bps`.
absl::optional<int> api_max_bitrate_bps;
if (encoder_config_.simulcast_layers[0].max_bitrate_bps > 0) {
api_max_bitrate_bps =
encoder_config_.simulcast_layers[0].max_bitrate_bps;
}
if (encoder_config_.max_bitrate_bps > 0) {
api_max_bitrate_bps = api_max_bitrate_bps.has_value()
? std::min(encoder_config_.max_bitrate_bps,
*api_max_bitrate_bps)
: encoder_config_.max_bitrate_bps;
}
if (!api_max_bitrate_bps.has_value()) {
max_bitrate_bps = encoder_bitrate_limits->max_bitrate_bps;
} else {
max_bitrate_bps = std::min(encoder_bitrate_limits->max_bitrate_bps,
@ -1138,7 +1147,7 @@ void VideoStreamEncoder::ReconfigureEncoder() {
<< ", max=" << encoder_bitrate_limits->max_bitrate_bps
<< ") do not intersect with limits set by app"
<< " (min=" << streams.back().min_bitrate_bps
<< ", max=" << encoder_config_.max_bitrate_bps
<< ", max=" << api_max_bitrate_bps.value_or(-1)
<< "). The app bitrate limits will be used.";
}
}