Closed Bug 1654112 Opened 1 year ago Closed 1 month ago

[meta] Update libwebrtc to new stable branch 2H2020

Categories

(Core :: WebRTC, enhancement, P2)

enhancement

Tracking

()

RESOLVED FIXED
96 Branch
Tracking Status
relnote-firefox --- 96+
firefox96 --- fixed

People

(Reporter: ng, Assigned: pehrsons)

References

(Depends on 21 open bugs, Blocks 25 open bugs, Regressed 2 open bugs)

Details

(Keywords: meta)

Attachments

(303 files, 40 obsolete files)

48 bytes, text/x-phabricator-request
Details | Review
48 bytes, text/x-phabricator-request
Details | Review
48 bytes, text/x-phabricator-request
Details | Review
48 bytes, text/x-phabricator-request
Details | Review
48 bytes, text/x-phabricator-request
Details | Review
48 bytes, text/x-phabricator-request
Details | Review
48 bytes, text/x-phabricator-request
Details | Review
48 bytes, text/x-phabricator-request
Details | Review
48 bytes, text/x-phabricator-request
Details | Review
48 bytes, text/x-phabricator-request
Details | Review
48 bytes, text/x-phabricator-request
Details | Review
48 bytes, text/x-phabricator-request
Details | Review
48 bytes, text/x-phabricator-request
Details | Review
48 bytes, text/x-phabricator-request
Details | Review
48 bytes, text/x-phabricator-request
Details | Review
48 bytes, text/x-phabricator-request
Details | Review
48 bytes, text/x-phabricator-request
Details | Review
48 bytes, text/x-phabricator-request
Details | Review
48 bytes, text/x-phabricator-request
Details | Review
48 bytes, text/x-phabricator-request
Details | Review
48 bytes, text/x-phabricator-request
Details | Review
48 bytes, text/x-phabricator-request
Details | Review
48 bytes, text/x-phabricator-request
Details | Review
48 bytes, text/x-phabricator-request
Details | Review
48 bytes, text/x-phabricator-request
Details | Review
48 bytes, text/x-phabricator-request
Details | Review
48 bytes, text/x-phabricator-request
Details | Review
48 bytes, text/x-phabricator-request
Details | Review
48 bytes, text/x-phabricator-request
Details | Review
48 bytes, text/x-phabricator-request
Details | Review
48 bytes, text/x-phabricator-request
Details | Review
48 bytes, text/x-phabricator-request
Details | Review
48 bytes, text/x-phabricator-request
Details | Review
48 bytes, text/x-phabricator-request
Details | Review
48 bytes, text/x-phabricator-request
Details | Review
48 bytes, text/x-phabricator-request
Details | Review
48 bytes, text/x-phabricator-request
Details | Review
48 bytes, text/x-phabricator-request
Details | Review
48 bytes, text/x-phabricator-request
Details | Review
48 bytes, text/x-phabricator-request
Details | Review
48 bytes, text/x-phabricator-request
Details | Review
48 bytes, text/x-phabricator-request
Details | Review
48 bytes, text/x-phabricator-request
Details | Review
48 bytes, text/x-phabricator-request
Details | Review
48 bytes, text/x-phabricator-request
Details | Review
48 bytes, text/x-phabricator-request
Details | Review
48 bytes, text/x-phabricator-request
Details | Review
48 bytes, text/x-phabricator-request
Details | Review
48 bytes, text/x-phabricator-request
Details | Review
48 bytes, text/x-phabricator-request
Details | Review
48 bytes, text/x-phabricator-request
Details | Review
48 bytes, text/x-phabricator-request
Details | Review
48 bytes, text/x-phabricator-request
Details | Review
48 bytes, text/x-phabricator-request
Details | Review
48 bytes, text/x-phabricator-request
Details | Review
48 bytes, text/x-phabricator-request
Details | Review
48 bytes, text/x-phabricator-request
Details | Review
48 bytes, text/x-phabricator-request
Details | Review
48 bytes, text/x-phabricator-request
Details | Review
48 bytes, text/x-phabricator-request
Details | Review
48 bytes, text/x-phabricator-request
Details | Review
48 bytes, text/x-phabricator-request
Details | Review
48 bytes, text/x-phabricator-request
Details | Review
48 bytes, text/x-phabricator-request
Details | Review
48 bytes, text/x-phabricator-request
Details | Review
48 bytes, text/x-phabricator-request
Details | Review
48 bytes, text/x-phabricator-request
Details | Review
48 bytes, text/x-phabricator-request
Details | Review
48 bytes, text/x-phabricator-request
Details | Review
48 bytes, text/x-phabricator-request
Details | Review
48 bytes, text/x-phabricator-request
Details | Review
48 bytes, text/x-phabricator-request
Details | Review
48 bytes, text/x-phabricator-request
Details | Review
48 bytes, text/x-phabricator-request
Details | Review
48 bytes, text/x-phabricator-request
Details | Review
48 bytes, text/x-phabricator-request
Details | Review
48 bytes, text/x-phabricator-request
Details | Review
48 bytes, text/x-phabricator-request
Details | Review
48 bytes, text/x-phabricator-request
Details | Review
48 bytes, text/x-phabricator-request
Details | Review
48 bytes, text/x-phabricator-request
Details | Review
48 bytes, text/x-phabricator-request
Details | Review
48 bytes, text/x-phabricator-request
Details | Review
48 bytes, text/x-phabricator-request
Details | Review
48 bytes, text/x-phabricator-request
Details | Review
48 bytes, text/x-phabricator-request
Details | Review
48 bytes, text/x-phabricator-request
Details | Review
48 bytes, text/x-phabricator-request
Details | Review
48 bytes, text/x-phabricator-request
Details | Review
48 bytes, text/x-phabricator-request
Details | Review
48 bytes, text/x-phabricator-request
Details | Review
48 bytes, text/x-phabricator-request
Details | Review
48 bytes, text/x-phabricator-request
Details | Review
48 bytes, text/x-phabricator-request
Details | Review
48 bytes, text/x-phabricator-request
Details | Review
48 bytes, text/x-phabricator-request
Details | Review
48 bytes, text/x-phabricator-request
Details | Review
48 bytes, text/x-phabricator-request
Details | Review
48 bytes, text/x-phabricator-request
Details | Review
48 bytes, text/x-phabricator-request
Details | Review
48 bytes, text/x-phabricator-request
Details | Review
48 bytes, text/x-phabricator-request
Details | Review
48 bytes, text/x-phabricator-request
Details | Review
48 bytes, text/x-phabricator-request
Details | Review
48 bytes, text/x-phabricator-request
Details | Review
48 bytes, text/x-phabricator-request
Details | Review
48 bytes, text/x-phabricator-request
Details | Review
48 bytes, text/x-phabricator-request
Details | Review
48 bytes, text/x-phabricator-request
Details | Review
48 bytes, text/x-phabricator-request
Details | Review
48 bytes, text/x-phabricator-request
Details | Review
48 bytes, text/x-phabricator-request
Details | Review
48 bytes, text/x-phabricator-request
Details | Review
48 bytes, text/x-phabricator-request
Details | Review
48 bytes, text/x-phabricator-request
Details | Review
48 bytes, text/x-phabricator-request
Details | Review
48 bytes, text/x-phabricator-request
Details | Review
48 bytes, text/x-phabricator-request
Details | Review
48 bytes, text/x-phabricator-request
Details | Review
48 bytes, text/x-phabricator-request
Details | Review
48 bytes, text/x-phabricator-request
Details | Review
48 bytes, text/x-phabricator-request
Details | Review
48 bytes, text/x-phabricator-request
Details | Review
48 bytes, text/x-phabricator-request
Details | Review
48 bytes, text/x-phabricator-request
Details | Review
48 bytes, text/x-phabricator-request
Details | Review
48 bytes, text/x-phabricator-request
Details | Review
48 bytes, text/x-phabricator-request
Details | Review
48 bytes, text/x-phabricator-request
Details | Review
48 bytes, text/x-phabricator-request
Details | Review
48 bytes, text/x-phabricator-request
Details | Review
48 bytes, text/x-phabricator-request
Details | Review
48 bytes, text/x-phabricator-request
Details | Review
48 bytes, text/x-phabricator-request
Details | Review
48 bytes, text/x-phabricator-request
Details | Review
48 bytes, text/x-phabricator-request
Details | Review
48 bytes, text/x-phabricator-request
Details | Review
48 bytes, text/x-phabricator-request
Details | Review
48 bytes, text/x-phabricator-request
Details | Review
48 bytes, text/x-phabricator-request
Details | Review
48 bytes, text/x-phabricator-request
Details | Review
48 bytes, text/x-phabricator-request
Details | Review
48 bytes, text/x-phabricator-request
Details | Review
48 bytes, text/x-phabricator-request
Details | Review
48 bytes, text/x-phabricator-request
Details | Review
48 bytes, text/x-phabricator-request
Details | Review
48 bytes, text/x-phabricator-request
Details | Review
48 bytes, text/x-phabricator-request
Details | Review
48 bytes, text/x-phabricator-request
Details | Review
48 bytes, text/x-phabricator-request
Details | Review
48 bytes, text/x-phabricator-request
Details | Review
48 bytes, text/x-phabricator-request
Details | Review
48 bytes, text/x-phabricator-request
Details | Review
48 bytes, text/x-phabricator-request
Details | Review
48 bytes, text/x-phabricator-request
Details | Review
48 bytes, text/x-phabricator-request
Details | Review
48 bytes, text/x-phabricator-request
Details | Review
48 bytes, text/x-phabricator-request
Details | Review
48 bytes, text/x-phabricator-request
Details | Review
48 bytes, text/x-phabricator-request
Details | Review
48 bytes, text/x-phabricator-request
Details | Review
48 bytes, text/x-phabricator-request
Details | Review
48 bytes, text/x-phabricator-request
Details | Review
48 bytes, text/x-phabricator-request
Details | Review
48 bytes, text/x-phabricator-request
Details | Review
48 bytes, text/x-phabricator-request
Details | Review
48 bytes, text/x-phabricator-request
Details | Review
48 bytes, text/x-phabricator-request
Details | Review
48 bytes, text/x-phabricator-request
Details | Review
48 bytes, text/x-phabricator-request
Details | Review
48 bytes, text/x-phabricator-request
Details | Review
48 bytes, text/x-phabricator-request
Details | Review
48 bytes, text/x-phabricator-request
Details | Review
48 bytes, text/x-phabricator-request
Details | Review
48 bytes, text/x-phabricator-request
Details | Review
48 bytes, text/x-phabricator-request
Details | Review
48 bytes, text/x-phabricator-request
Details | Review
48 bytes, text/x-phabricator-request
Details | Review
48 bytes, text/x-phabricator-request
Details | Review
48 bytes, text/x-phabricator-request
Details | Review
48 bytes, text/x-phabricator-request
Details | Review
48 bytes, text/x-phabricator-request
Details | Review
48 bytes, text/x-phabricator-request
Details | Review
48 bytes, text/x-phabricator-request
Details | Review
48 bytes, text/x-phabricator-request
Details | Review
48 bytes, text/x-phabricator-request
Details | Review
48 bytes, text/x-phabricator-request
Details | Review
48 bytes, text/x-phabricator-request
Details | Review
48 bytes, text/x-phabricator-request
Details | Review
48 bytes, text/x-phabricator-request
Details | Review
48 bytes, text/x-phabricator-request
Details | Review
48 bytes, text/x-phabricator-request
Details | Review
48 bytes, text/x-phabricator-request
Details | Review
48 bytes, text/x-phabricator-request
Details | Review
48 bytes, text/x-phabricator-request
Details | Review
48 bytes, text/x-phabricator-request
Details | Review
48 bytes, text/x-phabricator-request
Details | Review
48 bytes, text/x-phabricator-request
Details | Review
48 bytes, text/x-phabricator-request
Details | Review
48 bytes, text/x-phabricator-request
Details | Review
48 bytes, text/x-phabricator-request
Details | Review
48 bytes, text/x-phabricator-request
Details | Review
48 bytes, text/x-phabricator-request
Details | Review
48 bytes, text/x-phabricator-request
Details | Review
48 bytes, text/x-phabricator-request
Details | Review
48 bytes, text/x-phabricator-request
Details | Review
48 bytes, text/x-phabricator-request
Details | Review
48 bytes, text/x-phabricator-request
Details | Review
48 bytes, text/x-phabricator-request
Details | Review
48 bytes, text/x-phabricator-request
Details | Review
48 bytes, text/x-phabricator-request
Details | Review
48 bytes, text/x-phabricator-request
Details | Review
48 bytes, text/x-phabricator-request
Details | Review
48 bytes, text/x-phabricator-request
Details | Review
48 bytes, text/x-phabricator-request
Details | Review
48 bytes, text/x-phabricator-request
Details | Review
48 bytes, text/x-phabricator-request
Details | Review
48 bytes, text/x-phabricator-request
Details | Review
48 bytes, text/x-phabricator-request
Details | Review
48 bytes, text/x-phabricator-request
Details | Review
48 bytes, text/x-phabricator-request
Details | Review
48 bytes, text/x-phabricator-request
Details | Review
48 bytes, text/x-phabricator-request
Details | Review
48 bytes, text/x-phabricator-request
Details | Review
48 bytes, text/x-phabricator-request
Details | Review
48 bytes, text/x-phabricator-request
Details | Review
48 bytes, text/x-phabricator-request
Details | Review
48 bytes, text/x-phabricator-request
Details | Review
48 bytes, text/x-phabricator-request
Details | Review
48 bytes, text/x-phabricator-request
Details | Review
48 bytes, text/x-phabricator-request
Details | Review
48 bytes, text/x-phabricator-request
Details | Review
48 bytes, text/x-phabricator-request
Details | Review
48 bytes, text/x-phabricator-request
Details | Review
48 bytes, text/x-phabricator-request
Details | Review
48 bytes, text/x-phabricator-request
Details | Review
48 bytes, text/x-phabricator-request
Details | Review
48 bytes, text/x-phabricator-request
Details | Review
48 bytes, text/x-phabricator-request
Details | Review
48 bytes, text/x-phabricator-request
Details | Review
48 bytes, text/x-phabricator-request
Details | Review
48 bytes, text/x-phabricator-request
Details | Review
48 bytes, text/x-phabricator-request
Details | Review
48 bytes, text/x-phabricator-request
Details | Review
48 bytes, text/x-phabricator-request
Details | Review
48 bytes, text/x-phabricator-request
Details | Review
48 bytes, text/x-phabricator-request
Details | Review
48 bytes, text/x-phabricator-request
Details | Review
48 bytes, text/x-phabricator-request
Details | Review
48 bytes, text/x-phabricator-request
Details | Review
48 bytes, text/x-phabricator-request
Details | Review
48 bytes, text/x-phabricator-request
Details | Review
48 bytes, text/x-phabricator-request
Details | Review
48 bytes, text/x-phabricator-request
Details | Review
48 bytes, text/x-phabricator-request
Details | Review
48 bytes, text/x-phabricator-request
Details | Review
48 bytes, text/x-phabricator-request
Details | Review
48 bytes, text/x-phabricator-request
Details | Review
48 bytes, text/x-phabricator-request
Details | Review
48 bytes, text/x-phabricator-request
Details | Review
48 bytes, text/x-phabricator-request
Details | Review
48 bytes, text/x-phabricator-request
Details | Review
48 bytes, text/x-phabricator-request
Details | Review
48 bytes, text/x-phabricator-request
Details | Review
48 bytes, text/x-phabricator-request
Details | Review
48 bytes, text/x-phabricator-request
Details | Review
48 bytes, text/x-phabricator-request
Details | Review
48 bytes, text/x-phabricator-request
Details | Review
48 bytes, text/x-phabricator-request
Details | Review
48 bytes, text/x-phabricator-request
Details | Review
48 bytes, text/x-phabricator-request
Details | Review
48 bytes, text/x-phabricator-request
Details | Review
48 bytes, text/x-phabricator-request
Details | Review
48 bytes, text/x-phabricator-request
Details | Review
48 bytes, text/x-phabricator-request
Details | Review
48 bytes, text/x-phabricator-request
Details | Review
48 bytes, text/x-phabricator-request
Details | Review
48 bytes, text/x-phabricator-request
Details | Review
48 bytes, text/x-phabricator-request
Details | Review
48 bytes, text/x-phabricator-request
Details | Review
48 bytes, text/x-phabricator-request
Details | Review
48 bytes, text/x-phabricator-request
Details | Review
48 bytes, text/x-phabricator-request
Details | Review
48 bytes, text/x-phabricator-request
Details | Review
48 bytes, text/x-phabricator-request
Details | Review
48 bytes, text/x-phabricator-request
Details | Review
48 bytes, text/x-phabricator-request
Details | Review
48 bytes, text/x-phabricator-request
Details | Review
48 bytes, text/x-phabricator-request
Details | Review
48 bytes, text/x-phabricator-request
Details | Review
48 bytes, text/x-phabricator-request
Details | Review
48 bytes, text/x-phabricator-request
Details | Review
48 bytes, text/x-phabricator-request
Details | Review
48 bytes, text/x-phabricator-request
Details | Review
48 bytes, text/x-phabricator-request
Details | Review
48 bytes, text/x-phabricator-request
Details | Review
48 bytes, text/x-phabricator-request
Details | Review
48 bytes, text/x-phabricator-request
Details | Review
48 bytes, text/x-phabricator-request
Details | Review
48 bytes, text/x-phabricator-request
Details | Review
48 bytes, text/x-phabricator-request
Details | Review
48 bytes, text/x-phabricator-request
Details | Review
48 bytes, text/x-phabricator-request
Details | Review

