Add support for transport wide sequence numbers

Add support for transport wide sequence numbers

Also refactor packet router to use a map rather than iterate over all
rtp modules for each packet sent.

BUG=webrtc:4311
Committed: https://crrev.com/867fb5224e1ba6a1c2cd523c005499a93ed61a08
Cr-Commit-Position: refs/heads/master@{#9670}

[sprang_webrtc, 2015-07-22 08:30:12]

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

[stefan-webrtc, 2015-07-22 10:49:01]

https://codereview.webrtc.org/1247293002/diff/1/webrtc/modules/pacing/include...
File webrtc/modules/pacing/include/packet_router.h (right):

https://codereview.webrtc.org/1247293002/diff/1/webrtc/modules/pacing/include...
webrtc/modules/pacing/include/packet_router.h:21: #include
"webrtc/modules/bitrate_controller/send_time_history.h"
Maybe this isn't the right place for the send_time_history?

https://codereview.webrtc.org/1247293002/diff/1/webrtc/modules/pacing/packet_...
File webrtc/modules/pacing/packet_router.cc (right):

https://codereview.webrtc.org/1247293002/diff/1/webrtc/modules/pacing/packet_...
webrtc/modules/pacing/packet_router.cc:104: ++transport_seq_;
This isn't being used anywhere? Seems like we should pass it into the rtp module
somewhere?

Not knowing the details about transport_seq_, why don't you have to use
AtomicOps to modify it here?

https://codereview.webrtc.org/1247293002/diff/1/webrtc/modules/pacing/packet_...
webrtc/modules/pacing/packet_router.cc:149: 0xFFFF);
Isn't this implicit from the cast?

https://codereview.webrtc.org/1247293002/diff/1/webrtc/modules/rtp_rtcp/sourc...
File webrtc/modules/rtp_rtcp/source/rtp_sender.cc (right):

https://codereview.webrtc.org/1247293002/diff/1/webrtc/modules/rtp_rtcp/sourc...
webrtc/modules/rtp_rtcp/source/rtp_sender.cc:925:
UpdateTransportSequenceNumber(buffer_to_send_ptr, length, rtp_header);
You have probably thought about this, but is there any reason why we can't just
pass the seq num in through TimeToSend*()? I would really like to avoid exposing
the PacketRouter to RtpRtcp.

I do see that you need to expose it anyway for the time being to handle ssrc
changes. But down the road I believe we should be able to remove that, so I'd
like to not add additional dependencies if possible.

https://codereview.webrtc.org/1247293002/diff/1/webrtc/video/end_to_end_tests.cc
File webrtc/video/end_to_end_tests.cc (right):

https://codereview.webrtc.org/1247293002/diff/1/webrtc/video/end_to_end_tests...
webrtc/video/end_to_end_tests.cc:1146: // TODO(sprang): Cleanup when msvc
supports explicit initializers for array.
Hehe... Do you expect that any time soon? :) I don't think this is worth having
a todo for otherwise.

https://codereview.webrtc.org/1247293002/diff/1/webrtc/video/end_to_end_tests...
webrtc/video/end_to_end_tests.cc:1327: // is in place.
We could also do a test in rampup_tests.cc with the new extensions and feedback
messages. Later...

https://codereview.webrtc.org/1247293002/diff/1/webrtc/video_engine/vie_chann...
File webrtc/video_engine/vie_channel.cc (left):

https://codereview.webrtc.org/1247293002/diff/1/webrtc/video_engine/vie_chann...
webrtc/video_engine/vie_channel.cc:409:
send_payload_router_->SetSendingRtpModules(active_send_modules);
Seems like send_payload_router_ and active_send_modules should be under if
(sender_) as well?

https://codereview.webrtc.org/1247293002/diff/1/webrtc/video_engine/vie_chann...
File webrtc/video_engine/vie_channel.cc (right):

https://codereview.webrtc.org/1247293002/diff/1/webrtc/video_engine/vie_chann...
webrtc/video_engine/vie_channel.cc:711: for (RtpRtcp* rtp_rtcp :
rtp_rtcp_modules_)
{}

