When the shutdown timer has expired, the socket will abort/close and the
TCB is not valid after InternalClose.
Bug: webrtc:12614
Change-Id: I09a94a049f0cda4577225dd9c80a92a8ec7e0423
Reviewed-on: https://webrtc-review.googlesource.com/c/src/+/217767
Commit-Queue: Victor Boivie <boivie@webrtc.org>
Reviewed-by: Florent Castelli <orphis@webrtc.org>
Cr-Commit-Position: refs/heads/master@{#33956}
Pending messages on network thread for MediaChannel, will be dropped
when the MediaChannel object is deleted (without blocking).
Remove MessageHandler inheritance from Channel since Post-ing to the
network thread has been removed from there.
Copy/pasted code for SendRtp/SendRtcp in WebRtcVideoChannel and
WebRtcVoiceMediaChannel consolidated in MediaChannel.
Bug: webrtc:11993
Change-Id: I05320eb7f86b98adba50ca5eb8b76b78f4111263
Reviewed-on: https://webrtc-review.googlesource.com/c/src/+/217720
Commit-Queue: Tommi <tommi@webrtc.org>
Reviewed-by: Niels Moller <nisse@webrtc.org>
Cr-Commit-Position: refs/heads/master@{#33955}
It doesn't make sense to use negative values or 0 to disable the
feature, so we use an optional int value.
Values bigger than 65535 are clamped down.
Bug: webrtc:12730
Change-Id: I6bd9cd92f7d0a70a78cf5a7c91dca52c28d08ba1
Reviewed-on: https://webrtc-review.googlesource.com/c/src/+/217760
Reviewed-by: Harald Alvestrand <hta@webrtc.org>
Commit-Queue: Florent Castelli <orphis@webrtc.org>
Cr-Commit-Position: refs/heads/master@{#33954}
This CL mostly adds plumbing to get awareness of the network thread
to the media channel classes. Currently this pointer is only used
to DCHECK that `SetInterface` for the `NetworkInterface` pointer, is
called on the network thread. Follow up changes will establish that
most of the methods are called on the network thread and the mutex
in the MediaChannel base class, can be removed.
Most of the changes in the CL are in channel_unittest.cc. They're mostly
around updating the tests to incorporate the network thread in ways
that reflect how the classes are used in production. Another change is
to use accessor methods for the media channel instances instead of
caching potentially dangling pointers.
Bug: webrtc:11993
Change-Id: I8e2ed1bc23724e238554dbce386789d69660f7e4
Reviewed-on: https://webrtc-review.googlesource.com/c/src/+/217682
Reviewed-by: Harald Alvestrand <hta@webrtc.org>
Commit-Queue: Tommi <tommi@webrtc.org>
Cr-Commit-Position: refs/heads/master@{#33951}
This reverts commit 793bac569fdf1be16cbf24d7871d20d00bbec81b.
Reason for revert: rare compilation error fixed
Original change's description:
> Revert "Refactor the PlatformThread API."
>
> This reverts commit c89fdd716c4c8af608017c76f75bf27e4c3d602e.
>
> Reason for revert: Causes rare compilation error on win-libfuzzer-asan trybot.
> See https://ci.chromium.org/p/chromium/builders/try/win-libfuzzer-asan-rel/713745?
>
> Original change's description:
> > Refactor the PlatformThread API.
> >
> > PlatformThread's API is using old style function pointers, causes
> > casting, is unintuitive and forces artificial call sequences, and
> > is additionally possible to misuse in release mode.
> >
> > Fix this by an API face lift:
> > 1. The class is turned into a handle, which can be empty.
> > 2. The only way of getting a non-empty PlatformThread is by calling
> > SpawnJoinable or SpawnDetached, clearly conveying the semantics to the
> > code reader.
> > 3. Handles can be Finalized, which works differently for joinable and
> > detached threads:
> > a) Handles for detached threads are simply closed where applicable.
> > b) Joinable threads are joined before handles are closed.
> > 4. The destructor finalizes handles. No explicit call is needed.
> >
> > Fixed: webrtc:12727
> > Change-Id: Id00a0464edf4fc9e552b6a1fbb5d2e1280e88811
> > Reviewed-on: https://webrtc-review.googlesource.com/c/src/+/215075
> > Commit-Queue: Markus Handell <handellm@webrtc.org>
> > Reviewed-by: Harald Alvestrand <hta@webrtc.org>
> > Reviewed-by: Mirko Bonadei <mbonadei@webrtc.org>
> > Reviewed-by: Tommi <tommi@webrtc.org>
> > Cr-Commit-Position: refs/heads/master@{#33923}
>
> # Not skipping CQ checks because original CL landed > 1 day ago.
>
> TBR=handellm@webrtc.org
>
> Bug: webrtc:12727
> Change-Id: Ic0146be8866f6dd3ad9c364fb8646650b8e07419
> Reviewed-on: https://webrtc-review.googlesource.com/c/src/+/217583
> Reviewed-by: Guido Urdaneta <guidou@webrtc.org>
> Reviewed-by: Markus Handell <handellm@webrtc.org>
> Commit-Queue: Guido Urdaneta <guidou@webrtc.org>
> Cr-Commit-Position: refs/heads/master@{#33936}
# Not skipping CQ checks because this is a reland.
Bug: webrtc:12727
Change-Id: Ifd6f44eac72fed84474277a1be03eb84d2f4376e
Reviewed-on: https://webrtc-review.googlesource.com/c/src/+/217881
Commit-Queue: Mirko Bonadei <mbonadei@webrtc.org>
Reviewed-by: Mirko Bonadei <mbonadei@webrtc.org>
Reviewed-by: Markus Handell <handellm@webrtc.org>
Reviewed-by: Harald Alvestrand <hta@webrtc.org>
Cr-Commit-Position: refs/heads/master@{#33950}
If Shutdown is called when the socket is being established and while the
connection timers are running, it will put the socket in an inconsistent
state, which is verified in debug builds.
Bug: webrtc:12614
Change-Id: I66f07d1170ac8f0ad9fd485d77d6aef4c365f150
Reviewed-on: https://webrtc-review.googlesource.com/c/src/+/217765
Commit-Queue: Victor Boivie <boivie@webrtc.org>
Reviewed-by: Florent Castelli <orphis@webrtc.org>
Cr-Commit-Position: refs/heads/master@{#33949}
This makes it easier to understand which socket that experience an error
or abort. Aborts are now also logged, which was missed previously.
Bug: webrtc:12614
Change-Id: Ie5e4357b3e5450106cc6cc28c1e9578ad53d073a
Reviewed-on: https://webrtc-review.googlesource.com/c/src/+/217764
Commit-Queue: Victor Boivie <boivie@webrtc.org>
Reviewed-by: Florent Castelli <orphis@webrtc.org>
Cr-Commit-Position: refs/heads/master@{#33947}
As it will be heavily modified in a follow-up change, it's refactored
as a separate commit to make commits small:
* Extracted code to separate AddPacket and CullOldPackets methods
* BuildFeedbackPacket now returns a packet. In the next iteration, it
might not, so it needs to be able to decide when to increment the
packet sequence number.
* Documented some existing fields.
The follow-up is in change I3dd58105f9fbfb111f176833cd4aa6b040c0e01d.
Bug: webrtc:12689
Change-Id: I5ad0aee5a7da008f9e209f7c13bf299c12f9d1f3
Reviewed-on: https://webrtc-review.googlesource.com/c/src/+/217581
Commit-Queue: Victor Boivie <boivie@webrtc.org>
Reviewed-by: Sebastian Jansson <srte@webrtc.org>
Cr-Commit-Position: refs/heads/master@{#33942}
But keep option to disable it.
The fix has been tested using A/B experiment with Duo
with positive results.
Bug: webrtc:10707
Change-Id: Iaac72430bc52026ab5db9dd042c5fdc3832fdce5
Reviewed-on: https://webrtc-review.googlesource.com/c/src/+/217380
Reviewed-by: Harald Alvestrand <hta@webrtc.org>
Reviewed-by: Taylor Brandstetter <deadbeef@webrtc.org>
Commit-Queue: Jonas Oreland <jonaso@webrtc.org>
Cr-Commit-Position: refs/heads/master@{#33941}
This reverts commit c89fdd716c4c8af608017c76f75bf27e4c3d602e.
Reason for revert: Causes rare compilation error on win-libfuzzer-asan trybot.
See https://ci.chromium.org/p/chromium/builders/try/win-libfuzzer-asan-rel/713745?
Original change's description:
> Refactor the PlatformThread API.
>
> PlatformThread's API is using old style function pointers, causes
> casting, is unintuitive and forces artificial call sequences, and
> is additionally possible to misuse in release mode.
>
> Fix this by an API face lift:
> 1. The class is turned into a handle, which can be empty.
> 2. The only way of getting a non-empty PlatformThread is by calling
> SpawnJoinable or SpawnDetached, clearly conveying the semantics to the
> code reader.
> 3. Handles can be Finalized, which works differently for joinable and
> detached threads:
> a) Handles for detached threads are simply closed where applicable.
> b) Joinable threads are joined before handles are closed.
> 4. The destructor finalizes handles. No explicit call is needed.
>
> Fixed: webrtc:12727
> Change-Id: Id00a0464edf4fc9e552b6a1fbb5d2e1280e88811
> Reviewed-on: https://webrtc-review.googlesource.com/c/src/+/215075
> Commit-Queue: Markus Handell <handellm@webrtc.org>
> Reviewed-by: Harald Alvestrand <hta@webrtc.org>
> Reviewed-by: Mirko Bonadei <mbonadei@webrtc.org>
> Reviewed-by: Tommi <tommi@webrtc.org>
> Cr-Commit-Position: refs/heads/master@{#33923}
# Not skipping CQ checks because original CL landed > 1 day ago.
TBR=handellm@webrtc.org
Bug: webrtc:12727
Change-Id: Ic0146be8866f6dd3ad9c364fb8646650b8e07419
Reviewed-on: https://webrtc-review.googlesource.com/c/src/+/217583
Reviewed-by: Guido Urdaneta <guidou@webrtc.org>
Reviewed-by: Markus Handell <handellm@webrtc.org>
Commit-Queue: Guido Urdaneta <guidou@webrtc.org>
Cr-Commit-Position: refs/heads/master@{#33936}
An upcoming change will remove PeerConnection's involvement in
the RTCP packet path and keep the flow consistent with the RTP packet
path.
Bug: webrtc:11993
Change-Id: I7ce1bbe8bbc352a49310e2e55b55ca5d8d927935
Reviewed-on: https://webrtc-review.googlesource.com/c/src/+/217389
Reviewed-by: Harald Alvestrand <hta@webrtc.org>
Commit-Queue: Tommi <tommi@webrtc.org>
Cr-Commit-Position: refs/heads/master@{#33927}
This was caught in an integration test which had stricter assertions
than the FakeTimeout which is used in unit tests, so the first thing was
to add the same assertions to the FakeTimeout.
The issue is that when a Timer triggers, and if it's set to
automatically restart (possibly with an exponential backoff), the
`is_running_` field was set to true while the timer callback was called
to allow the client to know that the timer is in fact running, but the
timer was actually not started until the callback returned. Which made
sense, as the callback can with its return value override the duration,
which should affect the backoff algorithm.
The problem was when a timer was manually started within the callback.
As the Timer itself thought that it was already running, it first would
Stop() the underlying Timeout, then Start(). But calling Stop() on a
timeout that is not running is illegal, which set of assertions.
So the solution is to don't lie; Don't say that a timer is running when
it's not. Make sure that the timer is running when the timer callback is
triggered, which makes it consistent at all times. That may result in
unnecessary timeout invocations (stopping and starting), but that's not
too expensive.
Bug: webrtc:12614
Change-Id: I7b4447ccd88bd43d181e158f0d29b0770c8a3fd6
Reviewed-on: https://webrtc-review.googlesource.com/c/src/+/217522
Reviewed-by: Florent Castelli <orphis@webrtc.org>
Commit-Queue: Victor Boivie <boivie@webrtc.org>
Cr-Commit-Position: refs/heads/master@{#33926}
In real life, when a Timeout expires, the caller is supposed to call
DcSctpSocket::HandleTimeout directly, as the Timeout that just expired
is stopped (it just expired), but the Timer still believes it's running.
The system is not in a consistent state.
In tests, all timeouts were evaluated at the same time, which, if two
timeouts expired at the same time, would put them both as "not running",
and with their timers believing they were running. So if you would do
any operation on a timer whose timeout had just expired, the timeout
would assert saying that "you can't stop a stopped timeout" or similar.
This isn't relevant in non-test scenarios.
Solved by expiring timeouts one by one.
Bug: webrtc:12614
Change-Id: I79d006f4d3e96854d77cec3eb0080aa23b8569cb
Reviewed-on: https://webrtc-review.googlesource.com/c/src/+/217560
Reviewed-by: Florent Castelli <orphis@webrtc.org>
Commit-Queue: Victor Boivie <boivie@webrtc.org>
Cr-Commit-Position: refs/heads/master@{#33925}
PlatformThread's API is using old style function pointers, causes
casting, is unintuitive and forces artificial call sequences, and
is additionally possible to misuse in release mode.
Fix this by an API face lift:
1. The class is turned into a handle, which can be empty.
2. The only way of getting a non-empty PlatformThread is by calling
SpawnJoinable or SpawnDetached, clearly conveying the semantics to the
code reader.
3. Handles can be Finalized, which works differently for joinable and
detached threads:
a) Handles for detached threads are simply closed where applicable.
b) Joinable threads are joined before handles are closed.
4. The destructor finalizes handles. No explicit call is needed.
Fixed: webrtc:12727
Change-Id: Id00a0464edf4fc9e552b6a1fbb5d2e1280e88811
Reviewed-on: https://webrtc-review.googlesource.com/c/src/+/215075
Commit-Queue: Markus Handell <handellm@webrtc.org>
Reviewed-by: Harald Alvestrand <hta@webrtc.org>
Reviewed-by: Mirko Bonadei <mbonadei@webrtc.org>
Reviewed-by: Tommi <tommi@webrtc.org>
Cr-Commit-Position: refs/heads/master@{#33923}
Also take the opportunity to declare factory_ const.
(Bug reference is where I noticed the possibility; it is unlikely
to fix the bug.)
Bug: chromium:1205343
Change-Id: I6078f170cf68d94314ee184bdfd2dc6f4ffc1e71
Reviewed-on: https://webrtc-review.googlesource.com/c/src/+/217385
Reviewed-by: Tommi <tommi@webrtc.org>
Commit-Queue: Harald Alvestrand <hta@webrtc.org>
Cr-Commit-Position: refs/heads/master@{#33918}
The channel classes have stored the negotiated headers but a more
natural place to store them is in the RtpTransceiver class where
RtpHeaderExtension state is managed as well as the implementation of
the only method that depends on the stored state,
HeaderExtensionsNegotiated().
Also adding a TODO for further improvements where we're unnecessarily
storing state in the channel classes for the purposes of the transports.
Bug: webrtc:12726
Change-Id: If36668e3e49782ddeada23ebed126ee2c4935b8c
Reviewed-on: https://webrtc-review.googlesource.com/c/src/+/216691
Reviewed-by: Harald Alvestrand <hta@webrtc.org>
Commit-Queue: Tommi <tommi@webrtc.org>
Cr-Commit-Position: refs/heads/master@{#33917}
Before this CL, timestamps of received packets were rounded
to the nearest millisecond and stored as int64_t. Due to the
rounding it sometimes happened that timestamps later in the
pipeline that are not rounded seem to occur even before the
video frame was received.
Change-Id: I92d8f3540b23baae2d4a1dc6a7cb3f58bcdaad18
Bug: webrtc:12722
Reviewed-on: https://webrtc-review.googlesource.com/c/src/+/216398
Reviewed-by: Chen Xing <chxg@google.com>
Reviewed-by: Harald Alvestrand <hta@webrtc.org>
Commit-Queue: Johannes Kron <kron@webrtc.org>
Cr-Commit-Position: refs/heads/master@{#33916}
VideoCodecInitializer::SetupCodec never sets startBitrate,
so SetAv1SvcConfig shouldn't use it.
Bug: webrtc:12720
Change-Id: I04835dc27368f32c19132d93c72364173d7050fc
Reviewed-on: https://webrtc-review.googlesource.com/c/src/+/217382
Reviewed-by: Erik Språng <sprang@webrtc.org>
Commit-Queue: Danil Chapovalov <danilchap@webrtc.org>
Cr-Commit-Position: refs/heads/master@{#33915}
As INFO is aliased to LS_INFO, this didn't trigger any warnings or
compilation errors.
Bug: None
Change-Id: I1ed30c435d9ee6ea1b51d85a375d70135d3475e6
Reviewed-on: https://webrtc-review.googlesource.com/c/src/+/216689
Reviewed-by: Florent Castelli <orphis@webrtc.org>
Commit-Queue: Victor Boivie <boivie@webrtc.org>
Cr-Commit-Position: refs/heads/master@{#33912}