This bug tracks the work necessary for the next libwebrtc update (2H 2020).

Information needed to perform this update is documented at https://wiki.mozilla.org/Media/WebRTC/libwebrtc_Update_2H2020 .

The process is documented at https://wiki.mozilla.org/Media/WebRTC/libwebrtc_Update_Process .

Priority: P1 → P2
Depends on: 1646904
Depends on: 1654119
Depends on: 1654137
Summary: [meta] Update libwebrtc → [meta] Update libwebrtc to new stable branch (tbd)
Duplicate of this bug: 1607238
Summary: [meta] Update libwebrtc to new stable branch (tbd) → [meta] Update libwebrtc to new stable branch 2H2020
Blocks: 1654407
Blocks: 1671169
Depends on: 1677715
Blocks: 1412333
Blocks: 1654642
Blocks: 1576576
Blocks: 1685671
Blocks: 1685672

This code comes from the rollup of previous local patches to upstream, see
https://hg.mozilla.org/mozilla-central/rev/c8e80efab6fb

Assignee: nobody → apehrson
Status: NEW → ASSIGNED

This is needed to not regress stats. The milliseconds unit is more prevalent in
upstream stats code for timestamps.

This is similar to how it's already included for video send.

This switches those integrations to the new Call API, with AudioSendStream and
AudioReceiveStream as main API surface.