[sprang_webrtc, 2015-07-22 15:11:33]

https://codereview.webrtc.org/1247293002/diff/1/webrtc/modules/pacing/include...
File webrtc/modules/pacing/include/packet_router.h (right):

https://codereview.webrtc.org/1247293002/diff/1/webrtc/modules/pacing/include...
webrtc/modules/pacing/include/packet_router.h:21: #include
"webrtc/modules/bitrate_controller/send_time_history.h"
On 2015/07/22 10:49:00, stefan-webrtc (holmer) wrote:
> Maybe this isn't the right place for the send_time_history?

What would be a better place? modules/remote_bitrate_estimator?

https://codereview.webrtc.org/1247293002/diff/1/webrtc/modules/pacing/packet_...
File webrtc/modules/pacing/packet_router.cc (right):

https://codereview.webrtc.org/1247293002/diff/1/webrtc/modules/pacing/packet_...
webrtc/modules/pacing/packet_router.cc:104: ++transport_seq_;
On 2015/07/22 10:49:00, stefan-webrtc (holmer) wrote:
> This isn't being used anywhere? Seems like we should pass it into the rtp
module
> somewhere?
> 
> Not knowing the details about transport_seq_, why don't you have to use
> AtomicOps to modify it here?

That's a bug, that my test case evidently didn't catch. Will make sure it does.

All numbers should be assigned from the outside by calling
AllocateSequenceNumber().

https://codereview.webrtc.org/1247293002/diff/1/webrtc/modules/pacing/packet_...
webrtc/modules/pacing/packet_router.cc:149: 0xFFFF);
On 2015/07/22 10:49:00, stefan-webrtc (holmer) wrote:
> Isn't this implicit from the cast?

The static_cast, yes. This was a remnant from when I did a DCHECK here.
I've also updated it to not begin counting backwards after 2^31 packets...

https://codereview.webrtc.org/1247293002/diff/1/webrtc/modules/rtp_rtcp/sourc...
File webrtc/modules/rtp_rtcp/source/rtp_sender.cc (right):

https://codereview.webrtc.org/1247293002/diff/1/webrtc/modules/rtp_rtcp/sourc...
webrtc/modules/rtp_rtcp/source/rtp_sender.cc:925:
UpdateTransportSequenceNumber(buffer_to_send_ptr, length, rtp_header);
On 2015/07/22 10:49:00, stefan-webrtc (holmer) wrote:
> You have probably thought about this, but is there any reason why we can't
just
> pass the seq num in through TimeToSend*()? I would really like to avoid
exposing
> the PacketRouter to RtpRtcp.
> 
> I do see that you need to expose it anyway for the time being to handle ssrc
> changes. But down the road I believe we should be able to remove that, so I'd
> like to not add additional dependencies if possible.

Three reasons: 
1. When it's TimeToSendPadding<tm>, we don't know beforehand how many packets
will be sent. Sending a sequence number in and a number of packets back out
turned out to be quite messy (I tried).
2. As you said, OnSsrcChanged
3. Since we already had (2), it felt unnecessary to add another interface for
the OnPacketSent callback. And returning a vector of send times was even messier
than (1)...

https://codereview.webrtc.org/1247293002/diff/1/webrtc/video/end_to_end_tests.cc
File webrtc/video/end_to_end_tests.cc (right):

https://codereview.webrtc.org/1247293002/diff/1/webrtc/video/end_to_end_tests...
webrtc/video/end_to_end_tests.cc:1146: // TODO(sprang): Cleanup when msvc
supports explicit initializers for array.
On 2015/07/22 10:49:01, stefan-webrtc (holmer) wrote:
> Hehe... Do you expect that any time soon? :) I don't think this is worth
having
> a todo for otherwise.

When chromium starts using msvc 2015? No idea what the timeframe is for that.

