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}
This changes the following module directories:
* webrtc/modules/audio_conference_mixer/interface
* webrtc/modules/interface
* webrtc/modules/media_file/interface
* webrtc/modules/rtp_rtcp/interface
* webrtc/modules/utility/interface
To avoid breaking downstream, I followed this recipe:
1. Copy the interface dir to a new sibling directory: include
2. Update the header guards in the include directory to match the style guide.
3. Update the header guards in the interface directory to match the ones in include. This is required to avoid getting redefinitions in the not-yet-updated downstream code.
4. Add a pragma warning in the header files in the interface dir. Example:
#pragma message("WARNING: webrtc/modules/interface is DEPRECATED; "
"use webrtc/modules/include")
5. Search for all source references to webrtc/modules/interface and update them to webrtc/modules/include (*.c*,*.h,*.mm,*.S)
6. Update all GYP+GN files. This required manual inspection since many subdirectories of webrtc/modules referenced the interface dir using ../interface etc(*.gyp*,*.gn*)
BUG=5095
TESTED=Passing compile-trybots with --clobber flag:
git cl try --clobber --bot=win_compile_rel --bot=linux_compile_rel --bot=android_compile_rel --bot=mac_compile_rel --bot=ios_rel -m tryserver.webrtc
R=stefan@webrtc.org, tommi@webrtc.org
Review URL: https://codereview.webrtc.org/1417683006 .
Cr-Commit-Position: refs/heads/master@{#10500}
This change avoids calling neteq_->EnableVad() and DisableVad from the
AcmReceiver constructor. Instead, the new member
enable_post_decode_vad is added to NetEq's config struct. It is
disabled by defualt, but ACM sets it to enabled. This preserves the
behavior both of NetEq stand-alone (i.e., in tests) and of ACM.
BUG=webrtc:3520
Review URL: https://codereview.webrtc.org/1425133002
Cr-Commit-Position: refs/heads/master@{#10476}
This operation was relatively simple, since no one was doing anything
fishy with this enum. A large number of lines had to be changed
because the enum values now live in their own namespace, but this is
arguably worth it since it is now much clearer what sort of constant
they are.
BUG=webrtc:5028
Review URL: https://codereview.webrtc.org/1424083002
Cr-Commit-Position: refs/heads/master@{#10449}
Negative acknowledgement (NACK) has up to now been implemented in
ACM. But, since NetEq is in charge of the actual packet buffer, it
makes more sense to have the NACK functionlaity in there.
This CL does the following:
- Move nack.{h,cc} and the unit tests from main/acm2 to neteq.
- Move the NACK related code in ACM into NetEq.
- NACK related functions in AcmReceiver are changed to simple
forwarding APIs.
- Remove unused members in AcmReceiver.
- Remove unused API functions in NetEq.
BUG=webrtc:3520
Review URL: https://codereview.webrtc.org/1410073006
Cr-Commit-Position: refs/heads/master@{#10448}
Bug 4985 revealed two flaws
1. Opus duration estimate did not return correct length for DTX packets,
2. NetEq DoCodecInternalCng did not assign enough buffer.
P.S.
Generalizing problem 1, current NetEq decode function checks memory size by calling the duration estimate function. This is not ideal. A better way is to let codec's decode function to receive buffer size and return failure if it is not enough. This can be made in a separate CL.
BUG=webrtc:4985
R=henrik.lundin@webrtc.org
Review URL: https://codereview.webrtc.org/1334303005 .
Cr-Commit-Position: refs/heads/master@{#10031}
We must remove dependency on Chromium, i.e. we can't use Chromium's base/logging.h. That means we need to define these macros in WebRTC also when doing Chromium builds. And this causes redefinition.
Alternative solutions:
* Check if we already have defined e.g. CHECK, and don't define them in that case. This makes us depend on include order in Chromium, which is not acceptable.
* Don't allow using the macros in WebRTC headers. Error prone since if someone adds it there by mistake it may compile fine, but later break if a header in added or order is changed in Chromium. That will be confusing and hard to enforce.
* Ensure that headers that are included by an embedder don't include our macros. This would require some heavy refactoring to be maintainable and enforcable.
* Changes in Chromium for this is obviously not an option.
BUG=chromium:468375
NOTRY=true
Review URL: https://codereview.webrtc.org/1335923002
Cr-Commit-Position: refs/heads/master@{#9964}
In some cases, the number of samples (per channel) in NetEq's sync
buffer could fall below the allowed minimum (5 samples for narrowband,
scaling for other rates). If the number of samples extracted from the
buffer was smaller than the desired number, an error is
returned. However, if the decoder returns fewer samples than expected,
it could happen that the sync buffer level falls under the minimum,
but enough samples are extracted. This triggered an assert. With this
change, the minimum level of the sync buffer is always enforced.
A test is implemented to trigger the problem. It made the assert fire
without this fix, but it now passes.
BUG=webrtc:4840
R=minyue@webrtc.org
Review URL: https://codereview.webrtc.org/1324453002 .
Cr-Commit-Position: refs/heads/master@{#9828}
These asserts cover error cases that are also handled by the code
after the assert. Should not have both assert and error handling.
BUG=webrtc:4840
Review URL: https://codereview.webrtc.org/1321023002
Cr-Commit-Position: refs/heads/master@{#9804}
This is a bug that was introduced in
https://codereview.webrtc.org/1230503003, where the variable "int
temp_bufsize" was changed to a size_t. If the packet buffer was
flushed while inserting a packet, temp_bufsize became negative, which
was tested later in an if-statement. Now, with size_t instead, it
would just become very large, and the if-statement would never see a
negative value. The effect was that the packet size in samples could
be updated with a very large positive number, causing an overflow
which triggered rtc::checked_cast in
StatisticsCalculator::GetNetworkStatistics, line 220.
Also adding a test to reproduce the crash. Without the fix, the test
results in the above mentioned checked_cast to trigger. With the fix,
everything works fine.
BUG=chromium:525260
Review URL: https://codereview.webrtc.org/1307893004
Cr-Commit-Position: refs/heads/master@{#9802}
This was not implemented before. It returns the current total delay
(packet buffer and sync buffer) of NetEq. This is the same information
that was already available in
NetEqNetworkStatistics::current_buffer_size_ms, that can be obtained
through NetEq::NetworkStatistics(). But, since the current delay is a
key metric of NetEq, it is convenient to have it available in a
simpler way.
This is a re-landing of r9359,
https://webrtc-codereview.appspot.com/51149004, which was reverted in
r9360. The refactoring made in r9669 facilitated the relanding.
TBR=minyue@webrtc.org
Review URL: https://codereview.webrtc.org/1313873003
Cr-Commit-Position: refs/heads/master@{#9801}
The Init() method was previously used to initialize and reset
decoders, and returned an error code. The new Reset() method is used
for reset only; the constructor is now responsible for fully
initializing the AudioDecoder.
Reset() doesn't return an error code; it turned out that none of the
functions it ended up calling could actually fail, so this CL removes
their error return codes as well.
R=henrik.lundin@webrtc.org
Review URL: https://codereview.webrtc.org/1319683002 .
Cr-Commit-Position: refs/heads/master@{#9798}
With this change, the aggregates for packet waiting times are
calculated in NetEq's StatisticsCalculator insead of in
AcmReceiver. This simplifies things somewhat, and avoids having to
copy the raw data on polling.
R=ivoc@webrtc.org, minyue@webrtc.org
Review URL: https://codereview.webrtc.org/1296633002 .
Cr-Commit-Position: refs/heads/master@{#9778}
Measures the duration of each packet loss concealment (a.k.a. expand)
event that is not followed by a merge operation.
Having decoded and played packet m−1, the next expected packet is
m. If packet m arrives after some time of packet loss concealment, we
have a delayed packet outage event. However, if instead packet n>m
arrives, we have a lost packet outage event. In NetEq, the two outage
types results in different operations. Both types start with expand
operations to generate audio to play while the buffer is empty. When a
lost packet outage happens, the expand operation(s) are followed by
one merge operation. For delayed packet outages, merge is not done,
and the expand operations are immediately followed by normal
operations.
This change also includes unit tests for the new statistics.
BUG=webrtc:4915, chromium:488124
R=minyue@webrtc.org
Review URL: https://codereview.webrtc.org/1290113002 .
Cr-Commit-Position: refs/heads/master@{#9725}
