All downstream code have been updated to the new location.
In PRESUBMIT.py:
* Remove webrtc/rtc_base from CPP_BLACKLIST
* Add webrtc/rtc_base to LEGACY_API_DIRS
Fix some duplicated paths in
webrtc/modules/audio_processing/test/conversational_speech/BUILD.gn
BUG=webrtc:7634
TBR=kwiberg@webrtc.org
Review-Url: https://codereview.webrtc.org/2976293002
Cr-Commit-Position: refs/heads/master@{#19094}
Instead, use a TaskQueue in the only test that required it.
BUG=none
Review-Url: https://codereview.webrtc.org/2975883002
Cr-Commit-Position: refs/heads/master@{#18969}
All downstream code have been updated to the new location.
In PRESUBMIT.py:
* Remove webrtc/rtc_base from CPP_BLACKLIST
* Add webrtc/rtc_base to LEGACY_API_DIRS
Fix some duplicated paths in
webrtc/modules/audio_processing/test/conversational_speech/BUILD.gn
BUG=webrtc:7634
TBR=kwiberg@webrtc.org
Review-Url: https://codereview.webrtc.org/2973183002
Cr-Commit-Position: refs/heads/master@{#18948}
I used a command like this to update the paths:
perl -pi -e "s/webrtc\/base/webrtc\/rtc_base/g" `find webrtc/rtc_base -name "*.cc" -o -name "*.h"`
The only manual edit is to add an include of webrtc/rtc_base/checks.h in
webrtc/modules/audio_device/android/opensles_common.h, which likely
was needed due to changed include paths due to 'git cl format'.
BUG=webrtc:7634
NOTRY=True
NOPRESUBMIT=True
Review-Url: https://codereview.webrtc.org/2969653002
Cr-Commit-Position: refs/heads/master@{#18871}
const int16_t* data() const;
int16_t* mutable_data();
- data() returns a zeroed static buffer on muted frames (to avoid unnecessary zeroing of the member buffer) and directly returns AudioFrame::data_ on unmuted frames.
- mutable_data(), lazily zeroes AudioFrame::data_ if the frame is currently muted, sets muted=false, and returns AudioFrame::data_.
These accessors serve to "force" callers to be aware of the mute state field, i.e. lazy zeroing is not the primary motivation.
This change only optimizes handling of muted frames where it is somewhat trivial to do so. Other improvements requiring more significant structural changes will come later.
BUG=webrtc:7343
TBR=henrika
Review-Url: https://codereview.webrtc.org/2750783004
Cr-Commit-Position: refs/heads/master@{#18543}
AudioMixerImpl::CreateWithOutputRateCalculator has become
deprecated. Instead, either Create() or Create(OutputRateCalculator,
bool use_limiter) should be used. The first uses sensible default
values for missing arguments. The second takes all arguments. The old
CreateWithOutputRateCalculator is deprecated so that we don't have
different Create:s with all possible combinations of parameters.
Note that the factory methods may change in the future. The reason for
adding 'use_limiter' was that the limiter that was used had
questionable benefit and was very computationally expensive. Now work
is going on to replace it with a much cheaper version. After
the change, the factory method may change again to not allow for
disabling the limiter.
Bug: webrtc:7167
Change-Id: I0f9005e27e726fa552ee38dcbe965274e5006544
Reviewed-on: https://chromium-review.googlesource.com/528074
Reviewed-by: Alessio Bazzica <alessiob@webrtc.org>
Commit-Queue: Alex Loiko <aleloi@webrtc.org>
Cr-Commit-Position: refs/heads/master@{#18496}
The audio mixer has a subcomponent called FrameCombiner, which uses an
AudioProcessing instance as a limiter. The limiter smoothly increases
the volume to avoid causing clipping.
The limiter was created in a default configuration causing the
ResidualEchoDetector submodule of AudioProcessing to be
activated. That submodule operates in the band-split domain (see
AudioProcessingImpl::ApmSubmoduleStates::RenderMultiBandSubModulesActive()).
There is a goal to remove the (expensive and unnecessary)
band-splitting from AudioMixer. This change helps accomplish that. (It
can't be done yet, because the actual limiter sub-component of APM
also operates in the band-split domain).
BUG=webrtc:6185
Review-Url: https://codereview.webrtc.org/2875623002
Cr-Commit-Position: refs/heads/master@{#18090}
This target keeps track of .h the files under webrtc/modules/include/
that are not part of any target.
If a .h file is not part of a target the 'gn check' utility is not
able to spot if a target is missing a dependency because even if
it parses '#include' directives it is not able to find a target that
contains these headers.
BUG=webrtc:7513
NOTRY=True
Review-Url: https://codereview.webrtc.org/2838873002
Cr-Commit-Position: refs/heads/master@{#17880}
Make all rtc_source_test target that contains tests that
are included in a test executable only be visible to the
rtc_test target. Doing this exposed a couple of errors and
dependency problems that were resolved. Having this could
have prevented duplicated execution of tests like the case that
was recently fixed by deadbeef@ in
https://codereview.webrtc.org/2820263004
New targets:
* //webrtc/modules/rtp_rtcp:fec_test_helper
* //webrtc/modules/rtp_rtcp:mock_rtp_rtcp
* //webrtc/modules/remote_bitrate_estimator:mock_remote_bitrate_observer
The mock files and targets should probably be moved into webrtc/test in
the future, but that's out of the scope of this CL.
BUG=webrtc:5716
NOTRY=True
Review-Url: https://codereview.webrtc.org/2828793003
Cr-Commit-Position: refs/heads/master@{#17863}
Deletes left-over includes of trace.h and critical_section_wrapper.h.
BUG=webrtc:7035
Review-Url: https://codereview.webrtc.org/2784873002
Cr-Commit-Position: refs/heads/master@{#17460}
Before this change, the APM limiter used in FrameCombiner (a
sub-component of AudioMixer) only gets to process the data when the
number of non-muted streams is >1. If this number varies between <=1
and >1, the limiter's view of the data will have gaps during the
periods with <= 1 active stream.
This leads to discontinuities in the applied gain. These
discontinuities cause clicks in the output audio. This change
activates APM limiter processing based on the number of audio streams,
independently of their mutedness status.
BUG=chromium:695993
Review-Url: https://codereview.webrtc.org/2776113002
Cr-Commit-Position: refs/heads/master@{#17442}
AudioMixerImpl::CreateWithOutputRateCalculatorAndLimiter(rate_calculator, bool limiter)
was added to create a mixer without the limiter subcomponent. Calling
it "Create with ... *and* limiter" is counterintuitive.
Renamed to simply 'Create'.
TBR=solenberg@webrtc.org
BUG=webrtc:7167
Review-Url: https://codereview.webrtc.org/2709523006
Cr-Commit-Position: refs/heads/master@{#16755}
The APM limiter is a component for keeping the audio from clipping by smoothly reducing the amplitude of the audio samples. It can be rather expensive because of band-splitting & merging. Also, experiments indicate that it is of questionable benefit (adding several sources of human speech almost never cause clipping).
To optionally disable the limiter, this CL does some refactoring on the (quite large) AudioMixerImpl. Functionality related to actual addition of frames and handling AudioFrame meta-data (sample_rate, num_channels, samples_per_channel, time_stamp, elapsed_time_ms) is broken out in a new sub-component called FrameCombiner.
The FrameCombiner is initialized with a 'use_limiter' flag. To create a mixer without using the APM limiter
Inside of FrameCombiner, the meta-data handling and the audio sample addition are kept divided from each other.
This also fixes a few minor GN issues so that warnings do not have to be suppressed.
BUG=webrtc:7167
Review-Url: https://codereview.webrtc.org/2692333002
Cr-Commit-Position: refs/heads/master@{#16742}
(Or, in less flattering terms, fixing a performance issue introduced
a few months ago by me).
In GN release mode (is_debug = false), the version of the mixer code
before this CL generated code that multiplied each sample (tens of
thousands/second for each input stream) with a floating point number.
This number is almost always exactly 1.0f. The only situation when it's
not 1 is when an audio steam is added or removed.
For one input stream early return leads to a 30% improvement of audio
mixing time profiled on x86-64 under a release build (is_debug = false,
enable_profiling, enable_full_stack_frames_for_profiling) with 16kHz and no
APM limiter. There can be up to 3 streams.
BUG=chromium:687502
Review-Url: https://codereview.webrtc.org/2659423002
Cr-Commit-Position: refs/heads/master@{#16396}
The file was aldready pruned down to the point where it only included
webrtc/typedefs.h. Therefore, all includes of
voice_engine_configurations.h are replaced with typedefs.h, except on
two occasions where it was obvously not needed.
BUG=webrtc:6506
Review-Url: https://codereview.webrtc.org/2553583002
Cr-Commit-Position: refs/heads/master@{#15547}
This CL breaks out the output sample rate calculation from
webrtc::AudioMixerImpl. A new OutputRateCalculator interface is added
to make the sample rate configurable. There are at least three reasons
for this change:
1. The mixer will be used for an internal project, in which no
resampling is done after the mixing. There the sample rate should
be static. Currently, it can differ across mix iterations and
depends on the number of audio sources. If there are no sources,
the WebRTC mixer behavior is to produce silence at 48 kHz.
2. A planned change to WebRTC will make audio processing steps
happen at constant sample rates. A configurable sample rate
calculator will make the transition simpler for the mixer.
3. The current mixer design is a single large file. Behavior is not
always simple to change (e.g. as in this case to mix at a
constant rate), unrelated behavior can be broken, reusing the
mixer in internal projects is tricky. Using DI for the sample
rate calculation solves parts of these issues.
Changes:
The protected mixer c-tor now takes
unique_ptr<OutputRateCalculator>. The current output rate calculation
is moved to DefaultOutputRateCalculator. A new factory method
AudioMixerImpl::CreateWithOutputRateCalculator is added. The old
factory method passes the default rate calculator.
BUG=webrtc:6346
Review-Url: https://codereview.webrtc.org/2557713006
Cr-Commit-Position: refs/heads/master@{#15472}
This CL is in preparation to move the AudioFrame into webrtc/api. The
AudioFrame is a POD type used for representing 10ms of audio. It
appears as a parameter and return value of interfaces being migrated
to webrtc/api, in particular AudioMixer.
Here, methods operator+=, operator>>=, Mute are
moved into a new target webrtc/audio/utility/audio_frame_operations,
and dependencies are changed to use
the new versions. The old AudioFrame methods are marked deprecated.
The audio frame utilities in webrtc/modules/utility:audio_frame_operations
are also moved to the new location.
TBR=kjellander@webrtc.org
BUG=webrtc:6548
NOPRESUBMIT=True
Review-Url: https://codereview.webrtc.org/2424173003
Cr-Commit-Position: refs/heads/master@{#15413}
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}
In the AudioMixerImpl implementation, removing a source never fails
and the return value is always true (see audio_mixer/audio_mixer_impl.cc).
A return value of |false| signaled that removing a source failed for
some reason. We have come to the conclusion that
* we don't know how to handle a return value of |false|
* we can't think of why an alternative implementation would need to
signal failure when removing a stream.
To avoid having a status code that is never read, never acted upon and
probably never set to anything but |true|, we change ::RemoveSource to
not have a return value.
NOTRY=True
BUG=webrtc:6346
Review-Url: https://codereview.webrtc.org/2506173003
Cr-Commit-Position: refs/heads/master@{#15150}
This is a refactoring change in preparation for enabling AudioMixer
with the goal to have a small CL as possible for passing audio through
the new audio mixer in WebRTC. The dependent CL https://codereview.webrtc.org/2436033002/
enables the mixer.
An object of class AudioState is shared across different webrtc audio
connections. It is created in tests and in
WebRTCVoiceEngine. AudioState is constructed by passing a Config
struct, where one argument is scoped_refptr<AudioMixer>.
Populating this field has now been mandatory. Tests and
WebRTCVoiceEngine create and pass either a AudioMixerImpl.
WebRTCVoiceEngine passes a real AudioMixer, which is
currently unused.
An alternative would have tests pass a mocked audio mixer. We
chose not to do that, because we believe that tests should use
the real thing unless there are reasons against it. Construction
time is not an issue, because the real mixer is relatively
lightweight.
We couldn't find a way to test any mixer-related changes in AudioState
before the mixes is connected. The next dependent CL
https://codereview.webrtc.org/2436033002/ contains unit tests for
mixer usage.
BUG=webrtc:6346
Review-Url: https://codereview.webrtc.org/2469743002
Cr-Commit-Position: refs/heads/master@{#15134}
This change changes mixing to be done at the lowest possible
APM-native rate that does not lead to quality loss. An Audio
Processing-native rate is one of 8, 16, 32, or 48 kHz. Mixing at a
lower sampling rate and avoiding resampling can in many cases lead to
big efficiency improvements, as reported by experiments.
This CL also fixes a design issue with the AudioMixer: audio at
non-native rates is no longer fed to the APM instance which is the
limiter.
NOTRY=True
BUG=webrtc:6346
Review-Url: https://codereview.webrtc.org/2458703002
Cr-Commit-Position: refs/heads/master@{#14980}
This change will allow for a audio source to report its sampling rate
to the audio mixer. It is needed in order to mix at a lower sampling
rate. Mixing at a lower sampling rate can in many cases lead to big
efficiency improvements, as reported by experiments.
The code affected is all implementations of the Source interface:
AudioReceiveStream and a mock class. The AudioReceiveStream now
queries its underlying voe::Channel object for the needed frequency.
Note that the changes to the mixing algorithm are done in a later CL.
BUG=webrtc:6346
NOTRY=True
TBR=solenberg@webrtc.org
Review-Url: https://codereview.webrtc.org/2448113009
Cr-Commit-Position: refs/heads/master@{#14839}
Introduce a convention on categorizing GN targets:
1. Production code
2. Tests
3. Examples
4. Tools
The first two have targets spread out all over the tree,
while the latter are isolated to examples/ and tools/ directories.
Another new convention: Each directory's BUILD.gn file shall contain
a target named similar to the directory name. This target shall
contain the 'most common' production code, i.e. so that most
consumers of the directory can depend on only the directory
(which implicitly means that target in GN).
//webrtc:webrtc_tests is changed to depend on all WebRTC tests.
From now on, it's necessary to add new test targets to this dependency
tree in order to get them compiled.
Two new group targets are created:
//webrtc/modules/audio_coding:audio_coding_tests
//webrtc/modules/audio_processing:audio_processing_tests
to reduce the long list of tests in //webrtc:webrtc_tests.
Visibility on //webrtc:webrtc and //webrtc:webrtc_tests is restricted
to the root target, to avoid circular dependencies due to the monolithic
property of these targets (a problem we've had in the past).
The 'root' target at the top level is renamed to 'default', which means GN will
build this target instead of _all_ generated targets
(see https://chromium.googlesource.com/chromium/src/+/master/tools/gn/docs/faq.md#Can-I-control-what-targets-are-built-by-default).
This target now depends on everything we want to build, meaning all targets now
explicitly needs to be wired up from the root target in order to get build.
Having this, the number of compiled objects on Android is decreased from 8855 to 6276. It also gives us better control over our build.
BUG=webrtc:6440
TESTED=git cl try --clobber
NOTRY=True
Review-Url: https://codereview.webrtc.org/2441383002
Cr-Commit-Position: refs/heads/master@{#14821}
The mixer allocates an audio frame for each added data source. This
audio frame was deallocated when a source was removed from the
mixer. Source removal could happen during the mixing, and the existing
locking scheme (and the Clang thread checker) was not sufficient to
prevent a data race.
After this change, the mixer doesn't release its lock until it is
finished with the sources' Audio frames. Since multi-threaded access to
the mixer only happens when a source is added or removed, we believe
that this change wouldn't have any noticeable performance impact.
NOTRY=True
BUG=webrtc:6346
Review-Url: https://codereview.webrtc.org/2439283002
Cr-Commit-Position: refs/heads/master@{#14744}
Simplify the AudioMixer::Source interface and update the mixer
implementation to the new interface.
Instead of asking a mixer source to provide a pointer to an AudioFrame
during each mixing iteration, a mixer should supply a pointer to its
own AudioFrame.
This simplifies lifetime issues as sources do not give away an
internal pointer.
Implementation: when an audio source is added, the mixer allocates a
new AudioFrame. The audio frame is kept together in the internal class
SourceStatus together with the audio source pointer until the source
is removed.
NOTRY=True
BUG=webrtc:6346
Review-Url: https://codereview.webrtc.org/2420913002
Cr-Commit-Position: refs/heads/master@{#14713}
This change is due to an incorrect understanding of the threading
model in Chrome. The new AudioMixer has a thread checker to ensure
that mixing is always done from a single thread. Mixing is done on the
Audio Output Thread. When run in Chrome, it can change. Even if the thread
changes, there is never more than one audio thread, and mixing is done
sequentially.
The threading checks and variable access checks are replaced with
rtc::RaceChecker counterparts.
NOTRY=True
BUG=webrtc:6346
Review-Url: https://codereview.webrtc.org/2437913003
Cr-Commit-Position: refs/heads/master@{#14712}
The AudioMixer is now split in a mixer and audio source interface part, which has moved to webrtc/api, and a default implementation part, which lies in webrtc/modules.
This change makes it possible to create other mixer implementations and is a first step to facilitate passing down a mixer from outside of WebRTC.
It will also create less build dependencies when the new mixer has replaced the old one.
NOTRY=True
TBR=henrik.lundin@webrtc.org
BUG=webrtc:6346
Review-Url: https://codereview.webrtc.org/2411313003
Cr-Commit-Position: refs/heads/master@{#14705}
Changed mixability status into AddSource/RemoveSource. Added 'ssrc()'
method to the MixerSource interface. Removed unnecessary member 'num_audio_sources_' and made the mixer be refcounted.
BUG=webrtc:6346
NOTRY=True
Review-Url: https://codereview.webrtc.org/2408683002
Cr-Commit-Position: refs/heads/master@{#14612}
MixerAudioSource is moved to AudioMixerImpl::Source. Structures and methods of the MixerAudioSource interface have been renamed. The RemixFrame method has added checks and is moved to audio_frame_manipulator.h
BUG=webrtc:6346
Review-Url: https://codereview.webrtc.org/2396803004
Cr-Commit-Position: refs/heads/master@{#14600}
Reason for revert:
breaks chromium FYI
Original issue's description:
> Made MixerAudioSource a pure interface.
>
> This required quite a few small changes in the mixing algorithm
> structure, the mixer interface and the mixer unit tests.
>
> BUG=webrtc:6346
>
> Committed: https://crrev.com/2ae5fdff86b784545cbd724de54bb5ffedde1adf
> Cr-Commit-Position: refs/heads/master@{#14567}
TBR=ivoc@webrtc.org,solenberg@webrtc.org
# Skipping CQ checks because original CL landed less than 1 days ago.
NOPRESUBMIT=true
NOTREECHECKS=true
NOTRY=true
BUG=webrtc:6346
Review-Url: https://codereview.webrtc.org/2394253003
Cr-Commit-Position: refs/heads/master@{#14568}
This required quite a few small changes in the mixing algorithm
structure, the mixer interface and the mixer unit tests.
BUG=webrtc:6346
Review-Url: https://codereview.webrtc.org/2396483002
Cr-Commit-Position: refs/heads/master@{#14567}
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}