https://codereview.webrtc.org/1247293002/diff/1/webrtc/video/end_to_end_tests...
webrtc/video/end_to_end_tests.cc:1327: // is in place.
On 2015/07/22 10:49:01, stefan-webrtc (holmer) wrote:
> We could also do a test in rampup_tests.cc with the new extensions and
feedback
> messages. Later...

Definately. When everything is in place that is more important than this :)

https://codereview.webrtc.org/1247293002/diff/1/webrtc/video_engine/vie_chann...
File webrtc/video_engine/vie_channel.cc (left):

https://codereview.webrtc.org/1247293002/diff/1/webrtc/video_engine/vie_chann...
webrtc/video_engine/vie_channel.cc:409:
send_payload_router_->SetSendingRtpModules(active_send_modules);
On 2015/07/22 10:49:01, stefan-webrtc (holmer) wrote:
> Seems like send_payload_router_ and active_send_modules should be under if
> (sender_) as well?

Done.

https://codereview.webrtc.org/1247293002/diff/1/webrtc/video_engine/vie_chann...
File webrtc/video_engine/vie_channel.cc (right):

https://codereview.webrtc.org/1247293002/diff/1/webrtc/video_engine/vie_chann...
webrtc/video_engine/vie_channel.cc:711: for (RtpRtcp* rtp_rtcp :
rtp_rtcp_modules_)
On 2015/07/22 10:49:01, stefan-webrtc (holmer) wrote:
> {}

Done.

[stefan-webrtc, 2015-07-27 12:13:49]

https://codereview.webrtc.org/1247293002/diff/1/webrtc/modules/pacing/include...
File webrtc/modules/pacing/include/packet_router.h (right):

https://codereview.webrtc.org/1247293002/diff/1/webrtc/modules/pacing/include...
webrtc/modules/pacing/include/packet_router.h:21: #include
"webrtc/modules/bitrate_controller/send_time_history.h"
On 2015/07/22 15:11:32, språng wrote:
> On 2015/07/22 10:49:00, stefan-webrtc (holmer) wrote:
> > Maybe this isn't the right place for the send_time_history?
> 
> What would be a better place? modules/remote_bitrate_estimator?

I'm not sure... Maybe under pacing? We can keep it where it is if you'd like,
but it feels a bit weird since it's only used by the packet router / pacer?

https://codereview.webrtc.org/1247293002/diff/1/webrtc/modules/pacing/packet_...
File webrtc/modules/pacing/packet_router.cc (right):

https://codereview.webrtc.org/1247293002/diff/1/webrtc/modules/pacing/packet_...
webrtc/modules/pacing/packet_router.cc:104: ++transport_seq_;
On 2015/07/22 15:11:32, språng wrote:
> On 2015/07/22 10:49:00, stefan-webrtc (holmer) wrote:
> > This isn't being used anywhere? Seems like we should pass it into the rtp
> module
> > somewhere?
> > 
> > Not knowing the details about transport_seq_, why don't you have to use
> > AtomicOps to modify it here?
> 
> That's a bug, that my test case evidently didn't catch. Will make sure it
does.
> 
> All numbers should be assigned from the outside by calling
> AllocateSequenceNumber().

Acknowledged.

https://codereview.webrtc.org/1247293002/diff/1/webrtc/modules/rtp_rtcp/sourc...
File webrtc/modules/rtp_rtcp/source/rtp_sender.cc (right):

https://codereview.webrtc.org/1247293002/diff/1/webrtc/modules/rtp_rtcp/sourc...
webrtc/modules/rtp_rtcp/source/rtp_sender.cc:925:
UpdateTransportSequenceNumber(buffer_to_send_ptr, length, rtp_header);
On 2015/07/22 15:11:32, språng wrote:
> On 2015/07/22 10:49:00, stefan-webrtc (holmer) wrote:
> > You have probably thought about this, but is there any reason why we can't
> just
> > pass the seq num in through TimeToSend*()? I would really like to avoid
> exposing
> > the PacketRouter to RtpRtcp.
> > 
> > I do see that you need to expose it anyway for the time being to handle ssrc
> > changes. But down the road I believe we should be able to remove that, so
I'd
> > like to not add additional dependencies if possible.
> 
> Three reasons: 
> 1. When it's TimeToSendPadding<tm>, we don't know beforehand how many packets
> will be sent. Sending a sequence number in and a number of packets back out
> turned out to be quite messy (I tried).
> 2. As you said, OnSsrcChanged
> 3. Since we already had (2), it felt unnecessary to add another interface for
> the OnPacketSent callback. And returning a vector of send times was even
messier
> than (1)...