This will be used in PeerConnectionImpl for packetsReceived on the send side (from
rtcp).

For AudioConduit this patch:

  • Uses correct threads per upstream, pending changes due to the threading model
    update that's coming
  • Uses new upstream APIs for getting data for the stats

For VideoConduit this patch:

  • Removes the stats polling 1-second timer on main thread
  • Removes the internal stats classes for bridging stats between main and sts
    threads
  • Uses correct threads for direct access per upstream's thread checkers, pending
    changes due to the threading model

For PeerConnectionImpl and RTCRtpReceiver this patch:

  • Manages thread access into conduits for thread safety
  • Translates from the upstream Stats data models into our DOM data model

These fixes are made to reflect the current state of upstream.

This sets up our code for creating Call instances so that it works. A number of
things are left to do. Most notable fixing the threading model.

Until that is fixed we use main thread as a "worker" but this is not ideal.
It both leads to excessive work loads on main, and us having to set main as
"current" (using upstream's API for this) whenever we call into main from what
is meant to be the worker thread.

Doing this outside of the dtor, especially, removes the requirement of the
conduit having to be destroyed on the worker thread (main).

Not having threading requirements on the conduits' dtor means we can constify
conduit members everywhere, so this patch tries to do that, for added compiler
guarantees.

This assert was intended for when an input stream is started with processing on.
However, when an input stream is started with processing off, then later when
processing is turned on it triggers this assert because mSegment contains data
from InsertInGraph().

Depends on D102289

The new AudioProcessingImpl, and in particular AEC3 it seems, is more picky on
the ordering here. After an ApplyConfig() it needs ProcessStream() to properly
set up its internals. ProcessReverseStream sets off various asserts, for
instance because the processing sample rate is not divisible by 16k if
ProcessReverseStream() was fed 44.1k.

The input stream is not processed in passthrough, so we should skip feeding the
reverse stream too, to stay in sync.

This should also work with libwebrtc 64 currently in-tree.

This removes some prefs for things that are now unconfigurable, and adds some
others. This also re-works the plumbing for updating the config on the fly
through applyConstraints(). It is now simpler, with a single infallible call
to apply the new config.

Before this patch, if the data callback contained less data than was left over
from the last iteration, we'd skip iterating the graph again. The graph is
however is capable of 0-iterations.

This patch removes that optimization so that every data callback results in an
iteration.

This makes it easier to defer processing input data to the track processing
step, as is needed by the next patch in this stack, to keep input and output as
handed to AudioProcessing in sync as config changes are applied with
ControlMessages (in between input and output data notifications). With input
data processing deferred, ControlMessages are run first, then processing of
input data and last processing of output data.

These previously allowed a discontinuity at the beginning of the output stream.
This will no longer occur.

This is similar to the behavior prior to bug 1532898, which introduced telemetry
reporting every second. There's a bit of a difference in that we only report
data if we had collected some though the 1 second interval telemetry timer in
PeerConnectionCtx.

See Also: → 1646134
Blocks: 1689043
Assignee: apehrson → nobody
Status: ASSIGNED → NEW
Assignee: nobody → apehrson
Status: NEW → ASSIGNED

This patch uses mMutex to guarantee that the audio thread does not get things
pulled away from under its feet. When on the AudioProxyThread we allow blocking
the thread during negotiation. On the real audio thread we avoid blocking on
the mutex and return an error instead. This leads to us inserting silence --
which is fine since we're going through negotiation (call setup, teardown or
re-negotiation).

