Add a PacketOptions struct to webrtc::Transport.

Add a PacketOptions struct to webrtc::Transport.

This allows us to pass packet meta data, such as transport sequence
number, to libjingle and further down to the socket implementation. A
similar struct already exist in libjingle, see rtc::PacketOptions in asyncpacketsocket.h.

BUG=4173
Committed: https://crrev.com/1d8a506405734d0cef9653704b036ca4f1388960
Cr-Commit-Position: refs/heads/master@{#10144}

[stefan-webrtc, 2015-09-29 09:41:44]

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

[stefan-webrtc, 2015-09-29 09:44:29]

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

[commit-bot: I haz the power, 2015-09-29 09:44:38]

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

[pbos-webrtc, 2015-09-29 09:54:42]

lgtm

https://codereview.webrtc.org/1376673004/diff/20001/webrtc/modules/rtp_rtcp/s...
File webrtc/modules/rtp_rtcp/source/rtp_sender_unittest.cc (right):

https://codereview.webrtc.org/1376673004/diff/20001/webrtc/modules/rtp_rtcp/s...
webrtc/modules/rtp_rtcp/source/rtp_sender_unittest.cc:886: _))
cl format unless this is clean

https://codereview.webrtc.org/1376673004/diff/20001/webrtc/transport.h
File webrtc/transport.h (right):

https://codereview.webrtc.org/1376673004/diff/20001/webrtc/transport.h#newcode21
webrtc/transport.h:21: int transport_sequence_number = -1;
Shouldn't this one be a fixed-size datatype? Or is this an uint16_t or -1 which
always fits?

[stefan-webrtc, 2015-09-29 10:00:14]

https://codereview.webrtc.org/1376673004/diff/20001/webrtc/transport.h
File webrtc/transport.h (right):

https://codereview.webrtc.org/1376673004/diff/20001/webrtc/transport.h#newcode21
webrtc/transport.h:21: int transport_sequence_number = -1;
On 2015/09/29 09:54:42, pbos-webrtc wrote:
> Shouldn't this one be a fixed-size datatype? Or is this an uint16_t or -1
which
> always fits?

It is uint16_t or -1. We could make it int32_t instead if you'd like that
better.

[pbos-webrtc, 2015-09-29 10:02:09]

On 2015/09/29 10:00:14, stefan-webrtc (holmer) wrote:
> https://codereview.webrtc.org/1376673004/diff/20001/webrtc/transport.h
> File webrtc/transport.h (right):
> 
>
https://codereview.webrtc.org/1376673004/diff/20001/webrtc/transport.h#newcode21
> webrtc/transport.h:21: int transport_sequence_number = -1;
> On 2015/09/29 09:54:42, pbos-webrtc wrote:
> > Shouldn't this one be a fixed-size datatype? Or is this an uint16_t or -1
> which
> > always fits?
> 
> It is uint16_t or -1. We could make it int32_t instead if you'd like that
> better.

No, either keep it like this or uint16_t + bool flag if it's set or not.

[commit-bot: I haz the power, 2015-09-29 10:28:16]

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

[commit-bot: I haz the power, 2015-09-29 10:28: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/7972)

[stefan-webrtc, 2015-09-29 11:18:41]

Also, please comment on whether you want the member in PacketOptions to be
called packed_id (as in libjingle) or transport_sequence_number.

[the sun, 2015-09-29 11:50:38]

On 2015/09/29 11:18:41, stefan-webrtc (holmer) wrote:
> Also, please comment on whether you want the member in PacketOptions to be
> called packed_id (as in libjingle) or transport_sequence_number.

Where in libjingle is "packet_id" (or "packed_id") used?

What is this field? Is it a sequence number to identify individual *transports*
or individual *packets on a transport*?

[stefan-webrtc, 2015-09-29 11:56:06]

On 2015/09/29 11:50:38, the sun wrote:
> On 2015/09/29 11:18:41, stefan-webrtc (holmer) wrote:
> > Also, please comment on whether you want the member in PacketOptions to be
> > called packed_id (as in libjingle) or transport_sequence_number.
> 
> Where in libjingle is "packet_id" (or "packed_id") used?
> 
> What is this field? Is it a sequence number to identify individual
*transports*
> or individual *packets on a transport*?

It's not yet defined in libjingle, but is about to be here:
https://codereview.webrtc.org/1363573002/.

It is a sequence number increasing by one for each packet being sent on the same
transport, independent of being audio or video. Right now I'm assuming Call ==
transport. That is of course not correct in the long run, but maybe good enough
for now as it's what we do with abs-send-time?

[the sun, 2015-09-29 11:58:39]