Acknowledged.

https://codereview.webrtc.org/1247293002/diff/60001/webrtc/modules/pacing/pac...
File webrtc/modules/pacing/packet_router.cc (right):

https://codereview.webrtc.org/1247293002/diff/60001/webrtc/modules/pacing/pac...
webrtc/modules/pacing/packet_router.cc:140: do {
I think we should comment on this code. I have read it a couple of times and I'm
still not sure why we compare the result of CompareAndSwap() to prev_seq?

[sprang_webrtc, 2015-07-28 13:29:28]

I moved SendTimeHistory and removed some things, which make more sense to add in
a later wire-up cl.

https://codereview.webrtc.org/1247293002/diff/1/webrtc/modules/pacing/include...
File webrtc/modules/pacing/include/packet_router.h (right):

https://codereview.webrtc.org/1247293002/diff/1/webrtc/modules/pacing/include...
webrtc/modules/pacing/include/packet_router.h:21: #include
"webrtc/modules/bitrate_controller/send_time_history.h"
On 2015/07/27 12:13:49, stefan-webrtc (holmer) wrote:
> On 2015/07/22 15:11:32, språng wrote:
> > On 2015/07/22 10:49:00, stefan-webrtc (holmer) wrote:
> > > Maybe this isn't the right place for the send_time_history?
> > 
> > What would be a better place? modules/remote_bitrate_estimator?
> 
> I'm not sure... Maybe under pacing? We can keep it where it is if you'd like,
> but it feels a bit weird since it's only used by the packet router / pacer?

I've move it around a few times now, trying to fit it together with upcoming
parts of the transport feedback wireup. It ended up in remote_bitrate_estimator.
OK?

https://codereview.webrtc.org/1247293002/diff/60001/webrtc/modules/pacing/pac...
File webrtc/modules/pacing/packet_router.cc (right):

https://codereview.webrtc.org/1247293002/diff/60001/webrtc/modules/pacing/pac...
webrtc/modules/pacing/packet_router.cc:140: do {
On 2015/07/27 12:13:49, stefan-webrtc (holmer) wrote:
> I think we should comment on this code. I have read it a couple of times and
I'm
> still not sure why we compare the result of CompareAndSwap() to prev_seq?

Added comment and also reformulated code a bit.

[stefan-webrtc, 2015-07-29 09:04:11]

https://codereview.webrtc.org/1247293002/diff/70001/webrtc/modules/pacing/pac...
File webrtc/modules/pacing/packet_router.cc (right):

https://codereview.webrtc.org/1247293002/diff/70001/webrtc/modules/pacing/pac...
webrtc/modules/pacing/packet_router.cc:83: }
I'd say you could argue whether the code actually became better by switching to
a map from ssrc to modules instead of having the list. What do you think? Is it
worth doing?

https://codereview.webrtc.org/1247293002/diff/70001/webrtc/modules/rtp_rtcp/i...
File webrtc/modules/rtp_rtcp/interface/rtp_rtcp_defines.h (right):

https://codereview.webrtc.org/1247293002/diff/70001/webrtc/modules/rtp_rtcp/i...
webrtc/modules/rtp_rtcp/interface/rtp_rtcp_defines.h:303: virtual void
OnPacketSent(uint16_t sequence_number, int64_t send_time) = 0;
Maybe transport_sequence_number?

https://codereview.webrtc.org/1247293002/diff/70001/webrtc/modules/rtp_rtcp/s...
File webrtc/modules/rtp_rtcp/source/rtp_sender.cc (right):

https://codereview.webrtc.org/1247293002/diff/70001/webrtc/modules/rtp_rtcp/s...
webrtc/modules/rtp_rtcp/source/rtp_sender.cc:680:
send_time_observer_->OnPacketSent(transport_seq, now_ms);
Do you have any suggestion of how we should handle the case where we may get a
new timestamp from the lower layers (e.g., the socket)? Should we just update
this timestamp at that point?

https://codereview.webrtc.org/1247293002/diff/70001/webrtc/video/end_to_end_t...
File webrtc/video/end_to_end_tests.cc (right):

https://codereview.webrtc.org/1247293002/diff/70001/webrtc/video/end_to_end_t...
webrtc/video/end_to_end_tests.cc:1365: padding_observed_ &&
rtx_padding_observed_)
{}

