In older Chrome versions, the associated payload type in the RTX header
of retransmitted packets was always set to be the original media payload type,
regardless of the actual payload type of the packet. This meant that packets
encapsulated with RED headers had incorrect payload type information in the
RTX header. Due to an assumption in the receiver, this incorrect payload type
information would effectively be undone, leading to a working system.
Albeit working, this behaviour was undesired, and thus removed. In the interim,
several workarounds were introduced to not destroy interop between old and
new Chrome versions:
(1) https://codereview.webrtc.org/1649493004
- If no payload type mapping existed for RED over RTX, the payload type
of the underlying media would be used.
- If RED had been negotiated, received RTX packets would always be
assumed to contain RED.
(2) https://codereview.webrtc.org/1964473002
- If RED was removed from the remote description answer, it would be
disabled in the local receiver as well.
(3) https://codereview.webrtc.org/2033763002
- If RED was negotiated in the SDP, it would always be used, regardless
if ULPFEC was negotiated and used, or not.
Since the Chrome versions that exhibited the original bug now are very old,
this CL removes the workarounds from (1) and (2). In particular, after this
change, we will have the following behaviour:
- We assume that a payload type mapping for RED over RTX always is set.
If this is not the case, the RTX packet is not sent.
- The associated payload type of received RTX packets will always be obeyed.
- The (non)-existence of RED in the remote description does not affect the
local receiver.
The workaround in (3) still needs to exist, in order to interop with receivers
that did not have the workarounds in (1) and (2) removed. The change in (3)
can be removed in a couple of Chrome versions.
TESTED=Using AppRTC between patched Chrome (connected to ethernet) and standard Chrome M54 (connected to lossy internal Google WiFi), with and without FEC turned off using AppRTC flag. Also using "Munge SDP" sample on patched Chrome over loopback interface, with 100ms delay and 5% packet loss simulated using tc.
BUG=webrtc:6650
Review-Url: https://codereview.webrtc.org/2469093003
Cr-Commit-Position: refs/heads/master@{#15038}
Reason for revert:
Seems to be causing flakiness in perf test:
FullStackTest.ScreenshareSlidesVP8_2TL_LossyNet
Original issue's description:
> Reland of Issue 2434073003: Extract bitrate allocation ...
>
> This is a reland of https://codereview.webrtc.org/2434073003/ including
> some fixes for failing test cases.
>
> Original description:
>
> Extract bitrate allocation of spatial/temporal layers out of codec impl.
>
> This CL makes a number of intervowen changes:
>
> * Add BitrateAllocation struct, that contains a codec independent view
> of how the target bitrate is distributed over spatial and temporal
> layers.
>
> * Adds the BitrateAllocator interface, which takes a bitrate and frame
> rate and produces a BitrateAllocation.
>
> * A default (non layered) implementation is added, and
> SimulcastRateAllocator is extended to fully handle VP8 allocation.
> This includes capturing TemporalLayer instances created by the
> encoder.
>
> * ViEEncoder now owns both the bitrate allocator and the temporal layer
> factories for VP8. This allows allocation to happen fully outside of
> the encoder implementation.
>
> This refactoring will make it possible for ViEEncoder to signal the
> full picture of target bitrates to the RTCP module.
>
> BUG=webrtc:6301
>
> Committed: https://crrev.com/647bf43dcb2fd16fccf276bd94dc4400728bb405
> Cr-Commit-Position: refs/heads/master@{#15023}
TBR=mflodman@webrtc.org
# Skipping CQ checks because original CL landed less than 1 days ago.
NOPRESUBMIT=true
NOTREECHECKS=true
NOTRY=true
BUG=webrtc:6301
Review-Url: https://codereview.webrtc.org/2491393002
Cr-Commit-Position: refs/heads/master@{#15026}
This is a reland of https://codereview.webrtc.org/2434073003/ including
some fixes for failing test cases.
Original description:
Extract bitrate allocation of spatial/temporal layers out of codec impl.
This CL makes a number of intervowen changes:
* Add BitrateAllocation struct, that contains a codec independent view
of how the target bitrate is distributed over spatial and temporal
layers.
* Adds the BitrateAllocator interface, which takes a bitrate and frame
rate and produces a BitrateAllocation.
* A default (non layered) implementation is added, and
SimulcastRateAllocator is extended to fully handle VP8 allocation.
This includes capturing TemporalLayer instances created by the
encoder.
* ViEEncoder now owns both the bitrate allocator and the temporal layer
factories for VP8. This allows allocation to happen fully outside of
the encoder implementation.
This refactoring will make it possible for ViEEncoder to signal the
full picture of target bitrates to the RTCP module.
BUG=webrtc:6301
Review-Url: https://codereview.webrtc.org/2488833004
Cr-Commit-Position: refs/heads/master@{#15023}
Reason for revert:
This CL probably broke Chromium FYI.
Original issue's description:
> Stop caching supported codecs in WebRtcVideoEngine2
>
> We currently cache the result of GetSupportedCodecs in a member variable
> |video_codecs_| in WebRtcVideoEngine2. This means we need to keep
> |video_codecs_| and the result of GetSupportedCodecs in sync, which is
> error prone. It's simpler to just call GetSupportedCodecs when we need
> it, and we actually end up making fewer calls, so it's faster as well.
> This CL also returns all std::vectors by-value instead of by-ref. Move
> semantic together with in-place filtering of codecs actually end up with
> fewer copies, and it's also simpler to not return references.
>
> BUG=webrtc:6337
>
> Committed: https://crrev.com/9f71ec5a3e3175751f4475b126cfda89767363f2
> Cr-Commit-Position: refs/heads/master@{#15007}
TBR=tommi@webrtc.org
# Skipping CQ checks because original CL landed less than 1 days ago.
NOPRESUBMIT=true
NOTREECHECKS=true
NOTRY=true
BUG=webrtc:6337
Review-Url: https://codereview.webrtc.org/2489173004
Cr-Commit-Position: refs/heads/master@{#15014}
We currently cache the result of GetSupportedCodecs in a member variable
|video_codecs_| in WebRtcVideoEngine2. This means we need to keep
|video_codecs_| and the result of GetSupportedCodecs in sync, which is
error prone. It's simpler to just call GetSupportedCodecs when we need
it, and we actually end up making fewer calls, so it's faster as well.
This CL also returns all std::vectors by-value instead of by-ref. Move
semantic together with in-place filtering of codecs actually end up with
fewer copies, and it's also simpler to not return references.
BUG=webrtc:6337
Review-Url: https://codereview.webrtc.org/2492473002
Cr-Commit-Position: refs/heads/master@{#15007}
Reason for revert:
Breaks perf tests.
Original issue's description:
> Extract bitrate allocation of spatial/temporal layers out of codec impl.
>
> This CL makes a number of intervowen changes:
>
> * Add BitrateAllocation struct, that contains a codec independent view
> of how the target bitrate is distributed over spatial and temporal
> layers.
>
> * Adds the BitrateAllocator interface, which takes a bitrate and frame
> rate and produces a BitrateAllocation.
>
> * A default (non layered) implementation is added, and
> SimulcastRateAllocator is extended to fully handle VP8 allocation.
> This includes capturing TemporalLayer instances created by the
> encoder.
>
> * ViEEncoder now owns both the bitrate allocator and the temporal layer
> factories for VP8. This allows allocation to happen fully outside of
> the encoder implementation.
>
> This refactoring will make it possible for ViEEncoder to signal the
> full picture of target bitrates to the RTCP module.
>
> BUG=webrtc:6301
>
> Committed: https://crrev.com/8f46c679d24a05b3f08e02c6d91ec9637f34e24f
> Cr-Commit-Position: refs/heads/master@{#14998}
TBR=stefan@webrtc.org,perkj@webrtc.org,mflodman@webrtc.org
# Skipping CQ checks because original CL landed less than 1 days ago.
NOPRESUBMIT=true
NOTREECHECKS=true
NOTRY=true
BUG=webrtc:6301
Review-Url: https://codereview.webrtc.org/2489843002
Cr-Commit-Position: refs/heads/master@{#15001}
This CL makes a number of intervowen changes:
* Add BitrateAllocation struct, that contains a codec independent view
of how the target bitrate is distributed over spatial and temporal
layers.
* Adds the BitrateAllocator interface, which takes a bitrate and frame
rate and produces a BitrateAllocation.
* A default (non layered) implementation is added, and
SimulcastRateAllocator is extended to fully handle VP8 allocation.
This includes capturing TemporalLayer instances created by the
encoder.
* ViEEncoder now owns both the bitrate allocator and the temporal layer
factories for VP8. This allows allocation to happen fully outside of
the encoder implementation.
This refactoring will make it possible for ViEEncoder to signal the
full picture of target bitrates to the RTCP module.
BUG=webrtc:6301
Review-Url: https://codereview.webrtc.org/2434073003
Cr-Commit-Position: refs/heads/master@{#14998}
This CL removes WebRtcVideoReceiveStream::FilterSupportedCodecs and
adds more fine grained code for selecting and verifying codecs. This
also removes unnecessary copying of cricket::VideoCodecs.
BUG=webrtc:6337
Review-Url: https://codereview.webrtc.org/2474433012
Cr-Commit-Position: refs/heads/master@{#14964}
The cricket::Codec class contains std containers like
std::map<std::string, std::string> and is expensive to copy. This CL
adds move constructors and move assignment operators for it and all
subclasses.
This CL also:
* Implement functions with '= default' instead of doing it manually.
* Makes codec::Matches symmetric. We currently don't check if the
payload type of the callee is static, and might incorrectly return
true in these cases.
BUG=None
Review-Url: https://codereview.webrtc.org/2481193002
Cr-Commit-Position: refs/heads/master@{#14956}
The unit was kbps but the one default use of it is in bps. The inconsistency should be fixed.
BUG=webrtc:6670
Review-Url: https://codereview.webrtc.org/2247213005
Cr-Commit-Position: refs/heads/master@{#14955}
Reason for revert:
Reverting because of the reasons given in comment #16:
"This change breaks a scenario that is unfortunately not covered by unit tests,
but can easily happen in a real call.
The scenario that is broken by the change is this:
1. A sends an offer to B, with a set of codecs C_a (which is a subset of C_b,
the codecs supported by B)
2. B responds with an answer, and sets the receive codecs to C_a.
3. At a later time, B generates a new offer which by default includes all codecs
in C_b.
4. B calls SetLocalDescription() with this offer, that adds new receive codecs.
5. Adding the new codecs fails, because of the check at
https://cs.chromium.org/chromium/src/third_party/webrtc/voice_engine/channel.....
This causes SetLocalDescription() itself to fail. The net effect is that B
cannot set a local description it just generated.
Before the CL mentioned above, we'd stop playout before changing the codecs, and
the operation would succeed."
Original issue's description:
> Removed the legacy behavior of stopping playout when setting new receive codecs.
>
> BUG=webrtc:4690
>
> Committed: https://crrev.com/917d4e1e7131f35764cff932a8793151585e8179
> Cr-Commit-Position: refs/heads/master@{#14610}
TBR=solenberg@webrtc.org
# Not skipping CQ checks because original CL landed more than 1 days ago.
BUG=webrtc:4690
Review-Url: https://codereview.webrtc.org/2478433003
Cr-Commit-Position: refs/heads/master@{#14905}
Suppress WebRtcVideoEncoderFactory overloaded virtual function warning
in WebRtcSimulcastEncoderFactory and FakeWebRtcVideoEncoderFactory.
This warning is triggered by the change in this CL:
https://codereview.webrtc.org/2449993003/.
BUG=webrtc:6402, webrtc:6337
Review-Url: https://codereview.webrtc.org/2468253002
Cr-Commit-Position: refs/heads/master@{#14901}
This removes the VideoSendStream::LoadObserver interface and the implementation in WebrtcVideoSendStream and replace it with VideoSinkWants through the VideoSourceInterface.
To do that that, some stats for CPU adaptation is moved into VideoSendStream. Also handling of the CVO rtp header extension is moved to VideoSendStreamImpl.
BUG=webrtc:5687
TBR=mflodman@webrtc.org
Review-Url: https://codereview.webrtc.org/2304363002
Cr-Commit-Position: refs/heads/master@{#14877}
This CL introduces two new functions to the WebRtcVideoEncoderFactory
interface based on cricket::VideoFormat instead of
WebRtcVideoEncoderFactory::VideoCodec. The functions are:
WebRtcVideoEncoderFactory::CreateVideoEncoder() and
WebRtcVideoEncoderFactory::supported_codecs(). In order to make a smooth
transition to the new interface, the old functions are kept, and default
implementations are provided for both the old and new functions so that
external clients can switch from the old to the new functions in peace.
The default implementations will just convert between
cricket::VideoFormat and WebRtcVideoEncoderFactory::VideoCodec. Once all
external clients have updated their code, the plan is to remove the old
functions and all default implementations to make
WebRtcVideoEncoderFactory a pure interface again.
BUG=webrtc:6402,webrtc:6337
Review-Url: https://codereview.webrtc.org/2449993003
Cr-Commit-Position: refs/heads/master@{#14826}
- Update MockAudioProcessing to current APM interface.
- Replace calls to VoEAudioProcessing::Start/StopAecDump with direct calls on APM.
- Add AudioProcessing* in WVoE, get it from VoE, so we can call directly on APM.
BUG=webrtc:4690
Review-Url: https://codereview.webrtc.org/2446143002
Cr-Commit-Position: refs/heads/master@{#14786}
Since WebRtcVideoSendStream have reconfigures the send codec to match the incoming captured frames widht and height they have not been used.
Framerate has just been set when parsing sdp to 60fps and not changed elsewhere.
This cl require some upstream projects to change first.
BUG=webrtc:5332
Review-Url: https://codereview.webrtc.org/2408153002
Cr-Commit-Position: refs/heads/master@{#14733}
Call demultiplexes received RTP packets and delivers these to the
appropriate {Video,Flexfec}ReceiveStreams. A single video stream
could conceivably be protected by multiple FlexFEC streams.
BUG=webrtc:5654
Review-Url: https://codereview.webrtc.org/2388303009
Cr-Commit-Position: refs/heads/master@{#14727}
The stat is currently always set to zero until the residual echo detector has landed.
BUG=webrtc:6525
Review-Url: https://codereview.webrtc.org/2431443003
Cr-Commit-Position: refs/heads/master@{#14721}
An audio track with a level controller with the correct initialization
value can be created by a combination of
PeerConnectionFactory::CreateAudioTrack(..., audio_source) and
either
audio_source = PeerConnectionFactory::CreateAudioSource(constraints) or
audio_source = PeerConnectionFactory::CreateAudioSource(audio_options).
NOTRY=True
BUG=webrtc:6386
Review-Url: https://codereview.webrtc.org/2408143003
Cr-Commit-Position: refs/heads/master@{#14693}
to the functionality in the audio processing module.
Therefore, it should be a pure interface.
This CL ensures that is the case.
BUG=webrtc:6515
Review-Url: https://codereview.webrtc.org/2406193002
Cr-Commit-Position: refs/heads/master@{#14608}
This CL is a pure refactoring which should not result in any functinal
changes. It moves ownership of the RtcEventLog from webrtc::Call to the
webrtc::PeerConnection object.
This is done so that we can add RtcEventLog support for ICE events -
which will require the TransportController to have a pointer to the
RtcEventLog. PeerConnection is the closest common owner of both Call and
TransportController (through WebRtcSession).
BUG=webrtc:6393
Review-Url: https://codereview.webrtc.org/2353033005
Cr-Commit-Position: refs/heads/master@{#14578}
all methods but a few constructors. And similarly for the
subclass cricket::WebRtcVideoFrame.
TBR=tkchin@webrtc.org # Added an include line
BUG=webrtc:5682
Review-Url: https://codereview.webrtc.org/2315663002
Cr-Commit-Position: refs/heads/master@{#14576}
Also rename some related minor methods. No functional changes
are intended/expected.
BUG=webrtc:5654
Review-Url: https://codereview.webrtc.org/2391963002
Cr-Commit-Position: refs/heads/master@{#14513}
The original landed cl is in patchset 1.
The following patchset fix VideoQualityTest as well as fix the case where max_bitrate is set in the SendParams. A unit test is added for that as well.
Original cl description:
Let ViEEncoder handle resolution changes.
This cl move codec reconfiguration due to video frame size changes from WebRtcVideoSendStream to ViEEncoder.
With this change, many variables in WebRtcVideoSendStream no longer need to be locked.
BUG=webrtc:5687, webrtc:6371, webrtc:5332
Review-Url: https://codereview.webrtc.org/2386573002
Cr-Commit-Position: refs/heads/master@{#14467}