This also fixes the channel count when GetAudioFrame fails and we insert
silence, to use the same channel count as when we last appended real data. Since
changing the channel count can in the worst case cause a GraphDriver switch.

Prior to this patch, WebRtcCallWrapper would never clear the call instance,
meaning that it went away as the classes having references to WebRtcCallWrapper
also went away. These are mainly TransceiverImpl and PeerConnectionMedia. There
are other classes too, but these are the ones that have a strong-ref to
WebRtcCallWrapper and are cycle collected.

With the worker threads in libwebrtc now using our own TaskQueues on top of
our SharedThreadPool instances, the ownership model of the Call instances
becomes problematic.

On shutdown, xpcom-shutdown-threads happens before the cycle collector is shut
down.

xpcom-shutdown-threads will however, block shutdown until all SharedThreadPools
have been shut down.

If a TransceiverImpl sits around until shutdown it will only be destroyed
during cycle collector shutdown. And, alas, we reach a deadlock as we're stuck
in xpcom-shutdown-threads until that TransceiverImpl gets destroyed.

This patch decouples the Call instance lifetime from the cycle collector, and
avoids the shutdown hang.

This is especially prompted by a recurring task from libwebrtc that when run
will schedule itself again, which on a shut down TaskQueue will fail an assert.
The behavior with this patch better mirrors blink's behavior, and is what
libwebrtc expects.

