tl;dr - non-continuous frames (due to padding) would get stuck as incomplete if the previous complete frame arrived and was decoded before the padding arrived.This fix re-checks the incomplete frame list for continuous frames after old packets arrive.
When padding is enabled and RTX is not, padding is sent as empty RTP packets tacked onto the end of completed frames (meaning: same timestamp, but after a packet with the marker bit set). Given the following set of circumstances, codified in the new unit test method, a frame can get permanently stuck in the incomplete frames list:
- Frame A decoded (packets 94-95). Next expected sequence number is 96.
- Frame C arrives (packets 100-101) and is marked complete. It isn't continuous, since it starts at 100, so it's placed in the incomplete frame list.
- Frame B arrives (packets 96-97) and is complete, since 97 has a marker bit. Turns out that packets 98-99 are padding, but the receiver doesn't know that.
- Frame B is decoded, removed from the decodable frames list, and last decoded state is updated.
- Packets 98-99 arrive. They hit the IsOldPacket check and update the last decoded state, but they don't trigger FindAndInsertContinuousFrames.
- Further packets/frames arrive and complete, but FindAndInsertContinuousFrames only runs on frames that are newer than the newly completed frame.
In this state, Frame C is permanently stuck as incomplete, so the jitter buffer overall is stuck until max NACK age (default: 450 packets), the max NACK list size (default: 200 packets), or a keyframe arrives and IsContinuous returns true for the keyframe.
(Before the November refactoring, Frame B wouldn't have to have been decoded for the bug to trigger; just having a complete continuous frame at any time before the padding arrived would cause this state, as FindAndInsertContinuousFrames was only called when the frame originally became continuous and was inserted into the decodable frames list. Post refactoring, the frame is removed/re-added to the decodable list on every padding packet that arrives)
BUG=
R=stefan@webrtc.org
Review URL: https://webrtc-codereview.appspot.com/50959004
Cr-Commit-Position: refs/heads/master@{#9264}
Because of the Finch experiment, this will not affect Chrome's behaviour at all.
The SNRs in AudioProcessingTest.Formats were only increased to the next multiple of 5.
BUG=webrtc:3146
R=andrew@webrtc.org
Review URL: https://webrtc-codereview.appspot.com/43359004
Cr-Commit-Position: refs/heads/master@{#9263}
Previously, AudioEncoderCng required the speech encoder to not change
its mind regarding the number of 10 ms frames in the next packet
between calls to AudioEncoderCng::EncodeInternal()---specifically, it
could handle an upward but not a downward adjustment. With this patch,
it can handle a downward adjustment too, by simply saving the
overshoot data for the next call to EncodeInternal().
It will still not handle the case where the encoder's reported number
of 10 ms frames in the next packet is inconsistent with the behavior
of its Encode() function when called with no intervening changes to
the encoder.
R=henrik.lundin@webrtc.org
Review URL: https://webrtc-codereview.appspot.com/53469005
Cr-Commit-Position: refs/heads/master@{#9261}
1. move channel number of input file to the base class
2. limit channel number to be 1, since the resampler support only mono at the moment
3. adding a logging function
4. adding more switch to neteq_opus_quality_test
BUG=2692
R=henrik.lundin@webrtc.org
Review URL: https://webrtc-codereview.appspot.com/47239004
Cr-Commit-Position: refs/heads/master@{#9260}
This CL change the function declaration from uint16_t to size_t, and CHECKs that the size fits in uint16_t before proceeding.
BUG=484432
R=tommi@webrtc.org
Review URL: https://webrtc-codereview.appspot.com/47229004
Cr-Commit-Position: refs/heads/master@{#9246}
BUG=3056, 1320
TEST=AutoTest
Mainly add threadchecker and remove unnecessary lock.
And some more styling working.
- audio_device_pulse_linux.cc: wrap lines longer than 80 chars. And add '.' to some comments around. Not do it to all places.
- audio_mixer_manager_pulse_linux.cc: Here I adopt some chromium practice. We use to do many things to the failure of pulse operation, which causes most of the data race issue. In chromium, if we failed to call any pulse function, we just fail it w/o use the previous results. Here I did same. Please check if it's good.
R=bjornv@webrtc.org, henrika@webrtc.org, tommi@webrtc.org
Review URL: https://webrtc-codereview.appspot.com/52479004
Cr-Commit-Position: refs/heads/master@{#9243}
It is necessary for adding 48kHz support to the AudioProcessing::AnalyzeReverseStream int interface (It was not necessary for 32kHz since in that case the splitting filter is more efficient).
BUG=webrtc:3146
R=andrew@webrtc.org, bjornv@webrtc.org
Review URL: https://webrtc-codereview.appspot.com/56389004
Cr-Commit-Position: refs/heads/master@{#9241}
Avoids existing crash and ensures that error message is passed up to Libjingle. Will lead to the following logcat output:
E/libjingle(31404): Error(channel.cc:1514): Failed to SetSend 2 on voice channel
BUG=b/21273153
R=magjed@webrtc.org
Review URL: https://webrtc-codereview.appspot.com/54459004
Cr-Commit-Position: refs/heads/master@{#9236}
compile time.
The condition of static_assert() is evaluated at compile time which is safer and
more efficient.
Note that static_assert() requires C++11.
The changes were generated by the misc-static-assert ClangTidy check by alexfh@google.comR=tommi@webrtc.org
Review URL: https://webrtc-codereview.appspot.com/51019004
Cr-Commit-Position: refs/heads/master@{#9231}
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}
The class doesn't do anything in almost all cases except for grabbing and releasing locks + allocate memory. There are a couple of methods there such as WaitForKey and GetTimeInMs that are used, but those methods aren't specific to audio and we have implementations of these elsewhere. The third method, StringCompare isn't used anywhere (and also isn't specific to audio).
BUG=
R=henrika@webrtc.org
Review URL: https://webrtc-codereview.appspot.com/50009004
Cr-Commit-Position: refs/heads/master@{#9220}
From 6kHz-6.5kHz to 3kHz-5kHz. Previous range had unreliable mask values, letting high frequencies from all directions through. The new range is wider and lower, which results in better estimates.
R=andrew@webrtc.org
Review URL: https://webrtc-codereview.appspot.com/47089004
Cr-Commit-Position: refs/heads/master@{#9213}
BUG=4573,2982,2175,3590
TEST=modules_unittests --gtest_filter=AudioDevice*, AppRTCDemo and WebRTCDemo
Summary:
- Removes dependency of the 'enable_android_opensl' compiler flag.
Instead, OpenSL ES is always supported, and will enabled for devices that
supports low-latency output.
- WebRTC no longer supports OpenSL ES for the input/recording side.
- Removes old code and demos using OpenSL ES for audio input.
- Improves accuracy of total delay estimates (better AEC performance).
- Reduces roundtrip audio latency; especially when OpenSL can be used.
Performance verified on: Nexus 5, 6, 7 and 9. Samsung Galaxy S4 and S6.
Android One device.
R=magjed@webrtc.org, phoglund@webrtc.org, tommi@webrtc.org
Review URL: https://webrtc-codereview.appspot.com/51759004
Cr-Commit-Position: refs/heads/master@{#9208}
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}
The main purpose of this CL is to clean up RTCPSender::PrepareRTCP, but
it has quite a few ramifications. Notable changes:
* Removed the rtcpPacketTypeFlags bit vector and don't assume
RTCPPacketType values have a single unique bit set. This will allow
making this an enum class once rtcp_receiver has been overhauled.
* Flags are now stored in a map that is a member of the class. This
meant we could remove some bool flags (eg send_remb_) which was
previously masked into rtcpPacketTypeFlags and then masked out again
when testing if a remb packet should be sent.
* Make all build methods, eg. BuildREMB(), have the same signature.
An RtcpContext struct was introduced for this purpose. This allowed
the use of a map from RTCPPacketType to method pointer. Instead of
18 consecutive if-statements, there is now a single loop.
The context class also allowed some simplifications in the build
methods themselves.
* A few minor simplifications and cleanups.
The next step is to gradually replace the builder methods with the
builders from the new RtcpPacket classes.
BUG=2450
R=asapersson@webrtc.org, pbos@webrtc.org
Review URL: https://webrtc-codereview.appspot.com/48329004
Cr-Commit-Position: refs/heads/master@{#9166}
When compiling against an OSX 10.7+ SDK, explicitly redeclare methods only
available from an OSX 10.7+ SDK. This suppresses the clang warning
-Wpartial-availability, which will be turned on in the future.
BUG=chromium:471823
R=jiayl@webrtc.org, mark@chromium.org
Review URL: https://webrtc-codereview.appspot.com/44359004
Cr-Commit-Position: refs/heads/master@{#9163}
