Perf dashboard show a very minor change with the new pacer, for tests
that use flexfec. I have found that previously fec was in fact
prioritized at the same level as video, see eg PacketTypeToPriority()
in RTPSender.
With the new pacer we put fec in between video and padding.
Not sure if this is in fact an actual problem. In the non-loss case
the frame latency should actually be slighly lower, but on the other
hand if we have loss fec won't be applied until after the full frame
has been sent and so we may end up sending NACK before we apply the
FEC and recover a packet.
Just to avoid any problems let's revert to the old behavior.
Bug: webrtc:10633
Change-Id: I9a4210a64165a6e376c0c70ccaa07b0688cc58a5
Reviewed-on: https://webrtc-review.googlesource.com/c/src/+/146714
Reviewed-by: Sebastian Jansson <srte@webrtc.org>
Commit-Queue: Erik Språng <sprang@webrtc.org>
Cr-Commit-Position: refs/heads/master@{#28678}
This reverts commit fab3460a821abe336ab610c6d6dfc0d392dac263.
Reason for revert: fix downstream instead
Original change's description:
> Revert "Reland "Add plumbing of RtpPacketInfos to each AudioFrame as input for SourceTracker.""
>
> This reverts commit 9973933d2e606d64fcdc753acb9ba3afd6e30569.
>
> Reason for revert: breaking downstream projects and not reviewed by direct owners
>
> Original change's description:
> > Reland "Add plumbing of RtpPacketInfos to each AudioFrame as input for SourceTracker."
> >
> > This reverts commit 24192c267a40eb7d6b1850489ccdbf7a84f8ff0f.
> >
> > Reason for revert: Analyzed the performance regression in more detail.
> >
> > Most of the regression comes from the extra RtpPacketInfos-related memory allocations in every `NetEq::GetAudio()` call. Commit 1796a820f60cb9429bf4bcf13a40a41794ac8fb0 has removed roughly 2/3rds of the extra allocations from the impacted perf tests. Remaining perf impact is expected to be about "8 microseconds of CPU time per second" on the Linux benchmarking machines and "15 us per second" on Windows/Mac.
> >
> > There are options to optimize further but they are unlikely worth doing. Note for example that `NetEqPerformanceTest` uses the PCM codec while the real-world use cases would likely use the much heavier Opus codec. The numbers from `OpusSpeedTest` and `NetEqPerformanceTest` suggest that Opus decoding is about 10x as expensive as NetEq overall.
> >
> > Original change's description:
> > > Revert "Add plumbing of RtpPacketInfos to each AudioFrame as input for SourceTracker."
> > >
> > > This reverts commit 3e8ef940fe86cf6285afb80e68d2a0bedc631b9f.
> > >
> > > Reason for revert: This CL causes a performance regression in NetEq, see https://bugs.chromium.org/p/chromium/issues/detail?id=982260.
> > >
> > > Original change's description:
> > > > Add plumbing of RtpPacketInfos to each AudioFrame as input for SourceTracker.
> > > >
> > > > This change adds the plumbing of RtpPacketInfo from ChannelReceive::OnRtpPacket() to ChannelReceive::GetAudioFrameWithInfo() for audio. It is a step towards replacing the non-spec compliant ContributingSources that updates itself at packet-receive time, with the spec-compliant SourceTracker that will update itself at frame-delivery-to-track time.
> > > >
> > > > Bug: webrtc:10668
> > > > Change-Id: I03385d6865bbc7bfbef7634f88de820a934f787a
> > > > Reviewed-on: https://webrtc-review.googlesource.com/c/src/+/139890
> > > > Reviewed-by: Stefan Holmer <stefan@webrtc.org>
> > > > Reviewed-by: Minyue Li <minyue@webrtc.org>
> > > > Commit-Queue: Chen Xing <chxg@google.com>
> > > > Cr-Commit-Position: refs/heads/master@{#28434}
> > >
> > > TBR=kwiberg@webrtc.org,stefan@webrtc.org,minyue@webrtc.org,chxg@google.com
> > >
> > > Bug: webrtc:10668, chromium:982260
> > > Change-Id: I5e2cfde78c59d1123e21869564d76ed3f6193a5c
> > > Reviewed-on: https://webrtc-review.googlesource.com/c/src/+/145339
> > > Reviewed-by: Ivo Creusen <ivoc@webrtc.org>
> > > Commit-Queue: Ivo Creusen <ivoc@webrtc.org>
> > > Cr-Commit-Position: refs/heads/master@{#28561}
> >
> > TBR=kwiberg@webrtc.org,stefan@webrtc.org,ivoc@webrtc.org,minyue@webrtc.org,chxg@google.com
> >
> > # Not skipping CQ checks because original CL landed > 1 day ago.
> >
> > Bug: webrtc:10668, chromium:982260
> > Change-Id: Ie375a0b327ee368317bf3a04b2f1415c3a974470
> > Reviewed-on: https://webrtc-review.googlesource.com/c/src/+/146707
> > Reviewed-by: Stefan Holmer <stefan@webrtc.org>
> > Commit-Queue: Chen Xing <chxg@google.com>
> > Cr-Commit-Position: refs/heads/master@{#28664}
>
> TBR=kwiberg@webrtc.org,stefan@webrtc.org,ivoc@webrtc.org,minyue@webrtc.org,chxg@google.com
>
> Change-Id: I652cb0814d83b514d3bee34e65ca3bb693099b22
> No-Presubmit: true
> No-Tree-Checks: true
> No-Try: true
> Bug: webrtc:10668, chromium:982260
> Reviewed-on: https://webrtc-review.googlesource.com/c/src/+/146712
> Reviewed-by: Alessio Bazzica <alessiob@webrtc.org>
> Commit-Queue: Alessio Bazzica <alessiob@webrtc.org>
> Cr-Commit-Position: refs/heads/master@{#28671}
TBR=alessiob@webrtc.org,kwiberg@webrtc.org,stefan@webrtc.org,ivoc@webrtc.org,minyue@webrtc.org,chxg@google.com
Change-Id: Id43b7b3da79b4f48004b41767482bae1c1fa1e16
No-Presubmit: true
No-Tree-Checks: true
No-Try: true
Bug: webrtc:10668, chromium:982260
Reviewed-on: https://webrtc-review.googlesource.com/c/src/+/146713
Reviewed-by: Alessio Bazzica <alessiob@webrtc.org>
Commit-Queue: Alessio Bazzica <alessiob@webrtc.org>
Cr-Commit-Position: refs/heads/master@{#28672}
This reverts commit 9973933d2e606d64fcdc753acb9ba3afd6e30569.
Reason for revert: breaking downstream projects and not reviewed by direct owners
Original change's description:
> Reland "Add plumbing of RtpPacketInfos to each AudioFrame as input for SourceTracker."
>
> This reverts commit 24192c267a40eb7d6b1850489ccdbf7a84f8ff0f.
>
> Reason for revert: Analyzed the performance regression in more detail.
>
> Most of the regression comes from the extra RtpPacketInfos-related memory allocations in every `NetEq::GetAudio()` call. Commit 1796a820f60cb9429bf4bcf13a40a41794ac8fb0 has removed roughly 2/3rds of the extra allocations from the impacted perf tests. Remaining perf impact is expected to be about "8 microseconds of CPU time per second" on the Linux benchmarking machines and "15 us per second" on Windows/Mac.
>
> There are options to optimize further but they are unlikely worth doing. Note for example that `NetEqPerformanceTest` uses the PCM codec while the real-world use cases would likely use the much heavier Opus codec. The numbers from `OpusSpeedTest` and `NetEqPerformanceTest` suggest that Opus decoding is about 10x as expensive as NetEq overall.
>
> Original change's description:
> > Revert "Add plumbing of RtpPacketInfos to each AudioFrame as input for SourceTracker."
> >
> > This reverts commit 3e8ef940fe86cf6285afb80e68d2a0bedc631b9f.
> >
> > Reason for revert: This CL causes a performance regression in NetEq, see https://bugs.chromium.org/p/chromium/issues/detail?id=982260.
> >
> > Original change's description:
> > > Add plumbing of RtpPacketInfos to each AudioFrame as input for SourceTracker.
> > >
> > > This change adds the plumbing of RtpPacketInfo from ChannelReceive::OnRtpPacket() to ChannelReceive::GetAudioFrameWithInfo() for audio. It is a step towards replacing the non-spec compliant ContributingSources that updates itself at packet-receive time, with the spec-compliant SourceTracker that will update itself at frame-delivery-to-track time.
> > >
> > > Bug: webrtc:10668
> > > Change-Id: I03385d6865bbc7bfbef7634f88de820a934f787a
> > > Reviewed-on: https://webrtc-review.googlesource.com/c/src/+/139890
> > > Reviewed-by: Stefan Holmer <stefan@webrtc.org>
> > > Reviewed-by: Minyue Li <minyue@webrtc.org>
> > > Commit-Queue: Chen Xing <chxg@google.com>
> > > Cr-Commit-Position: refs/heads/master@{#28434}
> >
> > TBR=kwiberg@webrtc.org,stefan@webrtc.org,minyue@webrtc.org,chxg@google.com
> >
> > Bug: webrtc:10668, chromium:982260
> > Change-Id: I5e2cfde78c59d1123e21869564d76ed3f6193a5c
> > Reviewed-on: https://webrtc-review.googlesource.com/c/src/+/145339
> > Reviewed-by: Ivo Creusen <ivoc@webrtc.org>
> > Commit-Queue: Ivo Creusen <ivoc@webrtc.org>
> > Cr-Commit-Position: refs/heads/master@{#28561}
>
> TBR=kwiberg@webrtc.org,stefan@webrtc.org,ivoc@webrtc.org,minyue@webrtc.org,chxg@google.com
>
> # Not skipping CQ checks because original CL landed > 1 day ago.
>
> Bug: webrtc:10668, chromium:982260
> Change-Id: Ie375a0b327ee368317bf3a04b2f1415c3a974470
> Reviewed-on: https://webrtc-review.googlesource.com/c/src/+/146707
> Reviewed-by: Stefan Holmer <stefan@webrtc.org>
> Commit-Queue: Chen Xing <chxg@google.com>
> Cr-Commit-Position: refs/heads/master@{#28664}
TBR=kwiberg@webrtc.org,stefan@webrtc.org,ivoc@webrtc.org,minyue@webrtc.org,chxg@google.com
Change-Id: I652cb0814d83b514d3bee34e65ca3bb693099b22
No-Presubmit: true
No-Tree-Checks: true
No-Try: true
Bug: webrtc:10668, chromium:982260
Reviewed-on: https://webrtc-review.googlesource.com/c/src/+/146712
Reviewed-by: Alessio Bazzica <alessiob@webrtc.org>
Commit-Queue: Alessio Bazzica <alessiob@webrtc.org>
Cr-Commit-Position: refs/heads/master@{#28671}
In https://webrtc-review.googlesource.com/c/src/+/138275
the congestion window was recalculated during OnProcessInterval, as
to consider the case when downlink is down. However, this update
was not propagated to the congestion window pusback controller,
nor returned in the update.
This patch fixes that issue, as well as adding two tests to ensure
the behaviour works as expected.
Bug: None
Change-Id: Ic126d929dc7a7a3393a2f34a4682eea1ee1f2240
Reviewed-on: https://webrtc-review.googlesource.com/c/src/+/146704
Commit-Queue: Evan Shrubsole <eshr@google.com>
Reviewed-by: Sebastian Jansson <srte@webrtc.org>
Cr-Commit-Position: refs/heads/master@{#28667}
This reverts commit 24192c267a40eb7d6b1850489ccdbf7a84f8ff0f.
Reason for revert: Analyzed the performance regression in more detail.
Most of the regression comes from the extra RtpPacketInfos-related memory allocations in every `NetEq::GetAudio()` call. Commit 1796a820f60cb9429bf4bcf13a40a41794ac8fb0 has removed roughly 2/3rds of the extra allocations from the impacted perf tests. Remaining perf impact is expected to be about "8 microseconds of CPU time per second" on the Linux benchmarking machines and "15 us per second" on Windows/Mac.
There are options to optimize further but they are unlikely worth doing. Note for example that `NetEqPerformanceTest` uses the PCM codec while the real-world use cases would likely use the much heavier Opus codec. The numbers from `OpusSpeedTest` and `NetEqPerformanceTest` suggest that Opus decoding is about 10x as expensive as NetEq overall.
Original change's description:
> Revert "Add plumbing of RtpPacketInfos to each AudioFrame as input for SourceTracker."
>
> This reverts commit 3e8ef940fe86cf6285afb80e68d2a0bedc631b9f.
>
> Reason for revert: This CL causes a performance regression in NetEq, see https://bugs.chromium.org/p/chromium/issues/detail?id=982260.
>
> Original change's description:
> > Add plumbing of RtpPacketInfos to each AudioFrame as input for SourceTracker.
> >
> > This change adds the plumbing of RtpPacketInfo from ChannelReceive::OnRtpPacket() to ChannelReceive::GetAudioFrameWithInfo() for audio. It is a step towards replacing the non-spec compliant ContributingSources that updates itself at packet-receive time, with the spec-compliant SourceTracker that will update itself at frame-delivery-to-track time.
> >
> > Bug: webrtc:10668
> > Change-Id: I03385d6865bbc7bfbef7634f88de820a934f787a
> > Reviewed-on: https://webrtc-review.googlesource.com/c/src/+/139890
> > Reviewed-by: Stefan Holmer <stefan@webrtc.org>
> > Reviewed-by: Minyue Li <minyue@webrtc.org>
> > Commit-Queue: Chen Xing <chxg@google.com>
> > Cr-Commit-Position: refs/heads/master@{#28434}
>
> TBR=kwiberg@webrtc.org,stefan@webrtc.org,minyue@webrtc.org,chxg@google.com
>
> Bug: webrtc:10668, chromium:982260
> Change-Id: I5e2cfde78c59d1123e21869564d76ed3f6193a5c
> Reviewed-on: https://webrtc-review.googlesource.com/c/src/+/145339
> Reviewed-by: Ivo Creusen <ivoc@webrtc.org>
> Commit-Queue: Ivo Creusen <ivoc@webrtc.org>
> Cr-Commit-Position: refs/heads/master@{#28561}
TBR=kwiberg@webrtc.org,stefan@webrtc.org,ivoc@webrtc.org,minyue@webrtc.org,chxg@google.com
# Not skipping CQ checks because original CL landed > 1 day ago.
Bug: webrtc:10668, chromium:982260
Change-Id: Ie375a0b327ee368317bf3a04b2f1415c3a974470
Reviewed-on: https://webrtc-review.googlesource.com/c/src/+/146707
Reviewed-by: Stefan Holmer <stefan@webrtc.org>
Commit-Queue: Chen Xing <chxg@google.com>
Cr-Commit-Position: refs/heads/master@{#28664}
This interface is intended to only handle packet-sending parts of the
paced sender.
See https://webrtc-review.googlesource.com/c/src/+/145212 for context
Bug: webrtc:10809
Change-Id: I93f0b40e1865665c2d436db67021350a0ed0687b
Reviewed-on: https://webrtc-review.googlesource.com/c/src/+/145216
Commit-Queue: Erik Språng <sprang@webrtc.org>
Reviewed-by: Stefan Holmer <stefan@webrtc.org>
Reviewed-by: Sebastian Jansson <srte@webrtc.org>
Cr-Commit-Position: refs/heads/master@{#28662}
This makes the code path where packets are directly owned by PacedSender
rather that being temporarily put in the RtpPacketHistory the default.
Functionally, this should essentially be a noop, with only minor timing
differences.
The old code-path will stay around for a short while and then be
removed once we are certain there are no regressions.
Bug: webrtc:10633
Change-Id: Id6360dea48fd0c9d46fde6f5eee93726d4f11d13
Reviewed-on: https://webrtc-review.googlesource.com/c/src/+/146212
Commit-Queue: Erik Språng <sprang@webrtc.org>
Reviewed-by: Stefan Holmer <stefan@webrtc.org>
Cr-Commit-Position: refs/heads/master@{#28660}
If the SSRC of an RTP module is changed at runtime, we may get conflicts
with packets already there. Eg:
* Put seq# 123 in the history for SSRC 1.
* Change the SSRC to 2.
* Send a NACK for seq# 123 from SSRC 2.
Currently, we will respond with the packet belonging to SSRC 1 (and not
if the NACK specifies SSRC 1, to boot).
We can gen similar issues if the sequence number is changed, where
half frame are left in the buffer.
In these cases, the stream is likely being reset so we should just
clear the packet history too.
Bug: webrtc:10794
Change-Id: I28147c2532cf1c78840d4808c4366d4a647541f7
Reviewed-on: https://webrtc-review.googlesource.com/c/src/+/145729
Commit-Queue: Erik Språng <sprang@webrtc.org>
Reviewed-by: Stefan Holmer <stefan@webrtc.org>
Cr-Commit-Position: refs/heads/master@{#28658}
This CL fixes two issues related to the TransmissionOffset header
extension and the new (not yet active) pacer mode.
Previously capture time (if unset) would be populated when put into the
packet history before entering the pacer. Since the pacer now owns the
packets, this does not occur until packet is actually sent, if at all.
Capture has really nothing to do with the packet history, this should
be set by the RtpSender pre-pacing instead.
Furthermore, for retransmissions the old path would take the capture
time from the original packet, build the RTX-wrapped retransmission and
set the toffset extension of the RTX packet using that captured capture
time. Since RTX packets are now fully built before the pacer, this does
not work, and we need to transfer the capture time from the original to
the RTX packet instead.
Bug: webrtc:10633
Change-Id: I031e8b6cc4ab20fb094dbd46720829b78951e7f9
Reviewed-on: https://webrtc-review.googlesource.com/c/src/+/146218
Commit-Queue: Erik Språng <sprang@webrtc.org>
Reviewed-by: Stefan Holmer <stefan@webrtc.org>
Cr-Commit-Position: refs/heads/master@{#28657}
This adds the RemoteEstimate rtcp packet and wires it up to GoogCC where
it's used to improve congestion controller behavior.
The functionality is negotiated using SDP.
It's added with a field trial that allow disabling the functionality in
case there's any issues.
Bug: webrtc:10742
Change-Id: I1ea8e4216a27cd2b00505c99b42d1e38726256c8
Reviewed-on: https://webrtc-review.googlesource.com/c/src/+/146602
Reviewed-by: Stefan Holmer <stefan@webrtc.org>
Reviewed-by: Erik Språng <sprang@webrtc.org>
Commit-Queue: Sebastian Jansson <srte@webrtc.org>
Cr-Commit-Position: refs/heads/master@{#28654}
The code was doing nothing except for triggering thread sanitizer,
since concurrent writes weren't guarded:
* ReadRecordedData() through webrtc_audio_module_rec_thread
* InitPlayout() through main thread
Bug: webrtc:9751
Change-Id: I7ecf4fa436ff0695e5b998d7e3f159fb6c7e9214
Reviewed-on: https://webrtc-review.googlesource.com/c/src/+/146216
Reviewed-by: Henrik Andreassson <henrika@webrtc.org>
Commit-Queue: Yves Gerey <yvesg@google.com>
Cr-Commit-Position: refs/heads/master@{#28636}
Lifetime issue: "webrtc_audio_module_rec_thread" was still accessing
AudioTransport mock at and after its destruction.
Bug: webrtc:9751
Change-Id: I24308077cdeb77e570b8ec74098f1ae3397b7155
Reviewed-on: https://webrtc-review.googlesource.com/c/src/+/146217
Reviewed-by: Henrik Andreassson <henrika@webrtc.org>
Commit-Queue: Yves Gerey <yvesg@google.com>
Cr-Commit-Position: refs/heads/master@{#28635}
It's not currently used and it complicates receive side estimation.
Bug: webrtc:10742
Change-Id: Iaa3c86807c7b637aea3ff393e728dc91eac23db6
Reviewed-on: https://webrtc-review.googlesource.com/c/src/+/145724
Reviewed-by: Jonas Olsson <jonasolsson@webrtc.org>
Commit-Queue: Sebastian Jansson <srte@webrtc.org>
Cr-Commit-Position: refs/heads/master@{#28623}
Rationale:
* More explicit (you won't miss that when glancing at the code).
* More consistent (see MAYBE_* in other tests).
* Allow to re-activate tests via CLI (--gtest_also_run_disabled_tests).
* Tests won't wrongly show up as PASSING (bug/webrtc:10819),
since they won't show up at all.
Bug: webrtc:9778
Change-Id: Ic32e18cb8ee2352def95206c2aa66e1dea0cc1e3
Reviewed-on: https://webrtc-review.googlesource.com/c/src/+/146200
Reviewed-by: Henrik Andreassson <henrika@webrtc.org>
Commit-Queue: Yves Gerey <yvesg@google.com>
Cr-Commit-Position: refs/heads/master@{#28617}
This CL removes the field trial left in place as a kill-switch in case
there were any regressions related to selecting payload padding based
on the likelihood of being useful instead of matching size.
It also removes the functionality that was only enabled with the
kill-switch active.
The feature has been default-on since June 23rd 2019:
https://webrtc.googlesource.com/src.git/+/214f54365ec210db76218a35ead66c9ce23e068e
Since we have not observed any issues, let's clean this code up.
Bug: webrtc:8975
Change-Id: I7f49fe354227b3f6566a250332e56b6d70fe2f09
Reviewed-on: https://webrtc-review.googlesource.com/c/src/+/145821
Commit-Queue: Erik Språng <sprang@webrtc.org>
Reviewed-by: Stefan Holmer <stefan@webrtc.org>
Cr-Commit-Position: refs/heads/master@{#28616}
This CL fixes two things related to the (not yet active) new
PacedSender code path:
1. Make sure BWE header extensions are properly populated for all
padding packets.
2. When generating padding, don't hold the RtpSender critsect when
accessing the RtpPacketHistory as this may lead to a lock order
inversion.
Bug: webrtc:10633
Change-Id: I8650fbf5dafddbeae61837d2137338163e1c48ce
Reviewed-on: https://webrtc-review.googlesource.com/c/src/+/145723
Reviewed-by: Stefan Holmer <stefan@webrtc.org>
Commit-Queue: Erik Språng <sprang@webrtc.org>
Cr-Commit-Position: refs/heads/master@{#28613}
This is the last CL required to migrate WebRTC to ABSL_FLAG, rtc::Flag
will be removed soon after this one lands.
Bug: webrtc:10616
Change-Id: I2807cec39e28a2737d2c49e2dc23f2a6f98d08f0
Reviewed-on: https://webrtc-review.googlesource.com/c/src/+/145727
Reviewed-by: Tommi <tommi@webrtc.org>
Commit-Queue: Mirko Bonadei <mbonadei@webrtc.org>
Cr-Commit-Position: refs/heads/master@{#28606}
Landing with TBR given vacation times and the fact that none of this
code is active "in production". The ADM2 implementation can be seen
as experimental (non-default) code and it takes some work to enable it
and replace the existing ADM. Hence, extremely low risk to break
anything.
TBR: henrik.lundin
Bug: webrtc:9265
Change-Id: Ibc9a57f4851bf4b890b77b9eaef1dfbe3ca86f83
Reviewed-on: https://webrtc-review.googlesource.com/c/src/+/146084
Reviewed-by: Henrik Andreassson <henrika@webrtc.org>
Commit-Queue: Henrik Andreassson <henrika@webrtc.org>
Cr-Commit-Position: refs/heads/master@{#28601}
Landing with TBR given vacation times and the fact that none of this
code is active "in production". The ADM2 implementation can be seen
as experimental (non-default) code and it takes some work to enable it
and replace the existing ADM. Hence, extremely low risk to break
anything.
TBR: henrik.lundin
Bug: webrtc:9265
Change-Id: Ia5cfb2aaa8eaf9537b916b3375f55d8df6287071
Reviewed-on: https://webrtc-review.googlesource.com/c/src/+/145921
Reviewed-by: Henrik Andreassson <henrika@webrtc.org>
Commit-Queue: Henrik Andreassson <henrika@webrtc.org>
Cr-Commit-Position: refs/heads/master@{#28600}
The simulcast allocator would only set bitrates for the first 2 layers
in conference_screenshare_mode.
That would trigger an issue in the VP8 encoder initialization that expects
to have growing bitrates for the layers (3rd layer would have the same
bitrate as the 2nd one).
Bug: webrtc:8785
Change-Id: Ic6c940b78022387841b28074b373be6b2f45cb15
Reviewed-on: https://webrtc-review.googlesource.com/c/src/+/145922
Reviewed-by: Ilya Nikolaevskiy <ilnik@webrtc.org>
Commit-Queue: Florent Castelli <orphis@webrtc.org>
Cr-Commit-Position: refs/heads/master@{#28598}
Marking test as skipped is more honest than pretending it is successful!
Prevent confusion like in the following scenario for one given test:
- ubsan: launched and sometimes failing.
- tsan: never launched but always flagged OK.
Bug: webrtc:9778
Change-Id: Ie0be0759347eabd3c9d29dd5ea2de809511d1b97
Reviewed-on: https://webrtc-review.googlesource.com/c/src/+/145980
Reviewed-by: Henrik Andreassson <henrika@webrtc.org>
Reviewed-by: Sami Kalliomäki <sakal@webrtc.org>
Commit-Queue: Yves Gerey <yvesg@google.com>
Cr-Commit-Position: refs/heads/master@{#28597}
This CL fixes a bug affecting the new PacedSender code path (not yet
active): keepalive packets will always be sent, even if sending is
paused or network is congested.
Mainly though, it refactors all the PacedSender unittests and makes
sure all of them (as applicable) are run using both the old and the new
code paths.
Bug: webrtc:10633
Change-Id: I531939b719a11147a2a7dbab77dfcccaf7a42d83
Reviewed-on: https://webrtc-review.googlesource.com/c/src/+/145721
Reviewed-by: Sebastian Jansson <srte@webrtc.org>
Commit-Queue: Erik Språng <sprang@webrtc.org>
Cr-Commit-Position: refs/heads/master@{#28578}
Prevent clang-format to touch these two files,
as the result doesn't honor ColumnLimit setting.
Bug: webrtc:9340
Change-Id: I9b692a82df5385fa2d1216d915898439234b34b1
Reviewed-on: https://webrtc-review.googlesource.com/c/src/+/145720
Reviewed-by: Karl Wiberg <kwiberg@webrtc.org>
Reviewed-by: Oskar Sundbom <ossu@webrtc.org>
Commit-Queue: Yves Gerey <yvesg@google.com>
Cr-Commit-Position: refs/heads/master@{#28577}
New PacedSender code path should have the transport sequence numbers
start at 1, just like before. At least some integration tests seem to
depend on this behavior so just to make sure there aren't any parts
doing the same that will break, let's keep the old behavior.
Bug: webrtc:10633
Change-Id: I87290fbfd4ecb2ef04bd5e49a955d0b2cc8d59b6
Reviewed-on: https://webrtc-review.googlesource.com/c/src/+/145722
Reviewed-by: Sebastian Jansson <srte@webrtc.org>
Commit-Queue: Erik Språng <sprang@webrtc.org>
Cr-Commit-Position: refs/heads/master@{#28573}
On Win8.1 and Win10 when the window is occluded the cropping capturer
falls back to the PrintWindow API. But without a special flag it
fails when trying to capture apps that are using DirectComposition.
So just pass this undocumented flag named PW_RENDERFULLCONTENT to fix
the fallback case of the cropping capturer.
Due to new app framing on Win8 and Win10 the shadow of the window
are captured as black like if for the maximize case on Win7. So
just use the utility function webrtc::GetCroppedWindowRect and
remove the local GetWindowDrawableRect helper. The former returns
the same result as the later on Win7 so no real change is made here,
just that we make the WindowCapturerWin compatible with newer Windows.
Bug: webrtc:10734
Change-Id: Idb793ca0691261042569c30410669c4a5ad0c8ec
Reviewed-on: https://webrtc-review.googlesource.com/c/src/+/144960
Commit-Queue: Julien Isorce <julien.isorce@chromium.org>
Reviewed-by: Tommi <tommi@webrtc.org>
Cr-Commit-Position: refs/heads/master@{#28570}
This reverts commit 11820502b840f34aa9fccb1c273cefbcc457c962.
Reason for revert: The culprit was https://webrtc-review.googlesource.com/c/src/+/133169.
Original change's description:
> Revert "Make new pacer padding more like old one"
>
> This reverts commit bb7727211c535f8a9dce27891941b52b6ea8e750.
>
> Reason for revert: Speculative revert (some perf test are failing)
>
> Original change's description:
> > Make new pacer padding more like old one
> >
> > The (currently unused) new pacer code path was implemented with what
> > was intended as a more careful padding strategy.
> > Unfortunately this doesn't work as well as expected due to the fact
> > that the padding budget cannot build up underuse.
> >
> > I made another CL that could fix that, but I think it adds complexity
> > for dubious gains. It also will make it more difficult to find any
> > potential regression when switching to the new path, should one occur.
> > See https://webrtc-review.googlesource.com/c/src/+/144563
> >
> > Therefore, this CL makes the new code path choose RTX payload in the
> > same way as is currently done.
> >
> > Bug: webrtc:10633
> > Change-Id: If2115d4fa7463add959faa77c63101286c27e0f5
> > Reviewed-on: https://webrtc-review.googlesource.com/c/src/+/145202
> > Reviewed-by: Stefan Holmer <stefan@webrtc.org>
> > Commit-Queue: Erik Språng <sprang@webrtc.org>
> > Cr-Commit-Position: refs/heads/master@{#28537}
>
> TBR=sprang@webrtc.org,stefan@webrtc.org
>
> Change-Id: I99b72858414e0a245da596d94694449da88fd626
> No-Presubmit: true
> No-Tree-Checks: true
> No-Try: true
> Bug: webrtc:10633
> Reviewed-on: https://webrtc-review.googlesource.com/c/src/+/145324
> Reviewed-by: Mirko Bonadei <mbonadei@webrtc.org>
> Commit-Queue: Mirko Bonadei <mbonadei@webrtc.org>
> Cr-Commit-Position: refs/heads/master@{#28550}
TBR=mbonadei@webrtc.org,sprang@webrtc.org,stefan@webrtc.org
Change-Id: I1fff79d75dc888921c6fbfc7f3980395a67e1c1a
No-Presubmit: true
No-Tree-Checks: true
No-Try: true
Bug: webrtc:10633
Reviewed-on: https://webrtc-review.googlesource.com/c/src/+/145403
Reviewed-by: Mirko Bonadei <mbonadei@webrtc.org>
Commit-Queue: Mirko Bonadei <mbonadei@webrtc.org>
Cr-Commit-Position: refs/heads/master@{#28565}
This reverts commit 66147e892dd6b7b1beaddbcab456a1ce28b2ad22.
Reason for revert: The culprit was https://webrtc-review.googlesource.com/c/src/+/133169.
Original change's description:
> Revert "Optimize PacketRouter/RTPSender interactions."
>
> This reverts commit 6f129b3b7605dc69c8c188ca02d133250130570e.
>
> Reason for revert: Speculative revert (some perf test are failing)
>
> Original change's description:
> > Optimize PacketRouter/RTPSender interactions.
> >
> > The legacy code-path uses a hashmap as cache in order to speed up
> > finding the right rtp module to send on. The new path should use that
> > as well.
> > In addition, there are checks that verify if an RTP module can send
> > padding, in some cases payload based. These result in a number of
> > calls to methods in RTPSender requiring its lock to be taken. This CL
> > introduces a combined SupportsPadding() check method which performs
> > all those checks in one go.
> >
> > Bug: None
> > Change-Id: I2d18d0d6e7d8cfe92c81d08cef248a4daa7dcd4b
> > Reviewed-on: https://webrtc-review.googlesource.com/c/src/+/144780
> > Reviewed-by: Åsa Persson <asapersson@webrtc.org>
> > Reviewed-by: Sebastian Jansson <srte@webrtc.org>
> > Commit-Queue: Erik Språng <sprang@webrtc.org>
> > Cr-Commit-Position: refs/heads/master@{#28535}
>
> TBR=asapersson@webrtc.org,sprang@webrtc.org,srte@webrtc.org
>
> Change-Id: I8499dc0fd6e6d0b9fa7a0886c8754655e5589780
> No-Presubmit: true
> No-Tree-Checks: true
> No-Try: true
> Bug: None
> Reviewed-on: https://webrtc-review.googlesource.com/c/src/+/145326
> Reviewed-by: Mirko Bonadei <mbonadei@webrtc.org>
> Commit-Queue: Mirko Bonadei <mbonadei@webrtc.org>
> Cr-Commit-Position: refs/heads/master@{#28552}
TBR=mbonadei@webrtc.org,asapersson@webrtc.org,sprang@webrtc.org,srte@webrtc.org
Change-Id: I3bff3ecb2b776e30f77c1884f6faa72b21788017
No-Presubmit: true
No-Tree-Checks: true
No-Try: true
Bug: None
Reviewed-on: https://webrtc-review.googlesource.com/c/src/+/145401
Reviewed-by: Mirko Bonadei <mbonadei@webrtc.org>
Commit-Queue: Mirko Bonadei <mbonadei@webrtc.org>
Cr-Commit-Position: refs/heads/master@{#28563}
This reverts commit 8b3e4e2d1166464f6b309f4fc533a29607d2771f.
Reason for revert: The culprit was https://webrtc-review.googlesource.com/c/src/+/133169.
Original change's description:
> Revert "Reland "Add ability to set RTCP sender ssrc at construction time""
>
> This reverts commit 6f420e424885dab1d9f885365ea9abea5cc4a901.
>
> Reason for revert: Speculative revert (some perf test are failing)
>
> Original change's description:
> > Reland "Add ability to set RTCP sender ssrc at construction time"
> >
> > This is a reland of 94c58fd815f0c7c6429aa53a79621ea9ef39c770
> >
> > Patch set 1 is the original CL.
> > Patch set 2 introduced a trivial fix. In RtcpSender::SetSSRC(), check
> > if either current SSRC is 0 or if the SSRC is identical to the current
> > one. If so, don't schedule an early report.
> > This prevents a regression in which audio jitter became too low(?)
> >
> > Original change's description:
> > > Add ability to set RTCP sender ssrc at construction time
> > >
> > > Bug: webrtc:10774
> > > Change-Id: Iaf5857e24359e9795434bcd0cdbe1658a2f9f5d3
> > > Reviewed-on: https://webrtc-review.googlesource.com/c/src/+/144632
> > > Reviewed-by: Åsa Persson <asapersson@webrtc.org>
> > > Commit-Queue: Erik Språng <sprang@webrtc.org>
> > > Cr-Commit-Position: refs/heads/master@{#28506}
> >
> > Bug: webrtc:10774
> > Change-Id: I103dfa48719aa41d6ab633cdac8b3a5c46b54843
> > Reviewed-on: https://webrtc-review.googlesource.com/c/src/+/144565
> > Commit-Queue: Erik Språng <sprang@webrtc.org>
> > Reviewed-by: Åsa Persson <asapersson@webrtc.org>
> > Cr-Commit-Position: refs/heads/master@{#28520}
>
> TBR=asapersson@webrtc.org,sprang@webrtc.org,ilnik@webrtc.org
>
> # Not skipping CQ checks because original CL landed > 1 day ago.
>
> Bug: webrtc:10774
> Change-Id: I39238d942b2bbe0a9c8ca752387a35ed9dd70650
> Reviewed-on: https://webrtc-review.googlesource.com/c/src/+/145327
> Commit-Queue: Mirko Bonadei <mbonadei@webrtc.org>
> Reviewed-by: Mirko Bonadei <mbonadei@webrtc.org>
> Cr-Commit-Position: refs/heads/master@{#28555}
TBR=mbonadei@webrtc.org,ilnik@webrtc.org,asapersson@webrtc.org,sprang@webrtc.org
Change-Id: I2e5c17e8edfd938424f901222158643baa04866e
No-Presubmit: true
No-Tree-Checks: true
No-Try: true
Bug: webrtc:10774
Reviewed-on: https://webrtc-review.googlesource.com/c/src/+/145400
Reviewed-by: Mirko Bonadei <mbonadei@webrtc.org>
Commit-Queue: Mirko Bonadei <mbonadei@webrtc.org>
Cr-Commit-Position: refs/heads/master@{#28562}
This reverts commit 3e8ef940fe86cf6285afb80e68d2a0bedc631b9f.
Reason for revert: This CL causes a performance regression in NetEq, see https://bugs.chromium.org/p/chromium/issues/detail?id=982260.
Original change's description:
> Add plumbing of RtpPacketInfos to each AudioFrame as input for SourceTracker.
>
> This change adds the plumbing of RtpPacketInfo from ChannelReceive::OnRtpPacket() to ChannelReceive::GetAudioFrameWithInfo() for audio. It is a step towards replacing the non-spec compliant ContributingSources that updates itself at packet-receive time, with the spec-compliant SourceTracker that will update itself at frame-delivery-to-track time.
>
> Bug: webrtc:10668
> Change-Id: I03385d6865bbc7bfbef7634f88de820a934f787a
> Reviewed-on: https://webrtc-review.googlesource.com/c/src/+/139890
> Reviewed-by: Stefan Holmer <stefan@webrtc.org>
> Reviewed-by: Minyue Li <minyue@webrtc.org>
> Commit-Queue: Chen Xing <chxg@google.com>
> Cr-Commit-Position: refs/heads/master@{#28434}
TBR=kwiberg@webrtc.org,stefan@webrtc.org,minyue@webrtc.org,chxg@google.com
Bug: webrtc:10668, chromium:982260
Change-Id: I5e2cfde78c59d1123e21869564d76ed3f6193a5c
Reviewed-on: https://webrtc-review.googlesource.com/c/src/+/145339
Reviewed-by: Ivo Creusen <ivoc@webrtc.org>
Commit-Queue: Ivo Creusen <ivoc@webrtc.org>
Cr-Commit-Position: refs/heads/master@{#28561}
Code is not used in production. Need this change for a local test.
Using TBR.
TBR: henrik.lundin
Bug: webrtc:9265
Change-Id: I9f0cb265a51507de59ef2d7fd151465133687525
Reviewed-on: https://webrtc-review.googlesource.com/c/src/+/145330
Commit-Queue: Henrik Andreassson <henrika@webrtc.org>
Reviewed-by: Henrik Andreassson <henrika@webrtc.org>
Cr-Commit-Position: refs/heads/master@{#28557}
This partially reverts these 2 CLs:
1) Reland "Copy video frames metadata between encoded and plain frames in one place"
https://webrtc.googlesource.com/src/+/2ebf5239782bf6b46d4aa812f34fa9f9e5a02be9
2) Don't copy video frame metadata in each encoder/decoder
https://webrtc.googlesource.com/src/+/ab62b2ee51e622be6d0aade15e87e927fa60e6f2
The problem with them were that ColorSpace was made to always be copied from the
EncodedImage in the GenericDecoder, which overwrote ColorSpace information from
the decoder.
If decoder applied color space transition or bitstream color space information
was different from the WebRTC signaled one, the incorrect color space data were
passed to the renderer.
This CL removes introduced change regarding color space data: GenericDecoder
doesn't copy or store it and software decoders are restored to copy it.
Relevant tests are also removed.
Bug: chromium:982486
Change-Id: I989e01476ff7f7df376c05578ab8f540b95a1dd2
Reviewed-on: https://webrtc-review.googlesource.com/c/src/+/145323
Reviewed-by: Philip Eliasson <philipel@webrtc.org>
Commit-Queue: Ilya Nikolaevskiy <ilnik@webrtc.org>
Cr-Commit-Position: refs/heads/master@{#28556}
This reverts commit 6f420e424885dab1d9f885365ea9abea5cc4a901.
Reason for revert: Speculative revert (some perf test are failing)
Original change's description:
> Reland "Add ability to set RTCP sender ssrc at construction time"
>
> This is a reland of 94c58fd815f0c7c6429aa53a79621ea9ef39c770
>
> Patch set 1 is the original CL.
> Patch set 2 introduced a trivial fix. In RtcpSender::SetSSRC(), check
> if either current SSRC is 0 or if the SSRC is identical to the current
> one. If so, don't schedule an early report.
> This prevents a regression in which audio jitter became too low(?)
>
> Original change's description:
> > Add ability to set RTCP sender ssrc at construction time
> >
> > Bug: webrtc:10774
> > Change-Id: Iaf5857e24359e9795434bcd0cdbe1658a2f9f5d3
> > Reviewed-on: https://webrtc-review.googlesource.com/c/src/+/144632
> > Reviewed-by: Åsa Persson <asapersson@webrtc.org>
> > Commit-Queue: Erik Språng <sprang@webrtc.org>
> > Cr-Commit-Position: refs/heads/master@{#28506}
>
> Bug: webrtc:10774
> Change-Id: I103dfa48719aa41d6ab633cdac8b3a5c46b54843
> Reviewed-on: https://webrtc-review.googlesource.com/c/src/+/144565
> Commit-Queue: Erik Språng <sprang@webrtc.org>
> Reviewed-by: Åsa Persson <asapersson@webrtc.org>
> Cr-Commit-Position: refs/heads/master@{#28520}
TBR=asapersson@webrtc.org,sprang@webrtc.org,ilnik@webrtc.org
# Not skipping CQ checks because original CL landed > 1 day ago.
Bug: webrtc:10774
Change-Id: I39238d942b2bbe0a9c8ca752387a35ed9dd70650
Reviewed-on: https://webrtc-review.googlesource.com/c/src/+/145327
Commit-Queue: Mirko Bonadei <mbonadei@webrtc.org>
Reviewed-by: Mirko Bonadei <mbonadei@webrtc.org>
Cr-Commit-Position: refs/heads/master@{#28555}
This reverts commit 6f129b3b7605dc69c8c188ca02d133250130570e.
Reason for revert: Speculative revert (some perf test are failing)
Original change's description:
> Optimize PacketRouter/RTPSender interactions.
>
> The legacy code-path uses a hashmap as cache in order to speed up
> finding the right rtp module to send on. The new path should use that
> as well.
> In addition, there are checks that verify if an RTP module can send
> padding, in some cases payload based. These result in a number of
> calls to methods in RTPSender requiring its lock to be taken. This CL
> introduces a combined SupportsPadding() check method which performs
> all those checks in one go.
>
> Bug: None
> Change-Id: I2d18d0d6e7d8cfe92c81d08cef248a4daa7dcd4b
> Reviewed-on: https://webrtc-review.googlesource.com/c/src/+/144780
> Reviewed-by: Åsa Persson <asapersson@webrtc.org>
> Reviewed-by: Sebastian Jansson <srte@webrtc.org>
> Commit-Queue: Erik Språng <sprang@webrtc.org>
> Cr-Commit-Position: refs/heads/master@{#28535}
TBR=asapersson@webrtc.org,sprang@webrtc.org,srte@webrtc.org
Change-Id: I8499dc0fd6e6d0b9fa7a0886c8754655e5589780
No-Presubmit: true
No-Tree-Checks: true
No-Try: true
Bug: None
Reviewed-on: https://webrtc-review.googlesource.com/c/src/+/145326
Reviewed-by: Mirko Bonadei <mbonadei@webrtc.org>
Commit-Queue: Mirko Bonadei <mbonadei@webrtc.org>
Cr-Commit-Position: refs/heads/master@{#28552}
This reverts commit bb7727211c535f8a9dce27891941b52b6ea8e750.
Reason for revert: Speculative revert (some perf test are failing)
Original change's description:
> Make new pacer padding more like old one
>
> The (currently unused) new pacer code path was implemented with what
> was intended as a more careful padding strategy.
> Unfortunately this doesn't work as well as expected due to the fact
> that the padding budget cannot build up underuse.
>
> I made another CL that could fix that, but I think it adds complexity
> for dubious gains. It also will make it more difficult to find any
> potential regression when switching to the new path, should one occur.
> See https://webrtc-review.googlesource.com/c/src/+/144563
>
> Therefore, this CL makes the new code path choose RTX payload in the
> same way as is currently done.
>
> Bug: webrtc:10633
> Change-Id: If2115d4fa7463add959faa77c63101286c27e0f5
> Reviewed-on: https://webrtc-review.googlesource.com/c/src/+/145202
> Reviewed-by: Stefan Holmer <stefan@webrtc.org>
> Commit-Queue: Erik Språng <sprang@webrtc.org>
> Cr-Commit-Position: refs/heads/master@{#28537}
TBR=sprang@webrtc.org,stefan@webrtc.org
Change-Id: I99b72858414e0a245da596d94694449da88fd626
No-Presubmit: true
No-Tree-Checks: true
No-Try: true
Bug: webrtc:10633
Reviewed-on: https://webrtc-review.googlesource.com/c/src/+/145324
Reviewed-by: Mirko Bonadei <mbonadei@webrtc.org>
Commit-Queue: Mirko Bonadei <mbonadei@webrtc.org>
Cr-Commit-Position: refs/heads/master@{#28550}
This reverts commit 7325bc3917e6dd4c92e7a18fd879ba91f0b2851f.
Reason for revert: FecTest.UlpfecTest is consistently failing.
Original change's description:
> Refactor FEC code to use COW buffers
>
> This refactoring helps to reduce unnecessary memcpy calls on the receive
> side.
>
> This CL is the first stage of refactoring: it only replaces
> |uint8 data[IP_PACKET_SIZE]| with |rtc::CopyOnWriteBuffer data| and does
> necessary changes.
>
> A follow-up CL will remove length field of the Packet class.
>
>
> Bug: webrtc:10750
> Change-Id: Ie233da83ff33f6370f511955e4c65d59522389a7
> Reviewed-on: https://webrtc-review.googlesource.com/c/src/+/144881
> Reviewed-by: Artem Titov <titovartem@webrtc.org>
> Reviewed-by: Stefan Holmer <stefan@webrtc.org>
> Reviewed-by: Rasmus Brandt <brandtr@webrtc.org>
> Commit-Queue: Ilya Nikolaevskiy <ilnik@webrtc.org>
> Cr-Commit-Position: refs/heads/master@{#28539}
TBR=brandtr@webrtc.org,ilnik@webrtc.org,asapersson@webrtc.org,stefan@webrtc.org,titovartem@webrtc.org
Change-Id: I07c34256a76174f09a0d27eacbae6488e66f4b43
No-Presubmit: true
No-Tree-Checks: true
No-Try: true
Bug: webrtc:10750
Reviewed-on: https://webrtc-review.googlesource.com/c/src/+/145340
Reviewed-by: Qingsi Wang <qingsi@webrtc.org>
Commit-Queue: Ilya Nikolaevskiy <ilnik@webrtc.org>
Cr-Commit-Position: refs/heads/master@{#28545}
