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}
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}
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}
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}
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}
With this change, the value 0xFF is no longer used to flag that the RTP
type is unknown. Instead, an empty value for the rtc::Optional is used.
Review-Url: https://codereview.webrtc.org/2290153002
Cr-Commit-Position: refs/heads/master@{#13989}
The current_rtp_payload_type_ should only be updated when the packet is
actually inserted into the packet buffer, since then the payload type
has been validated. This CL removes an unvalidated setting of this value
that happened after SSRC change or upon first packet.
BUG=webrtc:5447
Review-Url: https://codereview.webrtc.org/2270793003
Cr-Commit-Position: refs/heads/master@{#13910}
If an error happens in the GetAudio call, for instance when corrupt
payloads are inserted, GetAudio wil return an error. In this case, the
audio frame is not always correctly populated, which is to be expected.
BUG=webrtc:5447
Review-Url: https://codereview.webrtc.org/2272963002
Cr-Commit-Position: refs/heads/master@{#13902}
The new method returns the current total delay (packet buffer and sync
buffer) in ms, with smoothing applied to even out short-time
fluctuations due to jitter. The packet buffer part of the delay is not
updated during DTX/CNG periods.
This CL also pipes the new metric through ACM and uses it in
VoiceEngine. It replaces the previous method of estimating the buffer
delay (where an inserted packet's RTP timestamp was compared with the
last played timestamp from NetEq). The new method works better under
periods of DTX/CNG.
Review-Url: https://codereview.webrtc.org/2262203002
Cr-Commit-Position: refs/heads/master@{#13855}
I'll be rewriting AcmReceiver soon and am trying to reduce the amount of
old stuff that needs to be supported.
I've manually checked the outputs of the AcmReceiver bitexactness
tests with this change. A large part of the tests are still bitexact,
with one section only differing slightly in timings. Nothing audible
unless playing the old and new versions back simultaneously.
The output of NetEqDecoderTest were also changed due to this CL, although only on android. I built and ran the test locally and compared the audio output manually - the changes were the same as for the other tests; i.e. very slight timing changes for a part of the output.
I updated the network stats checksum for android without analyzing it further. I expect it goes hand-in-hand with the changes to the output; i.e. the changes in it are fine because the audio output is fine. Likely, the stats will show changes in the usage of CNG, since that is what the code changes.
BUG=webrtc:1361
Review-Url: https://codereview.webrtc.org/2117763002
Cr-Commit-Position: refs/heads/master@{#13415}
This allows us to get rid of the function that computes it, which gets
us one step closer to getting rid of the NetEqDecoder type.
BUG=webrtc:5801
Review-Url: https://codereview.webrtc.org/2021063002
Cr-Commit-Position: refs/heads/master@{#12974}
The NetEqNetworkStatistics::expand_rate was not incremented during muted
state, which caused under-reporting of that metric. This change fixes
that.
BUG=chromium:613321, webrtc:5608
Review-Url: https://codereview.webrtc.org/2003203004
Cr-Commit-Position: refs/heads/master@{#12894}
Channel's API remains unchanged, but the creation of a BuiltinAudioDecoderFactory is now in Channel. The next step would be to amend Channel's API (through CreateChannel, I believe) to allow an AudioDecoderFactory to be sent along.
BUG=webrtc:5805
Review-Url: https://codereview.webrtc.org/1992763002
Cr-Commit-Position: refs/heads/master@{#12893}
This CL implements the muted output functionality in NetEq. Tests are
added. The feature is currently off by default, and AcmReceiver makes
sure that the muted state is not engaged.
BUG=webrtc:5608
Review-Url: https://codereview.webrtc.org/1965733002
Cr-Commit-Position: refs/heads/master@{#12711}
This is to prepare for implementation of NetEq muted state, which may
cause GetAudioInternal to make an early return just before the call to
GetDecision. With this change, the stats are updated in any case.
BUG=webrtc:5608
NOTRY=True
Review-Url: https://codereview.webrtc.org/1948663002
Cr-Commit-Position: refs/heads/master@{#12671}
The timescale_holdoff_ is a counter in the DecisionLogic class. The
purpose is to enforce a minimum number of GetAudio calls
between (successfull) time-scaling operations (i.e., Accelerate and
Pre-emptive Expand operations). With this change, the counter is
replaced with a Countdown timer obtained from a TickTimer object.
BUG=webrtc:5608
R=tina.legrand@webrtc.org
Review URL: https://codereview.webrtc.org/1945863002 .
Cr-Commit-Position: refs/heads/master@{#12670}
The counting is moved to NetEqImpl, and the new counter is realized as a
Stopwatch object. The DecisionLogic class still has to maintain record
of when the CNG period is shortened, in order to reduce the delay. This
is recorded in a new noise_fast_forward_ member in DecisionLogic.
BUG=webrtc:5608
Review-Url: https://codereview.webrtc.org/1914303004
Cr-Commit-Position: refs/heads/master@{#12608}
Later steps in the refactoring will have the factory injected from the
outside rather than owned by NetEq.
BUG=webrtc:5801
Review-Url: https://codereview.webrtc.org/1928293002
Cr-Commit-Position: refs/heads/master@{#12604}
This change replaces packet_iat_count_ms_ and max_timer_ms_, two
time-counting member variables in DelayManager, with Stopwatch objects
obtained from a TickTimer.
BUG=webrtc:5608
Review-Url: https://codereview.webrtc.org/1929863002
Cr-Commit-Position: refs/heads/master@{#12554}
Specifically, this change replaces peak_period_counter_ms_ with
peak_period_stopwatch_ which obtains a Stopwatch object from
TickTimer. Necessary plumbing to get the TickTimer through to the
DelayPeakDetector is also included.
BUG=webrtc:5608
NOTRY=True
Review-Url: https://codereview.webrtc.org/1921163003
Cr-Commit-Position: refs/heads/master@{#12542}
With this change, the NetEqImpl constructor takes a struct
(NetEqImpl::Dependencies) as input instead of a collection of
individual dependencies. The NetEqImpl unit test fixture is modified
to make better used of unique_ptrs.
Review URL: https://codereview.webrtc.org/1921243002
Cr-Commit-Position: refs/heads/master@{#12514}
This change makes use of the TickTimer::Stopwatch in Packets. When a
packet is inserted into the PacketBuffer, a Stopwatch object is
attached to it. When the packet is extracted from the buffer, the
Stopwatch is read to know how long the packet waited in the buffer.
BUG=webrtc:5608
Review URL: https://codereview.webrtc.org/1917913002
Cr-Commit-Position: refs/heads/master@{#12508}
The TickTimer is incremented on each call to GetAudioInternal(). Other
than that, the new object is not used yet.
Also adding a unit test in NetEqImplTest to verify that the tick timer
is incremented in the call to NetEq::GetAudio.
BUG=webrtc:5608
Review URL: https://codereview.webrtc.org/1903153005
Cr-Commit-Position: refs/heads/master@{#12493}
Broke out CNG from AudioDecoder as they didn't really share an interface.
Converted the CNG code to C++, to make initialization and resource handling easier. This includes several changes to the behavior, favoring RTC_CHECKs over returning error codes.
Review URL: https://codereview.webrtc.org/1868143002
Cr-Commit-Position: refs/heads/master@{#12491}
With this change, the return value from NetEq::GetPlayoutTimestamp is
empty if the latest call to NetEq::GetAudio resulted in comfort noise
(codec-internal or external) being played out. This is because the
playout timestamp is not updated during CNG, and can therefore not be
trusted.
A few unit tests were updated to reflect the change.
BUG=webrtc:5669
Review URL: https://codereview.webrtc.org/1861303002
Cr-Commit-Position: refs/heads/master@{#12268}
This was previously done in AcmReceiver, but belongs in NetEq where the
rest of the AudioFrame fields are populated.
BUG=webrtc:5669,webrtc:5607
Review URL: https://codereview.webrtc.org/1863993002
Cr-Commit-Position: refs/heads/master@{#12265}
This is in preparation for changes to when the playout timestamp is
valid.
BUG=webrtc:5669
Review URL: https://codereview.webrtc.org/1853183002
Cr-Commit-Position: refs/heads/master@{#12256}
This copies the contents (unittest excluded) of base/numerics in
chromium to base/numerics in webrtc. Files added:
- safe_conversions.h
- safe_conversions_impl.h
- safe_math.h
- safe_math_impl.h
A really old version of safe_conversions[_impl].h previously existed in
base/, this has been deleted and sources using it have been updated
to include the new base/numerics/safe_converions.h.
This CL also adds a DEPS file to webrtc/base.
NOPRESUBMIT=True
BUG=webrtc:5548, webrtc:5623
Review URL: https://codereview.webrtc.org/1753293002
Cr-Commit-Position: refs/heads/master@{#11907}
The type is included in the AudioFrame output parameter.
Rename the type NetEqOutputType to just OutputType, since it is now
internal to NetEq.
BUG=webrtc:5607
Review URL: https://codereview.webrtc.org/1769883002
Cr-Commit-Position: refs/heads/master@{#11903}
This change essentially does two things:
1. Remove the VAD-related methods from AcmReceiver. These are
EnableVad(), DisableVad(), and vad_enabled(). None of them were used
outside of unit tests.
2. Move the functionality to set AudioFrame::speech_type_ and
AudioFrame::vad_activity_ inside NetEq. This was previously done in
AcmReceiver, but based on information inherently owned by NetEq.
With the change in 2, NetEq's GetAudio interface can be simplified by
removing the output type parameter. This will be done in a follow-up
CL.
BUG=webrtc:5607
Review URL: https://codereview.webrtc.org/1772583002
Cr-Commit-Position: refs/heads/master@{#11902}
With this change, NetEq now uses AudioFrame as output type, like the
surrounding functions in ACM and VoiceEngine already do.
The computational savings is probably slim, since one memcpy is
removed while another one is added (both in AcmReceiver::GetAudio).
More simplifications and clean-up will be done in
AcmReceiver::GetAudio in future CLs.
BUG=webrtc:5607
Review URL: https://codereview.webrtc.org/1750353002
Cr-Commit-Position: refs/heads/master@{#11874}
The new fields are default-populated for built-in decoders, but for
external decoders, the name can now be given when registering the
decoder.
BUG=webrtc:3520
Review URL: https://codereview.webrtc.org/1484343003
Cr-Commit-Position: refs/heads/master@{#10952}
This change moves the logics for keeping track of the last ouput
sample rate from AcmReceiver to NetEq, where it fits better. The
getter function AcmReceiver::current_sample_rate_hz() is renamed to
last_output_sample_rate_hz().
BUG=webrtc:3520
Review URL: https://codereview.webrtc.org/1467163002
Cr-Commit-Position: refs/heads/master@{#10754}