Blocks: 1689340
No longer blocks: 1689340
Blocks: 1688342
Depends on: 1691457

This prepares VideoConduit for the decoder/encoder factory and its handling of
PluginID.

This hooks up VideoReceiveStream::Config with a decoder factory and
VideoSendStream::Config with both a bitrate allocator factory and an encoder
factory.

The factories are new and wraps all the types of decoders and encoders that we
could create prior to this patch. This patch lets libwebrtc create our codecs
on demand, allowing for reconfiguring the streams and changing codec on the fly.

Some extra care had to be taken to ensure that GMP PluginIDs can be properly
plumbed. This has been implemented with MediaEvent and MozPromise.

Some other changes are made too, as warranted by runtime or threading changes.

With the libwebrtc update codecs will be created off-main.
WebrtcGmpPCHandleSetter not only adds a lot of indirection, it is also
main-thread-only. With this patch we can create gmp codecs on any thread.

Attachment #9202383 - Attachment description: Bug 1654112 - Adapt the webrtc H264 codecs to the updated libwebrtc version. r?bwc! → Bug 1654112 - Adapt the webrtc codecs to the updated libwebrtc version. r?bwc!

The RTPFragmentationHeader is gone. Instead the packetizer scans for NALU start
sequences. OpenH264 encodes them natively, but converts them to NALU sizes
as expected by GMP.