On 2015/09/29 11:56:06, stefan-webrtc (holmer) wrote:
> On 2015/09/29 11:50:38, the sun wrote:
> > On 2015/09/29 11:18:41, stefan-webrtc (holmer) wrote:
> > > Also, please comment on whether you want the member in PacketOptions to be
> > > called packed_id (as in libjingle) or transport_sequence_number.
> > 
> > Where in libjingle is "packet_id" (or "packed_id") used?
> > 
> > What is this field? Is it a sequence number to identify individual
> *transports*
> > or individual *packets on a transport*?
> 
> It's not yet defined in libjingle, but is about to be here:
> https://codereview.webrtc.org/1363573002/.
> 
> It is a sequence number increasing by one for each packet being sent on the
same
> transport, independent of being audio or video. Right now I'm assuming Call ==
> transport. That is of course not correct in the long run, but maybe good
enough
> for now as it's what we do with abs-send-time?

Isn't packet_id or packet_sequence_number better then?

[pbos-webrtc, 2015-09-29 12:05:47]

On 2015/09/29 11:58:39, the sun wrote:
> On 2015/09/29 11:56:06, stefan-webrtc (holmer) wrote:
> > On 2015/09/29 11:50:38, the sun wrote:
> > > On 2015/09/29 11:18:41, stefan-webrtc (holmer) wrote:
> > > > Also, please comment on whether you want the member in PacketOptions to
be
> > > > called packed_id (as in libjingle) or transport_sequence_number.
> > > 
> > > Where in libjingle is "packet_id" (or "packed_id") used?
> > > 
> > > What is this field? Is it a sequence number to identify individual
> > *transports*
> > > or individual *packets on a transport*?
> > 
> > It's not yet defined in libjingle, but is about to be here:
> > https://codereview.webrtc.org/1363573002/.
> > 
> > It is a sequence number increasing by one for each packet being sent on the
> same
> > transport, independent of being audio or video. Right now I'm assuming Call
==
> > transport. That is of course not correct in the long run, but maybe good
> enough
> > for now as it's what we do with abs-send-time?
> 
> Isn't packet_id or packet_sequence_number better then?

Can they be the same or is packet id supposed to be more general?

[stefan-webrtc, 2015-09-29 14:04:40]

On 2015/09/29 12:05:47, pbos-webrtc wrote:
> On 2015/09/29 11:58:39, the sun wrote:
> > On 2015/09/29 11:56:06, stefan-webrtc (holmer) wrote:
> > > On 2015/09/29 11:50:38, the sun wrote:
> > > > On 2015/09/29 11:18:41, stefan-webrtc (holmer) wrote:
> > > > > Also, please comment on whether you want the member in PacketOptions
to
> be
> > > > > called packed_id (as in libjingle) or transport_sequence_number.
> > > > 
> > > > Where in libjingle is "packet_id" (or "packed_id") used?
> > > > 
> > > > What is this field? Is it a sequence number to identify individual
> > > *transports*
> > > > or individual *packets on a transport*?
> > > 
> > > It's not yet defined in libjingle, but is about to be here:
> > > https://codereview.webrtc.org/1363573002/.
> > > 
> > > It is a sequence number increasing by one for each packet being sent on
the
> > same
> > > transport, independent of being audio or video. Right now I'm assuming
Call
> ==
> > > transport. That is of course not correct in the long run, but maybe good
> > enough
> > > for now as it's what we do with abs-send-time?
> > 
> > Isn't packet_id or packet_sequence_number better then?
> 
> Can they be the same or is packet id supposed to be more general?

packet_id is fine with me. We call it "transport-wide sequence number" in our
draft, which is why it ended up being named transport sequence number in webrtc
code. But I guess outside it makes sense to rename it something with packet. :)

packet id is probably good.

[mflodman, 2015-10-02 09:05:28]

One comment, LGTM.

https://codereview.webrtc.org/1376673004/diff/40001/webrtc/transport.h
File webrtc/transport.h (right):

https://codereview.webrtc.org/1376673004/diff/40001/webrtc/transport.h#newcode21
webrtc/transport.h:21: int packet_id = -1;
Comment about range, wrap and -1 being invalid?

[stefan-webrtc, 2015-10-02 09:25:57]

https://codereview.webrtc.org/1376673004/diff/40001/webrtc/transport.h
File webrtc/transport.h (right):

https://codereview.webrtc.org/1376673004/diff/40001/webrtc/transport.h#newcode21
webrtc/transport.h:21: int packet_id = -1;
On 2015/10/02 09:05:28, mflodman wrote:
> Comment about range, wrap and -1 being invalid?

Done. Not mentioning wrap here since it was decided that we don't make the
assumption that this id is sequential between successive packets (even though it
is).

[stefan-webrtc, 2015-10-02 09:26:21]

Comment added

[stefan-webrtc, 2015-10-02 09:27:26]

The CQ bit was checked by stefan@webrtc.org

[stefan-webrtc, 2015-10-02 09:27:27]

The patchset sent to the CQ was uploaded after l-g-t-m from pbos@webrtc.org,
mflodman@webrtc.org
Link to the patchset: https://codereview.webrtc.org/1376673004/#ps80001 (title:
"Comment added")

[commit-bot: I haz the power, 2015-10-02 09:27:38]

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

[commit-bot: I haz the power, 2015-10-02 10:39:38]

Committed patchset #5 (id:80001)

[commit-bot: I haz the power, 2015-10-02 10:39:47]

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