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}
There are now multiple ways to configure VP9 L1Tx:
- Legacy API: configure legacy SVC and disable encodings, this gets
interpreted as disabling spatial layers (non-standard API hack).
- Standard API: configure scalability_mode. This can be done either
with a single encoding or multiple encodings. As long as only one
encoding is active we get a single L1Tx ssrc, same as legacy API.
Due to a bug, the ApplySpatialLayerBitrateLimits() logic which tweaks
bitrates was only applied in the legacy API code path, not the standard
API code path, despite both code paths configuring L1Tx.
The issue is that IsSimulcastOrMultipleSpatialLayers() was checking if
`number_of_streams == 1`. This is true in legacy code path but not
standard code path. The fix is to look at
`numberOfSimulcastStreams == 1` instead, which is set to the correct
value regardless of code path used.
This CL adds comments documenting the difference between
`number_of_streams` and `numberOfSimulcastStreams` to reduce the risk
of more mistakes like this in the future.
Bug: chromium:1455039, b:279161263
Change-Id: I69789b68cc5d45ef1b3becd310687c8dec8e7c87
Reviewed-on: https://webrtc-review.googlesource.com/c/src/+/308722
Reviewed-by: Ilya Nikolaevskiy <ilnik@webrtc.org>
Commit-Queue: Henrik Boström <hbos@webrtc.org>
Reviewed-by: Erik Språng <sprang@webrtc.org>
Cr-Commit-Position: refs/heads/main@{#40287}
The EncoderStreamFactory triggers different code paths depending on
`number_of_streams`: one for simulcast and one for non-simulcast.
The non-simulcast path is desired for both normal streams and SVC
streams.
The simulcast path gives sensible max bitrates for 4:2:1 scenarios, but
when encodings like {active,inactive,inactive} is specified in order to
do standard SVC, the max bps of the first encoding is so low that an
SVC stream will never send more than its first spatial layer (even when
scaleResolutionDownBy is 1).
Because of this, standard SVC is broken. This CL fixes this problem by
using the CreateDefaultVideoStreams() code path instead, which is the
same one that legacy SVC uses. With this fix, legacy and standard SVC
produce the same behavior regarding bitrate.
An added benefit of this is that numberOfSimulcastStreams == 1 in the
standard SVC path as well.
{active,inactive,inactive} tests are updated to verify the full
resolution is achieved after ramp-up. I've also confirmed that this
fixes the bug in Canary, see https://crbug.com/1428098#c2.
Bug: chromium:1428098, webrtc:15041, webrtc:15034
Change-Id: Ia1eb4ff59c4e2a56af833f7ac907a66bca8ea054
Reviewed-on: https://webrtc-review.googlesource.com/c/src/+/299147
Reviewed-by: Ilya Nikolaevskiy <ilnik@webrtc.org>
Commit-Queue: Henrik Boström <hbos@webrtc.org>
Cr-Commit-Position: refs/heads/main@{#39697}
This helps in figuring out which dependencies exist, and gets closer
to obeying the "one target per .cc file" rule.
Test failures seem unrelated, so using No-Try.
No-Try: true
Bug: webrtc:14775
Change-Id: Id25466c8b8fe628d05c819cf7c69ae6d8421c6cf
Reviewed-on: https://webrtc-review.googlesource.com/c/src/+/288020
Reviewed-by: Mirko Bonadei <mbonadei@webrtc.org>
Commit-Queue: Harald Alvestrand <hta@webrtc.org>
Cr-Commit-Position: refs/heads/main@{#38910}
This patch
1) modifies VideoAdapter to use requested_resolution
instead on OnOutputFormatRequest, iff there are no active encoders
that is not using requested_resolution (i.e all "old" encoder(s) are
not active).
2) modifies VideoBroadcaster to not broadcast wants from
encoders that are not active (iff there is an active encoder
using requested_resolution).
3) fixes a bug in encoder_stream_factor in that the
requested_resolution was not propagated to return value
(must have been lost in merge?).
Bug: webrtc:14451
Change-Id: I00e0907f0fe9329141ed169576fa46cdc5384886
Reviewed-on: https://webrtc-review.googlesource.com/c/src/+/278360
Reviewed-by: Ilya Nikolaevskiy <ilnik@webrtc.org>
Reviewed-by: Rasmus Brandt <brandtr@webrtc.org>
Commit-Queue: Jonas Oreland <jonaso@webrtc.org>
Cr-Commit-Position: refs/heads/main@{#38323}
This cl/ adds resource adapation to the requested_resolution
feature. The restrictions that are sent to the video source
are also saved inside video_stream_encoder and used when
determining layer resolution.
Anticipated further patches
4) Let VideoSource do adaption if possible
Bug: webrtc:14451
Change-Id: Ia9b990a6b92b76af7ff6665a562f84585f79c35b
Reviewed-on: https://webrtc-review.googlesource.com/c/src/+/277580
Reviewed-by: Ilya Nikolaevskiy <ilnik@webrtc.org>
Commit-Queue: Jonas Oreland <jonaso@webrtc.org>
Reviewed-by: Rasmus Brandt <brandtr@webrtc.org>
Cr-Commit-Position: refs/heads/main@{#38306}
This cl/ implements configuring of encode resolution
in the video_stream_encoder (webrtc_video_engine) in
a way that is independent of frame resolution (i.e
not using scale_resolution_down_by).
The cl/ reuses the VideoAdapter as is, and hence
the output resolution will be the same as it is today.
Anticipated further patches
3) Hook up resource adaptation
4) Let VideoSource do adaption if possible
Bug: webrtc:14451
Change-Id: I881b031c5b23be26cacfe138730154f1cb1b66a8
Reviewed-on: https://webrtc-review.googlesource.com/c/src/+/276742
Reviewed-by: Artem Titov <titovartem@webrtc.org>
Reviewed-by: Mirko Bonadei <mbonadei@webrtc.org>
Reviewed-by: Henrik Boström <hbos@webrtc.org>
Reviewed-by: Ilya Nikolaevskiy <ilnik@webrtc.org>
Reviewed-by: Rasmus Brandt <brandtr@webrtc.org>
Commit-Queue: Jonas Oreland <jonaso@webrtc.org>
Cr-Commit-Position: refs/heads/main@{#38245}
This cl move VideoEncoderConfig from api/ to video/config.
VideoStreamEncoderInterface and VideoStreamEncoderObserver
are moved as collateral.
brandt@ think that the reason these were in api/ in the
first place had to downstream project.
Functionality wise, this is a NOP, but it makes it easier
to modify the encoder (config).
Bug: webrtc:14451
Change-Id: I2610d815aeb186298498e7102cac773ecac8cd36
Reviewed-on: https://webrtc-review.googlesource.com/c/src/+/277002
Reviewed-by: Mirko Bonadei <mbonadei@webrtc.org>
Reviewed-by: Rasmus Brandt <brandtr@webrtc.org>
Reviewed-by: Artem Titov <titovartem@webrtc.org>
Reviewed-by: Ilya Nikolaevskiy <ilnik@webrtc.org>
Commit-Queue: Jonas Oreland <jonaso@webrtc.org>
Cr-Commit-Position: refs/heads/main@{#38242}
This cl is a preparation for https://webrtc-review.googlesource.com/c/src/+/277002
so that downstream tests can be modified first.
Bug: webrtc:14451
Change-Id: I03b613db695c533bd77ab4bd2f8b35677094443c
Reviewed-on: https://webrtc-review.googlesource.com/c/src/+/277003
Commit-Queue: Jonas Oreland <jonaso@webrtc.org>
Reviewed-by: Mirko Bonadei <mbonadei@webrtc.org>
Reviewed-by: Tomas Gunnarsson <tommi@webrtc.org>
Cr-Commit-Position: refs/heads/main@{#38231}
This cl/ is a NOP refactoring,
moving the EncoderStreamFactory from within webrtc_video_engine.cc
into own file in video/. simulcast.cc is collateral.
Bug: webrtc:14451
Change-Id: Ia69b9241d8cd8a12be6628d887701f2e244c07cc
Reviewed-on: https://webrtc-review.googlesource.com/c/src/+/276861
Reviewed-by: Artem Titov <titovartem@webrtc.org>
Reviewed-by: Mirko Bonadei <mbonadei@webrtc.org>
Reviewed-by: Rasmus Brandt <brandtr@webrtc.org>
Commit-Queue: Jonas Oreland <jonaso@webrtc.org>
Reviewed-by: Ilya Nikolaevskiy <ilnik@webrtc.org>
Cr-Commit-Position: refs/heads/main@{#38224}