https://codereview.webrtc.org/1247293002/diff/70001/webrtc/video_engine/vie_c...
File webrtc/video_engine/vie_channel_group.cc (right):

https://codereview.webrtc.org/1247293002/diff/70001/webrtc/video_engine/vie_c...
webrtc/video_engine/vie_channel_group.cc:175: */
Probably want to remove this comment? :)

[sprang_webrtc, 2015-07-29 10:03:25]

https://codereview.webrtc.org/1247293002/diff/70001/webrtc/modules/pacing/pac...
File webrtc/modules/pacing/packet_router.cc (right):

https://codereview.webrtc.org/1247293002/diff/70001/webrtc/modules/pacing/pac...
webrtc/modules/pacing/packet_router.cc:83: }
On 2015/07/29 09:04:11, stefan-webrtc (holmer) wrote:
> I'd say you could argue whether the code actually became better by switching
to
> a map from ssrc to modules instead of having the list. What do you think? Is
it
> worth doing?

Especially after it turned out that we actually need to hold the lock while
calling the rtp modules, no. This is not worth it. 
Will still keep some things though, like changes to vie_channel and asserts here
that we don't double-remove. Also change to allow padding on multiple modules.
WDYT?

https://codereview.webrtc.org/1247293002/diff/70001/webrtc/modules/rtp_rtcp/i...
File webrtc/modules/rtp_rtcp/interface/rtp_rtcp_defines.h (right):

https://codereview.webrtc.org/1247293002/diff/70001/webrtc/modules/rtp_rtcp/i...
webrtc/modules/rtp_rtcp/interface/rtp_rtcp_defines.h:303: virtual void
OnPacketSent(uint16_t sequence_number, int64_t send_time) = 0;
On 2015/07/29 09:04:11, stefan-webrtc (holmer) wrote:
> Maybe transport_sequence_number?

Done.

https://codereview.webrtc.org/1247293002/diff/70001/webrtc/modules/rtp_rtcp/s...
File webrtc/modules/rtp_rtcp/source/rtp_sender.cc (right):

https://codereview.webrtc.org/1247293002/diff/70001/webrtc/modules/rtp_rtcp/s...
webrtc/modules/rtp_rtcp/source/rtp_sender.cc:680:
send_time_observer_->OnPacketSent(transport_seq, now_ms);
On 2015/07/29 09:04:11, stefan-webrtc (holmer) wrote:
> Do you have any suggestion of how we should handle the case where we may get a
> new timestamp from the lower layers (e.g., the socket)? Should we just update
> this timestamp at that point?

Yes, that is my intent. I'll add a test for this in a follow-up CL where we
actually read the send times also :)
Also, in the unlikely case that this callback arrives later than the feedback
from the other side, this will just be sitting in the history until it times
out.

https://codereview.webrtc.org/1247293002/diff/70001/webrtc/video/end_to_end_t...
File webrtc/video/end_to_end_tests.cc (right):

https://codereview.webrtc.org/1247293002/diff/70001/webrtc/video/end_to_end_t...
webrtc/video/end_to_end_tests.cc:1365: padding_observed_ &&
rtx_padding_observed_)
On 2015/07/29 09:04:11, stefan-webrtc (holmer) wrote:
> {}

