As of [1], a single VP9 encoder instance can produce simulcast 4:2:1.
When it does, the EncodedImage has its simulcast index set (0, 1, 2).
The bug is that if you then go back to a single encoder instance,
either because you're doing singlecast or because you're doing
simulcast with scaling factors that are not power of two (not 4:2:1),
then the simulcast index which was previously set to 2 is not reset due
to the old code path never calling SetSimulcastIndex.
Example repro:
1. Send VP9 simulcast {180p, 360p, 720p}, i.e. 4:2.1.
2. Reconfigure to {180p, 360p, 540p}, i.e. no longer 4:2:1.
What should happen: all three layers are sent.
What actually happened: 180p is not sent and the 540p layer flips flops
between 180p and 540p because the EncodedImage says simulcast index is
2 for both encodings[0] and encodings[2].
The fix is a one-line change: `SetSimulcastIndex(std::nullopt)` in the
case that we don't have a `simulcast_to_svc_converter_` that sets it
(0, 1, 2) for us.
[1] https://webrtc-review.googlesource.com/c/src/+/360280
(cherry picked from commit a6fbb35ac1)
Bug: chromium:370299916
Change-Id: I94ce1a0bde43ef56cba930cb69b744877bbd4bf9
Reviewed-on: https://webrtc-review.googlesource.com/c/src/+/363941
Reviewed-by: Sergey Silkin <ssilkin@webrtc.org>
Commit-Queue: Henrik Boström <hbos@webrtc.org>
Cr-Original-Commit-Position: refs/heads/main@{#43109}
Reviewed-on: https://webrtc-review.googlesource.com/c/src/+/364302
Reviewed-by: Ilya Nikolaevskiy <ilnik@webrtc.org>
Cr-Commit-Position: refs/branch-heads/6723@{#2}
Cr-Branched-From: 13e377b804f68aa9c20ea5e449666ea5248e3286-refs/heads/main@{#43019}
This CL makes `requested_resolution`, which is the C++ name for what
the spec calls scaleResolutionDownTo, align with the latest PR[1].
The PR says to ignore scaleResolutionDownBy when scaleResolutionDownTo
is specified as to be backwards compatible with scaleResolutionDownBy's
default scaling factors (e.g. 4:2:1). Ignoring is different than what
the code does today which is to throw an InvalidModificationError.
We don't want to throw or else get+setParameters() would throw by
default due to 4:2:1 defaults so the app would have to remember to
delete these attributes every time even though it never specified them
(Chrome has a bug here but fixing that would expose this problem, see
https://crbug.com/344943229).
[1] https://github.com/w3c/webrtc-extensions/pull/221
Bug: none
Change-Id: I21165c9b9f9ee7259d88b89f9ae58b862ea4521e
Reviewed-on: https://webrtc-review.googlesource.com/c/src/+/362260
Commit-Queue: Henrik Boström <hbos@webrtc.org>
Reviewed-by: Ilya Nikolaevskiy <ilnik@webrtc.org>
Cr-Commit-Position: refs/heads/main@{#43002}
Disable the checks ensuring we reject mixed-codec simulcast
when the field trial is enabled.
The feature is however not yet implemented.
Bug: webrtc:362277533
Change-Id: Ib1601767c951d61aaa37a3d8767d0a81444d626c
Reviewed-on: https://webrtc-review.googlesource.com/c/src/+/361404
Reviewed-by: Harald Alvestrand <hta@webrtc.org>
Commit-Queue: Florent Castelli <orphis@webrtc.org>
Cr-Commit-Position: refs/heads/main@{#42942}
Normally (scaleResolutionDownBy) restrictions are applied at the source
which changes the input frame size which triggers reconfiguration with
appropriate scaling factors.
But when requested_resolution is used, encoder settings are by
definition not relative to the input frame size. In order for
restrictions to have an effect, they are applied inside
ReconfigureEncoder(): you get the minimum between the requested
resolution and the restricted resolution.
ReconfigureEncoder() happens when you SetParameters(), but the bug
here is that we don't do it again once the restrictions are updated.
So if restrictions are 540p when you ask for 720p, you get 540p and
after restrictions change to unlimited you're still stuck in 540p.
The fix is to also trigger ReconfigureEncoder() inside
OnVideoSourceRestrictionsUpdated() when the restricted resolution is
changing and a requested_resolution is configured.
To ensure reconfiguring the encoder "on the fly" like this does not
reset initial frame dropping logic, InitialFrameDropper caring about
input frame size changing is made conditional on not using
requested_resolution.
# Slow purple bots failing but they are not affected by this change.
NOTRY=True
Bug: webrtc:361477261
Change-Id: I1389aa16cf408b0d14e0b5b6f68c2442db955be9
Reviewed-on: https://webrtc-review.googlesource.com/c/src/+/360200
Commit-Queue: Henrik Boström <hbos@webrtc.org>
Reviewed-by: Ilya Nikolaevskiy <ilnik@webrtc.org>
Cr-Commit-Position: refs/heads/main@{#42882}
According to spec, if you ask for three encodings you get three
encodings (duh). But according to legacy code, if you ask for three
encodings AND codec is VP9, then surely you meant a single encoding that
is kSVC where the other encodings influence the scalability mode of the
first encoding.
Standard simulcast support in VP9 was shipped as an opt-in feature where
you have to specify `scalability_mode` and `scale_resolution_down_by` in
order to let WebRTC know that you want to disable the legacy path.
But `scale_resolution_down_by` is not the only way to configure
resolution, there is also the `requested_resolution` code path. This CL
adds standard simulcast support for this code path as well.
Prior to this change, our parameterized test would have passed in VP8
but failed in VP9. With this change the test passes for all codecs.
Bug: webrtc:361124448
Change-Id: Ic5a7136de8abf430813fd01342862775fca145fb
Reviewed-on: https://webrtc-review.googlesource.com/c/src/+/360100
Commit-Queue: Henrik Boström <hbos@webrtc.org>
Reviewed-by: Ilya Nikolaevskiy <ilnik@webrtc.org>
Cr-Commit-Position: refs/heads/main@{#42822}
Libvpx works without this, so the existing tests pass. However, other
encoder implementations (like rtc_video_encoder in Chrome) look at
different fields and get confused about the configuration.
Test: Integration tests with Chrome and windows hardware encoders.
Bug: webrtc:348342168
Change-Id: Id0d96cff34eb34c7e019a24255623f3aeeca5772
Reviewed-on: https://webrtc-review.googlesource.com/c/src/+/355500
Reviewed-by: Erik Språng <sprang@webrtc.org>
Reviewed-by: Åsa Persson <asapersson@webrtc.org>
Reviewed-by: Ilya Nikolaevskiy <ilnik@webrtc.org>
Commit-Queue: Evan Shrubsole <eshr@google.com>
Reviewed-by: Per Kjellander <perkj@webrtc.org>
Cr-Commit-Position: refs/heads/main@{#42555}
This changes the libvpx VP9 encoder to generate the scalability mode based on the current encoding parameters when using layer activation.
Tested: Ran with L3T3_KEY reduced to L2T3_KEY and L1T3 due to bandwidth or layer activation. Added unit tests.
Bug: webrtc:15892
Change-Id: Iaedca4ea5fc3a692996666ceaf0d6aa03fb058a1
Reviewed-on: https://webrtc-review.googlesource.com/c/src/+/344760
Commit-Queue: Evan Shrubsole <eshr@google.com>
Reviewed-by: Ilya Nikolaevskiy <ilnik@webrtc.org>
Reviewed-by: Harald Alvestrand <hta@webrtc.org>
Cr-Commit-Position: refs/heads/main@{#42007}
This is a reland of commit 1cce1d7ddc
after updating the WPT that broke on Mac.
Original change's description:
> Make setCodecPreferences only look at receive codecs
>
> which is what is noted in JSEP:
> https://www.rfc-editor.org/rfc/rfc8829.html#name-setcodecpreferences
>
> Some W3C spec modifications are required since the W3C specification
> currently takes into account send codecs as well.
>
> Spec issue:
> https://github.com/w3c/webrtc-pc/issues/2888
> Spec PR:
> https://github.com/w3c/webrtc-pc/pull/2926
>
> setCodecPreferences continues to modify the codecs in an offer.
>
> Also rename RtpSender::SetCodecPreferences to RtpSender::SetSendCodecs for consistent semantics.
>
> BUG=webrtc:15396
>
> Change-Id: I1e8fbe77cb2670575578a777ed1336567a1e4031
> Reviewed-on: https://webrtc-review.googlesource.com/c/src/+/328780
> Reviewed-by: Henrik Boström <hbos@webrtc.org>
> Commit-Queue: Philipp Hancke <phancke@microsoft.com>
> Reviewed-by: Harald Alvestrand <hta@webrtc.org>
> Cr-Commit-Position: refs/heads/main@{#41719}
Bug: webrtc:15396
Change-Id: I0c7b17f00de02286f176b500460e17980b83b35b
Reviewed-on: https://webrtc-review.googlesource.com/c/src/+/339541
Commit-Queue: Philipp Hancke <phancke@microsoft.com>
Reviewed-by: Harald Alvestrand <hta@webrtc.org>
Cr-Commit-Position: refs/heads/main@{#41807}
This reverts commit 1cce1d7ddc.
Reason for revert: Breaks WPTs
Original change's description:
> Make setCodecPreferences only look at receive codecs
>
> which is what is noted in JSEP:
> https://www.rfc-editor.org/rfc/rfc8829.html#name-setcodecpreferences
>
> Some W3C spec modifications are required since the W3C specification
> currently takes into account send codecs as well.
>
> Spec issue:
> https://github.com/w3c/webrtc-pc/issues/2888
> Spec PR:
> https://github.com/w3c/webrtc-pc/pull/2926
>
> setCodecPreferences continues to modify the codecs in an offer.
>
> Also rename RtpSender::SetCodecPreferences to RtpSender::SetSendCodecs for consistent semantics.
>
> BUG=webrtc:15396
>
> Change-Id: I1e8fbe77cb2670575578a777ed1336567a1e4031
> Reviewed-on: https://webrtc-review.googlesource.com/c/src/+/328780
> Reviewed-by: Henrik Boström <hbos@webrtc.org>
> Commit-Queue: Philipp Hancke <phancke@microsoft.com>
> Reviewed-by: Harald Alvestrand <hta@webrtc.org>
> Cr-Commit-Position: refs/heads/main@{#41719}
Bug: webrtc:15396
Change-Id: I7b545e91f820c3affc39841c6e93939eac75c363
No-Presubmit: true
No-Tree-Checks: true
No-Try: true
Reviewed-on: https://webrtc-review.googlesource.com/c/src/+/339520
Reviewed-by: Harald Alvestrand <hta@webrtc.org>
Commit-Queue: Harald Alvestrand <hta@webrtc.org>
Owners-Override: Henrik Boström <hbos@webrtc.org>
Reviewed-by: Henrik Boström <hbos@webrtc.org>
Auto-Submit: Henrik Boström <hbos@webrtc.org>
Bot-Commit: rubber-stamper@appspot.gserviceaccount.com <rubber-stamper@appspot.gserviceaccount.com>
Cr-Commit-Position: refs/heads/main@{#41725}
Instead, always use VideoSendStream::Start.
VideoSendStream::StartPerRtpStream was used for controlling if
individual rtp stream for a RtpEncodingParameter should be able to send RTP packets. It was not used for controlling the actual encoder layers.
With this change RtpEncodingParameter.active still controls actual encoder layers but it does not control if RTP packets can be sent or not.
The cleanup is done to simplify code and in the future allow sending
probe packet on a RtpTransceiver that allows sending, regardless of the
RtpEncodingParameter.active flag.
Bug: webrtc:14928
Change-Id: I896c055ed4de76db58d76f452147c29783f77ae1
Reviewed-on: https://webrtc-review.googlesource.com/c/src/+/335042
Reviewed-by: Henrik Boström <hbos@webrtc.org>
Reviewed-by: Erik Språng <sprang@webrtc.org>
Commit-Queue: Per Kjellander <perkj@webrtc.org>
Cr-Commit-Position: refs/heads/main@{#41619}
Same method, different name. Unblocks replacing RTCStatsMember<T> with
absl::optional<T>.
Bug: webrtc:15164
Change-Id: I251dd44d3b0f9576b3b68915fe0406d1b3381e5c
Reviewed-on: https://webrtc-review.googlesource.com/c/src/+/334641
Reviewed-by: Harald Alvestrand <hta@webrtc.org>
Commit-Queue: Henrik Boström <hbos@webrtc.org>
Cr-Commit-Position: refs/heads/main@{#41573}
This is a reland of commit 9e4a97bb02
Original change's description:
> [Stats] Add value_or() and migrate from ValueOrDefault().
>
> Yet another prerequisite for replacing RTCStatsMember<T> with
> absl::optional<T>, but this looks like the last one.
>
> Bug: webrtc:15164
> Change-Id: I2cde51e8c8c951f71b48ccd45e07146091a99616
> Reviewed-on: https://webrtc-review.googlesource.com/c/src/+/334647
> Commit-Queue: Henrik Boström <hbos@webrtc.org>
> Reviewed-by: Harald Alvestrand <hta@webrtc.org>
> Cr-Commit-Position: refs/heads/main@{#41541}
Bug: webrtc:15164
Change-Id: I5fdba499383e5d9efe0a1dcef6bf6c2e0a812857
Reviewed-on: https://webrtc-review.googlesource.com/c/src/+/335102
Commit-Queue: Mirko Bonadei <mbonadei@webrtc.org>
Reviewed-by: Harald Alvestrand <hta@webrtc.org>
Reviewed-by: Henrik Boström <hbos@webrtc.org>
Cr-Commit-Position: refs/heads/main@{#41564}
Yet another prerequisite for replacing RTCStatsMember<T> with
absl::optional<T>, but this looks like the last one.
Bug: webrtc:15164
Change-Id: I2cde51e8c8c951f71b48ccd45e07146091a99616
Reviewed-on: https://webrtc-review.googlesource.com/c/src/+/334647
Commit-Queue: Henrik Boström <hbos@webrtc.org>
Reviewed-by: Harald Alvestrand <hta@webrtc.org>
Cr-Commit-Position: refs/heads/main@{#41541}
Add ulpfec and flexfec to list of resiliency mechanisms taken
into account and in general exclude Comfort Noise (CN) from media
codecs.
Also introduce RtpCodecCapability::IsMediaCodec & ::IsResiliencyCodec
behaving like the MediaCodec methods.
BUG=webrtc:15396
Change-Id: I79041898928190bfdd33a06d8f6975d7556c46b1
Reviewed-on: https://webrtc-review.googlesource.com/c/src/+/330424
Reviewed-by: Florent Castelli <orphis@webrtc.org>
Commit-Queue: Philipp Hancke <phancke@microsoft.com>
Reviewed-by: Harald Alvestrand <hta@webrtc.org>
Cr-Commit-Position: refs/heads/main@{#41485}
This test drives the new tools_webrtc/remove_extra_namespace.py tool.
Bug: None
Change-Id: I9b590aa1213e4cace2d64d555f4dafd893f03606
Reviewed-on: https://webrtc-review.googlesource.com/c/src/+/327021
Reviewed-by: Mirko Bonadei <mbonadei@webrtc.org>
Commit-Queue: Harald Alvestrand <hta@webrtc.org>
Cr-Commit-Position: refs/heads/main@{#41141}
Because of our asymmetrical codec situation, it's possible to have
send only codecs that we cannot negotiate even with ourselves.
This means that we should not have a DCHECK, but just a plain error.
Bug: webrtc:15064
Change-Id: I0c170e5c7f356197bcb04bcecb8259c344423ccb
Reviewed-on: https://webrtc-review.googlesource.com/c/src/+/323183
Reviewed-by: Harald Alvestrand <hta@webrtc.org>
Commit-Queue: Florent Castelli <orphis@webrtc.org>
Cr-Commit-Position: refs/heads/main@{#40939}
The implementation covers the latest specification, but does not
support mixed-codec simulcast at the moment.
Changing codec for audio and video is supported.
Bug: webrtc:15064
Change-Id: I09082f39e2a7d54dd4a663a8a57bf9df5a851690
Reviewed-on: https://webrtc-review.googlesource.com/c/src/+/311663
Reviewed-by: Henrik Boström <hbos@webrtc.org>
Reviewed-by: Harald Alvestrand <hta@webrtc.org>
Commit-Queue: Florent Castelli <orphis@webrtc.org>
Cr-Commit-Position: refs/heads/main@{#40616}
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}
Previously this test only ran on VP9, now it runs for all codecs.
Bug: webrtc:15080
Change-Id: Id61a261cef3463a22062e3d313dc2725e051773d
Reviewed-on: https://webrtc-review.googlesource.com/c/src/+/300861
Commit-Queue: Henrik Boström <hbos@webrtc.org>
Reviewed-by: Ilya Nikolaevskiy <ilnik@webrtc.org>
Cr-Commit-Position: refs/heads/main@{#39812}
This CL introduces PeerConnectionEncodingsIntegrationParameterizedTest,
which is based on PeerConnectionEncodingsIntegrationTest but covers all
codecs using INSTANTIATE_TEST_SUITE_P (VP8, VP9, H264, AV1).
This applies to all standard paths, which in the case of VP9 and AV1
requires opting in to it by specifying scalabilityMode and
scaleResolutionDownBy. They are also limited to L1Tx because the other
codecs don't support SVC.
The VP9-only tests continue to run as TEST_F with
PeerConnectionEncodingsIntegrationTest.
Bug: webrtc:15079
Change-Id: I3429c90f2f79ff60adad0b33975bccdda31ce6d9
Reviewed-on: https://webrtc-review.googlesource.com/c/src/+/300900
Reviewed-by: Ilya Nikolaevskiy <ilnik@webrtc.org>
Commit-Queue: Henrik Boström <hbos@webrtc.org>
Cr-Commit-Position: refs/heads/main@{#39808}
This is a pure rename+move of PeerConnectionSimulcastMediaFlowTests.
The reason for renaming is to reflect that a) this is an integration
test, not a unit test, and b) not all of the tests use simulcast (some
use a single encoding, i.e. singlecast or SVC).
Shared helper functions between PeerConnectionEncodingsIntegrationTest
and PeerConnectionSimulcastTests are placed in a test-only util file.
# Already pass, no need to wait for chromium bots for webrtc testonly CL
NOTRY=True
Bug: webrtc:15063
Change-Id: Iec90d1a7ab712be1395c7644723422c8c6179974
Reviewed-on: https://webrtc-review.googlesource.com/c/src/+/300540
Reviewed-by: Jeremy Leconte <jleconte@google.com>
Commit-Queue: Henrik Boström <hbos@webrtc.org>
Cr-Commit-Position: refs/heads/main@{#39799}
