Attempting to ship "WebRTC-AllowDisablingLegacyScalability" revealed a
DCHECK that happens when negotiating 3 VP9 streams prior to the
setParameters() call:
1. By default, `scalability_mode` is missing, so those 3 streams
defaulted to legacy SVC, meaning only a single stream is used.
2. Then, setParameters() was called to make
`encodings[0].scalability_mode = "L2T2_KEY"` and
`encodings[1-2].active = false`. The inactive streams were just
dummies and never expected to exist.
Without simulcast support this is OK, because both 1) and 2) are
interpreted to have a single stream. But with simulcast support, 1) is
interpreted as single stream and 2) as three streams (1 active, 2
inactive). This should be roughly the same setup, but our code treats
them differently.
The DCHECK crash was a mismatch in number of streams in one of the
layers.
The fix is to re-create the streams when the number of streams change
for this reason. The new test revealed other issues and fixes too:
- Support for multiple spatial layers (e.g. "L2T2_KEY") when multiple
encodings exist but only one encoding is active.
- Allow inactive layers not to have a scalability mode set.
A laundry list (https://crbug.com/webrtc/15028) has been created to
update known places doing "if streams == 1" that need to do "if
active streams == 1" instead.
Credit:
The RecreateWebRtcStream() fix is based on eshr@'s POC from
https://webrtc-review.googlesource.com/c/src/+/298565.
Bug: webrtc:15016
Change-Id: I909a3f83a4ef53562894549ade0a870b208cec7e
Reviewed-on: https://webrtc-review.googlesource.com/c/src/+/298443
Reviewed-by: Ilya Nikolaevskiy <ilnik@webrtc.org>
Commit-Queue: Henrik Boström <hbos@webrtc.org>
Reviewed-by: Erik Språng <sprang@webrtc.org>
Reviewed-by: Evan Shrubsole <eshr@google.com>
Cr-Commit-Position: refs/heads/main@{#39651}
This reverts commit 75990b9a8f98ea2d597a31472fb778ec4d55f698.
Reason for revert: Breaks downstream, a use case of having three VP9
encodings, scalability mode only specified on the first layer
(L2T2_KEY) and the other two layers not having a scalability mode but
also being active=false appears to trigger a DCHECK in
call/rtp_video_sender.cc:501. More investigation needed
Original change's description:
> Ship ability to opt-in to VP9/AV1 simulcast.
>
> With this unflagging, an app can opt-in to simulcast when using multiple
> encodings by specifying RTCRtpEncodingParameters.scalabilityMode. This
> ensures backwards-compat with apps relying on 3 encodings to mean SVC
> who traditionally have not specified scalabilityMode.
>
> It fixes the spec/API bug of asking for simulcast and not getting
> simulcast. The field trial exists only as a kill-switch with a TODO to
> remove it.
>
> This ships initial support, however note that the VP9/AV1 simulcast uses
> SimulcastRateAllocator (just like VP8/H264 simulcast). This rate
> allocator uses more kbps than SvcRateAllocator. This should be revisited
> to avoid significant higher bitrates, for example when comparing VP9
> simulcast to VP9 SVC.
>
> Shipping the ability for apps to opt-in makes it easier to exercise
> these new code paths and allows initial feedback from developers, but
> due to the high bitrate (= same bitrate as VP8/H264 simulcast today)
> many apps may find that VP9 SVC is still more beneficial for BW reasons.
>
> Bug: webrtc:14884, webrtc:15005
> Change-Id: I748aae1adb47acc8a6b79b5852cff6aa47a46f5d
> Reviewed-on: https://webrtc-review.googlesource.com/c/src/+/298046
> Commit-Queue: Henrik Boström <hbos@webrtc.org>
> Reviewed-by: Erik Språng <sprang@webrtc.org>
> Reviewed-by: Evan Shrubsole <eshr@google.com>
> Reviewed-by: Ilya Nikolaevskiy <ilnik@webrtc.org>
> Cr-Commit-Position: refs/heads/main@{#39601}
Bug: webrtc:14884, webrtc:15005
Change-Id: Ic8f77e6a2971f493d6cd8c23faecd435058a8847
No-Presubmit: true
No-Tree-Checks: true
No-Try: true
Reviewed-on: https://webrtc-review.googlesource.com/c/src/+/298440
Bot-Commit: rubber-stamper@appspot.gserviceaccount.com <rubber-stamper@appspot.gserviceaccount.com>
Reviewed-by: Ilya Nikolaevskiy <ilnik@webrtc.org>
Commit-Queue: Henrik Boström <hbos@webrtc.org>
Cr-Commit-Position: refs/heads/main@{#39605}
With this unflagging, an app can opt-in to simulcast when using multiple
encodings by specifying RTCRtpEncodingParameters.scalabilityMode. This
ensures backwards-compat with apps relying on 3 encodings to mean SVC
who traditionally have not specified scalabilityMode.
It fixes the spec/API bug of asking for simulcast and not getting
simulcast. The field trial exists only as a kill-switch with a TODO to
remove it.
This ships initial support, however note that the VP9/AV1 simulcast uses
SimulcastRateAllocator (just like VP8/H264 simulcast). This rate
allocator uses more kbps than SvcRateAllocator. This should be revisited
to avoid significant higher bitrates, for example when comparing VP9
simulcast to VP9 SVC.
Shipping the ability for apps to opt-in makes it easier to exercise
these new code paths and allows initial feedback from developers, but
due to the high bitrate (= same bitrate as VP8/H264 simulcast today)
many apps may find that VP9 SVC is still more beneficial for BW reasons.
Bug: webrtc:14884, webrtc:15005
Change-Id: I748aae1adb47acc8a6b79b5852cff6aa47a46f5d
Reviewed-on: https://webrtc-review.googlesource.com/c/src/+/298046
Commit-Queue: Henrik Boström <hbos@webrtc.org>
Reviewed-by: Erik Språng <sprang@webrtc.org>
Reviewed-by: Evan Shrubsole <eshr@google.com>
Reviewed-by: Ilya Nikolaevskiy <ilnik@webrtc.org>
Cr-Commit-Position: refs/heads/main@{#39601}
The goal of the VP9 simulcast project is that when `scalability_mode`
is set, multiple encodings are always interpreted as simulcast, even
if VP9 or AV1 is used. This CL makes this so, but only if the flag
"WebRTC-AllowDisablingLegacyScalability" is "/Enabled/". This allows us
to make "SendingThreeEncodings_VP9_Simulcast" EXPECT VP9 simulcast.
When we are ready to ship we will remove the need to use the field
trial, but before we ship this we'll want to revisit if
SvcRateAllocator can be updated to support simulcast. (Today if we use
SvcRateAllocator when VP9 simulcast is used, all encodings except the
first one get bitrate=0, causing the test to fail because media is not
flowing on all layers.) For now, a TODO is added.
Bug: webrtc:14884
Change-Id: Ie20ae748b0c0405162f3a1b015ab94956ef83dae
Reviewed-on: https://webrtc-review.googlesource.com/c/src/+/297340
Reviewed-by: Evan Shrubsole <eshr@webrtc.org>
Reviewed-by: Erik Språng <sprang@webrtc.org>
Commit-Queue: Henrik Boström <hbos@webrtc.org>
Reviewed-by: Philip Eliasson <philipel@webrtc.org>
Cr-Commit-Position: refs/heads/main@{#39552}
This reverts commit 18c869bc36b342cd4a79947067e52a93a04a7808.
Reason for revert: Added a field trial that allows landing the code without affecting performance in prod.
This CL also incorporates subsequent CLs that also had to be reverted.
Original change's description:
> Revert "Use two MediaChannels for 2 directions."
>
> This reverts commit 8981a6fac3d665beac4a58b9453e6c39988a024f.
>
> Reason for revert: Quality regression detected.
>
> Original change's description:
> > Use two MediaChannels for 2 directions.
> >
> > This CL separates the two directions of MediaChannel into two separate objects that do not couple with each other.
> >
> > The notable API change is that receiver local SSRC now has to be set explicitly - before, it was done implicitly when the send-side MediaChannel had a stream added to it.
> >
> > Bug: webrtc:13931
> > Change-Id: I83c2e3c8e79f89872d5adda1bc2899f7049748b3
> > Reviewed-on: https://webrtc-review.googlesource.com/c/src/+/288400
> > Commit-Queue: Harald Alvestrand <hta@webrtc.org>
> > Reviewed-by: Henrik Boström <hbos@webrtc.org>
> > Cr-Commit-Position: refs/heads/main@{#39340}
>
> No-Try: true
> Bug: webrtc:13931
> Change-Id: I791997ad9eff75c3ac9cd2e4bbacf5bc6c3a3a79
> Reviewed-on: https://webrtc-review.googlesource.com/c/src/+/295663
> Commit-Queue: Harald Alvestrand <hta@webrtc.org>
> Reviewed-by: Mirko Bonadei <mbonadei@webrtc.org>
> Cr-Commit-Position: refs/heads/main@{#39445}
Bug: webrtc:13931
Change-Id: I1318910a685188e2b846c9040e1efc04c2c894ac
Reviewed-on: https://webrtc-review.googlesource.com/c/src/+/296080
Reviewed-by: Tomas Gunnarsson <tommi@webrtc.org>
Reviewed-by: Per Kjellander <perkj@webrtc.org>
Commit-Queue: Harald Alvestrand <hta@webrtc.org>
Reviewed-by: Henrik Boström <hbos@webrtc.org>
Cr-Commit-Position: refs/heads/main@{#39494}
Under the combined network/worker thread project, tasks
are unnecessarily posted to the same thread. Avoid this
by posting only if invoked on a diffferent sequence.
TESTED=presubmit + local Meet calls.
Bug: webrtc:137439
Change-Id: Ic5dd99e5fbb843ad4c54d4466138135ae81596cf
Reviewed-on: https://webrtc-review.googlesource.com/c/src/+/295867
Commit-Queue: Markus Handell <handellm@webrtc.org>
Reviewed-by: Harald Alvestrand <hta@webrtc.org>
Cr-Commit-Position: refs/heads/main@{#39471}
This reverts commit a087f6f1c842f1d70ad207b44c48321ab60d2d95.
Reason for revert: Needed to roll back other CL
Original change's description:
> Add plumbing for video NACK to be coupled between channels.
>
> Bug: webrtc:13931, webrtc:14920
> Change-Id: I451869e295e099a1d08c0c80e481decd53149f1b
> Reviewed-on: https://webrtc-review.googlesource.com/c/src/+/294382
> Reviewed-by: Henrik Boström <hbos@webrtc.org>
> Commit-Queue: Harald Alvestrand <hta@webrtc.org>
> Cr-Commit-Position: refs/heads/main@{#39373}
Bug: webrtc:13931, webrtc:14920
Change-Id: I19e176e75630313da470542e7ff1e89b6d717fc9
Reviewed-on: https://webrtc-review.googlesource.com/c/src/+/295664
Commit-Queue: Harald Alvestrand <hta@webrtc.org>
Reviewed-by: Mirko Bonadei <mbonadei@webrtc.org>
Cr-Commit-Position: refs/heads/main@{#39432}
Currently, send stats update `last_stats_log_ms_` causing receive stats
to never be logged.
This behavior was introduced in https://webrtc-review.googlesource.com/c/src/+/288750
Bug: b/270519075
Change-Id: Ie781082cfb212c1c903cbada5e393d2e7aa6150f
Reviewed-on: https://webrtc-review.googlesource.com/c/src/+/294743
Commit-Queue: Linus Nilsson <lnilsson@webrtc.org>
Reviewed-by: Harald Alvestrand <hta@webrtc.org>
Cr-Commit-Position: refs/heads/main@{#39381}
This allows MediaChannel to know whether it's being used
for sending, receiving, or both. This is a preparatory CL
for landing the split of MediaChannel usage into sending and
receiving objects.
Bug: webrtc:13931
Change-Id: If518c8b53d5256771200a42e1b5f2b3321d26d8c
Reviewed-on: https://webrtc-review.googlesource.com/c/src/+/292860
Reviewed-by: Florent Castelli <orphis@webrtc.org>
Commit-Queue: Harald Alvestrand <hta@webrtc.org>
Cr-Commit-Position: refs/heads/main@{#39283}
This is used when an unsignaled stream with a known payload type is received and later a RTX packet is received.
Bug: webrtc:14817
Change-Id: I29f43281cec17553e1ec2483e21b8847714d2931
Reviewed-on: https://webrtc-review.googlesource.com/c/src/+/291328
Reviewed-by: Henrik Boström <hbos@webrtc.org>
Reviewed-by: Erik Språng <sprang@webrtc.org>
Reviewed-by: Harald Alvestrand <hta@webrtc.org>
Commit-Queue: Per Kjellander <perkj@webrtc.org>
Cr-Commit-Position: refs/heads/main@{#39243}
These configurations are no longer used by call. Header extensions are identified once when demuxing packets in WebrtcVideoEngine::OnPacketReceived and WebrtcVoiceEngine::OnPacketReceived.
Change-Id: I49de9005f0aa9ab32f2c5d3abcdd8bd12343022d
Bug: webrtc:7135
Reviewed-on: https://webrtc-review.googlesource.com/c/src/+/291480
Owners-Override: Per Kjellander <perkj@webrtc.org>
Commit-Queue: Per Kjellander <perkj@webrtc.org>
Reviewed-by: Erik Språng <sprang@webrtc.org>
Reviewed-by: Henrik Boström <hbos@webrtc.org>
Cr-Commit-Position: refs/heads/main@{#39236}
This ensure transport feedback is sent for RTX packets that are received before media payload packets.
Bug: webrtc:14795, webrtc:14817
Change-Id: I6a2579b87c8863e003decb2b2559ef51a852cadb
Reviewed-on: https://webrtc-review.googlesource.com/c/src/+/291119
Commit-Queue: Per Kjellander <perkj@webrtc.org>
Reviewed-by: Henrik Boström <hbos@webrtc.org>
Cr-Commit-Position: refs/heads/main@{#39174}
optimizing for the fairly common case of many recv-only
mediasections.
BUG=webrtc:14808
Change-Id: Iae68c5bb7a5516d77f908f1effbb50a5ed750f92
Reviewed-on: https://webrtc-review.googlesource.com/c/src/+/290984
Reviewed-by: Harald Alvestrand <hta@webrtc.org>
Commit-Queue: Philipp Hancke <phancke@microsoft.com>
Reviewed-by: Henrik Boström <hbos@webrtc.org>
Cr-Commit-Position: refs/heads/main@{#39122}
instead of the full set of codecs that have been negotiated.
BUG=webrtc:14808
Change-Id: I464cc1d20e5b5227a09929c909615b432c6be041
Reviewed-on: https://webrtc-review.googlesource.com/c/src/+/290885
Reviewed-by: Harald Alvestrand <hta@webrtc.org>
Commit-Queue: Philipp Hancke <phancke@microsoft.com>
Reviewed-by: Henrik Boström <hbos@webrtc.org>
Cr-Commit-Position: refs/heads/main@{#39114}
Fallback to a default value if the scalability mode is unset or not supported by the codec.
The fallback logic is only enabled if the scalability mode is configured for any of the encodings for now (i.e. initial default values are not set).
Bug: webrtc:11607
Change-Id: Ie632767b627a1dbbef71c59f9340573daf386c14
Reviewed-on: https://webrtc-review.googlesource.com/c/src/+/287600
Reviewed-by: Erik Språng <sprang@webrtc.org>
Reviewed-by: Florent Castelli <orphis@webrtc.org>
Commit-Queue: Åsa Persson <asapersson@webrtc.org>
Cr-Commit-Position: refs/heads/main@{#39074}
With this cl, a packet is only parsed once in RtpTransport::DemuxPacket and the metadata is reused.
Extensions are still identified twice- one for demuxing based on mid. The second time in Channel::OnReceivedPacket in order to use extensions specific to that mid.
Bug: webrtc:7135, webrtc:14795
Change-Id: I50e3814af92ca4378f148876b20a54bcfac1e146
Reviewed-on: https://webrtc-review.googlesource.com/c/src/+/290540
Reviewed-by: Danil Chapovalov <danilchap@webrtc.org>
Commit-Queue: Per Kjellander <perkj@webrtc.org>
Cr-Commit-Position: refs/heads/main@{#39058}
This is a reland of commit 81aab488781c1a736c9d85ff1532631be2989523
See diff between Patch Set 1 and latest Patch Set.
The original CL broke this WPT[1] because getStats() with the receiver
as the selector stopped working in the event of unsignalled SSRCs due
to the receiver not knowing what the SSRC was.
This fix is to query media_channel_ for the unsignalled SSRC in the
event that the receiver does not know the SSRC.
[1] https://source.chromium.org/chromium/chromium/src/+/main:third_party/blink/web_tests/external/wpt/webrtc/simulcast/setParameters-active.https.html
Original change's description:
> Remove 'trackId' dependency in stats selector algorithm.
>
> In preparation for the deletion of deprecated 'track' stats, the
> stats selector algorithm needs to be rewritten not to use 'trackId'.
>
> This is achieved by finding RTP stats by their SSRC, as obtained via
> getParameters(). This unfortunately adds a block-invoke (in the sender
> case the block-invoke happens inside GetParametersInternal and in the
> receiver case the block-invoke is explicit at the calling place), but
> it can't be helped and it's just once per getStats() call and only if
> the selector argument is used.
>
> Bug: webrtc:14175
> Change-Id: If0e14cdbdc76d141e0042e43757970893bf32119
> Reviewed-on: https://webrtc-review.googlesource.com/c/src/+/289101
> Reviewed-by: Harald Alvestrand <hta@webrtc.org>
> Commit-Queue: Henrik Boström <hbos@webrtc.org>
> Cr-Commit-Position: refs/heads/main@{#38981}
Bug: webrtc:14175, webrtc:14811
Change-Id: I0d16724af4efeb93d50e36dbfcc798564daff5c0
Reviewed-on: https://webrtc-review.googlesource.com/c/src/+/290600
Commit-Queue: Henrik Boström <hbos@webrtc.org>
Reviewed-by: Harald Alvestrand <hta@webrtc.org>
Cr-Commit-Position: refs/heads/main@{#39010}
This is in preparation for splitting MediaChannel into sender and
receiver channels, with independent objects.
Bug: webrtc:13931
Change-Id: I8e34b0c80b4d76132394efcda658a8face3ab873
Reviewed-on: https://webrtc-review.googlesource.com/c/src/+/288750
Reviewed-by: Henrik Boström <hbos@webrtc.org>
Commit-Queue: Harald Alvestrand <hta@webrtc.org>
Cr-Commit-Position: refs/heads/main@{#38998}
This is a reland of commit 97ba853295578975a04fc504315cccd465f9f0bd
This cl did not cause the regression in Chrome rolls https://chromium-review.googlesource.com/c/chromium/src/+/4132644?tab=checks. Real culprit reverted in https://webrtc-review.googlesource.com/c/src/+/290502.
Original change's description:
> Remove use of ReceiveStreamRtpConfig:transport_cc
>
> With this change, webrtc will send RTCP transport feedback for all received packets that have a transport sequence number, if the header extension
> http://www.ietf.org/id/draft-holmer-rmcat-transport-wide-cc-extensions is negotiated.
> I.e the SDP attribute a=rtcp-fb:96 transport-cc is ignored.
>
>
> Change-Id: I95d8d4405dc86a2f872f7883b7bafd623d5f7841
>
> Bug: webrtc:14802
> Change-Id: I95d8d4405dc86a2f872f7883b7bafd623d5f7841
> Reviewed-on: https://webrtc-review.googlesource.com/c/src/+/290403
> Reviewed-by: Harald Alvestrand <hta@webrtc.org>
> Reviewed-by: Danil Chapovalov <danilchap@webrtc.org>
> Commit-Queue: Per Kjellander <perkj@webrtc.org>
> Cr-Commit-Position: refs/heads/main@{#38980}
Bug: webrtc:14802
Change-Id: Ib98e61413161d462da60144942cdb0140e12bc42
Reviewed-on: https://webrtc-review.googlesource.com/c/src/+/290503
Reviewed-by: Danil Chapovalov <danilchap@webrtc.org>
Commit-Queue: Per Kjellander <perkj@webrtc.org>
Reviewed-by: Harald Alvestrand <hta@webrtc.org>
Cr-Commit-Position: refs/heads/main@{#38997}
With this change, webrtc will send RTCP transport feedback for all received packets that have a transport sequence number, if the header extension
http://www.ietf.org/id/draft-holmer-rmcat-transport-wide-cc-extensions is negotiated.
I.e the SDP attribute a=rtcp-fb:96 transport-cc is ignored.
Change-Id: I95d8d4405dc86a2f872f7883b7bafd623d5f7841
Bug: webrtc:14802
Change-Id: I95d8d4405dc86a2f872f7883b7bafd623d5f7841
Reviewed-on: https://webrtc-review.googlesource.com/c/src/+/290403
Reviewed-by: Harald Alvestrand <hta@webrtc.org>
Reviewed-by: Danil Chapovalov <danilchap@webrtc.org>
Commit-Queue: Per Kjellander <perkj@webrtc.org>
Cr-Commit-Position: refs/heads/main@{#38980}
The implementation here has a number of changes that force the callers
that called the "channel" functions into specific interfaces rather than
just letting C++ take care of it; this should go away once there stops
being a common implementation class for those interfaces.
Bug: webrtc:13931
Change-Id: Ic4e279528a341bc0a0e88d2e1e76c90bc43a1035
Reviewed-on: https://webrtc-review.googlesource.com/c/src/+/287640
Reviewed-by: Florent Castelli <orphis@webrtc.org>
Commit-Queue: Harald Alvestrand <hta@webrtc.org>
Cr-Commit-Position: refs/heads/main@{#38888}
This is in the webrtc-stats spec at
https://www.w3.org/TR/webrtc-stats/#dom-rtcoutboundrtpstreamstats-scalabilitymode.
This adds the scalability mode to CodecSpecificInfo which is used to
plumb the modes for each simulcast layer.
TBR=orphis@webrtc.org
Tested: Compiled into Chrome and confirmed the scalability mode set for AV1, VP9, VP8 and H264 software encoders in chrome://webrtc-internals.
Bug: webrtc:14730
Change-Id: I71ceba8f6485a4f4a73e0856031b8d5f16f913f2
Reviewed-on: https://webrtc-review.googlesource.com/c/src/+/285085
Reviewed-by: Harald Alvestrand <hta@webrtc.org>
Reviewed-by: Åsa Persson <asapersson@webrtc.org>
Commit-Queue: Evan Shrubsole <eshr@webrtc.org>
Cr-Commit-Position: refs/heads/main@{#38847}
This reverts commit 75170be4acc90fece7c65f1a5b9bef03a5cc3880.
Reason for revert: Perf regression not affecting open source.
Original change's description:
> Revert "Remame VideoSendStream::UpdateActiveSimulcastLayers to StartPerRtpStream"
>
> This reverts commit d8c4de71722c9de38f942932be21d4015f32a3bc.
>
> Reason for revert: Tentative revert due to possible perf regression. b/260123362
>
> Original change's description:
> > Remame VideoSendStream::UpdateActiveSimulcastLayers to StartPerRtpStream
> >
> > VideoSendStreamImpl::Start and VideoSendStream::Start are not used by PeerConnections, only StartPerRtpStream.
> > Therefore this cl:
> > - Change implementation of VideoSendStream::Start to use VideoSendStream::StartPerRtpStream. VideoSendstream::Start is kept for convenience.
> > - Remove VideoSendStreamImpl::Start() since it was only used by tests that use call and is confusing.
> > - RtpVideoSender::SetActive is removed/changed to RtpVideoSender::Stop(). For normal operations RtpVideoSender::SetActiveModules is used.
> >
> > Bug: none
> > Change-Id: I43b153250b07c02fe63c84e3c4cec18d4ec0d47a
> > Reviewed-on: https://webrtc-review.googlesource.com/c/src/+/283660
> > Reviewed-by: Erik Språng <sprang@webrtc.org>
> > Commit-Queue: Per Kjellander <perkj@webrtc.org>
> > Cr-Commit-Position: refs/heads/main@{#38698}
>
> Bug: none
> Change-Id: I4f0d27679e51361b9ec54d2ae8e4d972527875d1
> Reviewed-on: https://webrtc-review.googlesource.com/c/src/+/284940
> Reviewed-by: Per Kjellander <perkj@webrtc.org>
> Commit-Queue: Erik Språng <sprang@webrtc.org>
> Auto-Submit: Per Kjellander <perkj@webrtc.org>
> Bot-Commit: rubber-stamper@appspot.gserviceaccount.com <rubber-stamper@appspot.gserviceaccount.com>
> Reviewed-by: Erik Språng <sprang@webrtc.org>
> Cr-Commit-Position: refs/heads/main@{#38725}
Bug: b/260400659
Change-Id: Ie8e545edcad85284a7d612183a8e4201672d0b5e
Reviewed-on: https://webrtc-review.googlesource.com/c/src/+/285900
Auto-Submit: Per Kjellander <perkj@webrtc.org>
Reviewed-by: Erik Språng <sprang@webrtc.org>
Commit-Queue: Erik Språng <sprang@webrtc.org>
Bot-Commit: rubber-stamper@appspot.gserviceaccount.com <rubber-stamper@appspot.gserviceaccount.com>
Cr-Commit-Position: refs/heads/main@{#38794}
This reverts commit d8c4de71722c9de38f942932be21d4015f32a3bc.
Reason for revert: Tentative revert due to possible perf regression. b/260123362
Original change's description:
> Remame VideoSendStream::UpdateActiveSimulcastLayers to StartPerRtpStream
>
> VideoSendStreamImpl::Start and VideoSendStream::Start are not used by PeerConnections, only StartPerRtpStream.
> Therefore this cl:
> - Change implementation of VideoSendStream::Start to use VideoSendStream::StartPerRtpStream. VideoSendstream::Start is kept for convenience.
> - Remove VideoSendStreamImpl::Start() since it was only used by tests that use call and is confusing.
> - RtpVideoSender::SetActive is removed/changed to RtpVideoSender::Stop(). For normal operations RtpVideoSender::SetActiveModules is used.
>
> Bug: none
> Change-Id: I43b153250b07c02fe63c84e3c4cec18d4ec0d47a
> Reviewed-on: https://webrtc-review.googlesource.com/c/src/+/283660
> Reviewed-by: Erik Språng <sprang@webrtc.org>
> Commit-Queue: Per Kjellander <perkj@webrtc.org>
> Cr-Commit-Position: refs/heads/main@{#38698}
Bug: none
Change-Id: I4f0d27679e51361b9ec54d2ae8e4d972527875d1
Reviewed-on: https://webrtc-review.googlesource.com/c/src/+/284940
Reviewed-by: Per Kjellander <perkj@webrtc.org>
Commit-Queue: Erik Språng <sprang@webrtc.org>
Auto-Submit: Per Kjellander <perkj@webrtc.org>
Bot-Commit: rubber-stamper@appspot.gserviceaccount.com <rubber-stamper@appspot.gserviceaccount.com>
Reviewed-by: Erik Språng <sprang@webrtc.org>
Cr-Commit-Position: refs/heads/main@{#38725}
VideoSendStreamImpl::Start and VideoSendStream::Start are not used by PeerConnections, only StartPerRtpStream.
Therefore this cl:
- Change implementation of VideoSendStream::Start to use VideoSendStream::StartPerRtpStream. VideoSendstream::Start is kept for convenience.
- Remove VideoSendStreamImpl::Start() since it was only used by tests that use call and is confusing.
- RtpVideoSender::SetActive is removed/changed to RtpVideoSender::Stop(). For normal operations RtpVideoSender::SetActiveModules is used.
Bug: none
Change-Id: I43b153250b07c02fe63c84e3c4cec18d4ec0d47a
Reviewed-on: https://webrtc-review.googlesource.com/c/src/+/283660
Reviewed-by: Erik Språng <sprang@webrtc.org>
Commit-Queue: Per Kjellander <perkj@webrtc.org>
Cr-Commit-Position: refs/heads/main@{#38698}
As the synchronous version only posts a task to recreate the encoder
later, it is not possible to catch errors and state changes that
could appear then.
The asynchronous version of SetParameters() aims to solve this by
providing a callback to wait for the completion of the encoder
reconfiguration, allowing any error to be propagate and subsequent
getParameters() call to have up to date information.
Bug: webrtc:11607
Change-Id: I5548e75aa14a97f8d9c0c94df1e72e9cd40887b2
Reviewed-on: https://webrtc-review.googlesource.com/c/src/+/278420
Reviewed-by: Harald Alvestrand <hta@webrtc.org>
Commit-Queue: Florent Castelli <orphis@webrtc.org>
Reviewed-by: Erik Språng <sprang@webrtc.org>
Cr-Commit-Position: refs/heads/main@{#38627}
The code for determining outbound-rtp.active assumed, as the spec says,
that we have one RtpEncodingParameters per RTP stream. Unfortunately
SVC is currently implemented as one RtpEncodingParameters per SVC
layer. This causes a discrepency where we do correctly only have one
outbound-rtp stats object, but the lookup to check whether or not we are
"active" needs to look at more than a single encoding.
The bug is that if SVC layers are {inactive, active, active} then
stats reports outbound-rtp.active: false. With this fix, active: true is
reported if ANY of the SVC layers are active.
For singlecast or simulcast this CL has no change in behavior. In these
cases we have the same number of outbound-rtp and encodings and a simple
ssrc lookup does work.
The fix is exercised by unit tests and has also manually been confirmed:
- Singlecast tested by https://jsfiddle.net/henbos/nvd6p4j1/.
- Simulcast tested by https://crbug.com/webrtc/14628#c11.
- SVC tested by Google Meet and chrome://webrtc-internals/.
Bug: webrtc:14628
Change-Id: Ib89945caf29c8f4b85dd8a1120dcf8279296e4a3
Reviewed-on: https://webrtc-review.googlesource.com/c/src/+/282222
Reviewed-by: Harald Alvestrand <hta@webrtc.org>
Commit-Queue: Henrik Boström <hbos@webrtc.org>
Cr-Commit-Position: refs/heads/main@{#38569}
and do the resolution of rids to layers. This has no effect yet
since the simulcast encoder adapter (SimulcastEncoderAdapter::Encode), the VP8 encoder (LibvpxVp8Encoder::Encode) and the OpenH264 encoder (H264EncoderImpl::Encode) all generate a key frame for all layers whenever a key frame is requested on one layer.
BUG=chromium:1354101
Change-Id: I13f5f1bf136839a68942b0f6bf4f2d5890415250
Reviewed-on: https://webrtc-review.googlesource.com/c/src/+/280945
Commit-Queue: Ilya Nikolaevskiy <ilnik@webrtc.org>
Commit-Queue: Philipp Hancke <phancke@microsoft.com>
Reviewed-by: Ilya Nikolaevskiy <ilnik@webrtc.org>
Reviewed-by: Harald Alvestrand <hta@webrtc.org>
Cr-Commit-Position: refs/heads/main@{#38565}
These metrics were not only non-standard, but residing in the
non-standard "track" stats object that we want to delete. As per
https://github.com/w3c/webrtc-stats/issues/695#issuecomment-1259611462
these metrics are no longer needed because we already have
inbound-rtp.totalInterFrameDelay/totalSquaredInterFrameDelay which is
basically the same thing.
// mac_rel infra failures are unrelated
NOTRY=True
Bug: webrtc:14522
Change-Id: I565da42514a93f15532ba8357dd006547a5296ee
Reviewed-on: https://webrtc-review.googlesource.com/c/src/+/278100
Commit-Queue: Henrik Boström <hbos@webrtc.org>
Reviewed-by: Harald Alvestrand <hta@webrtc.org>
Cr-Commit-Position: refs/heads/main@{#38509}
This metric was always supposed to be the spec's answer to
googBucketDelay, and is defined as "The total number of seconds that
packets have spent buffered locally before being transmitted onto the
network." But our implementation measured the time between capture and
send, including encode time. This is incorrect and yields a much larger
value than expected.
This CL updated the metric to do what the spec says. Implementation-wise
we measure the time between pushing and popping each packet from the
queue (in modules/pacing/prioritized_packet_queue.cc).
The spec says to increment the delay counter at the same time as we
increment the packet counter in order for the app to be able to do
"delta totalPacketSendDelay / delta packetSent". For this reason,
`total_packet_delay` is added to RtpPacketCounter. (Previously, the
two counters were incremented on different threads and observers.)
Running Google Meet on a good network, I could observe a 2-3 ms average
send delay per packet with this implementation compared to 20-30 ms
with the old implementation. See b/137014977#comment170 for comparison
with googBucketDelay which is a little bit different by design -
totalPacketSendDelay is clearly better than googBucketDelay.
Since none of this depend on the media kind, we can wire up this metric
for audio as well in a follow-up:
https://webrtc-review.googlesource.com/c/src/+/280523
Bug: webrtc:14593
Change-Id: If8fcd82fee74030d0923ee5df2c2aea2264600d4
Reviewed-on: https://webrtc-review.googlesource.com/c/src/+/280443
Reviewed-by: Erik Språng <sprang@webrtc.org>
Commit-Queue: Henrik Boström <hbos@webrtc.org>
Cr-Commit-Position: refs/heads/main@{#38480}
defined in
https://w3c.github.io/webrtc-encoded-transform/#rtcrtpsender-extension
Note: this does not implement the "rid(s)" parameter which will be done in a future CL.
VP8 still synchronizes keyframes on all layers even when asked for ones on individual layers while H264 (when implemented as three different encoders in SimulcastEncoderAdapter) can actually utilize this.
This does not change the behavior when receiving a RTCP PLI for a particular layer.
BUG=chromium:1354101
Change-Id: Ic8b14d155242e32c9aeafa55fe6652f346ac76b8
Reviewed-on: https://webrtc-review.googlesource.com/c/src/+/274169
Reviewed-by: Harald Alvestrand <hta@webrtc.org>
Reviewed-by: Ilya Nikolaevskiy <ilnik@webrtc.org>
Commit-Queue: Philipp Hancke <phancke@microsoft.com>
Cr-Commit-Position: refs/heads/main@{#38472}
ScalabilityMode should be validated against the currently
allowed codecs or the currently used codec.
Bug: webrtc:11607
Change-Id: Id2e6cbfad4f089de450150e1203657ed316e2f29
Reviewed-on: https://webrtc-review.googlesource.com/c/src/+/277403
Commit-Queue: Florent Castelli <orphis@webrtc.org>
Reviewed-by: Harald Alvestrand <hta@webrtc.org>
Cr-Commit-Position: refs/heads/main@{#38433}
This cl/ changes so that the EncoderStreamFactory is
not created inside WebRtcVideoSendStream (webrtc_video_engine).
The benifit of this is that the VideoStreamEncoder can then
amend the EncoderStreamFactory with state (and types)
w/o exposing it in VideoEncoderConfig.
I.e as an alternative to changes done inside
https://webrtc-review.googlesource.com/c/src/+/276742.
The fake_webrtc_call is modified to (if needed) create
it's own EncoderStreamFactory if needed.
Note: this cl/ will have to be merged with with
https://webrtc-review.googlesource.com/c/src/+/277002.
Bug: webrtc:14451
Change-Id: I3d896b227d39725ba6409622e8d09d14bd45d5fe
Reviewed-on: https://webrtc-review.googlesource.com/c/src/+/277160
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@{#38237}
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}
This moves the ownership away from VideoReceiveStream2 and closer to
VCMDecoderDataBase. That facilitates unregistration (upcoming change)
without recreating receive streams.
Bug: none
Change-Id: I812175134730a0ffbf7077fd149c8489481c73d8
Reviewed-on: https://webrtc-review.googlesource.com/c/src/+/272481
Reviewed-by: Danil Chapovalov <danilchap@webrtc.org>
Commit-Queue: Tomas Gunnarsson <tommi@webrtc.org>
Cr-Commit-Position: refs/heads/main@{#37866}
A follow-up change will combine the setters for ulpfec and red payload
types, since they're entwined.
Bug: webrtc:11993
Change-Id: Ifea7fe9f4ebc7ac88a62db6cd6748f4d3c20db4c
Reviewed-on: https://webrtc-review.googlesource.com/c/src/+/271482
Reviewed-by: Rasmus Brandt <brandtr@webrtc.org>
Commit-Queue: Tomas Gunnarsson <tommi@webrtc.org>
Cr-Commit-Position: refs/heads/main@{#37785}