Done.

https://codereview.webrtc.org/1247293002/diff/70001/webrtc/video_engine/vie_c...
File webrtc/video_engine/vie_channel_group.cc (right):

https://codereview.webrtc.org/1247293002/diff/70001/webrtc/video_engine/vie_c...
webrtc/video_engine/vie_channel_group.cc:175: */
On 2015/07/29 09:04:11, stefan-webrtc (holmer) wrote:
> Probably want to remove this comment? :)

Yes, even the whole method. Moved to a different CL :)

[stefan-webrtc, 2015-07-29 12:19:52]

A lot better, lgtm

https://codereview.webrtc.org/1247293002/diff/90001/webrtc/modules/pacing/pac...
File webrtc/modules/pacing/packet_router.cc (right):

https://codereview.webrtc.org/1247293002/diff/90001/webrtc/modules/pacing/pac...
webrtc/modules/pacing/packet_router.cc:38: rtp_modules_.erase(it);
rtp_modules_.remove(rtp_module); as before?

[sprang_webrtc, 2015-07-29 12:27:15]

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

[commit-bot: I haz the power, 2015-07-29 12:27:23]

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

[sprang_webrtc, 2015-07-29 12:27:27]

https://codereview.webrtc.org/1247293002/diff/90001/webrtc/modules/pacing/pac...
File webrtc/modules/pacing/packet_router.cc (right):

https://codereview.webrtc.org/1247293002/diff/90001/webrtc/modules/pacing/pac...
webrtc/modules/pacing/packet_router.cc:38: rtp_modules_.erase(it);
On 2015/07/29 12:19:52, stefan-webrtc (holmer) wrote:
> rtp_modules_.remove(rtp_module); as before?

erase() will directly remove that element from the list, while remove() will
iterate over the entire list and remove all instances where value matches the
argument. Since we have already asserted that there are no duplicates in the
list this is not necessary.

[commit-bot: I haz the power, 2015-07-29 12:28:21]

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

[commit-bot: I haz the power, 2015-07-29 12:28:21]

