in the ACMDump. The ACMDump interface itself is not updated, so there
is no way (yet) to actually write the configuration fields.
BUG=
Review URL: https://codereview.webrtc.org/1202833003
Cr-Commit-Position: refs/heads/master@{#9519}
This change introduces the sub-class ChangeLogger in AudioCodingModuleImpl. The class writes values to the named UMA histogram, but only if the value has changed since the last time (and always for the first call). This is to avoid the problem with audio codecs being registered but never used. Before this change, these codecs' bitrate was also logged, even though they were never used.
BUG=chromium:488124
R=kwiberg@webrtc.org
Review URL: https://codereview.webrtc.org/1203803004
Cr-Commit-Position: refs/heads/master@{#9506}
This CL logs the target audio bitrate to a UMA histogram called
WebRTC.Audio.TargetBitrateInKbps. It logs the rate when a codec is
created, and when the target is explicitly updated. Note that since
each codec implementation is free to change or ignore the target
value, there is no guarantee that the logged value will actually be
used as the target.
BUG=chromium:488124
Review URL: https://codereview.webrtc.org/1178053002
Cr-Commit-Position: refs/heads/master@{#9484}
Before this change, it could happen that a caller would get a pointer
to the encoder_ but not use it before another thread called the
Reconstruct method, changing the pointer. This of course resulted in
bad access crashes. With this change, each use of the pointer acquired
from the encoder() method is protected by the same lock that is
required to update the pointer. Note that this fix is probably too
aggressive, since it also affects the Opus implementation; the crash
has so far only been seen for iSAC.
Also adding a test to trigger the problem. The test did not trigger
the problem deterministically, but out would typically find it in less
than 1000 runs.
BUG=chromium:499468
R=jmarusic@webrtc.org, kwiberg@webrtc.org
Review URL: https://codereview.webrtc.org/1176303004.
Cr-Commit-Position: refs/heads/master@{#9436}
This includes changes like:
* Attempt to break lines at better positions
* Use "override" in more places, don't use "virtual" with it
* Use {} where the body is more than one line
* Make declaration and definition arg names match
* Eliminate unused code
* EXPECT_EQ(expected, actual) (but use (actual, expected) for e.g. _GT)
* Correct #include order
* Use anonymous namespaces in preference to "static" for file-scoping
* Eliminate unnecessary casts
* Update reference code in comments of ARM assembly sources to match actual current C code
* Fix indenting to be more style-guide compliant
* Use arraysize() in more places
* Use bool instead of int for "boolean" values (0/1)
* Shorten and simplify code
* Spaces around operators
* 80 column limit
* Use const more consistently
* Space goes after '*' in type name, not before
* Remove unnecessary return values
* Use "(var == const)", not "(const == var)"
* Spelling
* Prefer true, typed constants to "enum hack" constants
* Avoid "virtual" on non-overridden functions
* ASSERT(x == y) -> ASSERT_EQ(y, x)
BUG=none
R=andrew@webrtc.org, asapersson@webrtc.org, henrika@webrtc.org, juberti@webrtc.org, kjellander@webrtc.org, kwiberg@webrtc.org
Review URL: https://codereview.webrtc.org/1172163004
Cr-Commit-Position: refs/heads/master@{#9420}
This makes a variety of small changes to synchronize bits of code using different types, remove useless code or casts, and add explicit casts in some places previously doing implicit ones. For example:
* Change a few type declarations to better match how the majority of code uses those objects.
* Eliminate "< 0" check for unsigned values.
* Replace "(float)sin(x)", where |x| is also a float, with "sinf(x)", and similar.
* Add casts to uint32_t in many places timestamps were used and the existing code stored signed values into the unsigned objects.
* Remove downcasts when the results would be passed to a larger type, e.g. calling "foo((int16_t)x)" with an int |x| when foo() takes an int instead of an int16_t.
* Similarly, add casts when passing a larger type to a function taking a smaller one.
* Add casts to int16_t when doing something like "int16_t = int16_t + int16_t" as the "+" operation would implicitly upconvert to int, and similar.
* Use "false" instead of "0" for setting a bool.
* Shift a few temp types when doing a multi-stage calculation involving typecasts, so as to put the most logical/semantically correct type possible into the temps. For example, when doing "int foo = int + int; size_t bar = (size_t)foo + size_t;", we might change |foo| to a size_t and move the cast if it makes more sense for |foo| to be represented as a size_t.
BUG=none
R=andrew@webrtc.org, asapersson@webrtc.org, henrika@webrtc.org, juberti@webrtc.org, kwiberg@webrtc.org
TBR=andrew, asapersson, henrika
Review URL: https://codereview.webrtc.org/1168753002
Cr-Commit-Position: refs/heads/master@{#9419}
This makes some behaviorally-invariant changes to make certain code that
currently only works correctly with signed types work safely regardless of the
signedness of the types in question. This is preparation for a future change
that will convert a variety of types to size_t.
There are also some formatting changes (e.g. converting "enum hack" usage to real consts) to make it simpler to just change "int" to "size_t" in the future to change the types of those constants.
BUG=none
R=andrew@webrtc.org, juberti@webrtc.org, kwiberg@webrtc.org
TBR=ajm
Review URL: https://codereview.webrtc.org/1174813003
Cr-Commit-Position: refs/heads/master@{#9413}
Commit 7e0c7d49 ("Add support for external encoders in ACM") changed
things around so that we no longer recreate the speech encoder when
adding CNG or RED to an existing encoder. This isn't correct, since
those two expect to be in sync with the speech encoder they work with.
Solve the problem by resetting the speech encoder before hooking in
RED or CNG.
BUG=crbug/490368
R=jmarusic@webrtc.orgTBR=henrik.lundin@webrtc.org
Review URL: https://webrtc-codereview.appspot.com/53589004
Cr-Commit-Position: refs/heads/master@{#9307}
Merge WEBRTC_ARCH_ARM64_NEON and WEBRTC_ARCH_ARM_NEON into one
WEBRTC_HAS_NEON.
Replace WEBRTC_DETECT_ARM_NEON by WEBRTC_DETECT_NEON.
Replace WEBRTC_ARCH_ARM by WEBRTC_ARCH_ARM64 for arm64 cpu.
BUG=4002
R=andrew@webrtc.org, jridges@masque.com, kjellander@webrtc.org
Change-Id: I870a4d0682b80633b671c9aab733153f6d95a980
Review URL: https://webrtc-codereview.appspot.com/49309004
Cr-Commit-Position: refs/heads/master@{#9228}
Before this change, a decoder was registered into ACMReceiver through
the CodecOwner; the CodecOwner had to have a pointer back to the
AudioCodingModuleImpl object to make this call. With this change, the
AudioCodingModuleImpl object asks the CodecOwner for a decoder pointer
instead, making the chain of calls more straightforward.
COAUTHOR=henrik.lundin@webrtc.org
BUG=4474
R=jmarusic@webrtc.org, minyue@webrtc.org
Review URL: https://webrtc-codereview.appspot.com/52439004
Cr-Commit-Position: refs/heads/master@{#9204}
AudioCodingModuleImpl::Add10MsData() calls two private methods that
together do all the work: Add10MsDataInternal() and Encode(). They
each took locks internally in order to protect access to, among other
things, codec_manager_.
This turned out to be inadequate. Add10MsDataInternal() calls
codec_manager_.CurrentEncoder()->SampleRateHz() in order to be able to
resample the audio data to what the current encoder wants. When the
resampled data is fed to the encoder deep inside the Encode() call,
that sample rate must still be correct, but occasionally it wasn't,
which triggered a CHECK. (The specific test that failed was the
voe_auto_test subtest
CodecTest.OpusMaxPlaybackRateCannotBeSetForNonOpus, which changes the
current encoder while encoding is in progress.)
This CL solves the problem by covering all of
AudioCodingModuleImpl::Add10MsData() in a single critical section, so
that the sample rate obtained in Add10MsDataInternal() is guaranteed
to still be valid during the Encode() call.
BUG=4644
R=henrik.lundin@webrtc.org
Review URL: https://webrtc-codereview.appspot.com/52459004
Cr-Commit-Position: refs/heads/master@{#9174}
A better solution than forcing OPUS_APPLICATION_VOIP when enabling DTX has been found, which is to set OPUS_SIGNAL_VOICE.
This reduces the uncertainty of entering DTX over silence period of audio.
This CL contains the setup of OPUS_SIGNAL_VOICE and decoupling opus application mode with DTX.
BUG=4559
R=henrik.lundin@webrtc.org, henrika@webrtc.org
Review URL: https://webrtc-codereview.appspot.com/46959004
Cr-Commit-Position: refs/heads/master@{#9168}
CodecOwner is introduced here; AudioEncoderMutable was introduced in a
previous commit, but had no users until now. The only remaining task
for ACMGenericCodec was to construct and maintain the stack of speech,
CNG, and RED encoders. This task is now handled by the CodecOwner,
which is owned and used by the CodecManager.
COAUTHOR=henrik.lundin@webrtc.org
BUG=4228
R=jmarusic@webrtc.org, minyue@webrtc.org
Review URL: https://webrtc-codereview.appspot.com/43189004
Cr-Commit-Position: refs/heads/master@{#9152}
With this change, the currently used encoder is held in a scoped_ptr.
iSAC is a special case, since the encoder instance is also a decoder
instance, so it may have to be available also if another send codec is
used. This is accomplished by having a separate scoped_ptr for iSAC.
Remove mirror ID from ACM codec database functions, and remove unused
functions from the database.
COAUTHOR=kwiberg@webrtc.org
BUG=4228
R=tina.legrand@webrtc.org
Review URL: https://webrtc-codereview.appspot.com/48729004
Cr-Commit-Position: refs/heads/master@{#8982}
This change essentially divides AudioCodingModuleImpl into two parts:
one is the code related to managing codecs, now moved into CodecManager,
and the other is what remains in AudioCodingModuleImpl.
This change also removes AudioCodingModuleImpl::InitializeSender. The
function was essentially no-op, since it was always called immediately
after construction.
COAUTHOR=kwiberg@webrtc.org
BUG=4228
R=minyue@webrtc.org, tina.legrand@webrtc.org
Review URL: https://webrtc-codereview.appspot.com/51469004
Cr-Commit-Position: refs/heads/master@{#8893}
Change internal indexing of registered decoders. It makes sense because payload type is unique, while ACM codec ID may not be. This is a step towards allowing for addition of external decoders.
R=kwiberg@webrtc.org
Review URL: https://webrtc-codereview.appspot.com/44869004
Cr-Commit-Position: refs/heads/master@{#8867}
In SDP, RED audio codec has its own sample rate. Currently, we offer RED/8000 (8 kHz). But the actual send codec can violate this sample rate. The way to solve it is to introduce more RED payload types, e.g., RED/16000, RED/32000.
As a first step towards that, we, in this CL, limit the current RED (RED/8000) to work only with 8 kHz codecs.
BUG=3619
R=henrik.lundin@webrtc.org
Review URL: https://webrtc-codereview.appspot.com/43849004
Cr-Commit-Position: refs/heads/master@{#8830}
git-svn-id: http://webrtc.googlecode.com/svn/trunk@8830 4adac7df-926f-26a2-2b94-8c16560cd09d
A codec's packet-loss concealer is called once from NetEq before
decoding the first packet after a packet loss. The purpose is not to
use the PLC output, but to prepare the state of the decoder such that
it may recover faster after the loss. However, this effect is not
achieved by calling iSAC's PLC. Also, there are some problems with the
fixed-point implementation of the PLC (see the associated bug).
BUG=4423
R=kwiberg@webrtc.org
Review URL: https://webrtc-codereview.appspot.com/42849004
Cr-Commit-Position: refs/heads/master@{#8827}
git-svn-id: http://webrtc.googlecode.com/svn/trunk@8827 4adac7df-926f-26a2-2b94-8c16560cd09d
Move timestamp conversion out of ACMGenericCodec. Also remove lock
from ACMGenericCodec since the instance is always protected by
acm_crit_sect_ in AudioCodingModuleImpl.
Restructuring the code in AudioCodingModuleImpl::Encode to streamline
the use of locks.
R=minyue@webrtc.org
Review URL: https://webrtc-codereview.appspot.com/46479004
Cr-Commit-Position: refs/heads/master@{#8773}
git-svn-id: http://webrtc.googlecode.com/svn/trunk@8773 4adac7df-926f-26a2-2b94-8c16560cd09d
