1. It moves calculation of the needed padding to VideoSendStream instead of ViEEncoder and only does it once per send Stream instead of every time the network estimate changes.
2. The maximum amount of padding sent was prior to this cl calculated and updated based on network estimate changes. However, it can only change based on encoder configuration changes and if send streams are added or removed. This cl change the VideoSendStream/VieEncoder to notify the BitrateAllocator of changes to the needed padding bitrate and for BitrateAllocator to notify Call of these changes.
3. Fixed an issue in the SendPacer where it could send a padding packet before sending a real packet. This caused the test EndToEndTest.RestartingSendStreamPreservesRtpStatesWithRtx to fail with these refactorings since the pacer suddenly could send a padding packet before the encoder had produced its first frame.
BUG=webrtc:5687
Review-Url: https://codereview.webrtc.org/1993113003
Cr-Commit-Position: refs/heads/master@{#13149}
Reason for revert:
Reverting while we track down the issue on the Win10 bot.
Original issue's description:
> Split IncomingVideoStream into two implementations, with smoothing and without.
>
> This CL fixes an issue with the non-smoothing implementation where frames were delivered on the decoder thread. No-smoothing is now done in a separate class that uses a TaskQueue. The implementation may drop frames if the renderer doesn't keep up and it doesn't block the decoder thread.
>
> Further work done:
>
> * I added TODOs and documentation for VideoReceiveStream::OnFrame, where we today grab 5 locks.
>
> * I removed the Start/Stop methods from the IncomingVideoStream implementations. Now, when an instance is created, it should be considered to be "running" and when it is deleted, it's "not running". This saves on resources and also reduces the amount of locking required and I could remove one critical section altogether.
>
> * I changed the VideoStreamDecoder class to not depend on IncomingVideoStream but rather use the generic rtc::VideoSinkInterface<VideoFrame> interface. This means that any implementation of that interface can be used and the decoder can be made to just use the 'renderer' from the config. Once we do that, we can decouple the IncomingVideoStream implementations from the decoder and VideoReceiveStream implementations and leave it up to the application for how to do smoothing. The app can choose to use the Incoming* classes or roll its own (which may be preferable since applications often have their own scheduling mechanisms).
>
> * The non-smoothing IncomingVideoStream implementation currently allows only 1 outstanding pending frame. If we exceed that, the current frame won't be delivered to the renderer and instead we deliver the next one (since when this happens, the renderer is falling behind).
>
> * The lifetime of the VideoStreamDecoder instance is now bound to Start/Stop in VideoReceiveStream and not all of the lifetime of VideoReceiveStream.
>
> * Fixed VideoStreamDecoder to unregister callbacks in the dtor that were registered in the ctor. (this was open to a use-after-free regression)
>
> * Delay and callback pointers are now passed via the ctors to the IncomingVideoStream classes. The thread primitives in the IncomingVideoStream classes are also constructed/destructed at the same time as the owning object, which allowed me to remove one more lock.
>
> * Removed code in the VideoStreamDecoder that could overwrite the VideoReceiveStream render delay with a fixed value of 10ms on construction. This wasn't a problem with the previous implementation (it would be now though) but seemed to me like the wrong place to be setting that value.
>
> * Made the render delay value in VideoRenderFrames, const.
>
> BUG=
>
> Committed: https://crrev.com/1c7eef652b0aa22d8ebb0bfe2b547094a794be22
> Cr-Commit-Position: refs/heads/master@{#13129}
TBR=mflodman@webrtc.org
# Skipping CQ checks because original CL landed less than 1 days ago.
NOPRESUBMIT=true
NOTREECHECKS=true
NOTRY=true
BUG=
Review-Url: https://codereview.webrtc.org/2061363002
Cr-Commit-Position: refs/heads/master@{#13146}
Current number of threads selection code does not work well
for Android builds - middle and low end devices are having hard time
encoding VGA and QVGA with just one thread.
Increase the amount of vp8 encoder threads for 180p and above resolution.
Also limit maximum number of thread to 3, since for 8 core devices
most of time 4 cores are idle when thermal throttling kicks in.
BUG=b/27946721
R=marpan@webrtc.org
Review URL: https://codereview.webrtc.org/2058753003 .
Cr-Commit-Position: refs/heads/master@{#13142}
It appears there the encode and send operation can happen over multiple
threads. Also, padding data itself may be sent on a different thread.
Remove thread checker and protect all data with crit_sect.
BUG=webrtc:5998
TBR=stefan@webrtc.org
Review-Url: https://codereview.webrtc.org/2066863002
Cr-Commit-Position: refs/heads/master@{#13137}
This CL fixes an issue with the non-smoothing implementation where frames were delivered on the decoder thread. No-smoothing is now done in a separate class that uses a TaskQueue. The implementation may drop frames if the renderer doesn't keep up and it doesn't block the decoder thread.
Further work done:
* I added TODOs and documentation for VideoReceiveStream::OnFrame, where we today grab 5 locks.
* I removed the Start/Stop methods from the IncomingVideoStream implementations. Now, when an instance is created, it should be considered to be "running" and when it is deleted, it's "not running". This saves on resources and also reduces the amount of locking required and I could remove one critical section altogether.
* I changed the VideoStreamDecoder class to not depend on IncomingVideoStream but rather use the generic rtc::VideoSinkInterface<VideoFrame> interface. This means that any implementation of that interface can be used and the decoder can be made to just use the 'renderer' from the config. Once we do that, we can decouple the IncomingVideoStream implementations from the decoder and VideoReceiveStream implementations and leave it up to the application for how to do smoothing. The app can choose to use the Incoming* classes or roll its own (which may be preferable since applications often have their own scheduling mechanisms).
* The non-smoothing IncomingVideoStream implementation currently allows only 1 outstanding pending frame. If we exceed that, the current frame won't be delivered to the renderer and instead we deliver the next one (since when this happens, the renderer is falling behind).
* The lifetime of the VideoStreamDecoder instance is now bound to Start/Stop in VideoReceiveStream and not all of the lifetime of VideoReceiveStream.
* Fixed VideoStreamDecoder to unregister callbacks in the dtor that were registered in the ctor. (this was open to a use-after-free regression)
* Delay and callback pointers are now passed via the ctors to the IncomingVideoStream classes. The thread primitives in the IncomingVideoStream classes are also constructed/destructed at the same time as the owning object, which allowed me to remove one more lock.
* Removed code in the VideoStreamDecoder that could overwrite the VideoReceiveStream render delay with a fixed value of 10ms on construction. This wasn't a problem with the previous implementation (it would be now though) but seemed to me like the wrong place to be setting that value.
* Made the render delay value in VideoRenderFrames, const.
BUG=
Review-Url: https://codereview.webrtc.org/2035173002
Cr-Commit-Position: refs/heads/master@{#13129}
This CL eliminates repeated calls to AudioEffect.queryEffects() on Android when configuring the audio device. Each of these calls was taking 5-10 milliseconds on the devices I was testing (Nexus 4, Nexus 5), and setting up the audio device involved around 10 of these calls.
This change adds a method that checks the cached list of effects before calling the underlying operating system API; this eliminated about half of these calls. The other half happened inside static methods such as NoiseSuppressor.isAvailable(), which are just convenience wrappers for searching through the list of effects. These calls have been replaced with searching through the cached list of effects, reducing the time to configure audio processing effects from 60-80 ms to 5-10. This results in a similar improvement in call setup time.
BUG=
Review-Url: https://codereview.webrtc.org/2051323002
Cr-Commit-Position: refs/heads/master@{#13115}
Introduce a new method I420Buffer::CropAndScale, and a static
convenience helper I420Buffer::CenterCropAndScale. Use them for almost
all scaling needs.
Delete the Scaler class and the cricket::VideoFrame::Stretch* methods.
BUG=webrtc:5682
R=pbos@webrtc.org, perkj@webrtc.org, stefan@webrtc.org
Review URL: https://codereview.webrtc.org/2020593002 .
Cr-Commit-Position: refs/heads/master@{#13110}
Changes:
* Enabled protobuf for iOS globally.
* Set WEBRTC_INCLUDE_INTERNAL_AUDIO_DEVICE on a global
scope similar to GYP since tests depend on it.
* Added missing rtc_libvpx_build_vp9 variable.
* Moved out audio_coding defines into .gni file to avoid code duplication
* Renamed files to avoid object naming conflicts that GN disallows:
* webrtc/modules/audio_processing/{echo_cancellation_unittest.cc->echo_cancellation_bit_exact_unittest.cc}
* webrtc/modules/video_coding/codecs/vp9/{screenshare_layers_unittest.cc->vp9_screenshare_layers_unittest.cc}
BUG=webrtc:5949
TESTED=Built and ran the tests on Mac. Also ran:
gn gen out/Default --args="rtc_enable_bwe_test_logging=true"
and verified that more objects are being built (1885 vs 1883)
when compiling modules_unittests.
NOTRY=True
NOPRESUBMIT=True
Review-Url: https://codereview.webrtc.org/2041233006
Cr-Commit-Position: refs/heads/master@{#13108}
Changes:
* Set WEBRTC_INCLUDE_INTERNAL_AUDIO_DEVICE on a global scope
to match GYP.
* Enable sctpdataengine_unittest.cc for iOS, which should have
been done in https://codereview.webrtc.org/1587193006
* Renamed GN target rtc_base_test_utils -> rtc_base_tests_utils
to match GYP.
* Added dependencies on call, modules/video_coding and video for
rtc_media.
* Added dependency on audio for rtc_media_unitttests (couldn't be
added to rtc_media due to circular dependency problem).
BUG=webrtc:5949
TESTED=Built and ran the tests on Mac.
NOTRY=True
Review-Url: https://codereview.webrtc.org/2050313002
Cr-Commit-Position: refs/heads/master@{#13106}
Every message will now be traced with the location from which it was
posted, including function name, file and line number.
This CL also writes a normal LOG message when the dispatch took more
than a certain amount of time (currently 50ms).
This logging should help us identify messages that are taking
longer than expected to be dispatched.
R=pthatcher@webrtc.org, tommi@webrtc.org
Review URL: https://codereview.webrtc.org/2019423006 .
Cr-Commit-Position: refs/heads/master@{#13104}
To avoid the case where a single data point or too short window is used,
causing bad behavior due to bad stats, update RateStatistics to return
an Optional rather than a plain rate.
There was also a strange off by one bug where the rate was slightly
overestimated (N + 1 buckets, N ms time window).
These changes requires updates to a number of places, and may very well
cause seeming perf regressions (but the stats were probablty more wrong
previously).
BUG=
R=mflodman@webrtc.org, stefan@webrtc.org
Review URL: https://codereview.webrtc.org/2029593002 .
Cr-Commit-Position: refs/heads/master@{#13103}
NOTRY=True # two of the androids bots are not cooperating
BUG=
Review-Url: https://codereview.webrtc.org/2057533002
Cr-Commit-Position: refs/heads/master@{#13099}
These parts were commented out to avoid breaking the Chromium
WebRTC FYI bots. Include them in the WebRTC build to make our bots
build as many as possible of our GN targets.
BUG=webrtc:5949
NOTRY=True
TBR=phoglund@webrtc.org
Review-Url: https://codereview.webrtc.org/2054903002
Cr-Commit-Position: refs/heads/master@{#13097}
Sync the GYP and GN targets and update the name of the GN one
to 'remote_bitrate_estimator'.
Move the GYP variable 'enable_bwe_test_logging' into the local scope.
Remove redundant entries in modules.gyp.
These are preparations related to the GN migration.
BUG=webrtc:5949
TESTED=Ran GYP with the default variables and with
-Denable_bwe_test_logging=1. Compiled remote_bitrate_estimator
and verified that bwe_test_logging.cc is compiled only when
set.
NOTRY=True
Review-Url: https://codereview.webrtc.org/2040313004
Cr-Commit-Position: refs/heads/master@{#13087}
Previously RefCountedObject was passing all parameters by value.
This meant that it was hard to use it with movable types, such
as unique_ptr<>. Now there is a constructor that takes r-value,
which means that RefCountedObject<std::unique_ptr<foo>> can be
initialized by passing std::unique_ptr<foo> to the constructor.
Review-Url: https://codereview.webrtc.org/2036123002
Cr-Commit-Position: refs/heads/master@{#13079}
to fix projects that has own copy of base/thread_annotation.h
R=åsapersson
NOTRY=true
Review-Url: https://codereview.webrtc.org/2048113002
Cr-Commit-Position: refs/heads/master@{#13073}
This reduces its complexity by a factor of 2.7x total.
The mean error introduced by this is in the 6 different noise scenarios and 6 different speech signals tested is below -52dB.
Review-Url: https://codereview.webrtc.org/2035213002
Cr-Commit-Position: refs/heads/master@{#13072}
The changes are done in several patches in order to make
the review easier.
NOTRY=True
BUG=webrtc:5949
Review-Url: https://codereview.webrtc.org/2051443002
Cr-Commit-Position: refs/heads/master@{#13068}
WebRtcSpl_CrossCorrelation and WebRtcSpl_DotProductWithScale compute
the int32 sum of pairwise products from two int16 arrays. So as to
avoid overflow (which could otherwise happen when as little as two
products were summed), the products are right-shifted by an amount
specified by the caller.
This CL changes WebRtcIlbcfix_MyCorr and WebRtcIlbcfix_Smooth to give
sufficient right-shift amounts, instead of ones that may be too small
and cause overflow.
BUG=chromium:601787
Review-Url: https://codereview.webrtc.org/2014033002
Cr-Commit-Position: refs/heads/master@{#13066}
In https://codereview.webrtc.org/2034923003 it was discovered
that a test binary rtc_sdk_peerconnection_objc_tests was
a dependency to rtc_unittests. Unfortunately gtest doesn't
include dependent executables into the same test executable;
only libraries (so theses tests weren't run).
This CL incorporates those tests into rtc_unittests and
does the same changes to the GN build.
BUG=webrtc:5949
TESTED=Built and ran rtc_unittests locally on Mac.
NOTRY=True
Review-Url: https://codereview.webrtc.org/2041743003
Cr-Commit-Position: refs/heads/master@{#13060}
This CL adds support for an extension on RTP frames to allow the sender
to specify the minimum and maximum playout delay limits.
The receiver makes a best-effort attempt to keep the capture-to-render delay
within this range. This allows different types of application to specify
different end-to-end delay goals. For example gaming can support rendering
of frames as soon as received on receiver to minimize delay. A movie playback
application can specify a minimum playout delay to allow fixed buffering
in presence of network jitter.
There are no tests at this time and most of testing is done with chromium
webrtc prototype.
On chromoting performance tests, this extension helps bring down end-to-end
delay by about 150 ms on small frames.
BUG=webrtc:5895
Review-Url: https://codereview.webrtc.org/2007743003
Cr-Commit-Position: refs/heads/master@{#13059}
Previously raw pointers were used for owned DesktopFrame instances.
Updated all screen and window capturer implementations to use
std::unique_ptr<>.
Also includes some other cleanups in the capturers:
- s/NULL/nullptr
- moved default initializers to class definition.
BUG=webrtc:5950
Review-Url: https://codereview.webrtc.org/1988783003
Cr-Commit-Position: refs/heads/master@{#13058}
Reason for revert:
Broke chromium builder
Original issue's description:
> Use std::unique_ptr<> to pass frame ownership in DesktopCapturer impls.
>
> Previously raw pointers were used for owned DesktopFrame instances.
> Updated all screen and window capturer implementations to use
> std::unique_ptr<>.
>
> Also includes some other cleanups in the capturers:
> - s/NULL/nullptr
> - moved default initializers to class definition.
>
> BUG=webrtc:5950
>
> Committed: https://crrev.com/4a627a8c13554d12412cabb8f751caee6e61ee32
> Cr-Commit-Position: refs/heads/master@{#13032}
TBR=wez@chromium.org
# Skipping CQ checks because original CL landed less than 1 days ago.
NOPRESUBMIT=true
NOTREECHECKS=true
NOTRY=true
BUG=webrtc:5950
Review-Url: https://codereview.webrtc.org/2030333003
Cr-Commit-Position: refs/heads/master@{#13033}
Previously raw pointers were used for owned DesktopFrame instances.
Updated all screen and window capturer implementations to use
std::unique_ptr<>.
Also includes some other cleanups in the capturers:
- s/NULL/nullptr
- moved default initializers to class definition.
BUG=webrtc:5950
Review-Url: https://codereview.webrtc.org/1988783003
Cr-Commit-Position: refs/heads/master@{#13032}
This shouldn't be needed, but because the receiver assumes RTX packets
contain RED if configured to receive them (due to an incompatibility
issue), we also have to make sure we send them for now.
BUG=webrtc:5675
Review-Url: https://codereview.webrtc.org/2033763002
Cr-Commit-Position: refs/heads/master@{#13024}
This cl split the class MediaOptimization into two parts. One that deals with frame dropping and stats and one new class called ProtectionBitrateCalculator that deals with calculating the needed FEC parameters and how much of the estimated network bitrate that can be used by an encoder
Note that the logic of how FEC and the needed bitrates is not changed.
BUG=webrtc:5687
R=asapersson@webrtc.org, stefan@webrtc.org
Review URL: https://codereview.webrtc.org/1972083002 .
Cr-Commit-Position: refs/heads/master@{#13018}
This gets rid of the complex & icky state where the sample rate is not
yet determined.
BUG=webrtc:5801
Review-Url: https://codereview.webrtc.org/2020353003
Cr-Commit-Position: refs/heads/master@{#13011}
