diff --git a/media/engine/webrtc_video_engine.cc b/media/engine/webrtc_video_engine.cc index 35a5eb479d..95668de01c 100644 --- a/media/engine/webrtc_video_engine.cc +++ b/media/engine/webrtc_video_engine.cc @@ -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; diff --git a/video/config/encoder_stream_factory.cc b/video/config/encoder_stream_factory.cc index de475b90eb..c955602cfc 100644 --- a/video/config/encoder_stream_factory.cc +++ b/video/config/encoder_stream_factory.cc @@ -174,10 +174,27 @@ EncoderStreamFactory::CreateDefaultVideoStreams( const absl::optional& experimental_min_bitrate) const { std::vector 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 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 = diff --git a/video/video_stream_encoder.cc b/video/video_stream_encoder.cc index c63cf019ed..1e2409372a 100644 --- a/video/video_stream_encoder.cc +++ b/video/video_stream_encoder.cc @@ -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 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."; } }