From the logs I can't tell if close is happening or not on the bots.
Let's make it explicit just in case.
Bug: webrtc:15018
Change-Id: Icfa7fe8587d1516a9ef31e86ade920a6023e619b
Reviewed-on: https://webrtc-review.googlesource.com/c/src/+/300364
Commit-Queue: Henrik Boström <hbos@webrtc.org>
Auto-Submit: Henrik Boström <hbos@webrtc.org>
Reviewed-by: Jeremy Leconte <jleconte@google.com>
Commit-Queue: Jeremy Leconte <jleconte@google.com>
Cr-Commit-Position: refs/heads/main@{#39768}
This reverts commit fe53fec24e02d2d644220f913c3f9ae596bbb2d9.
Reason for revert: Speculative revert, may be breaking downstream project
Original change's description:
> [DataChannel] Send and receive packets on the network thread.
>
> This updates sctp channels, including work that happens between the
> data channel controller and the transport, to run on the network
> thread. Previously all network traffic related to data channels was
> routed through the signaling thread before going to either the network
> thread or the caller's thread (e.g. js thread in chrome). Now the
> calls can go straight from the network thread to the JS thread with
> enabling a special flag on the observer (see below) and similarly
> calls to send data, involve 2 threads instead of 3.
>
> * Custom data channel observer adapter implementation that
> maintains compatibility with existing observer implementations in
> that notifications are delivered on the signaling thread.
> The adapter can be explicitly disabled for implementations that
> want to optimize the callback path and promise to not block the
> network thread.
> * Remove the signaling thread copy of data channels in the controller.
> * Remove several PostTask operations that were needed to keep things
> in sync (but the need has gone away).
> * Update tests for the controller to consistently call
> TeardownDataChannelTransport_n to match with production.
> * Update stats collectors (current and legacy) to fetch the data
> channel stats on the network thread where they're maintained.
> * Remove the AsyncChannelCloseTeardown test since the async teardown
> step has gone away.
> * Remove `sid_s` in the channel code since we only need the network
> state now.
> * For the custom observer support (with and without data adapter) and
> maintain compatibility with existing implementations, added a new
> proxy macro that allows an implementation to selectively provide
> its own implementation without being proxied. This is used for
> registering/unregistering a data channel observer.
> * Update the data channel proxy to map most methods to the network
> thread, avoiding the interim jump to the signaling thread.
> * Update a plethora of thread checkers from signaling to network.
>
> Bug: webrtc:11547
> Change-Id: Ib4cff1482e31c46008e187189a79e967389bc518
> Reviewed-on: https://webrtc-review.googlesource.com/c/src/+/299142
> Commit-Queue: Tomas Gunnarsson <tommi@webrtc.org>
> Reviewed-by: Henrik Boström <hbos@webrtc.org>
> Cr-Commit-Position: refs/heads/main@{#39760}
Bug: webrtc:11547
Change-Id: Id0d65594bf727ccea5c49093c942b09714d101ad
No-Presubmit: true
No-Tree-Checks: true
No-Try: true
Reviewed-on: https://webrtc-review.googlesource.com/c/src/+/300341
Auto-Submit: Andrey Logvin <landrey@webrtc.org>
Owners-Override: Andrey Logvin <landrey@webrtc.org>
Bot-Commit: rubber-stamper@appspot.gserviceaccount.com <rubber-stamper@appspot.gserviceaccount.com>
Commit-Queue: Mirko Bonadei <mbonadei@webrtc.org>
Reviewed-by: Mirko Bonadei <mbonadei@webrtc.org>
Cr-Commit-Position: refs/heads/main@{#39764}
This handles a corner case whereby an OnStateChange implementation
synchronously calls UnregisterObserver, which would (before this CL)
delete the observer adapter.
(Using No-Try since an import bot won't pass until this CL lands)
No-Try: True
Bug: webrtc:11547
Change-Id: I33a13495aad6151fdd76becfa9a2c8672d80d825
Reviewed-on: https://webrtc-review.googlesource.com/c/src/+/300280
Reviewed-by: Mirko Bonadei <mbonadei@webrtc.org>
Commit-Queue: Tomas Gunnarsson <tommi@webrtc.org>
Cr-Commit-Position: refs/heads/main@{#39761}
This updates sctp channels, including work that happens between the
data channel controller and the transport, to run on the network
thread. Previously all network traffic related to data channels was
routed through the signaling thread before going to either the network
thread or the caller's thread (e.g. js thread in chrome). Now the
calls can go straight from the network thread to the JS thread with
enabling a special flag on the observer (see below) and similarly
calls to send data, involve 2 threads instead of 3.
* Custom data channel observer adapter implementation that
maintains compatibility with existing observer implementations in
that notifications are delivered on the signaling thread.
The adapter can be explicitly disabled for implementations that
want to optimize the callback path and promise to not block the
network thread.
* Remove the signaling thread copy of data channels in the controller.
* Remove several PostTask operations that were needed to keep things
in sync (but the need has gone away).
* Update tests for the controller to consistently call
TeardownDataChannelTransport_n to match with production.
* Update stats collectors (current and legacy) to fetch the data
channel stats on the network thread where they're maintained.
* Remove the AsyncChannelCloseTeardown test since the async teardown
step has gone away.
* Remove `sid_s` in the channel code since we only need the network
state now.
* For the custom observer support (with and without data adapter) and
maintain compatibility with existing implementations, added a new
proxy macro that allows an implementation to selectively provide
its own implementation without being proxied. This is used for
registering/unregistering a data channel observer.
* Update the data channel proxy to map most methods to the network
thread, avoiding the interim jump to the signaling thread.
* Update a plethora of thread checkers from signaling to network.
Bug: webrtc:11547
Change-Id: Ib4cff1482e31c46008e187189a79e967389bc518
Reviewed-on: https://webrtc-review.googlesource.com/c/src/+/299142
Commit-Queue: Tomas Gunnarsson <tommi@webrtc.org>
Reviewed-by: Henrik Boström <hbos@webrtc.org>
Cr-Commit-Position: refs/heads/main@{#39760}
The FakePeriodicVideoSource was not actually stopping its repeated
handle, which takes a raw pointer to the task queue. There could be a
race here where a repeated task was being posted at the same time as
the task queue was being destroyed due to the scoped safety flag being
tied to the repeated task rather than the task queue.
I'm still unable to repro locally, so this is a speculative fix.
# No need to wait for ios/android bots, all other bots green
NOTRY=True
Bug: webrtc:15018
Change-Id: Id6f9bda5f4fc641abc11068f5cf8aa0f1cf36d27
Reviewed-on: https://webrtc-review.googlesource.com/c/src/+/300264
Reviewed-by: Jeremy Leconte <jleconte@google.com>
Commit-Queue: Henrik Boström <hbos@webrtc.org>
Cr-Commit-Position: refs/heads/main@{#39758}
This updates DataChannelControllerTests to shut down the DCC in the
same way it's shut down by the owning PeerConnection instance:
* Call TeardownDataChannelTransport_n()
* Call PrepareForShutdown()
Also calling PrepareForShutdown() from PC's dtor to be consistent with
how `sdp_handler_->PrepareForShutdown()` is called since it appears
that many tests do not call PC::Close() before destruction.
Bug: b/276434297
Change-Id: I0379baa0df0e764bc255b83ae0667032acfe3db0
Reviewed-on: https://webrtc-review.googlesource.com/c/src/+/300220
Commit-Queue: Tomas Gunnarsson <tommi@webrtc.org>
Reviewed-by: Florent Castelli <orphis@webrtc.org>
Cr-Commit-Position: refs/heads/main@{#39756}
The Mode is currently redundant with the optional input_file_name.
Change-Id: Ib4f0a363e86d925107d61867a7f743d6663e7071
Bug: None
Reviewed-on: https://webrtc-review.googlesource.com/c/src/+/298743
Reviewed-by: Mirko Bonadei <mbonadei@webrtc.org>
Reviewed-by: Henrik Lundin <henrik.lundin@webrtc.org>
Commit-Queue: Jeremy Leconte <jleconte@google.com>
Cr-Commit-Position: refs/heads/main@{#39754}
Only in testing environments are the task queues shut down while sources
still have media flowing. It's still not clear why heap-use-after-free
happens, since it should be enough to close the PC, but it is clear that
the crash is happening due to frames flowing while the test is shutting
down, which is not something happening outside of testing.
In an attempt to deflake, this CL makes sure to manually stop the
test-only sources before closing the peer connection.
Bug: webrtc:15018
Change-Id: I48ee131a8994c9c4caee1bb4875580d255b97da1
Reviewed-on: https://webrtc-review.googlesource.com/c/src/+/299944
Commit-Queue: Henrik Boström <hbos@webrtc.org>
Auto-Submit: Henrik Boström <hbos@webrtc.org>
Reviewed-by: Jeremy Leconte <jleconte@google.com>
Cr-Commit-Position: refs/heads/main@{#39752}
Tests that caller BWE can rampup even if callee can not demux incoming RTP packets.
Bug: webrtc:14928
Change-Id: I3c89a14e67c6d781a26439980b5a99570a430dc7
Reviewed-on: https://webrtc-review.googlesource.com/c/src/+/299482
Commit-Queue: Per Kjellander <perkj@webrtc.org>
Reviewed-by: Henrik Boström <hbos@webrtc.org>
Reviewed-by: Erik Språng <sprang@webrtc.org>
Cr-Commit-Position: refs/heads/main@{#39749}
Before:
No attempt was made to figure out of the cursor was embedded into the
captured video frame when using DXGI on Windows as screen capturer.
Instead the cursor is superimposed on the frame by an external mouse
and cursor composer.
After:
We now check if the display adapter supports embedding the mouse
cursor and if so use it as is and thereby avoid adding it independently.
Bug: chromium:1421656
Change-Id: Ie07fe13e1c8f9583769961328bb41fbc689cd8e0
Reviewed-on: https://webrtc-review.googlesource.com/c/src/+/299241
Reviewed-by: Alexander Cooper <alcooper@chromium.org>
Commit-Queue: Henrik Andreassson <henrika@webrtc.org>
Cr-Commit-Position: refs/heads/main@{#39742}
If configured, the packet parser will allow packets with
a set checksum of zero. In that case, the correct checksum
will not even be calculated, avoiding a CPU intensive
calculation.
Also, if specified when building a packet, the checksum can
be opted to be not calculated and written to the packet.
This is to be used when draft-tuexen-tsvwg-sctp-zero-checksum
has been negotiated, except for some packets during association
establishment.
This is mainly a preparation CL and follow-up CL will enable
this feature.
Low-Coverage-Reason: Affects debug logging code not run in tests
Bug: webrtc:14997
Change-Id: I3207ac3a626df039ee2990403c2edd6429f17617
Reviewed-on: https://webrtc-review.googlesource.com/c/src/+/298481
Reviewed-by: Harald Alvestrand <hta@webrtc.org>
Commit-Queue: Victor Boivie <boivie@webrtc.org>
Cr-Commit-Position: refs/heads/main@{#39737}
(using no-try due to bot infra issue)
No-try: true
Bug: b/276434297
Change-Id: I33f796b501f96731c4ca76cb62c2331f10c795f1
Reviewed-on: https://webrtc-review.googlesource.com/c/src/+/299708
Reviewed-by: Björn Terelius <terelius@webrtc.org>
Commit-Queue: Tomas Gunnarsson <tommi@webrtc.org>
Cr-Commit-Position: refs/heads/main@{#39734}
This is needed in order to be able to update the legacy stats
collector to fetch data channel stats from the network thread, which
is part of an upcoming change to data channels.
Bug: webrtc:11547
Change-Id: Ic205b0314b9f11a024d36d714c223cbddd0f3df3
Reviewed-on: https://webrtc-review.googlesource.com/c/src/+/299462
Reviewed-by: Henrik Boström <hbos@webrtc.org>
Commit-Queue: Tomas Gunnarsson <tommi@webrtc.org>
Cr-Commit-Position: refs/heads/main@{#39732}
The goal is to reduce the amount of time stretching done in response
to network jitter. Specifically, we should be able to “ride” over delay
spikes if the current delay is sufficient, without decelerating
playout. We should also avoid accelerating immediately after a buffer
underrun, until we are reasonably sure that the jitter has passed.
This is achieved by increasing the deadband where we choose to do
normal playout, based on the maximum delay in the short term packet
arrival history.
The buffer level filter is still used to report the average delay for
A/V sync purposes.
The new behavior is behind a flag and will be experimented with before
it is made default.
Bug: webrtc:13322
Change-Id: I5fba0c9d46d835dbe5401669598fa031512ccced
Reviewed-on: https://webrtc-review.googlesource.com/c/src/+/299500
Reviewed-by: Henrik Lundin <henrik.lundin@webrtc.org>
Commit-Queue: Jakob Ivarsson <jakobi@webrtc.org>
Cr-Commit-Position: refs/heads/main@{#39730}
With this cl, packets that are discarded in RtpTransport now notifies Call, so that
they can be part of BWE even if they are dropped.
These packets have been recevied on the transport, and has bin decrypted
and parsed and thus can be accounted for.
The un demuxable packets are forwarded to Call similarly how RTCP packets are forwarded.
Bug: webrtc:14928
Change-Id: Ia53349c7b316c4442a3c7aac085a85ec4f4ab9ae
Reviewed-on: https://webrtc-review.googlesource.com/c/src/+/299262
Reviewed-by: Henrik Boström <hbos@webrtc.org>
Commit-Queue: Per Kjellander <perkj@webrtc.org>
Cr-Commit-Position: refs/heads/main@{#39727}
in order to not regress existing use-cases while following rules
described by the specification. This change now makes the existing
regression test pass after the spec-compliant modifications.
BUG=chromium:1051821
Change-Id: Ia384adf9a172ed88b5ec6a3cc5c478764a686cb9
Reviewed-on: https://webrtc-review.googlesource.com/c/src/+/299002
Reviewed-by: Harald Alvestrand <hta@webrtc.org>
Reviewed-by: Henrik Boström <hbos@webrtc.org>
Commit-Queue: Philipp Hancke <phancke@microsoft.com>
Cr-Commit-Position: refs/heads/main@{#39726}
The tests now have two ways of addressing the channel under test:
* channel_: All SctpDataChannelTest methods. This way calls get
executed as specified by the proxy and more accurately reflects
production.
* inner_channel: For SctpDataChannel methods that are not a part of
the SctpDataChannelTest interface. Use this to invoke event handlers
etc.
Upcoming changes will include threading changes and changes to the
proxy, so it's important to cover both.
Bug: webrtc:11547
Change-Id: I26c284ece82b9a58e2b5dc4468d124d54012d959
Reviewed-on: https://webrtc-review.googlesource.com/c/src/+/299264
Reviewed-by: Henrik Boström <hbos@webrtc.org>
Commit-Queue: Tomas Gunnarsson <tommi@webrtc.org>
Cr-Commit-Position: refs/heads/main@{#39725}
* Rename id_ -> id_s_, add id_n_ and thread guards.
* Same for getters, sid() -> sid_s(), add sid_n()
As more things migrate over to the network thread, we'll only need the
_n variant.
Bug: webrtc:11547
Change-Id: Ic998330f4c81b0f6833967631ac70edc2ca2301c
Reviewed-on: https://webrtc-review.googlesource.com/c/src/+/299141
Commit-Queue: Tomas Gunnarsson <tommi@webrtc.org>
Reviewed-by: Harald Alvestrand <hta@webrtc.org>
Cr-Commit-Position: refs/heads/main@{#39724}
It is a part of "encoding statistics" feature [1] available in Android SDK 33. Local testing revealed that for HW VP8/9 encoders we get QP in range [0,64] which is not what WebRTC quality scaler expects. Exclude VP8/9 encoders for now.
[1] https://developer.android.com/reference/android/media/MediaFormat#VIDEO_ENCODING_STATISTICS_LEVEL_1
Bug: webrtc:15015
Change-Id: I8af2fd96afb34e18cb3e2cc3562b10149324c16e
Reviewed-on: https://webrtc-review.googlesource.com/c/src/+/298306
Commit-Queue: Sergey Silkin <ssilkin@webrtc.org>
Reviewed-by: Erik Språng <sprang@webrtc.org>
Cr-Commit-Position: refs/heads/main@{#39722}
The reason is that:
* SDP/PeerConnection code should use HasUsedDataChannels() instead.
* HasDataChannels() touches the list of data channels on the signaling
thread, which will be a problem when it's moved to the network
thread.
* It's only needed by tests.
Bug: webrtc:11547
Change-Id: Idd47365c429e5f1d6e3812cf558c4e6fefbf733c
Reviewed-on: https://webrtc-review.googlesource.com/c/src/+/299481
Commit-Queue: Henrik Boström <hbos@webrtc.org>
Auto-Submit: Tomas Gunnarsson <tommi@webrtc.org>
Reviewed-by: Henrik Boström <hbos@webrtc.org>
Cr-Commit-Position: refs/heads/main@{#39721}
...when checking if negotiation is needed.
I noticed this call site after we recently updated PeerConnection here:
https://webrtc-review.googlesource.com/c/src/+/297860
Bug: chromium:1423562
Change-Id: Id37c938d731eadfccff44c95ef757a3cabd64936
Reviewed-on: https://webrtc-review.googlesource.com/c/src/+/299480
Commit-Queue: Tomas Gunnarsson <tommi@webrtc.org>
Reviewed-by: Harald Alvestrand <hta@webrtc.org>
Cr-Commit-Position: refs/heads/main@{#39720}
* Change data channel creation code to return RTCError for more
detailed/accurate errors.
* Move DataChannelController::sid_allocator_ to the network thread.
* Add a temporary duplicate vector of channels on the network thread.
This will eventually be the main vector.
* Delete one test that turns out to be racy (as long as we're using
both the signaling and network threads).
Bug: webrtc:11547, webrtc:12796
Change-Id: I93ab721a09872d075046a907df60e8aee4263371
Reviewed-on: https://webrtc-review.googlesource.com/c/src/+/298624
Reviewed-by: Harald Alvestrand <hta@webrtc.org>
Commit-Queue: Tomas Gunnarsson <tommi@webrtc.org>
Cr-Commit-Position: refs/heads/main@{#39719}