This patch puts the start sequences back in, in-place, and mitigates bug 1533001
by allowing the start sequences that leak through from OpenH264 straight through
to libwebrtc, as an audit of OpenH264 code shows this can happen when internally
OpenH264 reports a NAL count of 0.

Depends on D105012

  • Pull in sdk/objc/base and sdk/objc/helpers
  • Ensure that -fobjc-arc is set (have to use cflags, because our gn -> json scripting ignores things like cflags_objc)
  • Add gclient_args.gni to keep build happy.
  • Add a missing include path for libyuv
  • Support .m files in build.

Depends on D105014

Depends on D105016

Depends on D105018

Peer connections are cleaned up on shutdown through an xpcom-shutdown observer.
It doesn't block shutdown throughout async operations.

webrtc::Call instances are currently used on the main thread and so destroying
them could be a sync operation. However with upcoming threading model changes
they are moving to a worker thread, making destruction of them inevitably async.

Currently they are even destroyed async, even though still on the main thread,
to better simulate the behavior we'd see with the threading model updated.

Destroying Call instances async can lead to a race between destroying all
TaskQueues tied to the Call instance, and reaching xpcom-shutdown-threads.
xpcom-shutdown-threads will shut down the thread pools backing the TaskQueues,
making future dispatches fail. We have asserts checking that dispatches always
succeed.

This patch adds a ShutdownBlocker to the Call wrapper, so progressing to
xpcom-shutdown-threads is blocked until Call instances are cleaned up and no
more dispatches are expected.

Attachment #9198393 - Attachment description: Bug 1654112 - Reset AudioProcessing when turning it off to clear old state. r?padenot → Bug 1654112 - Reset AudioProcessing when turning it off to clear old state.
Attachment #9198394 - Attachment description: Bug 1654112 - Minor tidying around lambdas and ControlMessages in MediaEngineWebRTCAudio. r?padenot → Bug 1654112 - Minor tidying around lambdas and ControlMessages in MediaEngineWebRTCAudio.
Attachment #9198395 - Attachment description: Bug 1654112 - Update microphone DeviceChanged handler to new AudioProcessing API. r?padenot → Bug 1654112 - Update microphone DeviceChanged handler to new AudioProcessing API.
Attachment #9198396 - Attachment description: Bug 1654112 - Adapt to new AudioProcessing config. r?padenot → Bug 1654112 - Adapt to new AudioProcessing config.

This helps with ASAN link errors. Cherry-picked commit's commit message:

310c82cd97b3f1f0d1ee93a0ee2b0aee828b2a93 by Abseil Team <absl-team@google.com>:

Simplify unaligned memory access functions.

The #ifdef to produce calls to __sanitizer_unaligned_load16 etc were needed in past versions of this code, when we were lying to the compiler about the alignment of the loads/stores, by using a reinterpret_cast.

However, a year ago, absl switched to simply use memcpy. Sanitizers support this correctly by default, nothing extra is required.

PiperOrigin-RevId: 343159883

This patch fixes a linker error where __sanitizer_annotate_contiguous_container
is an undefined hidden symbol referenced by
third_party/libwebrtc/third_party/abseil-cpp/absl/container/fixed_array.h.

