Cast: Deduplicate packets in paced sender and always send older packets first

Cast: Deduplicate packets in paced sender and always send older packets first

Use a map sorted by <timestamp, ssrc, packet_id> to keep track of duplicate
packets. This map conveniently sorts such that map.begin() is the oldest packet
and should be sent first.

Also make sure that we never buffer more than one RTSP packet per ssrc.

Committed: https://src.chromium.org/viewvc/chrome?view=rev&revision=266488

[miu, 2014-04-24 00:09:05]

lgtm, but make sure all the data types for ssrc (and method args) are uint32 (in
paced_sender.*).  Also, a few nit comments:

https://codereview.chromium.org/248493002/diff/1/media/cast/transport/pacing/...
File media/cast/transport/pacing/paced_sender.cc (right):

https://codereview.chromium.org/248493002/diff/1/media/cast/transport/pacing/...
media/cast/transport/pacing/paced_sender.cc:122: *packet_type = i->second.first;
nit: Suggest DCHECK(i != packet_list_.end());

https://codereview.chromium.org/248493002/diff/1/media/cast/transport/pacing/...
File media/cast/transport/pacing/paced_sender.h (right):

https://codereview.chromium.org/248493002/diff/1/media/cast/transport/pacing/...
media/cast/transport/pacing/paced_sender.h:31: typedef
std::pair<base::TimeTicks, std::pair<unsigned int, uint16> > PacketKey;
ssrc is defined as uint32 in other parts of the code.

https://codereview.chromium.org/248493002/diff/1/media/cast/transport/pacing/...
media/cast/transport/pacing/paced_sender.h:31: typedef
std::pair<base::TimeTicks, std::pair<unsigned int, uint16> > PacketKey;
I'm wondering whether PacketKey should be in cast_transport_defines.h, since it
is used in other modules.

https://codereview.chromium.org/248493002/diff/1/media/cast/transport/pacing/...
media/cast/transport/pacing/paced_sender.h:111: std::map<PacketKey,
std::pair<PacketType, PacketRef> > packet_list_;
nit: Maybe comment here (or above, where PacketKey is defined) why the sort
order of the map causes packets to be emitted in the desired priority order.

[hubbe, 2014-04-24 18:36:41]

https://codereview.chromium.org/248493002/diff/1/media/cast/transport/pacing/...
File media/cast/transport/pacing/paced_sender.cc (right):

https://codereview.chromium.org/248493002/diff/1/media/cast/transport/pacing/...
media/cast/transport/pacing/paced_sender.cc:122: *packet_type = i->second.first;
On 2014/04/24 00:09:06, miu wrote:
> nit: Suggest DCHECK(i != packet_list_.end());

Done.

https://codereview.chromium.org/248493002/diff/1/media/cast/transport/pacing/...
File media/cast/transport/pacing/paced_sender.h (right):

https://codereview.chromium.org/248493002/diff/1/media/cast/transport/pacing/...
media/cast/transport/pacing/paced_sender.h:31: typedef
std::pair<base::TimeTicks, std::pair<unsigned int, uint16> > PacketKey;
On 2014/04/24 00:09:06, miu wrote:
> I'm wondering whether PacketKey should be in cast_transport_defines.h, since
it
> is used in other modules.

I don't think so. The sort order is specific to the pacer, anything else that
uses it is doing so because the pacer needs it.

https://codereview.chromium.org/248493002/diff/1/media/cast/transport/pacing/...
media/cast/transport/pacing/paced_sender.h:31: typedef
std::pair<base::TimeTicks, std::pair<unsigned int, uint16> > PacketKey;
On 2014/04/24 00:09:06, miu wrote:
> ssrc is defined as uint32 in other parts of the code.

Done.

https://codereview.chromium.org/248493002/diff/1/media/cast/transport/pacing/...
media/cast/transport/pacing/paced_sender.h:111: std::map<PacketKey,
std::pair<PacketType, PacketRef> > packet_list_;
On 2014/04/24 00:09:06, miu wrote:
> nit: Maybe comment here (or above, where PacketKey is defined) why the sort
> order of the map causes packets to be emitted in the desired priority order.

Done.

[miu, 2014-04-24 20:21:50]

lgtm

[hubbe, 2014-04-25 00:58:09]

The CQ bit was checked by hubbe@chromium.org

[commit-bot: I haz the power, 2014-04-25 00:59:49]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/hubbe@chromium.org/248493002/40001

[commit-bot: I haz the power, 2014-04-25 02:50:26]

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

[commit-bot: I haz the power, 2014-04-25 02:50:26]

Try jobs failed on following builders:
  tryserver.chromium on chromium_presubmit

[hubbe, 2014-04-25 05:46:05]

The CQ bit was checked by hubbe@chromium.org

[commit-bot: I haz the power, 2014-04-25 08:22:27]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/hubbe@chromium.org/248493002/40001

[commit-bot: I haz the power, 2014-04-25 23:02:54]

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

[commit-bot: I haz the power, 2014-04-25 23:02:54]

Try jobs failed on following builders:
  tryserver.chromium on chromium_presubmit

[hubbe, 2014-04-26 19:17:25]

The CQ bit was checked by hubbe@chromium.org

[commit-bot: I haz the power, 2014-04-26 19:18:18]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/hubbe@chromium.org/248493002/40001

[commit-bot: I haz the power, 2014-04-26 19:49:00]

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

[commit-bot: I haz the power, 2014-04-26 19:49:01]

Try jobs failed on following builders:
  tryserver.chromium on chromium_presubmit

[hubbe, 2014-04-27 01:16:34]

The CQ bit was checked by hubbe@chromium.org

[commit-bot: I haz the power, 2014-04-27 01:17:15]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/hubbe@chromium.org/248493002/60001

[commit-bot: I haz the power, 2014-04-27 01:19:54]

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

[commit-bot: I haz the power, 2014-04-27 01:19:55]

Try jobs failed on following builders:
  tryserver.chromium on linux_chromium_gn_rel

[hubbe, 2014-04-27 19:35:41]

The CQ bit was checked by hubbe@chromium.org

[commit-bot: I haz the power, 2014-04-27 19:36:20]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/hubbe@chromium.org/248493002/60001

[commit-bot: I haz the power, 2014-04-28 08:15:12]

Change committed as 266488