Cast: cancel re-sending if a later nack packet says the receiver doesn't want it anymore

Cast: cancel re-sending if a later nack packet says the receiver doesn't want it anymore

When we receive a nack for a frame, we can cancel re-sending of any packet that is NOT missing.
This is a fairly minor optimzation, but helps when we're close to the available bandwidth.
(According to simulation.)

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

[miu, 2014-05-29 06:20:37]

lgtm

https://codereview.chromium.org/302913004/diff/1/media/cast/transport/rtp_sen...
File media/cast/transport/rtp_sender/rtp_sender.cc (right):

https://codereview.chromium.org/302913004/diff/1/media/cast/transport/rtp_sen...
media/cast/transport/rtp_sender/rtp_sender.cc:91: if (success) {
You can remove this if-statement now.  With your change, success is never false
at this point.

https://codereview.chromium.org/302913004/diff/1/media/cast/transport/rtp_sen...
media/cast/transport/rtp_sender/rtp_sender.cc:97: // Set the size as correspond
to each frame.
I don't understand this comment.  Should this LOC be deleted?

[hubbe, 2014-05-29 19:17:23]

Found a bug in my previous change. PTAL.

https://codereview.chromium.org/302913004/diff/1/media/cast/transport/rtp_sen...
File media/cast/transport/rtp_sender/rtp_sender.cc (right):

https://codereview.chromium.org/302913004/diff/1/media/cast/transport/rtp_sen...
media/cast/transport/rtp_sender/rtp_sender.cc:91: if (success) {
On 2014/05/29 06:20:37, miu wrote:
> You can remove this if-statement now.  With your change, success is never
false
> at this point.

Done.

https://codereview.chromium.org/302913004/diff/1/media/cast/transport/rtp_sen...
media/cast/transport/rtp_sender/rtp_sender.cc:97: // Set the size as correspond
to each frame.
On 2014/05/29 06:20:37, miu wrote:
> I don't understand this comment.  Should this LOC be deleted?

I looked up the change that added this comment, I still don't understand it:
Deleted.

[miu, 2014-05-29 20:28:34]

https://codereview.chromium.org/302913004/diff/20001/media/cast/transport/rtp...
File media/cast/transport/rtp_sender/rtp_sender.cc (right):

https://codereview.chromium.org/302913004/diff/20001/media/cast/transport/rtp...
media/cast/transport/rtp_sender/rtp_sender.cc:86: if (!packets_set.empty() &&
I'm a little confused about the semantics of |packets_set|.  What exactly does
|packets_set| contain?  I thought it contained the set of packets sent at least
once, but not yet ack'ed.

Also, now I'm suggesting the variable be renamed to make this clearer.  ;-)

[hubbe, 2014-05-29 20:46:21]

https://codereview.chromium.org/302913004/diff/20001/media/cast/transport/rtp...
File media/cast/transport/rtp_sender/rtp_sender.cc (right):

https://codereview.chromium.org/302913004/diff/20001/media/cast/transport/rtp...
media/cast/transport/rtp_sender/rtp_sender.cc:86: if (!packets_set.empty() &&
On 2014/05/29 20:28:34, miu wrote:
> I'm a little confused about the semantics of |packets_set|.  What exactly does
> |packets_set| contain?  I thought it contained the set of packets sent at
least
> once, but not yet ack'ed.
> 
> Also, now I'm suggesting the variable be renamed to make this clearer.  ;-)

Better?

[miu, 2014-05-29 21:30:35]

lgtm

Much clearer now.  Thanks.

[commit-bot: I haz the power, 2014-05-29 21:31:45]

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

[commit-bot: I haz the power, 2014-05-30 02:34:57]

FYI, CQ is re-trying this CL (attempt #1).
The failing builders are:
  android_dbg_triggered_tests on tryserver.chromium
(http://build.chromium.org/p/tryserver.chromium/builders/android_dbg_triggered...)

[commit-bot: I haz the power, 2014-05-30 05:33:25]

Change committed as 273729