Attachment #9204241 - Attachment is obsolete: true
Attachment #9204242 - Attachment is obsolete: true
Attachment #9204244 - Attachment is obsolete: true
Depends on: 1695580
Attachment #9200073 - Attachment description: Bug 1654112 - Fix up AudioConduit audio thread access model, and the channel count for silence. r?padenot → Bug 1654112 - Fix up AudioConduit audio thread access model, and the channel count for silence.
Attachment #9198390 - Attachment description: Bug 1654112 - Remove overzealous assert. r?padenot → Bug 1654112 - Remove overzealous assert.
Attachment #9198523 - Attachment description: Bug 1654112 - Always iterate the graph on a data callback. r?padenot → Bug 1654112 - Always iterate the graph on a data callback.
Attachment #9198391 - Attachment description: Bug 1654112 - Always start feeding input to audio processing, and always end on output. r?padenot → Bug 1654112 - Always start feeding input to audio processing, and always end on output.
Attachment #9198392 - Attachment description: Bug 1654112 - Don't feed the reverse stream in passthrough. r?padenot → Bug 1654112 - Don't feed the reverse stream in passthrough.
Attachment #9198524 - Attachment description: Bug 1654112 - Tighten AudioTrackGraph gtest discontinuity checks, and fix related comment/log. r?padenot → Bug 1654112 - Tighten AudioTrackGraph gtest discontinuity checks, and fix related comment/log.
Attachment #9200077 - Attachment is obsolete: true
Attachment #9204919 - Attachment description: Bug 1654112 - Add a compile-time error if WEBRTC_USE_X11 is not defined when compiling X11 desktop capture. r?ng! → Bug 1654112 - Add compile-time errors if WEBRTC_USE_X11 or MOZ_X11 are not defined when compiling X11 desktop capture. r?ng!
Attachment #9205532 - Attachment is obsolete: true
Attachment #9205533 - Attachment is obsolete: true
Attachment #9205534 - Attachment is obsolete: true
Attachment #9205535 - Attachment is obsolete: true
Attachment #9204918 - Attachment description: Bug 1654112 - Add sanitizer/common_interface_defs.h as system-header. r?bwc! → Bug 1654112 - Add sanitizer/common_interface_defs.h as system-header. r?glandium!
Blocks: 1697369
Blocks: 1697385

WIP Note: originally I commented this block out instead of checking build_with_mozilla. This needs to be checked.

Depends on D107900

Blocks: 1697862

This removes media/signaling/src from ThirdPartyPaths.txt as that directory no
longer exists.

It also adds media/signaling/gtest/MockCall.h as the new SetAndGetRecordingState
from upstream libwebrtc causes a static-analysis error because RecordingState is
a non-param type.

Blocks: 1509669

The cache can be a lot simpler than the old one, since libwebrtc handles
pruning old entries for us, and since we're now telling libwebrtc when we
render frames (so we don't have to make our own stats with guesses of when
frames will be rendered). We also now have a single implementation of the
cache in the base class.

This patch also removes the old CSRC stat insertion stuff we hacked into
libwebrtc for testing, and handles that stuff in our cache directly.

Involves teaching MediaPipelineFilter to learn ssrc->rid mappings more
aggressively since libwebrtc does not emit RTP RID more than once.

Also involves modifying a couple of mochitests to look for RTP RID in
the first packet, instead of whichever one comes in next once we care.

  • Make sure this is 0 if we have not received a RTCP RR yet.
  • Subtract the packet lost count from the result.

The divisor here was already in ticks/sec, which results in a conversion to seconds.

libwebrtc has stopped surfacing these, and Chromium does not support
these stats at all.

We've never supported the spec spelling (packetsDiscarded), and libwebrtc
no longer supports discarded packet count anyway.

Alsol, make sure errors note what kind of stats (audio/video) are being checked.

Attachment #9204243 - Attachment description: Bug 1654112: Fix comparison so capture timestamps aren't allowed to be 0. → Bug 1654112: libwebrtc modification: Fix comparison so capture timestamps aren't allowed to be 0.
Blocks: 1698995
Attachment #9198524 - Attachment is obsolete: true

This makes the latest changes to base_capturer_pipewire in our tree compile
with the libwebrtc 86 update.

Depends on D107902

Depends on D110895

Attachment #9208510 - Attachment is obsolete: true
Attachment #9208887 - Attachment description: Bug 1654112 - Add protobuff for AEC dumping;r?pehrsons → Abandoned - Bug 1654112 - Add protobuff for AEC dumping;r?pehrsons
Attachment #9208214 - Attachment is obsolete: true
Attachment #9208887 - Attachment is obsolete: true
Attachment #9214461 - Attachment is obsolete: true

It turns out these were in a specific order. This restores the order from a previous patch.

Attachment #9213645 - Attachment is obsolete: true

Additionally:

  • Trys to detect non-generated build file changes to the tree and exits to prevent a clean checkout and clobber
  • Adds the DEBUG_GEN variable that prints what is happening in the script
  • tee(s) the output of mach into the logs so one doesn't have to check them to see what went wrong
  • Removes the symlinked directories if they were left over by a failed run
  • Exits as soon as anything exits unexpectedly
  • Adds an optional variable so that the git checkout and the gclient sync checkout can live in different locations
Attachment #9215958 - Attachment description: Bug 1654112 - Switching to a precomputed list of valuse for win setup_tools.py;r?dminor → Bug 1654112 - Switching to a precomputed list of values for win setup_tools.py;r?dminor

This patch also renames it WebrtcCallWrapper, to better match neighboring
classes starting on "Webrtc".

Ideally TaskQueueWrapper would implement both webrtc::TaskQueueBase and
AbstractThread, but webrtc::TaskQueueBase is not refcounted so implementing both
in the same class is not possible.

