Call OnSentPacket for all packets sent in the test framework.

Call OnSentPacket for all packets sent in the test framework.

Required a bit of refactoring to make it possible to pass a Call to DirectTransport on construction. This also lead to me having to remove the shared lock between PacketTransport and RtpRtcpObserver. Now RtpRtcpObserver has a SetTransports method instead of a SetReceivers method.

BUG=webrtc:4173
Committed: https://crrev.com/f116bd0d7a3cdad20bb638d5a87427bd920c8904
Cr-Commit-Position: refs/heads/master@{#10430}

[stefan-webrtc, 2015-10-23 13:24:43]

stefan@webrtc.org changed reviewers:
+ pbos@webrtc.org

[stefan-webrtc, 2015-10-23 13:24:51]

The CQ bit was checked by stefan@webrtc.org to run a CQ dry run

[commit-bot: I haz the power, 2015-10-23 13:25:01]

Dry run: CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/1419193002/1
View timeline at
 https://chromium-cq-status.appspot.com/patch-timeline/1419193002/1

[commit-bot: I haz the power, 2015-10-23 14:01:44]

The CQ bit was unchecked by commit-bot@chromium.org

[commit-bot: I haz the power, 2015-10-23 14:01:45]

Dry run: Try jobs failed on following builders:
  android_arm64_rel on tryserver.webrtc (JOB_FAILED,
http://build.chromium.org/p/tryserver.webrtc/builders/android_arm64_rel/build...)

[stefan-webrtc, 2015-10-27 09:17:16]

Description was changed from

==========
Wire up OnSentPacket for loopback video applications.

BUG=4173
==========

to

==========
Call OnSentPacket for all packets sent in the test framework.

Required a bit of refactoring to make it possible to pass a Call to
DirectTransport on construction. This also lead to me having to remove the
shared lock between PacketTransport and RtpRtcpObserver. Now RtpRtcpObserver has
a SetTransports method instead of a SetReceivers method.

BUG=4173
==========

[stefan-webrtc, 2015-10-27 09:23:18]

The CQ bit was checked by stefan@webrtc.org to run a CQ dry run

[commit-bot: I haz the power, 2015-10-27 09:23:25]

Dry run: CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/1419193002/40001
View timeline at
 https://chromium-cq-status.appspot.com/patch-timeline/1419193002/40001

[commit-bot: I haz the power, 2015-10-27 10:26:17]

The CQ bit was unchecked by commit-bot@chromium.org

[commit-bot: I haz the power, 2015-10-27 10:26:17]

Dry run: Try jobs failed on following builders:
  android_rel on tryserver.webrtc (JOB_FAILED,
http://build.chromium.org/p/tryserver.webrtc/builders/android_rel/builds/8566)

[stefan-webrtc, 2015-10-27 11:54:15]

temporary to investigate build issue.

[pbos-webrtc, 2015-10-27 12:28:22]

Description was changed from

==========
Call OnSentPacket for all packets sent in the test framework.

Required a bit of refactoring to make it possible to pass a Call to
DirectTransport on construction. This also lead to me having to remove the
shared lock between PacketTransport and RtpRtcpObserver. Now RtpRtcpObserver has
a SetTransports method instead of a SetReceivers method.

BUG=4173
==========

to

==========
Call OnSentPacket for all packets sent in the test framework.

Required a bit of refactoring to make it possible to pass a Call to
DirectTransport on construction. This also lead to me having to remove the
shared lock between PacketTransport and RtpRtcpObserver. Now RtpRtcpObserver has
a SetTransports method instead of a SetReceivers method.

BUG=webrtc:4173
==========

[stefan-webrtc, 2015-10-27 13:46:17]

Patchset #4 (id:60001) has been deleted

[stefan-webrtc, 2015-10-27 13:46:24]

Patchset #4 (id:80001) has been deleted

[stefan-webrtc, 2015-10-27 13:47:24]

The CQ bit was checked by stefan@webrtc.org to run a CQ dry run

[commit-bot: I haz the power, 2015-10-27 13:47:32]

Dry run: CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/1419193002/100001
View timeline at
 https://chromium-cq-status.appspot.com/patch-timeline/1419193002/100001

[pbos-webrtc, 2015-10-27 14:06:30]

https://codereview.webrtc.org/1419193002/diff/100001/webrtc/call/call_perf_te...
File webrtc/call/call_perf_tests.cc (right):

https://codereview.webrtc.org/1419193002/diff/100001/webrtc/call/call_perf_te...
webrtc/call/call_perf_tests.cc:129: : SyncRtcpObserver(),
Remove from ctor list

https://codereview.webrtc.org/1419193002/diff/100001/webrtc/call/call_perf_te...
webrtc/call/call_perf_tests.cc:250:
audio_send_transport.SetReceiver(&voe_packet_receiver);
consider adding this to ctor if possible in a follow-up CL

https://codereview.webrtc.org/1419193002/diff/100001/webrtc/call/call_perf_te...
webrtc/call/call_perf_tests.cc:459: Clock* clock_;
const

https://codereview.webrtc.org/1419193002/diff/100001/webrtc/call/call_perf_te...
webrtc/call/call_perf_tests.cc:460: int threshold_ms_;
thread annotate what you need

https://codereview.webrtc.org/1419193002/diff/100001/webrtc/call/call_perf_te...
webrtc/call/call_perf_tests.cc:556: Action OnSendRtcp(const uint8_t* packet,
size_t length) override {
Make a TODO to make this into something that runs every n milliseconds instead
of per-RTCP.

https://codereview.webrtc.org/1419193002/diff/100001/webrtc/test/call_test.cc
File webrtc/test/call_test.cc (right):

https://codereview.webrtc.org/1419193002/diff/100001/webrtc/test/call_test.cc...
webrtc/test/call_test.cc:39: test->SetTransports(send_transport_.get(),
receive_transport_.get());
Comment on why this one is needed (I assume it's not used by tests directly, if
so call it OnTransportsCreated or something)

https://codereview.webrtc.org/1419193002/diff/100001/webrtc/test/direct_trans...
File webrtc/test/direct_transport.cc (right):

https://codereview.webrtc.org/1419193002/diff/100001/webrtc/test/direct_trans...
webrtc/test/direct_transport.cc:66: rtc::SentPacket
sent_packet(options.packet_id, clock_->TimeInMilliseconds());
Move inside if()

https://codereview.webrtc.org/1419193002/diff/100001/webrtc/test/rtp_rtcp_obs...
File webrtc/test/rtp_rtcp_observer.h (right):

https://codereview.webrtc.org/1419193002/diff/100001/webrtc/test/rtp_rtcp_obs...
webrtc/test/rtp_rtcp_observer.h:38: virtual void SetTransports(PacketTransport*
send_transport,
if so remove here

https://codereview.webrtc.org/1419193002/diff/100001/webrtc/test/rtp_rtcp_obs...
webrtc/test/rtp_rtcp_observer.h:75: PacketTransport* send_transport_;
dead code? if so remove

https://codereview.webrtc.org/1419193002/diff/100001/webrtc/video/end_to_end_...
File webrtc/video/end_to_end_tests.cc (right):

https://codereview.webrtc.org/1419193002/diff/100001/webrtc/video/end_to_end_...
webrtc/video/end_to_end_tests.cc:447: rtc::CriticalSection crit_;
annotate

https://codereview.webrtc.org/1419193002/diff/100001/webrtc/video/end_to_end_...
webrtc/video/end_to_end_tests.cc:777: rtc::CriticalSection crit_;
guarded_by

https://codereview.webrtc.org/1419193002/diff/100001/webrtc/video/end_to_end_...
webrtc/video/end_to_end_tests.cc:987: rtc::CriticalSection crit_;
guarded_by

https://codereview.webrtc.org/1419193002/diff/100001/webrtc/video/end_to_end_...
webrtc/video/end_to_end_tests.cc:1266: virtual test::DirectTransport*
CreateSendTransport(Call* sender_call) {
Inline if this isn't overriding anything. If so tag as override.

https://codereview.webrtc.org/1419193002/diff/100001/webrtc/video/end_to_end_...
webrtc/video/end_to_end_tests.cc:1518: virtual test::DirectTransport*
CreateSendTransport(Call* sender_call) {
Mark as override or inline if not overriding.

https://codereview.webrtc.org/1419193002/diff/100001/webrtc/video/end_to_end_...
webrtc/video/end_to_end_tests.cc:1587: virtual test::DirectTransport*
CreateReceiveTransport(Call* receiver_call) {
override?

https://codereview.webrtc.org/1419193002/diff/100001/webrtc/video/video_send_...
File webrtc/video/video_send_stream_tests.cc (right):

https://codereview.webrtc.org/1419193002/diff/100001/webrtc/video/video_send_...
webrtc/video/video_send_stream_tests.cc:891: void
SetTransports(test::PacketTransport* send_transport,
If you need this in the tests then call it OnTransportsCreated or something that
matches the other style. Woohoo.

[stefan-webrtc, 2015-10-27 14:33:16]

https://codereview.webrtc.org/1419193002/diff/100001/webrtc/call/call_perf_te...
File webrtc/call/call_perf_tests.cc (right):

https://codereview.webrtc.org/1419193002/diff/100001/webrtc/call/call_perf_te...
webrtc/call/call_perf_tests.cc:129: : SyncRtcpObserver(),
On 2015/10/27 14:06:29, pbos-webrtc wrote:
> Remove from ctor list

Done.

https://codereview.webrtc.org/1419193002/diff/100001/webrtc/call/call_perf_te...
webrtc/call/call_perf_tests.cc:250:
audio_send_transport.SetReceiver(&voe_packet_receiver);
On 2015/10/27 14:06:29, pbos-webrtc wrote:
> consider adding this to ctor if possible in a follow-up CL

TODO for now.

https://codereview.webrtc.org/1419193002/diff/100001/webrtc/call/call_perf_te...
webrtc/call/call_perf_tests.cc:460: int threshold_ms_;
On 2015/10/27 14:06:29, pbos-webrtc wrote:
> thread annotate what you need

Done.

https://codereview.webrtc.org/1419193002/diff/100001/webrtc/test/call_test.cc
File webrtc/test/call_test.cc (right):

https://codereview.webrtc.org/1419193002/diff/100001/webrtc/test/call_test.cc...
webrtc/test/call_test.cc:39: test->SetTransports(send_transport_.get(),
receive_transport_.get());
On 2015/10/27 14:06:29, pbos-webrtc wrote:
> Comment on why this one is needed (I assume it's not used by tests directly,
if
> so call it OnTransportsCreated or something)

Done.

https://codereview.webrtc.org/1419193002/diff/100001/webrtc/test/direct_trans...
File webrtc/test/direct_transport.cc (right):

https://codereview.webrtc.org/1419193002/diff/100001/webrtc/test/direct_trans...
webrtc/test/direct_transport.cc:66: rtc::SentPacket
sent_packet(options.packet_id, clock_->TimeInMilliseconds());
On 2015/10/27 14:06:30, pbos-webrtc wrote:
> Move inside if()

Done.

https://codereview.webrtc.org/1419193002/diff/100001/webrtc/test/rtp_rtcp_obs...
File webrtc/test/rtp_rtcp_observer.h (right):

https://codereview.webrtc.org/1419193002/diff/100001/webrtc/test/rtp_rtcp_obs...
webrtc/test/rtp_rtcp_observer.h:38: virtual void SetTransports(PacketTransport*
send_transport,
On 2015/10/27 14:06:30, pbos-webrtc wrote:
> if so remove here

Done.

https://codereview.webrtc.org/1419193002/diff/100001/webrtc/test/rtp_rtcp_obs...
webrtc/test/rtp_rtcp_observer.h:75: PacketTransport* send_transport_;
On 2015/10/27 14:06:30, pbos-webrtc wrote:
> dead code? if so remove

Done.

https://codereview.webrtc.org/1419193002/diff/100001/webrtc/video/end_to_end_...
File webrtc/video/end_to_end_tests.cc (right):

https://codereview.webrtc.org/1419193002/diff/100001/webrtc/video/end_to_end_...
webrtc/video/end_to_end_tests.cc:447: rtc::CriticalSection crit_;
On 2015/10/27 14:06:30, pbos-webrtc wrote:
> annotate

Done.

https://codereview.webrtc.org/1419193002/diff/100001/webrtc/video/end_to_end_...
webrtc/video/end_to_end_tests.cc:777: rtc::CriticalSection crit_;
On 2015/10/27 14:06:30, pbos-webrtc wrote:
> guarded_by

Done.

https://codereview.webrtc.org/1419193002/diff/100001/webrtc/video/end_to_end_...
webrtc/video/end_to_end_tests.cc:987: rtc::CriticalSection crit_;
On 2015/10/27 14:06:30, pbos-webrtc wrote:
> guarded_by

Done.

https://codereview.webrtc.org/1419193002/diff/100001/webrtc/video/end_to_end_...
webrtc/video/end_to_end_tests.cc:1266: virtual test::DirectTransport*
CreateSendTransport(Call* sender_call) {
On 2015/10/27 14:06:30, pbos-webrtc wrote:
> Inline if this isn't overriding anything. If so tag as override.

Done.

https://codereview.webrtc.org/1419193002/diff/100001/webrtc/video/end_to_end_...
webrtc/video/end_to_end_tests.cc:1518: virtual test::DirectTransport*
CreateSendTransport(Call* sender_call) {
On 2015/10/27 14:06:30, pbos-webrtc wrote:
> Mark as override or inline if not overriding.

Done.

https://codereview.webrtc.org/1419193002/diff/100001/webrtc/video/end_to_end_...
webrtc/video/end_to_end_tests.cc:1587: virtual test::DirectTransport*
CreateReceiveTransport(Call* receiver_call) {
On 2015/10/27 14:06:30, pbos-webrtc wrote:
> override?

Done.

https://codereview.webrtc.org/1419193002/diff/100001/webrtc/video/video_send_...
File webrtc/video/video_send_stream_tests.cc (right):

https://codereview.webrtc.org/1419193002/diff/100001/webrtc/video/video_send_...
webrtc/video/video_send_stream_tests.cc:891: void
SetTransports(test::PacketTransport* send_transport,
On 2015/10/27 14:06:30, pbos-webrtc wrote:
> If you need this in the tests then call it OnTransportsCreated or something
that
> matches the other style. Woohoo.

Done.

[stefan-webrtc, 2015-10-27 14:33:48]

Comments addresded.

[pbos-webrtc, 2015-10-27 14:37:09]

lgtm

[stefan-webrtc, 2015-10-27 14:37:45]

The CQ bit was checked by stefan@webrtc.org

[commit-bot: I haz the power, 2015-10-27 14:37:49]

CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/1419193002/120001
View timeline at
 https://chromium-cq-status.appspot.com/patch-timeline/1419193002/120001

[commit-bot: I haz the power, 2015-10-27 15:29:46]

Committed patchset #5 (id:120001)

[commit-bot: I haz the power, 2015-10-27 15:29:53]

Patchset 5 (id:??) landed as
https://crrev.com/f116bd0d7a3cdad20bb638d5a87427bd920c8904
Cr-Commit-Position: refs/heads/master@{#10430}