Dry run: Try jobs failed on following builders:
  android_clang on tryserver.webrtc (JOB_FAILED,
http://build.chromium.org/p/tryserver.webrtc/builders/android_clang/builds/6800)
  android_gn on tryserver.webrtc (JOB_FAILED,
http://build.chromium.org/p/tryserver.webrtc/builders/android_gn/builds/4872)
  linux_gn on tryserver.webrtc (JOB_FAILED,
http://build.chromium.org/p/tryserver.webrtc/builders/linux_gn/builds/4805)
  linux_gn_rel on tryserver.webrtc (JOB_FAILED,
http://build.chromium.org/p/tryserver.webrtc/builders/linux_gn_rel/builds/4754)
  linux_rel on tryserver.webrtc (JOB_FAILED,
http://build.chromium.org/p/tryserver.webrtc/builders/linux_rel/builds/8612)
  linux_tsan2 on tryserver.webrtc (JOB_FAILED,
http://build.chromium.org/p/tryserver.webrtc/builders/linux_tsan2/builds/5975)
  win_x64_rel on tryserver.webrtc (JOB_FAILED,
http://build.chromium.org/p/tryserver.webrtc/builders/win_x64_rel/builds/8581)

[sprang_webrtc, 2015-07-29 12:42:05]

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

[commit-bot: I haz the power, 2015-07-29 12:42:17]

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

[commit-bot: I haz the power, 2015-07-29 13:42:04]

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

[commit-bot: I haz the power, 2015-07-29 13:42:05]

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

[sprang_webrtc, 2015-07-29 15:15:27]

The CQ bit was checked by sprang@webrtc.org

[sprang_webrtc, 2015-07-29 15:15:27]

The patchset sent to the CQ was uploaded after l-g-t-m from stefan@webrtc.org
Link to the patchset: https://codereview.webrtc.org/1247293002/#ps130001 (title:
" ")

[commit-bot: I haz the power, 2015-07-29 15:15:41]

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

[commit-bot: I haz the power, 2015-07-29 15:17:41]

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

[commit-bot: I haz the power, 2015-07-29 15:17:42]

Try jobs failed on following builders:
  presubmit on tryserver.webrtc (JOB_FAILED,
http://build.chromium.org/p/tryserver.webrtc/builders/presubmit/builds/277)

[sprang_webrtc, 2015-07-29 15:45:21]

sprang@webrtc.org changed reviewers:
+ niklas.enbom@webrtc.org

[sprang_webrtc, 2015-07-29 15:45:22]

+niklas.enbom : Can you please review webrtc/config.(cc|h) ? All other root
owners seem to be on holiday :/
pbos: Can you please check webrtc/test/frame_generator_capturer.*

[niklas.enbom, 2015-07-29 15:52:52]

Are you planning to add documentation to
http://www.webrtc.org/experiments/rtp-hdrext/transport-sequence-number ?

On 2015/07/29 15:45:22, språng wrote:
> +niklas.enbom : Can you please review webrtc/config.(cc|h) ? All other root
> owners seem to be on holiday :/
> pbos: Can you please check webrtc/test/frame_generator_capturer.*

[niklas.enbom, 2015-07-29 20:25:08]

lgtm
On 2015/07/29 15:52:52, niklas.enbom wrote:
> Are you planning to add documentation to
> http://www.webrtc.org/experiments/rtp-hdrext/transport-sequence-number ?
> 
> On 2015/07/29 15:45:22, språng wrote:
> > +niklas.enbom : Can you please review webrtc/config.(cc|h) ? All other root
> > owners seem to be on holiday :/
> > pbos: Can you please check webrtc/test/frame_generator_capturer.*

[pbos-webrtc, 2015-07-30 13:04:50]

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

[pbos-webrtc, 2015-07-30 13:04:50]

lgtm for webrtc/test/. Haven't had time to look at the rest. +R mflodman@, I
think this could be worth getting another pair of eyes on, so take a look if you
have time when you get back even if it's after the fact.

[sprang_webrtc, 2015-08-03 10:56:13]

The CQ bit was checked by sprang@webrtc.org

[sprang_webrtc, 2015-08-03 10:56:14]

The patchset sent to the CQ was uploaded after l-g-t-m from
niklas.enbom@webrtc.org, pbos@webrtc.org, stefan@webrtc.org
Link to the patchset: https://codereview.webrtc.org/1247293002/#ps150001 (title:
"Rebase, again")

[commit-bot: I haz the power, 2015-08-03 10:56:19]

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

[commit-bot: I haz the power, 2015-08-03 11:38:46]

Committed patchset #9 (id:150001)

[commit-bot: I haz the power, 2015-08-03 11:38:54]

Patchset 9 (id:??) landed as
https://crrev.com/867fb5224e1ba6a1c2cd523c005499a93ed61a08
Cr-Commit-Position: refs/heads/master@{#9670}

[tommi, 2015-08-04 09:39:43]

On 2015/08/03 11:38:54, commit-bot: I haz the power wrote:
> Patchset 9 (id:??) landed as
> https://crrev.com/867fb5224e1ba6a1c2cd523c005499a93ed61a08
> Cr-Commit-Position: refs/heads/master@{#9670}

Could this CL have caused this failure?
https://build.chromium.org/p/client.webrtc/builders/Win%20DrMemory%20Full/bui...

[sprang_webrtc, 2015-08-04 11:06:24]

On 2015/08/04 09:39:43, tommi (webrtc) wrote:
> On 2015/08/03 11:38:54, commit-bot: I haz the power wrote:
> > Patchset 9 (id:??) landed as
> > https://crrev.com/867fb5224e1ba6a1c2cd523c005499a93ed61a08
> > Cr-Commit-Position: refs/heads/master@{#9670}
> 
> Could this CL have caused this failure?
>
https://build.chromium.org/p/client.webrtc/builders/Win%20DrMemory%20Full/bui...

Yes, fix in CQ: https://codereview.webrtc.org/1269743004/