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}
There's no longer any need to make the two arguments have the same
signedness, so we can drop the "u" suffix on literal integer
arguments.
NOPRESUBMIT=true
BUG=webrtc:6645
Review-Url: https://codereview.webrtc.org/2535593002
Cr-Commit-Position: refs/heads/master@{#15280}
This is undefined behavior, according to specification.
BUG=chromium:661133
Review-Url: https://codereview.webrtc.org/2500953003
Cr-Commit-Position: refs/heads/master@{#15078}
This can happen in rare and strange cases.
Also taking the opportunity to replace all asserts with DCHECKs in
that file.
BUG=chromium:659225
Review-Url: https://codereview.webrtc.org/2499013002
Cr-Commit-Position: refs/heads/master@{#15070}
With this change, instead of
RTC_DCHECK_GE(unsigned_var, 17u);
we can simply write
RTC_DCHECK_GE(unsigned_var, 17);
or even
RTC_DCHECK_GE(unsigned_var, -17); // Always true.
and the mathematically sensible thing will happen.
Perhaps more importantly, we can replace checks like
// index is size_t, num_channels is int.
RTC_DCHECK(num_channels >= 0
&& index < static_cast<size_t>(num_channels));
or, even worse, just
// Surely num_channels isn't negative. That would be absurd!
RTC_DCHECK_LT(index, static_cast<size_t>(num_channels));
with simply
RTC_DCHECK_LT(index, num_channels);
In short, you no longer have to keep track of the signedness of the arguments, because the sensible thing will happen.
BUG=webrtc:6645
Review-Url: https://codereview.webrtc.org/2459793002
Cr-Commit-Position: refs/heads/master@{#14878}
The timestamps are 32 bit and can (conceivably) be spaced far enough
apart for the calculation, which is done in Q4, to overflow.
BUG=chromium:653268
Review-Url: https://codereview.webrtc.org/2460393002
Cr-Commit-Position: refs/heads/master@{#14856}
PacketList is now list<Packet> instead of list<Packet*>.
Splicing the lists in NetEqImpl::InsertPacketInternal instead of
moving packets. Avoid moving the packet when doing Rfc3389Cng.
Removed PacketBuffer::DeleteFirstPacket and DeleteAllPackets.
BUG=chromium:657300
Review-Url: https://codereview.webrtc.org/2425223002
Cr-Commit-Position: refs/heads/master@{#14747}
Only three items in the (rather large) header were actually used after
InsertPacket: payloadType, timestamp and sequenceNumber. They are now
put directly into Packet. This saves 129 bytes per Packet that no
longer need to be allocated and deallocated.
This also works towards decoupling NetEq from RTP. As part of that,
I've moved the NACK code earlier in InsertPacketInternal, together
with other things that directly reference the RTPHeader.
BUG=webrtc:6549
Review-Url: https://codereview.webrtc.org/2411183003
Cr-Commit-Position: refs/heads/master@{#14658}
With this change, the calculations inside AverageIAT are changed to be
in double-precision floating point instead of in fixed point. Also,
the method's name is changed to EstimatedClockDriftPpm to better
reflect what it returns.
A few unit tests had to be updated because of minor numerical
differences.
Also removing the UBSan suppression related to this issue.
BUG=webrtc:5889
Review-Url: https://codereview.webrtc.org/2408653002
Cr-Commit-Position: refs/heads/master@{#14628}
This was an ill tested special case which turned out to be more problem
than benefit. The special case was only triggered when the decoder frame
size was smaller than 10 ms, which is more or less unsupported by NetEq.
Also fixed a bug in a test, a bug which was exposed by the code change.
BUG=chromium:654983
Review-Url: https://codereview.webrtc.org/2412883002
Cr-Commit-Position: refs/heads/master@{#14627}
This CL deletes the old and not used video defines in
engine_configurations.h and pre-pends voice_ to indicate there are only
voice/audio defines left in the file.
BUG=none
R=solenberg@webrtc.org
Review URL: https://codereview.webrtc.org/2401673002 .
Cr-Commit-Position: refs/heads/master@{#14558}
The original CL (https://codereview.webrtc.org/2315633002) was
reverted since the fuzzer depended on gflags and files in the
resources folder; neither of this is allowed for a fuzzer test in
Chromium. This new version streamlines the dependencies, and changes
the test to generate a sinusoid input audio signal instead of reading
from a file.
Original commit message:
This CL introduces a new fuzzer target neteq_rtp_fuzzer that
manipulates the RTP header fields before inserting the packets into
NetEq. A few helper classes are also introduced.
BUG=webrtc:5447
CQ_INCLUDE_TRYBOTS=master.tryserver.chromium.win:win_chromium_rel_ng;master.tryserver.chromium.android:android_compile_dbg,linux_android_rel_ng;master.tryserver.chromium.linux:linux_chromium_rel_ng;master.tryserver.chromium.mac:mac_chromium_rel_ng,ios-device
Review-Url: https://codereview.webrtc.org/2384423002
Cr-Commit-Position: refs/heads/master@{#14523}
NetEq already uses SdpAudioFormat internally; this CL adds an
AudioCodingModule::RegisterReceiveCodec overload that accepts
SdpAudioFormat, and propagates it through AcmReceiver into NetEq.
The intention is to get rid of the other ways to specify decoders and
always use SdpAudioFormat. (And eventually to do the same for encoders
too.)
NOTRY=true
BUG=5801
Review-Url: https://codereview.webrtc.org/2365653004
Cr-Commit-Position: refs/heads/master@{#14506}
Addresses a regression in the NetEq performance test.
# Added NOTRY due to android_arm64_rel being swamped.
NOTRY=True
BUG=chromium:651426
Review-Url: https://codereview.webrtc.org/2383723002
Cr-Commit-Position: refs/heads/master@{#14495}
The RtcEventLog headers need to be accessible from any place which needs
logging, and the implementation needs access to data structures that are
logged.
After a discussion in the code review, we all agreed to move the RtcEventLog implementation into its own top level directory - which I called "logging/" in expectation that other types of logging may have similar requirements. The directory contains two main build targets - "rtc_event_log_api", which is just rtc_event_log.h, that has no external dependencies and can be used from anywhere, and "rtc_event_log_impl" which contains the rest of the implementation and has many dependencies (more in the future).
The "api" target can be referenced from anywhere, while the "impl" target is only needed at the place of instantiation (currently Call, soon to be moved to PeerConnection by https://codereview.webrtc.org/2353033005/).
This change allows using RtcEventLog in the p2p/ directory, so that we
can log STUN pings and ICE state transitions.
BUG=webrtc:6393
R=kjellander@webrtc.org, kwiberg@webrtc.org, solenberg@webrtc.org, stefan@webrtc.org, terelius@webrtc.org
Review URL: https://codereview.webrtc.org/2380683005 .
Cr-Commit-Position: refs/heads/master@{#14485}
NetEqDecoder is still used in the external interfaces, but this change
opens up the ability to use SdpAudioFormats directly, once appropriate
interfaces have been added.
BUG=webrtc:5805
Review-Url: https://codereview.webrtc.org/2355503002
Cr-Commit-Position: refs/heads/master@{#14368}
The biggest change to NetEq is the move from a primary flag, to a
Priority with two separate levels: one set by RED splitting and one
set by the codec itself. This allows us to unambigously prioritize
"fallback" packets from these two sources. I've chosen what I believe
is the sensible ordering: packets that the codec prioritizes are
chosen first, regardless of if they are secondary RED packets or
not. So if we were to use Opus w/ FEC in RED, we'd only do Opus FEC
decoding if there was no RED packet that could cover the time slot.
With this change, PayloadSplitter now only deals with RED
packets. Maybe it should be renamed RedPayloadSplitter?
BUG=webrtc:5805
Review-Url: https://codereview.webrtc.org/2342443005
Cr-Commit-Position: refs/heads/master@{#14347}
There's still some code run specifically for Opus w/ FEC. It will be
addressed in a separate CL.
BUG=webrtc:5805
Review-Url: https://codereview.webrtc.org/2326003002
Cr-Commit-Position: refs/heads/master@{#14319}
Verifies that NetEq doesn't enter muted state when CNG mode is active
and the packet stream is suspended for a long time.
BUG=webrtc:5608
Review-Url: https://codereview.webrtc.org/2335343011
Cr-Commit-Position: refs/heads/master@{#14308}
It requires a new NetEq method, but it can no longer fail. And we no
longer need to use AcmReceiver::decoders_, which we're trying to
eliminate.
(This is a re-land of https://codereview.webrtc.org/2342313002.)
BUG=webrtc:5801
Review-Url: https://codereview.webrtc.org/2348233002
Cr-Commit-Position: refs/heads/master@{#14304}
It allows the decoder to split the input up into usable chunks before
they are put into NetEq's PacketBuffer. Eventually, all packet splitting
will move into ParsePayload.
There's currently a base implementation of ParsePayload. It will
generate a single Frame that calls the underlying AudioDecoder for
getting Duration() and to Decode.
BUG=webrtc:5805
BUG=chromium:428099
Review-Url: https://codereview.webrtc.org/2326953003
Cr-Commit-Position: refs/heads/master@{#14300}
Reason for revert:
Seems to have broken Chromium tests.
Original issue's description:
> AcmReceiver: Look up last decoder in NetEq's table of decoders
>
> AcmReceiver::decoders_ is now one step closer to being unused.
>
> BUG=webrtc:5801
>
> Committed: https://crrev.com/1e4d8b574cde64d93b98d89c7b817fb93185a307
> Cr-Commit-Position: refs/heads/master@{#14274}
TBR=ossu@webrtc.org,henrik.lundin@webrtc.org
# Skipping CQ checks because original CL landed less than 1 days ago.
NOPRESUBMIT=true
NOTREECHECKS=true
NOTRY=true
BUG=webrtc:5801
Review-Url: https://codereview.webrtc.org/2348123002
Cr-Commit-Position: refs/heads/master@{#14279}
Reason for revert:
Seems to have broken Chromium tests.
Original issue's description:
> AcmReceiver: Ask NetEq to delete all decoders at once instead of one by one
>
> It requires a new NetEq method, but it can no longer fail. And we no
> longer need to use AcmReceiver::decoders_, which we're trying to
> eliminate.
>
> BUG=webrtc:5801
>
> Committed: https://crrev.com/f6232b43a176e1717354b671a0a52b887d70de59
> Cr-Commit-Position: refs/heads/master@{#14275}
TBR=ossu@webrtc.org,henrik.lundin@webrtc.org
# Skipping CQ checks because original CL landed less than 1 days ago.
NOPRESUBMIT=true
NOTREECHECKS=true
NOTRY=true
BUG=webrtc:5801
Review-Url: https://codereview.webrtc.org/2349973002
Cr-Commit-Position: refs/heads/master@{#14278}
It requires a new NetEq method, but it can no longer fail. And we no
longer need to use AcmReceiver::decoders_, which we're trying to
eliminate.
BUG=webrtc:5801
Review-Url: https://codereview.webrtc.org/2342313002
Cr-Commit-Position: refs/heads/master@{#14275}
AcmReceiver::decoders_ is now one step closer to being unused.
BUG=webrtc:5801
Review-Url: https://codereview.webrtc.org/2339953002
Cr-Commit-Position: refs/heads/master@{#14274}
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}
Reason for revert:
Broke all Chromium libFuzzer builds
https://bugs.chromium.org/p/chromium/issues/detail?id=645069
Original issue's description:
> Setting up an RTP input fuzzer for NetEq
>
> This CL introduces a new fuzzer target neteq_rtp_fuzzer that
> manipulates the RTP header fields before inserting the packets into
> NetEq. A few helper classes are also introduced.
>
> BUG=webrtc:5447
> NOTRY=True
>
> Committed: https://crrev.com/2d273f1e97cd5030ed1686f27ce1118291b66395
> Cr-Commit-Position: refs/heads/master@{#14103}
TBR=ivoc@webrtc.org,kjellander@webrtc.org
# Skipping CQ checks because original CL landed less than 1 days ago.
NOPRESUBMIT=true
NOTREECHECKS=true
NOTRY=true
BUG=webrtc:5447
Review-Url: https://codereview.webrtc.org/2328483002
Cr-Commit-Position: refs/heads/master@{#14131}
I could not find a single place it was used, outside of the unittests
for the sync packet support itself.
Review-Url: https://codereview.webrtc.org/2309303002
Cr-Commit-Position: refs/heads/master@{#14130}
With this CL, the NetEqReplacementInput class handles reordered and
missing packets in a better way than before, by storing the last
confirmed packet size and using that when the next packet size cannot
be calculated.
NOTRY=True
Review-Url: https://codereview.webrtc.org/2319553003
Cr-Commit-Position: refs/heads/master@{#14122}
This CL introduces a new fuzzer target neteq_rtp_fuzzer that
manipulates the RTP header fields before inserting the packets into
NetEq. A few helper classes are also introduced.
BUG=webrtc:5447
NOTRY=True
Review-Url: https://codereview.webrtc.org/2315633002
Cr-Commit-Position: refs/heads/master@{#14103}
If neteq_rtpplay is invoked with the --ssrc option to select packets
matching a specific SSRC, but no matching packets are found, this CL
provides a meaningful error message.
BUG=webrtc:2692
NOTRY=True
TBR=ivoc@webrtc.org
Review-Url: https://codereview.webrtc.org/2318503002
Cr-Commit-Position: refs/heads/master@{#14083}
The sample uses are from when I debugged bug 617124. The change in neteq_network_stats_unittest.cc is a fix for a minor unrelated bug found by the try bots when I tried to land this CL (a test was passing uninitialized packet data to NetEq).
BUG=chromium:617124
Review-Url: https://codereview.webrtc.org/2293893002
Cr-Commit-Position: refs/heads/master@{#14034}
If a CNG packet is received first, followed by a speech packet with
another sample rate, NetEq should treat this as a change of codec, flush
out the CNG packet and reset the sample rate to that of the speech
packet.
BUG=webrtc:5447
NOTRY=True
Review-Url: https://codereview.webrtc.org/2307493002
Cr-Commit-Position: refs/heads/master@{#14032}
That is, rather than keeping a separate pointer and size.
This helps automate memory management in NetEq and will be useful in the
work to minimize the AudioDecoder interface as part of the injectable
audio codec work.
I'm planning a follow-up that will change the current management of Packet* to wrapping them in unique_ptr instead.
Review-Url: https://codereview.webrtc.org/2289093003
Cr-Commit-Position: refs/heads/master@{#14002}
