Previously: Failed to setup RTCP mux filter.
Now: rtcpMuxPolicy is 'require', but media description does not
contain 'a=rtcp-mux'.
BUG=webrtc:6966
Review-Url: https://codereview.webrtc.org/2622553003
Cr-Commit-Position: refs/heads/master@{#16062}
Previously, BaseChannel supported a "no RTCP" mode, which wasn't
being used any more and is being deleted.
Also, "RTCP mux required" previously worked by calling "ActivateRtcpMux"
after construction. Now it works by explicitly passing a
"require_rtcp_mux" parameter into the constructor.
BUG=None
Review-Url: https://codereview.webrtc.org/2622613004
Cr-Commit-Position: refs/heads/master@{#16045}
The BaseChannel can set the transport directly without depending on
TransportController.
When initializing the network of the BaseChannel, the ChannelManager will
create TransportChannels with the TransportController.
When enabling bundling, WebRtcSession will get or create TransportChannels
with the TransportController.
When a TransportChannel of the BaseChannel needs to be destroyed, it will
fire a signal to notify the WebRtcSession.
BUG=none.
Review-Url: https://codereview.webrtc.org/2614263002
Cr-Commit-Position: refs/heads/master@{#16043}
Bulk of the changes were produced using
git grep -l ' ASSERT(' | grep -v test | grep -v 'common\.h' |\
xargs -n1 sed -i 's/ ASSERT(/ RTC_DCHECK(/'
followed by additional includes of base/checks.h in affected files,
and git cl format.
Also had to do some tweaks to #if !defined(NDEBUG) logic in the
taskrunner code (webrtc/base/task.cc, webrtc/base/taskparent.cc,
webrtc/base/taskparent.h, webrtc/base/taskrunner.cc), replaced to
consistently use RTC_DCHECK_IS_ON, and some of the checks needed
additional #if protection.
Test code was excluded, because it should probably use RTC_CHECK
rather than RTC_DCHECK.
BUG=webrtc:6424
Review-Url: https://codereview.webrtc.org/2620303003
Cr-Commit-Position: refs/heads/master@{#16030}
Bulk of changes done using
git grep -l 'RTC_DCHECK(false)' | \
xargs sed -i 's/RTC_DCHECK(false)/RTC_NOTREACHED()/'
peerconnection.cc also used RTC_DCHECK(false && "msg") in two places,
which were updated manually.
BUG=webrtc:6424
Review-Url: https://codereview.webrtc.org/2623313004
Cr-Commit-Position: refs/heads/master@{#16026}
This cl was produced by
git grep -l 'ASSERT(false)' |\
xargs -n1 sed -i 's/ASSERT(false)/RTC_NOTREACHED()/'
followed by additional includes of base/checks.h in affected files,
git cl format to adjust spacing in webrtc/base/transformadapter.cc.
Finally, to make presubmit happy, one unnamed TODO marker was deleted
in that file.
This is a step towards deletion of base/common.h.
BUG=webrtc:6424
Review-Url: https://codereview.webrtc.org/2625003003
Cr-Commit-Position: refs/heads/master@{#16009}
The BaseChannel code is geared around RTP; the presence of media engines,
send and receive streams, SRTP, SDP directional attribute negotiation, etc.
It doesn't make sense to use it for SCTP as well. This separation should make
future work both on BaseChannel and the SCTP code paths easier.
SctpDataEngine now becomes SctpTransport, and is used by WebRtcSession
directly. cricket::DataChannel is also renamed, to RtpDataChannel, so it
doesn't get confused with webrtc::DataChannel any more.
Beyond just moving code around, some consequences of this CL:
- We'll now stop using the worker thread for SCTP. Packets will be
processed right on the network thread instead.
- The SDP directional attribute is ignored, as it's supposed to be.
BUG=None
Review-Url: https://codereview.webrtc.org/2564333002
Cr-Original-Commit-Position: refs/heads/master@{#15906}
Committed: 67b3bbe639
Review-Url: https://codereview.webrtc.org/2564333002
Cr-Commit-Position: refs/heads/master@{#15973}
Reason for revert:
Hitting DCHECK in chromium's WebrtcTransportTest.TerminateDataChannel and WebrtcTransportTest.DataStreamLate. Will investigate and reland.
Original issue's description:
> Separating SCTP code from BaseChannel/MediaChannel.
>
> The BaseChannel code is geared around RTP; the presence of media engines,
> send and receive streams, SRTP, SDP directional attribute negotiation, etc.
> It doesn't make sense to use it for SCTP as well. This separation should make
> future work both on BaseChannel and the SCTP code paths easier.
>
> SctpDataEngine now becomes SctpTransport, and is used by WebRtcSession
> directly. cricket::DataChannel is also renamed, to RtpDataChannel, so it
> doesn't get confused with webrtc::DataChannel any more.
>
> Beyond just moving code around, some consequences of this CL:
> - We'll now stop using the worker thread for SCTP. Packets will be
> processed right on the network thread instead.
> - The SDP directional attribute is ignored, as it's supposed to be.
>
> BUG=None
>
> Review-Url: https://codereview.webrtc.org/2564333002
> Cr-Commit-Position: refs/heads/master@{#15906}
> Committed: 67b3bbe639TBR=pthatcher@webrtc.org
# Skipping CQ checks because original CL landed less than 1 days ago.
NOPRESUBMIT=true
NOTREECHECKS=true
NOTRY=true
BUG=None
Review-Url: https://codereview.webrtc.org/2614813003
Cr-Commit-Position: refs/heads/master@{#15908}
The BaseChannel code is geared around RTP; the presence of media engines,
send and receive streams, SRTP, SDP directional attribute negotiation, etc.
It doesn't make sense to use it for SCTP as well. This separation should make
future work both on BaseChannel and the SCTP code paths easier.
SctpDataEngine now becomes SctpTransport, and is used by WebRtcSession
directly. cricket::DataChannel is also renamed, to RtpDataChannel, so it
doesn't get confused with webrtc::DataChannel any more.
Beyond just moving code around, some consequences of this CL:
- We'll now stop using the worker thread for SCTP. Packets will be
processed right on the network thread instead.
- The SDP directional attribute is ignored, as it's supposed to be.
BUG=None
Review-Url: https://codereview.webrtc.org/2564333002
Cr-Commit-Position: refs/heads/master@{#15906}
Reason for revert:
Doing a reland where systeminfo.cc includes basictypes.h so that CPU_X86 etc. are defined when they are checked/used.
Original issue's description:
> Revert of Replace basictypes.h with stdint.h for int_t types. (patchset #1 id:1 of https://codereview.webrtc.org/2604043002/ )
>
> Reason for revert:
> Very likely cause of Chromium import bot breakage (unused function '__cpuid'), TBD why.
>
> Original issue's description:
> > Replace basictypes.h with stdint.h for int_t types.
> >
> > Removes basictypes.h for types that only makes use of it for fixed-size-int
> > typedefs and replaces it with stdint.h.
> >
> > BUG=webrtc:6853
> > R=tommi@webrtc.org
> >
> > Review-Url: https://codereview.webrtc.org/2604043002
> > Cr-Commit-Position: refs/heads/master@{#15867}
> > Committed: 7fd1a75300
>
> TBR=tommi@webrtc.org
> # Skipping CQ checks because original CL landed less than 1 days ago.
> NOPRESUBMIT=true
> NOTREECHECKS=true
> NOTRY=true
> BUG=webrtc:6853
>
> Review-Url: https://codereview.webrtc.org/2603203003
> Cr-Commit-Position: refs/heads/master@{#15869}
> Committed: 7eb0e23bcf
BUG=webrtc:6853
TBR=tommi@webrtc.org
Review-Url: https://codereview.webrtc.org/2609783002
Cr-Commit-Position: refs/heads/master@{#15873}
Reason for revert:
Very likely cause of Chromium import bot breakage (unused function '__cpuid'), TBD why.
Original issue's description:
> Replace basictypes.h with stdint.h for int_t types.
>
> Removes basictypes.h for types that only makes use of it for fixed-size-int
> typedefs and replaces it with stdint.h.
>
> BUG=webrtc:6853
> R=tommi@webrtc.org
>
> Review-Url: https://codereview.webrtc.org/2604043002
> Cr-Commit-Position: refs/heads/master@{#15867}
> Committed: 7fd1a75300TBR=tommi@webrtc.org
# Skipping CQ checks because original CL landed less than 1 days ago.
NOPRESUBMIT=true
NOTREECHECKS=true
NOTRY=true
BUG=webrtc:6853
Review-Url: https://codereview.webrtc.org/2603203003
Cr-Commit-Position: refs/heads/master@{#15869}
Removes basictypes.h for types that only makes use of it for fixed-size-int
typedefs and replaces it with stdint.h.
BUG=webrtc:6853
R=tommi@webrtc.org
Review-Url: https://codereview.webrtc.org/2604043002
Cr-Commit-Position: refs/heads/master@{#15867}
This allows building without SCTP support (and even building/running
tests). The "HAVE_SCTP" define has been functional for a while, but there
wasn't any easy way to turn it on/off.
NOTRY=True
BUG=webrtc:6933
Review-Url: https://codereview.webrtc.org/2593313002
Cr-Commit-Position: refs/heads/master@{#15763}
"Crypto required" is a property of the PeerConnection of construction
time; it has nothing to do with SDP. So I'm moving it out of
MediaContentDescription and putting it in the BaseChannel constructor
instead. This is more intuitive, and provides the added assurance that
"secure_required_" can't be flipped from "true" to "false".
BUG=None
Review-Url: https://codereview.webrtc.org/2537343003
Cr-Commit-Position: refs/heads/master@{#15579}
Their base class, Transport, still exists, but it now has a more specific
role: a helper class that applies TransportDescriptions. And is renamed
to JsepTransport as a result.
TransportController is now the entity primarily responsible for managing
TransportChannels. It also starts storing pointers to the DTLS and ICE
chanels separately, which will make it easier to remove
TransportChannel/TransportChannelImpl in a subsequent CL.
BUG=None
Review-Url: https://codereview.webrtc.org/2517883002
Cr-Commit-Position: refs/heads/master@{#15453}
Reason for revert:
Deletion of transport.h broke downstream builds.
Going to reland with transport.h containing enums/etc.
Original issue's description:
> Refactoring that removes P2PTransport and DtlsTransport classes.
>
> Their base class, Transport, still exists, but it now has a more specific
> role: a helper class that applies TransportDescriptions. And is renamed
> to JsepTransport as a result.
>
> TransportController is now the entity primarily responsible for managing
> TransportChannels. It also starts storing pointers to the DTLS and ICE
> chanels separately, which will make it easier to remove
> TransportChannel/TransportChannelImpl in a subsequent CL.
>
> BUG=None
>
> Committed: https://crrev.com/bd28681d02dee8c185aeb39207e8154f0ad14a37
> Cr-Commit-Position: refs/heads/master@{#15450}
TBR=pthatcher@webrtc.org
# Skipping CQ checks because original CL landed less than 1 days ago.
NOPRESUBMIT=true
NOTREECHECKS=true
NOTRY=true
BUG=None
Review-Url: https://codereview.webrtc.org/2553043004
Cr-Commit-Position: refs/heads/master@{#15452}
Their base class, Transport, still exists, but it now has a more specific
role: a helper class that applies TransportDescriptions. And is renamed
to JsepTransport as a result.
TransportController is now the entity primarily responsible for managing
TransportChannels. It also starts storing pointers to the DTLS and ICE
chanels separately, which will make it easier to remove
TransportChannel/TransportChannelImpl in a subsequent CL.
BUG=None
Review-Url: https://codereview.webrtc.org/2517883002
Cr-Commit-Position: refs/heads/master@{#15450}
This isn't a performance test, so it may be running in a slow
environment, and shouldn't be subject to strict timeouts.
BUG=webrtc:6801
TBR=pthatcher@webrtc.org
Review-Url: https://codereview.webrtc.org/2539183005
Cr-Commit-Position: refs/heads/master@{#15370}
We still DCHECK for RTP, but not RTCP. RTCP packets can be sent before
offer/answer negotiation is complete, due to this bug:
https://bugs.chromium.org/p/webrtc/issues/detail?id=6809
This bug can only occur if the RTCP mux policy is "require", which is
why we started hitting it recently (the default in unit tests was
recently changed to "require").
BUG=webrtc:6776
TBR=pthatcher@webrtc.org
Review-Url: https://codereview.webrtc.org/2542233002
Cr-Commit-Position: refs/heads/master@{#15369}
There's no longer any need to make the two arguments have the same
signedness, so we can remove a bunch of superfluous (and sometimes
dangerous) casts.
It turned out I also had to fix the safe_cmp functions to properly handle
enums that are implicitly convertible to integers.
NOPRESUBMIT=true
BUG=webrtc:6645
Review-Url: https://codereview.webrtc.org/2534683002
Cr-Commit-Position: refs/heads/master@{#15281}
A WebRtcVideoEngine2 object seems to be reused between PeerConnections,
which means that the field trial added in
https://codereview.webrtc.org/2511703002/ may not activate/deactivate
as intended between calls. This CL removes the caching of video codecs,
which gets rid of this problem.
BUG=webrtc:5654
Review-Url: https://codereview.webrtc.org/2521393004
Cr-Commit-Position: refs/heads/master@{#15265}
This CL will start to distinguish H264 profiles during SDP negotiation.
We currently don't look at the H264 profile at all and assume they are
all Constrained Baseline Level 3.1. This CL will start to check profiles
for equality when matching, and will generate the correct answer H264
level.
Each local supported H264 profile needs to be listed explicitly in the
list of local supported codecs, even if they are redundant. For example,
Baseline profile should be listed explicitly even though another profile
that is a superset of Baseline is also listed. The reason for this is to
simplify the code and avoid profile intersection during matching. So
VideoCodec::Matches will check for profile equality, and not check if
one codec is a subset of the other. This also leads to the nice property
that VideoCodec::Matches is symmetric, i.e. iif a.Matches(b) then
b.Matches(a).
BUG=webrtc:6337
TBR=tkchin@webrtc.org
Review-Url: https://codereview.webrtc.org/2483173002
Cr-Commit-Position: refs/heads/master@{#15051}
These functions currently copy cricket::Codec classes by value which is
expensive since they contain e.g. std::map<std::string, std::string>
containers with parameters. This CL avoids copying them altogether.
BUG=webrtc:6337
Review-Url: https://codereview.webrtc.org/2493733003
Cr-Commit-Position: refs/heads/master@{#15040}
Reason for revert:
Relanding after known downstream breakages have been fixed.
Original issue's description:
> Revert of Delete all use of cricket::VideoFrame and cricket::WebRtcVideoFrame. (patchset #7 id:120001 of https://codereview.webrtc.org/2383093002/ )
>
> Reason for revert:
> Breaks chrome, see https://build.chromium.org/p/chromium.webrtc.fyi/builders/Mac%20Builder/builds/19019/steps/compile/logs/stdio
>
> Analysis: Chrome uses cricket::VideoFrame, without explicitly including webrtc/media/base/videoframe.h, and breaks when that file is no longer included by any other webrtc headers. Will reland after updating Chrome.
>
> Original issue's description:
> > Delete all use of cricket::VideoFrame and cricket::WebRtcVideoFrame.
> >
> > Replaced with webrtc::VideoFrame.
> >
> > TBR=mflodman@webrtc.org
> > BUG=webrtc:5682
> >
> > Committed: https://crrev.com/45c8b8940042bd2574c39920804ade8343cefdba
> > Cr-Commit-Position: refs/heads/master@{#14885}
>
> TBR=perkj@webrtc.org,pthatcher@webrtc.org,tkchin@webrtc.org,mflodman@webrtc.org,stefan@webrtc.org
> # Skipping CQ checks because original CL landed less than 1 days ago.
> NOPRESUBMIT=true
> NOTREECHECKS=true
> NOTRY=true
> BUG=webrtc:5682
>
> Committed: https://crrev.com/7341ab8e2505c9763d208e069bda269018357e7d
> Cr-Commit-Position: refs/heads/master@{#14886}
TBR=perkj@webrtc.org,pthatcher@webrtc.org,tkchin@webrtc.org,mflodman@webrtc.org,stefan@webrtc.org
# Not skipping CQ checks because original CL landed more than 1 days ago.
BUG=webrtc:5682
Review-Url: https://codereview.webrtc.org/2487633002
Cr-Commit-Position: refs/heads/master@{#15039}
Reason for revert:
This CL probably broke Chromium FYI.
Original issue's description:
> Stop caching supported codecs in WebRtcVideoEngine2
>
> We currently cache the result of GetSupportedCodecs in a member variable
> |video_codecs_| in WebRtcVideoEngine2. This means we need to keep
> |video_codecs_| and the result of GetSupportedCodecs in sync, which is
> error prone. It's simpler to just call GetSupportedCodecs when we need
> it, and we actually end up making fewer calls, so it's faster as well.
> This CL also returns all std::vectors by-value instead of by-ref. Move
> semantic together with in-place filtering of codecs actually end up with
> fewer copies, and it's also simpler to not return references.
>
> BUG=webrtc:6337
>
> Committed: https://crrev.com/9f71ec5a3e3175751f4475b126cfda89767363f2
> Cr-Commit-Position: refs/heads/master@{#15007}
TBR=tommi@webrtc.org
# Skipping CQ checks because original CL landed less than 1 days ago.
NOPRESUBMIT=true
NOTREECHECKS=true
NOTRY=true
BUG=webrtc:6337
Review-Url: https://codereview.webrtc.org/2489173004
Cr-Commit-Position: refs/heads/master@{#15014}
We currently cache the result of GetSupportedCodecs in a member variable
|video_codecs_| in WebRtcVideoEngine2. This means we need to keep
|video_codecs_| and the result of GetSupportedCodecs in sync, which is
error prone. It's simpler to just call GetSupportedCodecs when we need
it, and we actually end up making fewer calls, so it's faster as well.
This CL also returns all std::vectors by-value instead of by-ref. Move
semantic together with in-place filtering of codecs actually end up with
fewer copies, and it's also simpler to not return references.
BUG=webrtc:6337
Review-Url: https://codereview.webrtc.org/2492473002
Cr-Commit-Position: refs/heads/master@{#15007}
Reason for revert:
Breaks chrome, see https://build.chromium.org/p/chromium.webrtc.fyi/builders/Mac%20Builder/builds/19019/steps/compile/logs/stdio
Analysis: Chrome uses cricket::VideoFrame, without explicitly including webrtc/media/base/videoframe.h, and breaks when that file is no longer included by any other webrtc headers. Will reland after updating Chrome.
Original issue's description:
> Delete all use of cricket::VideoFrame and cricket::WebRtcVideoFrame.
>
> Replaced with webrtc::VideoFrame.
>
> TBR=mflodman@webrtc.org
> BUG=webrtc:5682
>
> Committed: https://crrev.com/45c8b8940042bd2574c39920804ade8343cefdba
> Cr-Commit-Position: refs/heads/master@{#14885}
TBR=perkj@webrtc.org,pthatcher@webrtc.org,tkchin@webrtc.org,mflodman@webrtc.org,stefan@webrtc.org
# Skipping CQ checks because original CL landed less than 1 days ago.
NOPRESUBMIT=true
NOTREECHECKS=true
NOTRY=true
BUG=webrtc:5682
Review-Url: https://codereview.webrtc.org/2471783002
Cr-Commit-Position: refs/heads/master@{#14886}
Introduce rtc::PacketTransportInterface. Refactor cricket::TransportChannel.
Fix signal slots parameter types in all related code.
BUG=webrtc:6531
Review-Url: https://codereview.webrtc.org/2416023002
Cr-Commit-Position: refs/heads/master@{#14778}
Now that Chromium has taken libsrtp2, remove any compatibility bridge code in WebRTC that was only needed for libsrtp1.
Remove SRTP_RELATIVE_PATH now that Google's internal copy of libsrtp and the Chromium copy have the same directory structure.
Fix some include orderings per the Chromium C++ style guide.
Remove the `extern "C"` blocks now that the libsrtp headers include them (https://github.com/cisco/libsrtp/pull/195).
BUG=webrtc:6376
Review-Url: https://codereview.webrtc.org/2447893002
Cr-Commit-Position: refs/heads/master@{#14776}
Since WebRtcVideoSendStream have reconfigures the send codec to match the incoming captured frames widht and height they have not been used.
Framerate has just been set when parsing sdp to 60fps and not changed elsewhere.
This cl require some upstream projects to change first.
BUG=webrtc:5332
Review-Url: https://codereview.webrtc.org/2408153002
Cr-Commit-Position: refs/heads/master@{#14733}
It would be enough to say we're removing EnableSrtpDebugging because
it's never called, but the story is a bit more interesting.
libsrtp's debugging facilities are gated behind the reasonably-named
ENABLE_DEBUGGING macro:
b17c065a8a/srtp/crypto/include/err.h (186)
This code was imported to WebRTC from libjingle, but neither WebRTC or
Chromium ever set ENABLE_DEBUGGING. Even if someone had ever called
EnableSrtpDebugging, it wouldn't have done anything.
BUG=0
Review-Url: https://codereview.webrtc.org/2409513002
Cr-Commit-Position: refs/heads/master@{#14592}
This was missed in the first pass because this code only compiles in
Chromium.
BUG=webrtc:6376
Review-Url: https://codereview.webrtc.org/2407743002
Cr-Commit-Position: refs/heads/master@{#14591}
- Rename the data codec payload types to end with "PlType" instead of "Id", for consistency.
BUG=webrtc:2795
Review-Url: https://codereview.webrtc.org/2397413002
Cr-Commit-Position: refs/heads/master@{#14581}
This CL is a pure refactoring which should not result in any functinal
changes. It moves ownership of the RtcEventLog from webrtc::Call to the
webrtc::PeerConnection object.
This is done so that we can add RtcEventLog support for ICE events -
which will require the TransportController to have a pointer to the
RtcEventLog. PeerConnection is the closest common owner of both Call and
TransportController (through WebRtcSession).
BUG=webrtc:6393
Review-Url: https://codereview.webrtc.org/2353033005
Cr-Commit-Position: refs/heads/master@{#14578}
Also update gyp dependency from rtc_base to rtc_base_approved.
BUG=None.
Review-Url: https://codereview.webrtc.org/2368203002
Cr-Commit-Position: refs/heads/master@{#14497}
This changes most non-test related rtc_source_set targets to be
rtc_static_library instead. Targets without any .cc files are excluded.
This should bring back the build behavior we used to have with GYP
(i.e. same symbols exported in the libjingle_peerconnection.a file, which
are used by some downstream projects).
After doing an Android build with these changes:
$ nm --defined-only -g -C out/Release/lib.unstripped/libjingle_peerconnection_so.so | grep -i createpeerconnectionf
00077c51 T Java_org_webrtc_PeerConnectionFactory_nativeCreatePeerConnectionFactory
$ nm --defined-only -g -C out/Release/obj/webrtc/api/libjingle_peerconnection.a | grep -i createpeerconnectionf
00000001 T webrtc::CreatePeerConnectionFactory(rtc::Thread*, rtc::Thread*, rtc::Thread*, webrtc::AudioDeviceModule*, cricket::WebRtcVideoEncoderFactory*, cricket::WebRtcVideoDecoderFactory*)
00000001 T webrtc::CreatePeerConnectionFactory()
See https://chromium.googlesource.com/chromium/src/+/master/tools/gn/docs/cookbook.md#Note-on-static-libraries
for more details on this.
NOTICE: This should be further cleaned up in the future, to reduce
binary bloat and unnecessary linking time. Right now it's more
important to restore the desired build output though.
BUG=webrtc:6410, chromium:630755
Review-Url: https://codereview.webrtc.org/2361623004
Cr-Commit-Position: refs/heads/master@{#14364}
Remove a large number of targets that are no longer built, to reduce maintenance.
Only targets that have a GN version were removed.
BUG=webrtc:6323
NOTRY=True
NOPRESUBMIT=True
Review-Url: https://codereview.webrtc.org/2340773003
Cr-Commit-Position: refs/heads/master@{#14231}
During GN vs GYP auditing it was discovered that some
GN targets that had public_configs were not exposing them
to dependents where the dependent depended on a group, which
in turn included that target as a dependency. Instead of
changing those public_configs to all_dependent_configs
(which would be a change from GYP), it's better to just change
those group targets to use public_deps instead.
BUG=webrtc:6323
NOTRY=True
TESTED=Generated GYP and GN project files on Mac and ran the
tools/gyp_flag_compare.py script before and after this patch was
applied. The file in question used for inspection was the
webrtc/api/webrtcsessiondescriptionfactory.cc
which is a part of the libjingle_peerconnection target.
Review-Url: https://codereview.webrtc.org/2344623002
Cr-Commit-Position: refs/heads/master@{#14222}