This patch implements AbstractThread in a refcounted class CallWorkerThread on
top of TaskQueueWrapper. Because of SharedModuleThread not having a thread-safe
refcount, the same CallWorkerThread must be used for all Call instances.
PeerConnectionCtx's SharedWebrtcState becomes the canonical owner of the
CallWorkerThread, but WebrtcCallWrappers will all hold a strong reference to it.

The new AbstractThread allows us to use webrtc TaskQueues with Mozilla
specific higher order threading functions, like MozPromise, MediaEventSource,
and whatever else could come in handy, without having to use an explicit
CurrentTaskQueueSetter (which is not public) everywhere.

This patch makes mediapipeline_unittest pass an event target to the Call wrapper
but keeps using main thread for that event target, so that it can keep the test
cases simpler by allowing sybnc calls into the conduits.

TaskQueueWrapper::MainAsCurrent is no longer needed as mediapipeline_unittest
was the last user of it. A simpler version remains dedicated in
mediapipeline_unittest instead.

On its own this patch should mainly be seen as a refactor of the way we handle
races in MediaPipelineTransmit::SetTrack. Namely that Stop() immediately
followed by SetTrack is still exposed to racing.

This patch makes MediaPipelines use a WatchManager to manage their tracks.
For receive pipelines, mActive triggers whether we enable the receive listener.
For transmit pipelines, mActive and mDomTrack triggers whether mDomTrack's
underlying track is hooked up to feed data to the conduit. A hole is also
punched through this allowing for unittests to set an overriden send track
to avoid mocking a MediaStreamTrack (DOM object) and MediaTrackGraph (several
non-virtual methods we'd need to override).

This patch also moves the responsibility of starting and stopping conduits to
the transceiver/receiver objects, where most of the interaction with the
conduits is already happening.

This patch mainly sets the stage for using codec configs with StateMirroring.
It also makes the code a bit simpler, and removes the need to handle the nullptr
case.

SharedWebrtcState when managed by PeerConnectionCtx is destroyed on main thread.
With interactions with call instances being async, there is no guarantee that
the objects owned by SharedWebrtcState (and referenced through raw pointers by
Call instances) are alive until all Call instances have been destroyed.

Making the WebrtcCallWrapper keep a strong-ref to the SharedWebrtcState will
ensure it can manage the lifetime of the SharedWebrtcState for as long as
necessary.

The logic used for choosing codec in this patch mimics the logic of Chromium.

The case added by this patch will be used to destroy the worker thread on
shutdown, and have it release its resources. In particular the reference to its
underlying thread pool, which could be blocking shutdown.

Unittests might still use the case where responsibility of destroying the worker
thread is left external, to allow using SharedWebrtcState with main thread as
the worker thread.

This patch makes the Call worker thread support tail dispatch, since it is
required for state mirroring. Since tail dispatch is only used when both the
source and target threads support it, this will only be used for dispatch to or
from other Mozilla threads.

TaskQueueWrappers created by libwebrtc are made to not support tail dispatch,
since upstream code assumes that dispatches are direct. In some places this is
seen by one webrtc::TaskQueue dispatching a task to another webrtc::TaskQueue
and blocks the source thread until the task has run on the target.

This patch makes AudioConduit mimic VideoConduit. We no longer do any
thread-specific actions in the dtor, so this assert is unnecessary.

With VideoConduit setters being called on a worker thread rather than main, we
no longer have a guarantee that the first SendVideoFrame comes in after ssrcs
or the send codec config are set.

Setters have so far been called on main, and the listener that triggers
SendVideoFrame is also managed by main. Using a call worker thread would change
the thread the setters run on, but not the SendVideoFrame triggering listener.
This breaks the guarantee.

Assignee: apehrson → nobody
Status: ASSIGNED → NEW
Assignee: nobody → apehrson
Status: NEW → ASSIGNED
  • Our build didn't like the non-explictness of these conversion
    operators.
  • Changing to explicit required casts in various places.
Depends on: 1707769
  • pipefail doesn't work in sh on linux, so we're running under bash now.
  • removing a link for third_party/libwebrtc/.git didn't work due to a typo.
  • needed export on MOZCONFIG line.
  • only grab the specific .json file based on the build config.

Depends on D113439

Depends on D113605

Blocks: 1702781

Only works on Chrome because they zero out alloced memory?

Involves calling CFRunLoopRunInMode on the capture thread (so the RunLoop gets
cycles), and also calling Start() on the capture thread (so the callbacks are
connected to the same RunLoop we're giving cycles to).

Fixing error: format specifies type 'long' but the argument has type 'webrtc::ScreenId' (aka 'int')

Attachment #9218517 - Attachment is obsolete: true

Andrew, I do not believe any of the work in this bug has landed. There is still much work to be done. We are doing a number of try builds as we try to green this up.

Flags: needinfo?(continuation)

Oops, I meant to post that in another bug. Sorry.

Flags: needinfo?(kwright)
Flags: needinfo?(continuation)
Blocks: 1710676