All graphs for this bug:
  https://chromeperf.appspot.com/group_report?bug_id=597332

Original alerts at time of bug-filing:
  https://chromeperf.appspot.com/group_report?keys=agxzfmNocm9tZXBlcmZyFAsSB0Fub21hbHkYgICg0OPPqQoM,agxzfmNocm9tZXBlcmZyFAsSB0Fub21hbHkYgICg0PumtgoM,agxzfmNocm9tZXBlcmZyFAsSB0Fub21hbHkYgICg0Pu8pwoM,agxzfmNocm9tZXBlcmZyFAsSB0Fub21hbHkYgICg0MWq4woM,agxzfmNocm9tZXBlcmZyFAsSB0Fub21hbHkYgICg0JyboAoM,agxzfmNocm9tZXBlcmZyFAsSB0Fub21hbHkYgICgkNubpgoM,agxzfmNocm9tZXBlcmZyFAsSB0Fub21hbHkYgICg0NippwoM,agxzfmNocm9tZXBlcmZyFAsSB0Fub21hbHkYgICg0OXAuQsM,agxzfmNocm9tZXBlcmZyFAsSB0Fub21hbHkYgICg0PK1vQoM,agxzfmNocm9tZXBlcmZyFAsSB0Fub21hbHkYgICg0PLhrAoM,agxzfmNocm9tZXBlcmZyFAsSB0Fub21hbHkYgICg0OOloQkM,agxzfmNocm9tZXBlcmZyFAsSB0Fub21hbHkYgICg0PL6tQkM,agxzfmNocm9tZXBlcmZyFAsSB0Fub21hbHkYgICgkN_tpQoM,agxzfmNocm9tZXBlcmZyFAsSB0Fub21hbHkYgICg0JyougoM,agxzfmNocm9tZXBlcmZyFAsSB0Fub21hbHkYgICg0JWRtwkM,agxzfmNocm9tZXBlcmZyFAsSB0Fub21hbHkYgICgkKGwzwkM,agxzfmNocm9tZXBlcmZyFAsSB0Fub21hbHkYgICg0JHsqAoM,agxzfmNocm9tZXBlcmZyFAsSB0Fub21hbHkYgICg0PL6tQoM,agxzfmNocm9tZXBlcmZyFAsSB0Fub21hbHkYgICgkN-JuAoM,agxzfmNocm9tZXBlcmZyFAsSB0Fub21hbHkYgICg0OzjsgoM,agxzfmNocm9tZXBlcmZyFAsSB0Fub21hbHkYgICg0Oz5owoM,agxzfmNocm9tZXBlcmZyFAsSB0Fub21hbHkYgICgkN-WqQoM,agxzfmNocm9tZXBlcmZyFAsSB0Fub21hbHkYgICg0JXcuwoM,agxzfmNocm9tZXBlcmZyFAsSB0Fub21hbHkYgICg0OyevAoM,agxzfmNocm9tZXBlcmZyFAsSB0Fub21hbHkYgICg0OOloQoM,agxzfmNocm9tZXBlcmZyFAsSB0Fub21hbHkYgICg0JjLvAoM,agxzfmNocm9tZXBlcmZyFAsSB0Fub21hbHkYgICg0OyKrQoM,agxzfmNocm9tZXBlcmZyFAsSB0Fub21hbHkYgICg0PKcpAoM,agxzfmNocm9tZXBlcmZyFAsSB0Fub21hbHkYgICg0JHwsQoM


Bot(s) for this bug's original alert(s):

webrtc-linux-large-tests
webrtc-mac-large-tests
webrtc-win-large-tests

It looks like the test is using a fake transport which starts in the "up" state and always remains like that. Before the CL in question, we would start sending RTP right away, but the RTCP stream would remain down until we get an explicit "transport is up" event.

With the change, both RTP and RTCP are started immediately. Since now more packets are competing for the same bandwidth, it takes a longer time - and more packets - to ramp up to max bandwidth.

The specific part of the change responsible for the regression is https://codereview.webrtc.org/1757683002/diff/40001/webrtc/call/call.cc#newcode500. To me this looks like a bug that was fixed - so the new behavior is correct, however, it might have also been a special case.

Adding pbos@ who wrote the original state change code, he should be able to clarify the intended behavior.

I think it sounds reasonable. We should definitely not have been sending only RTP and not RTCP.

+holmer@ to get a second look, I'm not sure if the number of packets/increased time is of reasonable size.

By the RTCP stream do you mean the receiver or just outgoing/incoming RTCP for both sender/receiver?

Looks like the problem is deeper than I thought: the RTCP sender is enabled by both the Send and Receive streams, so even if the receive stream doesn't get a network state change event - it shouldn't change the behavior.
However, it looks like every time RTCPSender::SendRTCPStatus() is called with a parameter other than kOff, it recomputes the time to send the next RTCP packet. When it gets called by the VideoReceiveStream, it reschedules the next RTCP packet 400 ms later than it would otherwise be - which changes the timing of the entire test. 

https://bugs.chromium.org/p/chromium/issues/detail?id=597714 is another bug sharing the same root cause.

 Issue 597714  has been merged into this issue.

https://codereview.webrtc.org/1826093004/ is the proposed fix - which only makes RTCPSender::SendRTCPStatus() reschedule the packet if it was previously off. 

@skvlad: Any update on the above issue ?

Appreciate your response.

Thank you!

@ashejole: waiting for one of the owners of the affected file to sign off on the code review. Once that's done I'll commit the fix.

The following revision refers to this bug:
  https://chromium.googlesource.com/external/webrtc.git/+/1c392cc5cf83630eaaa0401954b66149c1760ebc

commit 1c392cc5cf83630eaaa0401954b66149c1760ebc
Author: skvlad <skvlad@webrtc.org>
Date: Fri Apr 01 21:46:44 2016

Avoid rescheduling the next RTCP packet if the RTCP sender status doesn't change.

The change made in https://codereview.webrtc.org/1757683002 introduced an extra call to RTCPSender::SetRTCPStatus after the video receive stream is created. The SetRTCPStatus call results in no state change, as the RTCP sender is already enabled, however, it reschedules the next RTCP packet to be RTCP_INTERVAL_VIDEO_MS/2 (500) ms in the future.
Before the change, the next packet time was only set by the previous call to RTCPSender::SetSSRC, which placed it 100 ms in the future. The change, therefore, changed the timing of multiple performance tests - as it now takes a different length of time to ramp up to the same bandwidth.

BUG= chromium:597332 

Review URL: https://codereview.webrtc.org/1826093004

Cr-Commit-Position: refs/heads/master@{#12203}

[modify] https://crrev.com/1c392cc5cf83630eaaa0401954b66149c1760ebc/webrtc/modules/rtp_rtcp/source/rtcp_sender.cc

This change should restore previous performance characteristics.

The performance graphs went back to normal when this change landed.

A spot check of the original graphs also lgtm.